From 316c763316ae8f9c9cbaf9cb7f997a9faf149085 Mon Sep 17 00:00:00 2001 From: Yanting Yang Date: Wed, 17 Apr 2019 00:28:18 +0800 Subject: [PATCH] Improve the latency of NotificationChannelSlice Get the notification data in parallel to reduce the latency. Fixes: 123065955 Test: visual, robotests Change-Id: I434ec68197af214f540ba39ae9155ee7625a2742 --- .../slices/NotificationChannelSlice.java | 58 ++++++------- .../NotificationMultiChannelAppRow.java | 53 ++++++++++++ .../slices/NotificationChannelSliceTest.java | 1 + .../NotificationMultiChannelAppRowTest.java | 81 +++++++++++++++++++ 4 files changed, 164 insertions(+), 29 deletions(-) create mode 100644 src/com/android/settings/homepage/contextualcards/slices/NotificationMultiChannelAppRow.java create mode 100644 tests/robotests/src/com/android/settings/homepage/contextualcards/slices/NotificationMultiChannelAppRowTest.java diff --git a/src/com/android/settings/homepage/contextualcards/slices/NotificationChannelSlice.java b/src/com/android/settings/homepage/contextualcards/slices/NotificationChannelSlice.java index 07fc8996d96..a262191d395 100644 --- a/src/com/android/settings/homepage/contextualcards/slices/NotificationChannelSlice.java +++ b/src/com/android/settings/homepage/contextualcards/slices/NotificationChannelSlice.java @@ -66,7 +66,12 @@ import com.android.settingslib.applications.ApplicationsState; import java.util.ArrayList; import java.util.Comparator; import java.util.List; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.stream.Collectors; public class NotificationChannelSlice implements CustomSliceable { @@ -98,6 +103,7 @@ public class NotificationChannelSlice implements CustomSliceable { private static final String PACKAGE_NAME = "package_name"; private static final String PACKAGE_UID = "package_uid"; private static final String CHANNEL_ID = "channel_id"; + private static final long TASK_TIMEOUT_MS = 100; /** * Sort notification channel with weekly average sent count by descending. @@ -129,6 +135,7 @@ public class NotificationChannelSlice implements CustomSliceable { }; protected final Context mContext; + private final ExecutorService mExecutorService; @VisibleForTesting NotificationBackend mNotificationBackend; private NotificationBackend.AppRow mAppRow; @@ -138,6 +145,7 @@ public class NotificationChannelSlice implements CustomSliceable { public NotificationChannelSlice(Context context) { mContext = context; mNotificationBackend = new NotificationBackend(); + mExecutorService = Executors.newCachedThreadPool(); } @Override @@ -151,10 +159,7 @@ public class NotificationChannelSlice implements CustomSliceable { * 2. Multiple channels. * 3. Sent at least ~10 notifications. */ - // TODO(b/123065955): Review latency of NotificationChannelSlice - final List multiChannelPackages = getMultiChannelPackages( - getRecentlyInstalledPackages()); - mPackageName = getMaxSentNotificationsPackage(multiChannelPackages); + mPackageName = getEligibleNotificationsPackage(getRecentlyInstalledPackages()); if (mPackageName == null) { // Return a header with IsError flag, if package is not found. return listBuilder.setHeader(getNoSuggestedAppHeader()) @@ -306,25 +311,6 @@ public class NotificationChannelSlice implements CustomSliceable { return PendingIntent.getBroadcast(mContext, intent.hashCode(), intent, 0); } - private List getMultiChannelPackages(List packageInfoList) { - final List multiChannelPackages = new ArrayList<>(); - - if (packageInfoList.isEmpty()) { - return multiChannelPackages; - } - - for (PackageInfo packageInfo : packageInfoList) { - final int channelCount = mNotificationBackend.getChannelCount(packageInfo.packageName, - getApplicationUid(packageInfo.packageName)); - if (channelCount > 1) { - multiChannelPackages.add(packageInfo); - } - } - - // TODO(b/119831690): Filter the packages which doesn't have any configurable channel. - return multiChannelPackages; - } - private List getRecentlyInstalledPackages() { final long startTime = System.currentTimeMillis() - DURATION_START_DAYS; final long endTime = System.currentTimeMillis() - DURATION_END_DAYS; @@ -383,19 +369,33 @@ public class NotificationChannelSlice implements CustomSliceable { .collect(Collectors.toList()); } - private String getMaxSentNotificationsPackage(List packageInfoList) { + private String getEligibleNotificationsPackage(List packageInfoList) { if (packageInfoList.isEmpty()) { return null; } + // Create tasks to get notification data for multi-channel packages. + final List> appRowTasks = new ArrayList<>(); + for (PackageInfo packageInfo : packageInfoList) { + final NotificationMultiChannelAppRow future = new NotificationMultiChannelAppRow( + mContext, mNotificationBackend, packageInfo); + appRowTasks.add(mExecutorService.submit(future)); + } + // Get the package which has sent at least ~10 notifications and not turn off channels. int maxSentCount = 0; String maxSentCountPackage = null; - for (PackageInfo packageInfo : packageInfoList) { - final NotificationBackend.AppRow appRow = mNotificationBackend.loadAppRow(mContext, - mContext.getPackageManager(), packageInfo); + for (Future appRowTask : appRowTasks) { + NotificationBackend.AppRow appRow = null; + try { + appRow = appRowTask.get(TASK_TIMEOUT_MS, TimeUnit.MILLISECONDS); + } catch (ExecutionException | InterruptedException | TimeoutException e) { + Log.w(TAG, "Failed to get notification data.", e); + } + // Ignore packages which are banned notifications or block all displayable channels. - if (appRow.banned || isAllChannelsBlocked(getDisplayableChannels(appRow))) { + if (appRow == null || appRow.banned || isAllChannelsBlocked( + getDisplayableChannels(appRow))) { continue; } @@ -403,7 +403,7 @@ public class NotificationChannelSlice implements CustomSliceable { final int sentCount = appRow.sentByApp.sentCount; if (sentCount >= MIN_NOTIFICATION_SENT_COUNT && sentCount > maxSentCount) { maxSentCount = sentCount; - maxSentCountPackage = packageInfo.packageName; + maxSentCountPackage = appRow.pkg; mAppRow = appRow; } } diff --git a/src/com/android/settings/homepage/contextualcards/slices/NotificationMultiChannelAppRow.java b/src/com/android/settings/homepage/contextualcards/slices/NotificationMultiChannelAppRow.java new file mode 100644 index 00000000000..4edce14dda3 --- /dev/null +++ b/src/com/android/settings/homepage/contextualcards/slices/NotificationMultiChannelAppRow.java @@ -0,0 +1,53 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License + */ + +package com.android.settings.homepage.contextualcards.slices; + +import android.content.Context; +import android.content.pm.PackageInfo; + +import com.android.settings.notification.NotificationBackend; + +import java.util.concurrent.Callable; + +/** + * This class is responsible for getting notification app row from package which has multiple + * notification channels.{@link NotificationChannelSlice} uses it to improve latency. + */ +class NotificationMultiChannelAppRow implements Callable { + + private final Context mContext; + private final NotificationBackend mNotificationBackend; + private final PackageInfo mPackageInfo; + + public NotificationMultiChannelAppRow(Context context, NotificationBackend notificationBackend, + PackageInfo packageInfo) { + mContext = context; + mNotificationBackend = notificationBackend; + mPackageInfo = packageInfo; + } + + @Override + public NotificationBackend.AppRow call() throws Exception { + final int channelCount = mNotificationBackend.getChannelCount( + mPackageInfo.applicationInfo.packageName, mPackageInfo.applicationInfo.uid); + if (channelCount > 1) { + return mNotificationBackend.loadAppRow(mContext, mContext.getPackageManager(), + mPackageInfo); + } + return null; + } +} diff --git a/tests/robotests/src/com/android/settings/homepage/contextualcards/slices/NotificationChannelSliceTest.java b/tests/robotests/src/com/android/settings/homepage/contextualcards/slices/NotificationChannelSliceTest.java index 81f5797c659..aee1305ed58 100644 --- a/tests/robotests/src/com/android/settings/homepage/contextualcards/slices/NotificationChannelSliceTest.java +++ b/tests/robotests/src/com/android/settings/homepage/contextualcards/slices/NotificationChannelSliceTest.java @@ -318,6 +318,7 @@ public class NotificationChannelSliceTest { applicationInfo.name = APP_LABEL; applicationInfo.uid = UID; applicationInfo.flags = flags; + applicationInfo.packageName = PACKAGE_NAME; final PackageInfo packageInfo = new PackageInfo(); packageInfo.packageName = PACKAGE_NAME; diff --git a/tests/robotests/src/com/android/settings/homepage/contextualcards/slices/NotificationMultiChannelAppRowTest.java b/tests/robotests/src/com/android/settings/homepage/contextualcards/slices/NotificationMultiChannelAppRowTest.java new file mode 100644 index 00000000000..d722af67a1d --- /dev/null +++ b/tests/robotests/src/com/android/settings/homepage/contextualcards/slices/NotificationMultiChannelAppRowTest.java @@ -0,0 +1,81 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License + */ + +package com.android.settings.homepage.contextualcards.slices; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; + +import android.content.Context; +import android.content.pm.ApplicationInfo; +import android.content.pm.PackageInfo; +import android.content.pm.PackageManager; + +import com.android.settings.notification.NotificationBackend; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.RuntimeEnvironment; + +@RunWith(RobolectricTestRunner.class) +public class NotificationMultiChannelAppRowTest { + + @Mock + private NotificationBackend mNotificationBackend; + private Context mContext; + private NotificationMultiChannelAppRow mNotificationMultiChannelAppRow; + private PackageInfo mPackageInfo; + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + + mContext = RuntimeEnvironment.application; + mPackageInfo = new PackageInfo(); + mPackageInfo.applicationInfo = new ApplicationInfo(); + mPackageInfo.applicationInfo.packageName = "com.android.test"; + mNotificationMultiChannelAppRow = new NotificationMultiChannelAppRow(mContext, + mNotificationBackend, mPackageInfo); + } + + @Test + public void call_isMultiChannel_shouldLoadAppRow() throws Exception { + doReturn(3).when(mNotificationBackend).getChannelCount(any(String.class), + any(int.class)); + + mNotificationMultiChannelAppRow.call(); + + verify(mNotificationBackend).loadAppRow(any(Context.class), any(PackageManager.class), + any(PackageInfo.class)); + } + + @Test + public void call_isNotMultiChannel_shouldNotLoadAppRow() throws Exception { + doReturn(1).when(mNotificationBackend).getChannelCount(any(String.class), + any(int.class)); + + mNotificationMultiChannelAppRow.call(); + + verify(mNotificationBackend, never()).loadAppRow(any(Context.class), + any(PackageManager.class), any(PackageInfo.class)); + } +}