From 0f98a6d1141be3f1fbc7ff308bf2bfa7b14cc989 Mon Sep 17 00:00:00 2001 From: Kevin Han Date: Fri, 13 Dec 2019 16:28:54 -0800 Subject: [PATCH 1/2] Convert NotificationContentInflater to singleton Convert NotificationContentInflater into a singleton since it has almost no state, making it easier to inject and re-use. Move the only per-row state it has, cached remote views, to a separate class NotifRemoteViewCacheImpl that manages the lifetime of the cached remote views based off the notification's lifecycle. This has the added benefit of being re-usable for content recycling later on. Bug: 145749521 Test: atest SystemUITests Test: atest NotificationContentInflaterTest w/o @Suppress locally Test: smoke test (add, remove, cancel, expand, groups, HUN) Change-Id: I7c05b19a0e12bced02bfdaa21e0243f35d64029f --- .../NotificationRemoteInputManager.java | 10 +- .../StatusBarDependenciesModule.java | 4 +- .../collection/NotificationRowBinderImpl.java | 10 +- .../row/ExpandableNotificationRow.java | 28 ++- .../row/NotifRemoteViewCache.java | 75 ++++++++ .../row/NotifRemoteViewCacheImpl.java | 110 ++++++++++++ .../row/NotificationContentInflater.java | 168 ++++++++++-------- .../row/NotificationRowModule.java | 44 +++++ .../NotificationGroupAlertTransferHelper.java | 3 +- .../statusbar/NotificationTestHelper.java | 7 +- .../NotificationEntryManagerTest.java | 11 +- .../row/NotifRemoteViewCacheImplTest.java | 112 ++++++++++++ .../row/NotificationContentInflaterTest.java | 86 ++++++++- 13 files changed, 566 insertions(+), 102 deletions(-) create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotifRemoteViewCache.java create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotifRemoteViewCacheImpl.java create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationRowModule.java create mode 100644 packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotifRemoteViewCacheImplTest.java diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationRemoteInputManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationRemoteInputManager.java index 43d0399c6d627..667e721ae37dd 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationRemoteInputManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationRemoteInputManager.java @@ -584,7 +584,15 @@ public class NotificationRemoteInputManager implements Dumpable { public void bindRow(ExpandableNotificationRow row) { row.setRemoteInputController(mRemoteInputController); - row.setRemoteViewClickHandler(mOnClickHandler); + } + + /** + * Return on-click handler for notification remote views + * + * @return on-click handler + */ + public RemoteViews.OnClickHandler getRemoteViewsOnClickHandler() { + return mOnClickHandler; } @VisibleForTesting diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarDependenciesModule.java b/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarDependenciesModule.java index d1f6ebf3826da..ec8dbead7de25 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarDependenciesModule.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarDependenciesModule.java @@ -18,6 +18,8 @@ package com.android.systemui.statusbar; import android.content.Context; +import com.android.systemui.statusbar.notification.row.NotificationRowModule; + import javax.inject.Singleton; import dagger.Module; @@ -26,7 +28,7 @@ import dagger.Provides; /** * Dagger Module providing common dependencies of StatusBar. */ -@Module +@Module(includes = {NotificationRowModule.class}) public class StatusBarDependenciesModule { /** * Provides our instance of CommandQueue which is considered optional. diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationRowBinderImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationRowBinderImpl.java index 6a2774b1842df..d75362c8e5ed2 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationRowBinderImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationRowBinderImpl.java @@ -41,8 +41,8 @@ import com.android.systemui.statusbar.notification.NotificationClicker; import com.android.systemui.statusbar.notification.NotificationInterruptionStateProvider; import com.android.systemui.statusbar.notification.logging.NotificationLogger; import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow; -import com.android.systemui.statusbar.notification.row.NotificationContentInflater; import com.android.systemui.statusbar.notification.row.NotificationGutsManager; +import com.android.systemui.statusbar.notification.row.NotificationRowContentBinder; import com.android.systemui.statusbar.notification.row.RowInflaterTask; import com.android.systemui.statusbar.notification.stack.NotificationListContainer; import com.android.systemui.statusbar.phone.KeyguardBypassController; @@ -67,6 +67,7 @@ public class NotificationRowBinderImpl implements NotificationRowBinder { private final NotificationInterruptionStateProvider mNotificationInterruptionStateProvider; private final Context mContext; + private final NotificationRowContentBinder mRowContentBinder; private final NotificationMessagingUtil mMessagingUtil; private final ExpandableNotificationRow.ExpansionLogger mExpansionLogger = this::logNotificationExpansion; @@ -79,7 +80,7 @@ public class NotificationRowBinderImpl implements NotificationRowBinder { private NotificationPresenter mPresenter; private NotificationListContainer mListContainer; private HeadsUpManager mHeadsUpManager; - private NotificationContentInflater.InflationCallback mInflationCallback; + private NotificationRowContentBinder.InflationCallback mInflationCallback; private ExpandableNotificationRow.OnAppOpsClickListener mOnAppOpsClickListener; private BindRowCallback mBindRowCallback; private NotificationClicker mNotificationClicker; @@ -90,6 +91,7 @@ public class NotificationRowBinderImpl implements NotificationRowBinder { Context context, NotificationRemoteInputManager notificationRemoteInputManager, NotificationLockscreenUserManager notificationLockscreenUserManager, + NotificationRowContentBinder rowContentBinder, @Named(ALLOW_NOTIFICATION_LONG_PRESS_NAME) boolean allowLongPress, KeyguardBypassController keyguardBypassController, StatusBarStateController statusBarStateController, @@ -98,6 +100,7 @@ public class NotificationRowBinderImpl implements NotificationRowBinder { NotificationInterruptionStateProvider notificationInterruptionStateProvider, NotificationLogger logger) { mContext = context; + mRowContentBinder = rowContentBinder; mMessagingUtil = new NotificationMessagingUtil(context); mNotificationRemoteInputManager = notificationRemoteInputManager; mNotificationLockscreenUserManager = notificationLockscreenUserManager; @@ -124,7 +127,7 @@ public class NotificationRowBinderImpl implements NotificationRowBinder { mOnAppOpsClickListener = mGutsManager::openGuts; } - public void setInflationCallback(NotificationContentInflater.InflationCallback callback) { + public void setInflationCallback(NotificationRowContentBinder.InflationCallback callback) { mInflationCallback = callback; } @@ -169,6 +172,7 @@ public class NotificationRowBinderImpl implements NotificationRowBinder { row.setGroupManager(mGroupManager); row.setHeadsUpManager(mHeadsUpManager); row.setOnExpandClickListener(mPresenter); + row.setContentBinder(mRowContentBinder); row.setInflationCallback(mInflationCallback); if (mAllowLongPress) { row.setLongPressListener(mGutsManager::openGuts); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRow.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRow.java index 3c247df692f49..50f21990eaeb8 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRow.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRow.java @@ -65,7 +65,6 @@ import android.view.accessibility.AccessibilityNodeInfo.AccessibilityAction; import android.widget.Chronometer; import android.widget.FrameLayout; import android.widget.ImageView; -import android.widget.RemoteViews; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.logging.MetricsLogger; @@ -150,7 +149,7 @@ public class ExpandableNotificationRow extends ActivatableNotificationView private StatusBarStateController mStatusbarStateController; private KeyguardBypassController mBypassController; private LayoutListener mLayoutListener; - private final NotificationContentInflater mNotificationInflater; + private NotificationRowContentBinder mNotificationContentBinder; private int mIconTransformContentShift; private int mIconTransformContentShiftNoIcon; private int mMaxHeadsUpHeightBeforeN; @@ -464,7 +463,7 @@ public class ExpandableNotificationRow extends ActivatableNotificationView * Inflate views based off the inflation flags set. Inflation happens asynchronously. */ public void inflateViews() { - mNotificationInflater.bindContent(mEntry, this, mInflationFlags, mBindParams, + mNotificationContentBinder.bindContent(mEntry, this, mInflationFlags, mBindParams, false /* forceInflate */, mInflationCallback); } @@ -478,7 +477,7 @@ public class ExpandableNotificationRow extends ActivatableNotificationView // View should not be reinflated in the future clearInflationFlags(inflationFlag); Runnable freeViewRunnable = - () -> mNotificationInflater.unbindContent(mEntry, this, inflationFlag); + () -> mNotificationContentBinder.unbindContent(mEntry, this, inflationFlag); switch (inflationFlag) { case FLAG_CONTENT_VIEW_HEADS_UP: getPrivateLayout().performWhenContentInactive(VISIBLE_TYPE_HEADSUP, @@ -852,7 +851,7 @@ public class ExpandableNotificationRow extends ActivatableNotificationView mIsChildInGroup = isChildInGroup; if (mIsLowPriority) { int flags = FLAG_CONTENT_VIEW_CONTRACTED | FLAG_CONTENT_VIEW_EXPANDED; - mNotificationInflater.bindContent(mEntry, this, flags, mBindParams, + mNotificationContentBinder.bindContent(mEntry, this, flags, mBindParams, false /* forceInflate */, mInflationCallback); } } @@ -1259,7 +1258,7 @@ public class ExpandableNotificationRow extends ActivatableNotificationView l.reInflateViews(); } mEntry.getSbn().clearPackageContext(); - mNotificationInflater.bindContent(mEntry, this, mInflationFlags, mBindParams, + mNotificationContentBinder.bindContent(mEntry, this, mInflationFlags, mBindParams, true /* forceInflate */, mInflationCallback); } @@ -1634,10 +1633,6 @@ public class ExpandableNotificationRow extends ActivatableNotificationView mBindParams.usesIncreasedHeadsUpHeight = use; } - public void setRemoteViewClickHandler(RemoteViews.OnClickHandler remoteViewClickHandler) { - mNotificationInflater.setRemoteViewClickHandler(remoteViewClickHandler); - } - /** * Set callback for notification content inflation * @@ -1652,7 +1647,7 @@ public class ExpandableNotificationRow extends ActivatableNotificationView mNeedsRedaction = needsRedaction; if (needsRedaction) { setInflationFlags(FLAG_CONTENT_VIEW_PUBLIC); - mNotificationInflater.bindContent(mEntry, this, FLAG_CONTENT_VIEW_PUBLIC, + mNotificationContentBinder.bindContent(mEntry, this, FLAG_CONTENT_VIEW_PUBLIC, mBindParams, false /* forceInflate */, mInflationCallback); } else { clearInflationFlags(FLAG_CONTENT_VIEW_PUBLIC); @@ -1661,9 +1656,13 @@ public class ExpandableNotificationRow extends ActivatableNotificationView } } - @VisibleForTesting - public NotificationContentInflater getNotificationInflater() { - return mNotificationInflater; + /** + * Set the binder implementation for notification content views. + * + * @param contentBinder content binder + */ + public void setContentBinder(NotificationRowContentBinder contentBinder) { + mNotificationContentBinder = contentBinder; } public interface ExpansionLogger { @@ -1672,7 +1671,6 @@ public class ExpandableNotificationRow extends ActivatableNotificationView public ExpandableNotificationRow(Context context, AttributeSet attrs) { super(context, attrs); - mNotificationInflater = new NotificationContentInflater(); mMenuRow = new NotificationMenuRow(mContext); mImageResolver = new NotificationInlineImageResolver(context, new NotificationInlineImageCache()); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotifRemoteViewCache.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotifRemoteViewCache.java new file mode 100644 index 0000000000000..c11c60fcdd044 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotifRemoteViewCache.java @@ -0,0 +1,75 @@ +/* + * Copyright (C) 2020 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.systemui.statusbar.notification.row; + +import android.widget.RemoteViews; + +import androidx.annotation.Nullable; + +import com.android.systemui.statusbar.notification.collection.NotificationEntry; +import com.android.systemui.statusbar.notification.row.NotificationRowContentBinder.InflationFlag; + +/** + * Caches {@link RemoteViews} for a notification's content views. + */ +public interface NotifRemoteViewCache { + + /** + * Whether the notification has the remote view cached + * + * @param entry notification + * @param flag inflation flag for content view + * @return true if the remote view is cached + */ + boolean hasCachedView(NotificationEntry entry, @InflationFlag int flag); + + /** + * Get the remote view for the content flag specified. + * + * @param entry notification + * @param flag inflation flag for the content view + * @return the remote view if it is cached, null otherwise + */ + @Nullable RemoteViews getCachedView(NotificationEntry entry, @InflationFlag int flag); + + /** + * Cache a remote view for a given content flag on a notification. + * + * @param entry notification + * @param flag inflation flag for the content view + * @param remoteView remote view to store + */ + void putCachedView( + NotificationEntry entry, + @InflationFlag int flag, + RemoteViews remoteView); + + /** + * Remove a cached remote view for a given content flag on a notification. + * + * @param entry notification + * @param flag inflation flag for the content view + */ + void removeCachedView(NotificationEntry entry, @InflationFlag int flag); + + /** + * Clear a notification's remote view cache. + * + * @param entry notification + */ + void clearCache(NotificationEntry entry); +} diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotifRemoteViewCacheImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotifRemoteViewCacheImpl.java new file mode 100644 index 0000000000000..a6e5c2b79968f --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotifRemoteViewCacheImpl.java @@ -0,0 +1,110 @@ +/* + * Copyright (C) 2020 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.systemui.statusbar.notification.row; + +import android.util.ArrayMap; +import android.util.SparseArray; +import android.widget.RemoteViews; + +import androidx.annotation.Nullable; + +import com.android.internal.statusbar.NotificationVisibility; +import com.android.systemui.statusbar.notification.NotificationEntryListener; +import com.android.systemui.statusbar.notification.NotificationEntryManager; +import com.android.systemui.statusbar.notification.collection.NotificationEntry; +import com.android.systemui.statusbar.notification.row.NotificationRowContentBinder.InflationFlag; + +import java.util.Map; + +import javax.inject.Inject; + +/** + * Implementation of remote view cache that keeps remote views cached for all active notifications. + */ +public class NotifRemoteViewCacheImpl implements NotifRemoteViewCache { + private final Map> mNotifCachedContentViews = + new ArrayMap<>(); + + @Inject + NotifRemoteViewCacheImpl(NotificationEntryManager entryManager) { + entryManager.addNotificationEntryListener(mEntryListener); + } + + @Override + public boolean hasCachedView(NotificationEntry entry, @InflationFlag int flag) { + return getCachedView(entry, flag) != null; + } + + @Override + public @Nullable RemoteViews getCachedView(NotificationEntry entry, @InflationFlag int flag) { + SparseArray contentViews = mNotifCachedContentViews.get(entry); + if (contentViews == null) { + return null; + } + return contentViews.get(flag); + } + + @Override + public void putCachedView( + NotificationEntry entry, + @InflationFlag int flag, + RemoteViews remoteView) { + /** + * TODO: We should be more strict here in the future (i.e. throw an exception) if the + * content views aren't created. We don't do that right now because we have edge cases + * where we may bind/unbind content after a notification is removed. + */ + SparseArray contentViews = mNotifCachedContentViews.get(entry); + if (contentViews == null) { + return; + } + contentViews.put(flag, remoteView); + } + + @Override + public void removeCachedView(NotificationEntry entry, @InflationFlag int flag) { + SparseArray contentViews = mNotifCachedContentViews.get(entry); + if (contentViews == null) { + return; + } + contentViews.remove(flag); + } + + @Override + public void clearCache(NotificationEntry entry) { + SparseArray contentViews = mNotifCachedContentViews.get(entry); + if (contentViews == null) { + return; + } + contentViews.clear(); + } + + private final NotificationEntryListener mEntryListener = new NotificationEntryListener() { + @Override + public void onPendingEntryAdded(NotificationEntry entry) { + mNotifCachedContentViews.put(entry, new SparseArray<>()); + } + + @Override + public void onEntryRemoved( + NotificationEntry entry, + @Nullable NotificationVisibility visibility, + boolean removedByUser) { + mNotifCachedContentViews.remove(entry); + } + }; +} diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationContentInflater.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationContentInflater.java index 30f22ac5e1618..e1a6747b5398f 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationContentInflater.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationContentInflater.java @@ -16,6 +16,7 @@ package com.android.systemui.statusbar.notification.row; +import static com.android.internal.annotations.VisibleForTesting.Visibility.PACKAGE; import static com.android.systemui.statusbar.notification.row.NotificationContentView.VISIBLE_TYPE_CONTRACTED; import static com.android.systemui.statusbar.notification.row.NotificationContentView.VISIBLE_TYPE_HEADSUP; @@ -26,7 +27,6 @@ import android.content.Context; import android.os.AsyncTask; import android.os.CancellationSignal; import android.service.notification.StatusBarNotification; -import android.util.ArrayMap; import android.util.Log; import android.view.View; import android.widget.RemoteViews; @@ -35,6 +35,7 @@ import com.android.internal.annotations.VisibleForTesting; import com.android.internal.widget.ImageMessageConsumer; import com.android.systemui.Dependency; import com.android.systemui.statusbar.InflationTask; +import com.android.systemui.statusbar.NotificationRemoteInputManager; import com.android.systemui.statusbar.SmartReplyController; import com.android.systemui.statusbar.notification.InflationException; import com.android.systemui.statusbar.notification.MediaNotificationProcessor; @@ -49,17 +50,30 @@ import com.android.systemui.util.Assert; import java.util.HashMap; +import javax.inject.Inject; +import javax.inject.Singleton; + /** * {@link NotificationContentInflater} binds content to a {@link ExpandableNotificationRow} by * asynchronously building the content's {@link RemoteViews} and applying it to the row. */ +@Singleton +@VisibleForTesting(visibility = PACKAGE) public class NotificationContentInflater implements NotificationRowContentBinder { public static final String TAG = "NotifContentInflater"; - private RemoteViews.OnClickHandler mRemoteViewClickHandler; private boolean mInflateSynchronously = false; - private final ArrayMap mCachedContentViews = new ArrayMap<>(); + private final NotificationRemoteInputManager mRemoteInputManager; + private final NotifRemoteViewCache mRemoteViewCache; + + @Inject + public NotificationContentInflater( + NotifRemoteViewCache remoteViewCache, + NotificationRemoteInputManager remoteInputManager) { + mRemoteViewCache = remoteViewCache; + mRemoteInputManager = remoteInputManager; + } @Override public void bindContent( @@ -76,27 +90,27 @@ public class NotificationContentInflater implements NotificationRowContentBinder return; } - StatusBarNotification sbn = row.getEntry().getSbn(); + StatusBarNotification sbn = entry.getSbn(); // To check if the notification has inline image and preload inline image if necessary. row.getImageResolver().preloadImages(sbn.getNotification()); if (forceInflate) { - mCachedContentViews.clear(); + mRemoteViewCache.clearCache(entry); } AsyncInflationTask task = new AsyncInflationTask( - sbn, mInflateSynchronously, contentToBind, - mCachedContentViews, + mRemoteViewCache, + entry, row, bindParams.isLowPriority, bindParams.isChildInGroup, bindParams.usesIncreasedHeight, bindParams.usesIncreasedHeadsUpHeight, callback, - mRemoteViewClickHandler); + mRemoteInputManager.getRemoteViewsOnClickHandler()); if (mInflateSynchronously) { task.onPostExecute(task.doInBackground()); } else { @@ -123,13 +137,15 @@ public class NotificationContentInflater implements NotificationRowContentBinder result = inflateSmartReplyViews(result, reInflateFlags, entry, row.getContext(), packageContext, row.getHeadsUpManager(), row.getExistingSmartRepliesAndActions()); + apply( inflateSynchronously, result, reInflateFlags, - mCachedContentViews, + mRemoteViewCache, + entry, row, - mRemoteViewClickHandler, + mRemoteInputManager.getRemoteViewsOnClickHandler(), null); return result; } @@ -149,42 +165,33 @@ public class NotificationContentInflater implements NotificationRowContentBinder int curFlag = 1; while (contentToUnbind != 0) { if ((contentToUnbind & curFlag) != 0) { - freeNotificationView(row, curFlag); + freeNotificationView(entry, row, curFlag); } contentToUnbind &= ~curFlag; curFlag = curFlag << 1; } } - /** - * Set click handler for notification remote views - * - * @param remoteViewClickHandler click handler for remote views - */ - public void setRemoteViewClickHandler(RemoteViews.OnClickHandler remoteViewClickHandler) { - mRemoteViewClickHandler = remoteViewClickHandler; - } - /** * Frees the content view associated with the inflation flag. Will only succeed if the * view is safe to remove. * * @param inflateFlag the flag corresponding to the content view which should be freed */ - private void freeNotificationView(ExpandableNotificationRow row, + private void freeNotificationView(NotificationEntry entry, ExpandableNotificationRow row, @InflationFlag int inflateFlag) { switch (inflateFlag) { case FLAG_CONTENT_VIEW_HEADS_UP: if (row.getPrivateLayout().isContentViewInactive(VISIBLE_TYPE_HEADSUP)) { row.getPrivateLayout().setHeadsUpChild(null); - mCachedContentViews.remove(FLAG_CONTENT_VIEW_HEADS_UP); + mRemoteViewCache.removeCachedView(entry, FLAG_CONTENT_VIEW_HEADS_UP); row.getPrivateLayout().setHeadsUpInflatedSmartReplies(null); } break; case FLAG_CONTENT_VIEW_PUBLIC: if (row.getPublicLayout().isContentViewInactive(VISIBLE_TYPE_CONTRACTED)) { row.getPublicLayout().setContractedChild(null); - mCachedContentViews.remove(FLAG_CONTENT_VIEW_PUBLIC); + mRemoteViewCache.removeCachedView(entry, FLAG_CONTENT_VIEW_PUBLIC); } break; case FLAG_CONTENT_VIEW_CONTRACTED: @@ -245,11 +252,12 @@ public class NotificationContentInflater implements NotificationRowContentBinder return result; } - public static CancellationSignal apply( + private static CancellationSignal apply( boolean inflateSynchronously, InflationProgress result, @InflationFlag int reInflateFlags, - ArrayMap cachedContentViews, + NotifRemoteViewCache remoteViewCache, + NotificationEntry entry, ExpandableNotificationRow row, RemoteViews.OnClickHandler remoteViewClickHandler, @Nullable InflationCallback callback) { @@ -261,7 +269,7 @@ public class NotificationContentInflater implements NotificationRowContentBinder if ((reInflateFlags & flag) != 0) { boolean isNewView = !canReapplyRemoteView(result.newContentView, - cachedContentViews.get(FLAG_CONTENT_VIEW_CONTRACTED)); + remoteViewCache.getCachedView(entry, FLAG_CONTENT_VIEW_CONTRACTED)); ApplyCallback applyCallback = new ApplyCallback() { @Override public void setResultView(View v) { @@ -273,8 +281,8 @@ public class NotificationContentInflater implements NotificationRowContentBinder return result.newContentView; } }; - applyRemoteView(inflateSynchronously, result, reInflateFlags, flag, cachedContentViews, - row, isNewView, remoteViewClickHandler, callback, privateLayout, + applyRemoteView(inflateSynchronously, result, reInflateFlags, flag, remoteViewCache, + entry, row, isNewView, remoteViewClickHandler, callback, privateLayout, privateLayout.getContractedChild(), privateLayout.getVisibleWrapper( NotificationContentView.VISIBLE_TYPE_CONTRACTED), runningInflations, applyCallback); @@ -285,7 +293,7 @@ public class NotificationContentInflater implements NotificationRowContentBinder if (result.newExpandedView != null) { boolean isNewView = !canReapplyRemoteView(result.newExpandedView, - cachedContentViews.get(FLAG_CONTENT_VIEW_EXPANDED)); + remoteViewCache.getCachedView(entry, FLAG_CONTENT_VIEW_EXPANDED)); ApplyCallback applyCallback = new ApplyCallback() { @Override public void setResultView(View v) { @@ -297,8 +305,8 @@ public class NotificationContentInflater implements NotificationRowContentBinder return result.newExpandedView; } }; - applyRemoteView(inflateSynchronously, result, reInflateFlags, flag, - cachedContentViews, row, isNewView, remoteViewClickHandler, + applyRemoteView(inflateSynchronously, result, reInflateFlags, flag, remoteViewCache, + entry, row, isNewView, remoteViewClickHandler, callback, privateLayout, privateLayout.getExpandedChild(), privateLayout.getVisibleWrapper( NotificationContentView.VISIBLE_TYPE_EXPANDED), runningInflations, @@ -311,7 +319,7 @@ public class NotificationContentInflater implements NotificationRowContentBinder if (result.newHeadsUpView != null) { boolean isNewView = !canReapplyRemoteView(result.newHeadsUpView, - cachedContentViews.get(FLAG_CONTENT_VIEW_HEADS_UP)); + remoteViewCache.getCachedView(entry, FLAG_CONTENT_VIEW_HEADS_UP)); ApplyCallback applyCallback = new ApplyCallback() { @Override public void setResultView(View v) { @@ -323,8 +331,8 @@ public class NotificationContentInflater implements NotificationRowContentBinder return result.newHeadsUpView; } }; - applyRemoteView(inflateSynchronously, result, reInflateFlags, flag, - cachedContentViews, row, isNewView, remoteViewClickHandler, + applyRemoteView(inflateSynchronously, result, reInflateFlags, flag, remoteViewCache, + entry, row, isNewView, remoteViewClickHandler, callback, privateLayout, privateLayout.getHeadsUpChild(), privateLayout.getVisibleWrapper( VISIBLE_TYPE_HEADSUP), runningInflations, @@ -336,7 +344,7 @@ public class NotificationContentInflater implements NotificationRowContentBinder if ((reInflateFlags & flag) != 0) { boolean isNewView = !canReapplyRemoteView(result.newPublicView, - cachedContentViews.get(FLAG_CONTENT_VIEW_PUBLIC)); + remoteViewCache.getCachedView(entry, FLAG_CONTENT_VIEW_PUBLIC)); ApplyCallback applyCallback = new ApplyCallback() { @Override public void setResultView(View v) { @@ -348,15 +356,16 @@ public class NotificationContentInflater implements NotificationRowContentBinder return result.newPublicView; } }; - applyRemoteView(inflateSynchronously, result, reInflateFlags, flag, cachedContentViews, - row, isNewView, remoteViewClickHandler, callback, + applyRemoteView(inflateSynchronously, result, reInflateFlags, flag, remoteViewCache, + entry, row, isNewView, remoteViewClickHandler, callback, publicLayout, publicLayout.getContractedChild(), publicLayout.getVisibleWrapper(NotificationContentView.VISIBLE_TYPE_CONTRACTED), runningInflations, applyCallback); } // Let's try to finish, maybe nobody is even inflating anything - finishIfDone(result, reInflateFlags, cachedContentViews, runningInflations, callback, row); + finishIfDone(result, reInflateFlags, remoteViewCache, runningInflations, callback, entry, + row); CancellationSignal cancellationSignal = new CancellationSignal(); cancellationSignal.setOnCancelListener( () -> runningInflations.values().forEach(CancellationSignal::cancel)); @@ -369,7 +378,8 @@ public class NotificationContentInflater implements NotificationRowContentBinder final InflationProgress result, final @InflationFlag int reInflateFlags, @InflationFlag int inflationId, - final ArrayMap cachedContentViews, + final NotifRemoteViewCache remoteViewCache, + final NotificationEntry entry, final ExpandableNotificationRow row, boolean isNewView, RemoteViews.OnClickHandler remoteViewClickHandler, @@ -422,8 +432,8 @@ public class NotificationContentInflater implements NotificationRowContentBinder existingWrapper.onReinflated(); } runningInflations.remove(inflationId); - finishIfDone(result, reInflateFlags, cachedContentViews, runningInflations, - callback, row); + finishIfDone(result, reInflateFlags, remoteViewCache, runningInflations, + callback, entry, row); } @Override @@ -488,11 +498,11 @@ public class NotificationContentInflater implements NotificationRowContentBinder * @return true if the inflation was finished */ private static boolean finishIfDone(InflationProgress result, - @InflationFlag int reInflateFlags, ArrayMap cachedContentViews, + @InflationFlag int reInflateFlags, NotifRemoteViewCache remoteViewCache, HashMap runningInflations, - @Nullable InflationCallback endListener, ExpandableNotificationRow row) { + @Nullable InflationCallback endListener, NotificationEntry entry, + ExpandableNotificationRow row) { Assert.isMainThread(); - NotificationEntry entry = row.getEntry(); NotificationContentView privateLayout = row.getPrivateLayout(); NotificationContentView publicLayout = row.getPublicLayout(); if (runningInflations.isEmpty()) { @@ -500,23 +510,27 @@ public class NotificationContentInflater implements NotificationRowContentBinder if (result.inflatedContentView != null) { // New view case privateLayout.setContractedChild(result.inflatedContentView); - cachedContentViews.put(FLAG_CONTENT_VIEW_CONTRACTED, result.newContentView); - } else if (cachedContentViews.get(FLAG_CONTENT_VIEW_CONTRACTED) != null) { + remoteViewCache.putCachedView(entry, FLAG_CONTENT_VIEW_CONTRACTED, + result.newContentView); + } else if (remoteViewCache.hasCachedView(entry, FLAG_CONTENT_VIEW_CONTRACTED)) { // Reinflation case. Only update if it's still cached (i.e. view has not been // freed while inflating). - cachedContentViews.put(FLAG_CONTENT_VIEW_CONTRACTED, result.newContentView); + remoteViewCache.putCachedView(entry, FLAG_CONTENT_VIEW_CONTRACTED, + result.newContentView); } } if ((reInflateFlags & FLAG_CONTENT_VIEW_EXPANDED) != 0) { if (result.inflatedExpandedView != null) { privateLayout.setExpandedChild(result.inflatedExpandedView); - cachedContentViews.put(FLAG_CONTENT_VIEW_EXPANDED, result.newExpandedView); + remoteViewCache.putCachedView(entry, FLAG_CONTENT_VIEW_EXPANDED, + result.newExpandedView); } else if (result.newExpandedView == null) { privateLayout.setExpandedChild(null); - cachedContentViews.put(FLAG_CONTENT_VIEW_EXPANDED, null); - } else if (cachedContentViews.get(FLAG_CONTENT_VIEW_EXPANDED) != null) { - cachedContentViews.put(FLAG_CONTENT_VIEW_EXPANDED, result.newExpandedView); + remoteViewCache.removeCachedView(entry, FLAG_CONTENT_VIEW_EXPANDED); + } else if (remoteViewCache.hasCachedView(entry, FLAG_CONTENT_VIEW_EXPANDED)) { + remoteViewCache.putCachedView(entry, FLAG_CONTENT_VIEW_EXPANDED, + result.newExpandedView); } if (result.newExpandedView != null) { privateLayout.setExpandedInflatedSmartReplies( @@ -530,12 +544,14 @@ public class NotificationContentInflater implements NotificationRowContentBinder if ((reInflateFlags & FLAG_CONTENT_VIEW_HEADS_UP) != 0) { if (result.inflatedHeadsUpView != null) { privateLayout.setHeadsUpChild(result.inflatedHeadsUpView); - cachedContentViews.put(FLAG_CONTENT_VIEW_HEADS_UP, result.newHeadsUpView); + remoteViewCache.putCachedView(entry, FLAG_CONTENT_VIEW_HEADS_UP, + result.newHeadsUpView); } else if (result.newHeadsUpView == null) { privateLayout.setHeadsUpChild(null); - cachedContentViews.put(FLAG_CONTENT_VIEW_HEADS_UP, null); - } else if (cachedContentViews.get(FLAG_CONTENT_VIEW_HEADS_UP) != null) { - cachedContentViews.put(FLAG_CONTENT_VIEW_HEADS_UP, result.newHeadsUpView); + remoteViewCache.removeCachedView(entry, FLAG_CONTENT_VIEW_HEADS_UP); + } else if (remoteViewCache.hasCachedView(entry, FLAG_CONTENT_VIEW_HEADS_UP)) { + remoteViewCache.putCachedView(entry, FLAG_CONTENT_VIEW_HEADS_UP, + result.newHeadsUpView); } if (result.newHeadsUpView != null) { privateLayout.setHeadsUpInflatedSmartReplies( @@ -548,16 +564,18 @@ public class NotificationContentInflater implements NotificationRowContentBinder if ((reInflateFlags & FLAG_CONTENT_VIEW_PUBLIC) != 0) { if (result.inflatedPublicView != null) { publicLayout.setContractedChild(result.inflatedPublicView); - cachedContentViews.put(FLAG_CONTENT_VIEW_PUBLIC, result.newPublicView); - } else if (cachedContentViews.get(FLAG_CONTENT_VIEW_PUBLIC) != null) { - cachedContentViews.put(FLAG_CONTENT_VIEW_PUBLIC, result.newPublicView); + remoteViewCache.putCachedView(entry, FLAG_CONTENT_VIEW_PUBLIC, + result.newPublicView); + } else if (remoteViewCache.hasCachedView(entry, FLAG_CONTENT_VIEW_PUBLIC)) { + remoteViewCache.putCachedView(entry, FLAG_CONTENT_VIEW_PUBLIC, + result.newPublicView); } } entry.headsUpStatusBarText = result.headsUpStatusBarText; entry.headsUpStatusBarTextPublic = result.headsUpStatusBarTextPublic; if (endListener != null) { - endListener.onAsyncInflationFinished(row.getEntry(), reInflateFlags); + endListener.onAsyncInflationFinished(entry, reInflateFlags); } return true; } @@ -615,7 +633,7 @@ public class NotificationContentInflater implements NotificationRowContentBinder public static class AsyncInflationTask extends AsyncTask implements InflationCallback, InflationTask { - private final StatusBarNotification mSbn; + private final NotificationEntry mEntry; private final Context mContext; private final boolean mInflateSynchronously; private final boolean mIsLowPriority; @@ -624,17 +642,17 @@ public class NotificationContentInflater implements NotificationRowContentBinder private final InflationCallback mCallback; private final boolean mUsesIncreasedHeadsUpHeight; private @InflationFlag int mReInflateFlags; - private final ArrayMap mCachedContentViews; + private final NotifRemoteViewCache mRemoteViewCache; private ExpandableNotificationRow mRow; private Exception mError; private RemoteViews.OnClickHandler mRemoteViewClickHandler; private CancellationSignal mCancellationSignal; private AsyncInflationTask( - StatusBarNotification notification, boolean inflateSynchronously, @InflationFlag int reInflateFlags, - ArrayMap cachedContentViews, + NotifRemoteViewCache cache, + NotificationEntry entry, ExpandableNotificationRow row, boolean isLowPriority, boolean isChildInGroup, @@ -642,11 +660,11 @@ public class NotificationContentInflater implements NotificationRowContentBinder boolean usesIncreasedHeadsUpHeight, InflationCallback callback, RemoteViews.OnClickHandler remoteViewClickHandler) { + mEntry = entry; mRow = row; - mSbn = notification; mInflateSynchronously = inflateSynchronously; mReInflateFlags = reInflateFlags; - mCachedContentViews = cachedContentViews; + mRemoteViewCache = cache; mContext = mRow.getContext(); mIsLowPriority = isLowPriority; mIsChildInGroup = isChildInGroup; @@ -654,7 +672,6 @@ public class NotificationContentInflater implements NotificationRowContentBinder mUsesIncreasedHeadsUpHeight = usesIncreasedHeadsUpHeight; mRemoteViewClickHandler = remoteViewClickHandler; mCallback = callback; - NotificationEntry entry = row.getEntry(); entry.setInflationTask(this); } @@ -667,12 +684,13 @@ public class NotificationContentInflater implements NotificationRowContentBinder @Override protected InflationProgress doInBackground(Void... params) { try { + final StatusBarNotification sbn = mEntry.getSbn(); final Notification.Builder recoveredBuilder = Notification.Builder.recoverBuilder(mContext, - mSbn.getNotification()); + sbn.getNotification()); - Context packageContext = mSbn.getPackageContext(mContext); - Notification notification = mSbn.getNotification(); + Context packageContext = sbn.getPackageContext(mContext); + Notification notification = sbn.getNotification(); if (notification.isMediaNotification()) { MediaNotificationProcessor processor = new MediaNotificationProcessor(mContext, packageContext); @@ -681,7 +699,7 @@ public class NotificationContentInflater implements NotificationRowContentBinder InflationProgress inflationProgress = createRemoteViews(mReInflateFlags, recoveredBuilder, mIsLowPriority, mIsChildInGroup, mUsesIncreasedHeight, mUsesIncreasedHeadsUpHeight, packageContext); - return inflateSmartReplyViews(inflationProgress, mReInflateFlags, mRow.getEntry(), + return inflateSmartReplyViews(inflationProgress, mReInflateFlags, mEntry, mRow.getContext(), packageContext, mRow.getHeadsUpManager(), mRow.getExistingSmartRepliesAndActions()); } catch (Exception e) { @@ -694,15 +712,15 @@ public class NotificationContentInflater implements NotificationRowContentBinder protected void onPostExecute(InflationProgress result) { if (mError == null) { mCancellationSignal = apply(mInflateSynchronously, result, mReInflateFlags, - mCachedContentViews, mRow, mRemoteViewClickHandler, this); + mRemoteViewCache, mEntry, mRow, mRemoteViewClickHandler, this); } else { handleError(mError); } } private void handleError(Exception e) { - mRow.getEntry().onInflationTaskFinished(); - StatusBarNotification sbn = mRow.getEntry().getSbn(); + mEntry.onInflationTaskFinished(); + StatusBarNotification sbn = mEntry.getSbn(); final String ident = sbn.getPackageName() + "/0x" + Integer.toHexString(sbn.getId()); Log.e(StatusBar.TAG, "couldn't inflate view for notification " + ident, e); @@ -736,10 +754,10 @@ public class NotificationContentInflater implements NotificationRowContentBinder @Override public void onAsyncInflationFinished(NotificationEntry entry, @InflationFlag int inflatedFlags) { - mRow.getEntry().onInflationTaskFinished(); + mEntry.onInflationTaskFinished(); mRow.onNotificationUpdated(); if (mCallback != null) { - mCallback.onAsyncInflationFinished(mRow.getEntry(), inflatedFlags); + mCallback.onAsyncInflationFinished(mEntry, inflatedFlags); } // Notify the resolver that the inflation task has finished, diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationRowModule.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationRowModule.java new file mode 100644 index 0000000000000..df8653cf24060 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationRowModule.java @@ -0,0 +1,44 @@ +/* + * Copyright (C) 2020 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.systemui.statusbar.notification.row; + +import javax.inject.Singleton; + +import dagger.Binds; +import dagger.Module; + +/** + * Dagger Module containing notification row and view inflation implementations. + */ +@Module +public abstract class NotificationRowModule { + /** + * Provides notification row content binder instance. + */ + @Binds + @Singleton + public abstract NotificationRowContentBinder provideNotificationRowContentBinder( + NotificationContentInflater contentBinderImpl); + + /** + * Provides notification remote view cache instance. + */ + @Binds + @Singleton + public abstract NotifRemoteViewCache provideNotifRemoteViewCache( + NotifRemoteViewCacheImpl cacheImpl); +} diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationGroupAlertTransferHelper.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationGroupAlertTransferHelper.java index fe0739f9088c4..896b6e570da2e 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationGroupAlertTransferHelper.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationGroupAlertTransferHelper.java @@ -32,7 +32,6 @@ import com.android.systemui.statusbar.InflationTask; import com.android.systemui.statusbar.notification.NotificationEntryListener; import com.android.systemui.statusbar.notification.NotificationEntryManager; import com.android.systemui.statusbar.notification.collection.NotificationEntry; -import com.android.systemui.statusbar.notification.row.NotificationContentInflater.AsyncInflationTask; import com.android.systemui.statusbar.notification.row.NotificationRowContentBinder.InflationFlag; import com.android.systemui.statusbar.phone.NotificationGroupManager.NotificationGroup; import com.android.systemui.statusbar.phone.NotificationGroupManager.OnGroupChangeListener; @@ -428,7 +427,7 @@ public class NotificationGroupAlertTransferHelper implements OnHeadsUpChangedLis * The notification is still pending inflation but we've decided that we no longer need * the content view (e.g. suppression might have changed and we decided we need to transfer * back). However, there is no way to abort just this inflation if other inflation requests - * have started (see {@link AsyncInflationTask#supersedeTask(InflationTask)}). So instead + * have started (see {@link InflationTask#supersedeTask(InflationTask)}). So instead * we just flag it as aborted and free when it's inflated. */ boolean mAbortOnInflation; diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationTestHelper.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationTestHelper.java index 77659df738c39..fbfd92314e6f7 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationTestHelper.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationTestHelper.java @@ -47,6 +47,7 @@ import com.android.systemui.plugins.statusbar.StatusBarStateController; import com.android.systemui.statusbar.notification.collection.NotificationEntry; import com.android.systemui.statusbar.notification.collection.NotificationEntryBuilder; import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow; +import com.android.systemui.statusbar.notification.row.NotifRemoteViewCache; import com.android.systemui.statusbar.notification.row.NotificationContentInflater; import com.android.systemui.statusbar.notification.row.NotificationRowContentBinder.InflationFlag; import com.android.systemui.statusbar.phone.HeadsUpManagerPhone; @@ -329,6 +330,11 @@ public class NotificationTestHelper { entry.setRow(row); entry.createIcons(mContext, entry.getSbn()); row.setEntry(entry); + NotificationContentInflater contentBinder = new NotificationContentInflater( + mock(NotifRemoteViewCache.class), + mock(NotificationRemoteInputManager.class)); + contentBinder.setInflateSynchronously(true); + row.setContentBinder(contentBinder); row.setInflationFlags(extraInflationFlags); inflateAndWait(row); @@ -341,7 +347,6 @@ public class NotificationTestHelper { private static void inflateAndWait(ExpandableNotificationRow row) throws Exception { CountDownLatch countDownLatch = new CountDownLatch(1); - row.getNotificationInflater().setInflateSynchronously(true); NotificationContentInflater.InflationCallback callback = new NotificationContentInflater.InflationCallback() { @Override diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationEntryManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationEntryManagerTest.java index a60fd52b7c7f2..1e0179dc92f15 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationEntryManagerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationEntryManagerTest.java @@ -84,7 +84,10 @@ import com.android.systemui.statusbar.notification.logging.NotifLog; import com.android.systemui.statusbar.notification.logging.NotificationLogger; import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier; import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow; +import com.android.systemui.statusbar.notification.row.NotifRemoteViewCache; +import com.android.systemui.statusbar.notification.row.NotificationContentInflater; import com.android.systemui.statusbar.notification.row.NotificationGutsManager; +import com.android.systemui.statusbar.notification.row.NotificationRowContentBinder; import com.android.systemui.statusbar.notification.row.RowInflaterTask; import com.android.systemui.statusbar.notification.stack.NotificationListContainer; import com.android.systemui.statusbar.phone.KeyguardBypassController; @@ -200,9 +203,15 @@ public class NotificationEntryManagerTest extends SysuiTestCase { mEntry.expandedIcon = mock(StatusBarIconView.class); + NotificationRowContentBinder contentBinder = new NotificationContentInflater( + mock(NotifRemoteViewCache.class), + mRemoteInputManager); + NotificationRowBinderImpl notificationRowBinder = new NotificationRowBinderImpl(mContext, - mRemoteInputManager, mLockscreenUserManager, + mRemoteInputManager, + mLockscreenUserManager, + contentBinder, true, /* allowLongPress */ mock(KeyguardBypassController.class), mock(StatusBarStateController.class), diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotifRemoteViewCacheImplTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotifRemoteViewCacheImplTest.java new file mode 100644 index 0000000000000..d7214f3b92287 --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotifRemoteViewCacheImplTest.java @@ -0,0 +1,112 @@ +/* + * Copyright (C) 2020 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.systemui.statusbar.notification.row; + +import static com.android.systemui.statusbar.notification.row.NotificationRowContentBinder.FLAG_CONTENT_VIEW_CONTRACTED; +import static com.android.systemui.statusbar.notification.row.NotificationRowContentBinder.FLAG_CONTENT_VIEW_EXPANDED; + +import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertFalse; +import static junit.framework.Assert.assertTrue; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; + +import android.testing.AndroidTestingRunner; +import android.widget.RemoteViews; + +import androidx.test.filters.SmallTest; + +import com.android.systemui.SysuiTestCase; +import com.android.systemui.statusbar.notification.NotificationEntryListener; +import com.android.systemui.statusbar.notification.NotificationEntryManager; +import com.android.systemui.statusbar.notification.collection.NotificationEntry; +import com.android.systemui.statusbar.notification.collection.NotificationEntryBuilder; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +@SmallTest +@RunWith(AndroidTestingRunner.class) +public class NotifRemoteViewCacheImplTest extends SysuiTestCase { + + private NotifRemoteViewCacheImpl mNotifRemoteViewCache; + private NotificationEntry mEntry; + private NotificationEntryListener mEntryListener; + @Mock private RemoteViews mRemoteViews; + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + mEntry = new NotificationEntryBuilder().build(); + + NotificationEntryManager entryManager = mock(NotificationEntryManager.class); + mNotifRemoteViewCache = new NotifRemoteViewCacheImpl(entryManager); + ArgumentCaptor entryListenerCaptor = + ArgumentCaptor.forClass(NotificationEntryListener.class); + verify(entryManager).addNotificationEntryListener(entryListenerCaptor.capture()); + mEntryListener = entryListenerCaptor.getValue(); + } + + @Test + public void testPutCachedView() { + // GIVEN an initialized cache for an entry. + mEntryListener.onPendingEntryAdded(mEntry); + + // WHEN a notification's cached remote views is put in. + mNotifRemoteViewCache.putCachedView(mEntry, FLAG_CONTENT_VIEW_CONTRACTED, mRemoteViews); + + // THEN the remote view is cached. + assertTrue(mNotifRemoteViewCache.hasCachedView(mEntry, FLAG_CONTENT_VIEW_CONTRACTED)); + assertEquals( + "Cached remote view is not the one we put in.", + mRemoteViews, + mNotifRemoteViewCache.getCachedView(mEntry, FLAG_CONTENT_VIEW_CONTRACTED)); + } + + @Test + public void testRemoveCachedView() { + // GIVEN a cache with a cached view. + mEntryListener.onPendingEntryAdded(mEntry); + mNotifRemoteViewCache.putCachedView(mEntry, FLAG_CONTENT_VIEW_CONTRACTED, mRemoteViews); + + // WHEN we remove the cached view. + mNotifRemoteViewCache.removeCachedView(mEntry, FLAG_CONTENT_VIEW_CONTRACTED); + + // THEN the remote view is not cached. + assertFalse(mNotifRemoteViewCache.hasCachedView(mEntry, FLAG_CONTENT_VIEW_CONTRACTED)); + } + + @Test + public void testClearCache() { + // GIVEN a non-empty cache. + mEntryListener.onPendingEntryAdded(mEntry); + mNotifRemoteViewCache.putCachedView(mEntry, FLAG_CONTENT_VIEW_CONTRACTED, mRemoteViews); + mNotifRemoteViewCache.putCachedView(mEntry, FLAG_CONTENT_VIEW_EXPANDED, mRemoteViews); + + // WHEN we clear the cache. + mNotifRemoteViewCache.clearCache(mEntry); + + // THEN the cache is empty. + assertFalse(mNotifRemoteViewCache.hasCachedView(mEntry, FLAG_CONTENT_VIEW_CONTRACTED)); + assertFalse(mNotifRemoteViewCache.hasCachedView(mEntry, FLAG_CONTENT_VIEW_EXPANDED)); + } +} diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationContentInflaterTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationContentInflaterTest.java index f916fe5ad7fb1..cb9da6a40cb95 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationContentInflaterTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationContentInflaterTest.java @@ -22,11 +22,16 @@ import static com.android.systemui.statusbar.notification.row.NotificationRowCon import static com.android.systemui.statusbar.notification.row.NotificationRowContentBinder.FLAG_CONTENT_VIEW_HEADS_UP; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import android.app.Notification; import android.content.Context; @@ -35,16 +40,17 @@ import android.os.Handler; import android.os.Looper; import android.testing.AndroidTestingRunner; import android.testing.TestableLooper.RunWithLooper; -import android.util.ArrayMap; import android.view.View; import android.view.ViewGroup; import android.widget.RemoteViews; +import android.widget.TextView; import androidx.test.filters.SmallTest; import androidx.test.filters.Suppress; import com.android.systemui.SysuiTestCase; import com.android.systemui.statusbar.InflationTask; +import com.android.systemui.statusbar.NotificationRemoteInputManager; import com.android.systemui.statusbar.NotificationTestHelper; import com.android.systemui.statusbar.notification.collection.NotificationEntry; import com.android.systemui.statusbar.notification.row.NotificationRowContentBinder.BindParams; @@ -57,6 +63,8 @@ import org.junit.Before; import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; import java.util.HashMap; import java.util.concurrent.CountDownLatch; @@ -73,8 +81,11 @@ public class NotificationContentInflaterTest extends SysuiTestCase { private Notification.Builder mBuilder; private ExpandableNotificationRow mRow; + @Mock private NotifRemoteViewCache mCache; + @Before public void setUp() throws Exception { + MockitoAnnotations.initMocks(this); mBuilder = new Notification.Builder(mContext).setSmallIcon( R.drawable.ic_person) .setContentTitle("Title") @@ -83,7 +94,9 @@ public class NotificationContentInflaterTest extends SysuiTestCase { ExpandableNotificationRow row = new NotificationTestHelper(mContext, mDependency).createRow( mBuilder.build()); mRow = spy(row); - mNotificationInflater = new NotificationContentInflater(); + mNotificationInflater = new NotificationContentInflater( + mCache, + mock(NotificationRemoteInputManager.class)); } @Test @@ -174,7 +187,9 @@ public class NotificationContentInflaterTest extends SysuiTestCase { result, FLAG_CONTENT_VIEW_EXPANDED, 0, - new ArrayMap() /* cachedContentViews */, mRow, + mock(NotifRemoteViewCache.class), + mRow.getEntry(), + mRow, true /* isNewView */, (v, p, r) -> true, new InflationCallback() { @Override @@ -244,6 +259,71 @@ public class NotificationContentInflaterTest extends SysuiTestCase { NotificationContentInflater.canReapplyRemoteView(mediaView, decoratedMediaView)); } + @Test + public void testUsesSameViewWhenCachedPossibleToReuse() throws Exception { + // GIVEN a cached view. + RemoteViews contractedRemoteView = mBuilder.createContentView(); + when(mCache.hasCachedView(mRow.getEntry(), FLAG_CONTENT_VIEW_CONTRACTED)) + .thenReturn(true); + when(mCache.getCachedView(mRow.getEntry(), FLAG_CONTENT_VIEW_CONTRACTED)) + .thenReturn(contractedRemoteView); + + // GIVEN existing bound view with same layout id. + View view = contractedRemoteView.apply(mContext, null /* parent */); + mRow.getPrivateLayout().setContractedChild(view); + + // WHEN inflater inflates + inflateAndWait(mNotificationInflater, FLAG_CONTENT_VIEW_CONTRACTED, mRow); + + // THEN the view should be re-used + assertEquals("Binder inflated a new view even though the old one was cached and usable.", + view, mRow.getPrivateLayout().getContractedChild()); + } + + @Test + public void testInflatesNewViewWhenCachedNotPossibleToReuse() throws Exception { + // GIVEN a cached remote view. + RemoteViews contractedRemoteView = mBuilder.createHeadsUpContentView(); + when(mCache.hasCachedView(mRow.getEntry(), FLAG_CONTENT_VIEW_CONTRACTED)) + .thenReturn(true); + when(mCache.getCachedView(mRow.getEntry(), FLAG_CONTENT_VIEW_CONTRACTED)) + .thenReturn(contractedRemoteView); + + // GIVEN existing bound view with different layout id. + View view = new TextView(mContext); + mRow.getPrivateLayout().setContractedChild(view); + + // WHEN inflater inflates + inflateAndWait(mNotificationInflater, FLAG_CONTENT_VIEW_CONTRACTED, mRow); + + // THEN the view should be a new view + assertNotEquals("Binder (somehow) used the same view when inflating.", + view, mRow.getPrivateLayout().getContractedChild()); + } + + @Test + public void testInflationCachesCreatedRemoteView() throws Exception { + // WHEN inflater inflates + inflateAndWait(mNotificationInflater, FLAG_CONTENT_VIEW_CONTRACTED, mRow); + + // THEN inflater informs cache of the new remote view + verify(mCache).putCachedView( + eq(mRow.getEntry()), + eq(FLAG_CONTENT_VIEW_CONTRACTED), + any()); + } + + @Test + public void testUnbindRemovesCachedRemoteView() { + // WHEN inflated unbinds content + mNotificationInflater.unbindContent(mRow.getEntry(), mRow, FLAG_CONTENT_VIEW_HEADS_UP); + + // THEN inflated informs cache to remove remote view + verify(mCache).removeCachedView( + eq(mRow.getEntry()), + eq(FLAG_CONTENT_VIEW_HEADS_UP)); + } + private static void inflateAndWait(NotificationContentInflater inflater, @InflationFlag int contentToInflate, ExpandableNotificationRow row) From 5182c9f5d734205b1d266306d8bdc1a24953bc8c Mon Sep 17 00:00:00 2001 From: Kevin Han Date: Tue, 7 Jan 2020 17:54:43 -0800 Subject: [PATCH 2/2] Move a bunch of row setters into an init method. We have a bunch of setters for stuff that is only initialized once on a row. So let's just go ahead and move these into an initialize method so it's more readable and harder to mis-use. There are a few others that are used in tests that should probably be moved into this method or just moved out of ExpandableNotificationRow entirely. Test: builds Test: atest SystemUITests Change-Id: Ifa55ca19fa2fd1f2030caa0676e6aa3e1713f134 --- .../collection/NotificationRowBinderImpl.java | 38 ++++++------ .../row/ExpandableNotificationRow.java | 59 ++++++++----------- .../statusbar/NotificationTestHelper.java | 20 +++++-- 3 files changed, 60 insertions(+), 57 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationRowBinderImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationRowBinderImpl.java index d75362c8e5ed2..80b5b8af28ac7 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationRowBinderImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationRowBinderImpl.java @@ -166,20 +166,6 @@ public class NotificationRowBinderImpl implements NotificationRowBinder { private void bindRow(NotificationEntry entry, PackageManager pmUser, StatusBarNotification sbn, ExpandableNotificationRow row, Runnable onDismissRunnable) { - row.setExpansionLogger(mExpansionLogger, entry.getSbn().getKey()); - row.setBypassController(mKeyguardBypassController); - row.setStatusBarStateController(mStatusBarStateController); - row.setGroupManager(mGroupManager); - row.setHeadsUpManager(mHeadsUpManager); - row.setOnExpandClickListener(mPresenter); - row.setContentBinder(mRowContentBinder); - row.setInflationCallback(mInflationCallback); - if (mAllowLongPress) { - row.setLongPressListener(mGutsManager::openGuts); - } - mListContainer.bindRow(row); - mNotificationRemoteInputManager.bindRow(row); - // Get the app name. // Note that Notification.Builder#bindHeaderAppName has similar logic // but since this field is used in the guts, it must be accurate. @@ -197,15 +183,33 @@ public class NotificationRowBinderImpl implements NotificationRowBinder { } catch (PackageManager.NameNotFoundException e) { // Do nothing } - row.setAppName(appname); + + row.initialize( + appname, + sbn.getKey(), + mExpansionLogger, + mKeyguardBypassController, + mGroupManager, + mHeadsUpManager, + mRowContentBinder, + mPresenter); + + // TODO: Either move these into ExpandableNotificationRow#initialize or out of row entirely + row.setStatusBarStateController(mStatusBarStateController); + row.setInflationCallback(mInflationCallback); + row.setAppOpsOnClickListener(mOnAppOpsClickListener); + if (mAllowLongPress) { + row.setLongPressListener(mGutsManager::openGuts); + } + mListContainer.bindRow(row); + mNotificationRemoteInputManager.bindRow(row); + row.setOnDismissRunnable(onDismissRunnable); row.setDescendantFocusability(ViewGroup.FOCUS_BLOCK_DESCENDANTS); if (ENABLE_REMOTE_INPUT) { row.setDescendantFocusability(ViewGroup.FOCUS_BEFORE_DESCENDANTS); } - row.setAppOpsOnClickListener(mOnAppOpsClickListener); - mBindRowCallback.onBindRow(entry, pmUser, sbn, row); } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRow.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRow.java index 50f21990eaeb8..a8a35d07b3f09 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRow.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRow.java @@ -741,23 +741,10 @@ public class ExpandableNotificationRow extends ActivatableNotificationView return mIsHeadsUp || mHeadsupDisappearRunning; } - - public void setGroupManager(NotificationGroupManager groupManager) { - mGroupManager = groupManager; - mPrivateLayout.setGroupManager(groupManager); - } - public void setRemoteInputController(RemoteInputController r) { mPrivateLayout.setRemoteInputController(r); } - public void setAppName(String appName) { - mAppName = appName; - if (mMenuRow != null && mMenuRow.getMenuView() != null) { - mMenuRow.setAppName(mAppName); - } - } - public void addChildNotification(ExpandableNotificationRow row) { addChildNotification(row, -1); } @@ -1104,10 +1091,6 @@ public class ExpandableNotificationRow extends ActivatableNotificationView return mPrivateLayout.getContractedNotificationHeader(); } - public void setOnExpandClickListener(OnExpandClickListener onExpandClickListener) { - mOnExpandClickListener = onExpandClickListener; - } - public void setLongPressListener(LongPressListener longPressListener) { mLongPressListener = longPressListener; } @@ -1130,10 +1113,6 @@ public class ExpandableNotificationRow extends ActivatableNotificationView } } - public void setHeadsUpManager(HeadsUpManager headsUpManager) { - mHeadsUpManager = headsUpManager; - } - public HeadsUpManager getHeadsUpManager() { return mHeadsUpManager; } @@ -1656,15 +1635,6 @@ public class ExpandableNotificationRow extends ActivatableNotificationView } } - /** - * Set the binder implementation for notification content views. - * - * @param contentBinder content binder - */ - public void setContentBinder(NotificationRowContentBinder contentBinder) { - mNotificationContentBinder = contentBinder; - } - public interface ExpansionLogger { void logNotificationExpansion(String key, boolean userAction, boolean expanded); } @@ -1678,8 +1648,30 @@ public class ExpandableNotificationRow extends ActivatableNotificationView initDimens(); } - public void setBypassController(KeyguardBypassController bypassController) { + /** + * Initialize row. + */ + public void initialize( + String appName, + String notificationKey, + ExpansionLogger logger, + KeyguardBypassController bypassController, + NotificationGroupManager groupManager, + HeadsUpManager headsUpManager, + NotificationRowContentBinder rowContentBinder, + OnExpandClickListener onExpandClickListener) { + mAppName = appName; + if (mMenuRow != null && mMenuRow.getMenuView() != null) { + mMenuRow.setAppName(mAppName); + } + mLogger = logger; + mLoggingKey = notificationKey; mBypassController = bypassController; + mGroupManager = groupManager; + mPrivateLayout.setGroupManager(groupManager); + mHeadsUpManager = headsUpManager; + mNotificationContentBinder = rowContentBinder; + mOnExpandClickListener = onExpandClickListener; } public void setStatusBarStateController(StatusBarStateController statusBarStateController) { @@ -2918,11 +2910,6 @@ public class ExpandableNotificationRow extends ActivatableNotificationView return 0; } - public void setExpansionLogger(ExpansionLogger logger, String key) { - mLogger = logger; - mLoggingKey = key; - } - public void onExpandedByGesture(boolean userExpanded) { int event = MetricsEvent.ACTION_NOTIFICATION_GESTURE_EXPANDER; if (mGroupManager.isSummaryOfGroup(mEntry.getSbn())) { diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationTestHelper.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationTestHelper.java index fbfd92314e6f7..3fdbd3edfcaa1 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationTestHelper.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationTestHelper.java @@ -47,6 +47,8 @@ import com.android.systemui.plugins.statusbar.StatusBarStateController; import com.android.systemui.statusbar.notification.collection.NotificationEntry; import com.android.systemui.statusbar.notification.collection.NotificationEntryBuilder; import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow; +import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow.ExpansionLogger; +import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow.OnExpandClickListener; import com.android.systemui.statusbar.notification.row.NotifRemoteViewCache; import com.android.systemui.statusbar.notification.row.NotificationContentInflater; import com.android.systemui.statusbar.notification.row.NotificationRowContentBinder.InflationFlag; @@ -73,6 +75,7 @@ public class NotificationTestHelper { public static final UserHandle USER_HANDLE = UserHandle.of(ActivityManager.getCurrentUser()); private static final String GROUP_KEY = "gruKey"; + private static final String APP_NAME = "appName"; private final Context mContext; private int mId; @@ -304,9 +307,6 @@ public class NotificationTestHelper { null /* root */, false /* attachToRoot */); ExpandableNotificationRow row = mRow; - row.setGroupManager(mGroupManager); - row.setHeadsUpManager(mHeadsUpManager); - row.setAboveShelfChangedListener(aboveShelf -> {}); final NotificationChannel channel = new NotificationChannel( @@ -330,11 +330,23 @@ public class NotificationTestHelper { entry.setRow(row); entry.createIcons(mContext, entry.getSbn()); row.setEntry(entry); + NotificationContentInflater contentBinder = new NotificationContentInflater( mock(NotifRemoteViewCache.class), mock(NotificationRemoteInputManager.class)); contentBinder.setInflateSynchronously(true); - row.setContentBinder(contentBinder); + + row.initialize( + APP_NAME, + entry.getKey(), + mock(ExpansionLogger.class), + mock(KeyguardBypassController.class), + mGroupManager, + mHeadsUpManager, + contentBinder, + mock(OnExpandClickListener.class)); + row.setAboveShelfChangedListener(aboveShelf -> { }); + row.setInflationFlags(extraInflationFlags); inflateAndWait(row);