From 5759f87bf6f4a25e67cc86f0404834e26faec8a3 Mon Sep 17 00:00:00 2001 From: Gustav Sennton Date: Wed, 13 Feb 2019 17:25:26 +0000 Subject: Move SmartReplyView inflation off the UI thread. To avoid blocking the UI thread we here move the inflation of SmartReplyView and its smart suggestion buttons off the UI thread. We move these inflations into NotificationContentInflater where they are performed before the inflations of other notification content views - reusing the existing AsyncTask to run these inflations on a background thread. More specifically: for both the expanded state and the heads-up state of a notification we 1. inflate a SmartReplyView, 2. inflate the buttons within that view (reply buttons and action buttons), and 3. create a SmartRepliesAndActions object from the AsyncTask (on a background thread). We then pass these objects back to the UI thread where we attach them to the view hierarchy or use them to make certain decisions. In terms of thread-safety the background thread reads constants from SmartReplyConstants so that class must be thread-safe (at least so that the background thread doesn't store a local copy of the constants). We also pass a SmartReplyController reference on the background thread, that reference is only used on the UI thread (in onClick listeners). Bug:119801785 Test: post message notification containing smart replies and actions. Post both heads-up and notification shade notifications. Click on replies and actions. Ensure the smart-buttons are updated when a notification is updated. Change-Id: I4698a6895cf65c46461cc429b2b6eb5905acb629 --- .../row/NotificationContentInflater.java | 49 ++++++++++- .../notification/row/NotificationContentView.java | 95 ++++++++++++++++------ .../statusbar/policy/InflatedSmartReplies.java | 61 +++++++++++++- .../systemui/statusbar/policy/SmartReplyView.java | 90 ++++++++++++-------- .../statusbar/policy/SmartReplyViewTest.java | 28 ++++--- 5 files changed, 251 insertions(+), 72 deletions(-) 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 b65c4a5f71d8..b4dd1144e761 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 @@ -34,12 +34,17 @@ import android.widget.RemoteViews; 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.SmartReplyController; import com.android.systemui.statusbar.notification.InflationException; import com.android.systemui.statusbar.notification.MediaNotificationProcessor; import com.android.systemui.statusbar.notification.collection.NotificationEntry; import com.android.systemui.statusbar.notification.row.wrapper.NotificationViewWrapper; import com.android.systemui.statusbar.phone.StatusBar; +import com.android.systemui.statusbar.policy.HeadsUpManager; +import com.android.systemui.statusbar.policy.InflatedSmartReplies; +import com.android.systemui.statusbar.policy.SmartReplyConstants; import com.android.systemui.util.Assert; import java.lang.annotation.Retention; @@ -278,6 +283,8 @@ public class NotificationContentInflater { InflationProgress result = createRemoteViews(reInflateFlags, builder, mIsLowPriority, mIsChildInGroup, mUsesIncreasedHeight, mUsesIncreasedHeadsUpHeight, mRedactAmbient, packageContext); + result = inflateSmartReplyViews(result, reInflateFlags, mRow.getEntry(), + mRow.getContext(), mRow.getHeadsUpManager()); apply( inflateSynchronously, result, @@ -306,6 +313,7 @@ public class NotificationContentInflater { if (mRow.getPrivateLayout().isContentViewInactive(VISIBLE_TYPE_HEADSUP)) { mRow.getPrivateLayout().setHeadsUpChild(null); mCachedContentViews.remove(FLAG_CONTENT_VIEW_HEADS_UP); + mRow.getPrivateLayout().setHeadsUpInflatedSmartReplies(null); } break; case FLAG_CONTENT_VIEW_AMBIENT: @@ -336,12 +344,33 @@ public class NotificationContentInflater { } } + private static InflationProgress inflateSmartReplyViews(InflationProgress result, + @InflationFlag int reInflateFlags, NotificationEntry entry, Context context, + HeadsUpManager headsUpManager) { + SmartReplyConstants smartReplyConstants = Dependency.get(SmartReplyConstants.class); + SmartReplyController smartReplyController = Dependency.get(SmartReplyController.class); + if ((reInflateFlags & FLAG_CONTENT_VIEW_EXPANDED) != 0 && result.newExpandedView != null) { + result.expandedInflatedSmartReplies = + InflatedSmartReplies.inflate( + context, entry, smartReplyConstants, smartReplyController, + headsUpManager); + } + if ((reInflateFlags & FLAG_CONTENT_VIEW_HEADS_UP) != 0 && result.newHeadsUpView != null) { + result.headsUpInflatedSmartReplies = + InflatedSmartReplies.inflate( + context, entry, smartReplyConstants, smartReplyController, + headsUpManager); + } + return result; + } + private static InflationProgress createRemoteViews(@InflationFlag int reInflateFlags, Notification.Builder builder, boolean isLowPriority, boolean isChildInGroup, boolean usesIncreasedHeight, boolean usesIncreasedHeadsUpHeight, boolean redactAmbient, Context packageContext) { InflationProgress result = new InflationProgress(); isLowPriority = isLowPriority && !isChildInGroup; + if ((reInflateFlags & FLAG_CONTENT_VIEW_CONTRACTED) != 0) { result.newContentView = createContentView(builder, isLowPriority, usesIncreasedHeight); } @@ -661,6 +690,12 @@ public class NotificationContentInflater { } else if (result.newExpandedView == null) { privateLayout.setExpandedChild(null); } + if (result.newExpandedView != null) { + privateLayout.setExpandedInflatedSmartReplies( + result.expandedInflatedSmartReplies); + } else { + privateLayout.setExpandedInflatedSmartReplies(null); + } cachedContentViews.put(FLAG_CONTENT_VIEW_EXPANDED, result.newExpandedView); row.setExpandable(result.newExpandedView != null); } @@ -671,6 +706,12 @@ public class NotificationContentInflater { } else if (result.newHeadsUpView == null) { privateLayout.setHeadsUpChild(null); } + if (result.newHeadsUpView != null) { + privateLayout.setHeadsUpInflatedSmartReplies( + result.headsUpInflatedSmartReplies); + } else { + privateLayout.setHeadsUpInflatedSmartReplies(null); + } cachedContentViews.put(FLAG_CONTENT_VIEW_HEADS_UP, result.newHeadsUpView); } @@ -846,9 +887,12 @@ public class NotificationContentInflater { packageContext); processor.processNotification(notification, recoveredBuilder); } - return createRemoteViews(mReInflateFlags, recoveredBuilder, mIsLowPriority, + InflationProgress inflationProgress = createRemoteViews(mReInflateFlags, + recoveredBuilder, mIsLowPriority, mIsChildInGroup, mUsesIncreasedHeight, mUsesIncreasedHeadsUpHeight, mRedactAmbient, packageContext); + return inflateSmartReplyViews(inflationProgress, mReInflateFlags, mRow.getEntry(), + mRow.getContext(), mRow.getHeadsUpManager()); } catch (Exception e) { mError = e; return null; @@ -927,6 +971,9 @@ public class NotificationContentInflater { private View inflatedPublicView; private CharSequence headsUpStatusBarText; private CharSequence headsUpStatusBarTextPublic; + + private InflatedSmartReplies expandedInflatedSmartReplies; + private InflatedSmartReplies headsUpInflatedSmartReplies; } @VisibleForTesting diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationContentView.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationContentView.java index dae14debd683..646617c72128 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationContentView.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationContentView.java @@ -93,6 +93,8 @@ public class NotificationContentView extends FrameLayout { private SmartReplyView mExpandedSmartReplyView; private SmartReplyView mHeadsUpSmartReplyView; private SmartReplyController mSmartReplyController; + private InflatedSmartReplies mExpandedInflatedSmartReplies; + private InflatedSmartReplies mHeadsUpInflatedSmartReplies; private NotificationViewWrapper mContractedWrapper; private NotificationViewWrapper mExpandedWrapper; @@ -1316,8 +1318,22 @@ public class NotificationContentView extends FrameLayout { return; } - SmartRepliesAndActions smartRepliesAndActions = - InflatedSmartReplies.chooseSmartRepliesAndActions(mSmartReplyConstants, entry); + applyRemoteInput(entry, InflatedSmartReplies.hasFreeformRemoteInput(entry)); + + if (mExpandedInflatedSmartReplies == null && mHeadsUpInflatedSmartReplies == null) { + if (DEBUG) { + Log.d(TAG, "Both expanded, and heads-up InflatedSmartReplies are null, " + + "don't add smart replies."); + } + return; + } + // The inflated smart-reply objects for the expanded view and the heads-up view both contain + // the same SmartRepliesAndActions to avoid discrepancies between the two views. We here + // reuse that object for our local SmartRepliesAndActions to avoid discrepancies between + // this class and the InflatedSmartReplies classes. + SmartRepliesAndActions smartRepliesAndActions = mExpandedInflatedSmartReplies != null + ? mExpandedInflatedSmartReplies.getSmartRepliesAndActions() + : mHeadsUpInflatedSmartReplies.getSmartRepliesAndActions(); if (DEBUG) { Log.d(TAG, String.format("Adding suggestions for %s, %d actions, and %d replies.", entry.notification.getKey(), @@ -1326,7 +1342,6 @@ public class NotificationContentView extends FrameLayout { smartRepliesAndActions.smartReplies == null ? 0 : smartRepliesAndActions.smartReplies.choices.length)); } - applyRemoteInput(entry, InflatedSmartReplies.hasFreeformRemoteInput(entry)); applySmartReplyView(smartRepliesAndActions, entry); } @@ -1429,11 +1444,12 @@ public class NotificationContentView extends FrameLayout { return null; } - private void applySmartReplyView(SmartRepliesAndActions smartRepliesAndActions, + private void applySmartReplyView( + SmartRepliesAndActions smartRepliesAndActions, NotificationEntry entry) { if (mExpandedChild != null) { - mExpandedSmartReplyView = - applySmartReplyView(mExpandedChild, smartRepliesAndActions, entry); + mExpandedSmartReplyView = applySmartReplyView(mExpandedChild, smartRepliesAndActions, + entry, mExpandedInflatedSmartReplies); if (mExpandedSmartReplyView != null) { if (smartRepliesAndActions.smartReplies != null || smartRepliesAndActions.smartActions != null) { @@ -1455,18 +1471,21 @@ public class NotificationContentView extends FrameLayout { } } if (mHeadsUpChild != null && mSmartReplyConstants.getShowInHeadsUp()) { - mHeadsUpSmartReplyView = - applySmartReplyView(mHeadsUpChild, smartRepliesAndActions, entry); + mHeadsUpSmartReplyView = applySmartReplyView(mHeadsUpChild, smartRepliesAndActions, + entry, mHeadsUpInflatedSmartReplies); } } + @Nullable private SmartReplyView applySmartReplyView(View view, - SmartRepliesAndActions smartRepliesAndActions, NotificationEntry entry) { + SmartRepliesAndActions smartRepliesAndActions, + NotificationEntry entry, InflatedSmartReplies inflatedSmartReplyView) { View smartReplyContainerCandidate = view.findViewById( com.android.internal.R.id.smart_reply_container); if (!(smartReplyContainerCandidate instanceof LinearLayout)) { return null; } + LinearLayout smartReplyContainer = (LinearLayout) smartReplyContainerCandidate; if (!InflatedSmartReplies.shouldShowSmartReplyView(entry, smartRepliesAndActions)) { smartReplyContainer.setVisibility(View.GONE); @@ -1474,31 +1493,57 @@ public class NotificationContentView extends FrameLayout { } SmartReplyView smartReplyView = null; - if (smartReplyContainer.getChildCount() == 0) { - smartReplyView = SmartReplyView.inflate(mContext, smartReplyContainer); + if (smartReplyContainer.getChildCount() == 1 + && smartReplyContainer.getChildAt(0) instanceof SmartReplyView) { + // If we already have a SmartReplyView - replace it with the newly inflated one. The + // newly inflated one is connected to the new inflated smart reply/action buttons. + smartReplyContainer.removeAllViews(); + } + if (smartReplyContainer.getChildCount() == 0 + && inflatedSmartReplyView != null + && inflatedSmartReplyView.getSmartReplyView() != null) { + smartReplyView = inflatedSmartReplyView.getSmartReplyView(); smartReplyContainer.addView(smartReplyView); - } else if (smartReplyContainer.getChildCount() == 1) { - View child = smartReplyContainer.getChildAt(0); - if (child instanceof SmartReplyView) { - smartReplyView = (SmartReplyView) child; - } } if (smartReplyView != null) { smartReplyView.resetSmartSuggestions(smartReplyContainer); - if (smartRepliesAndActions.smartReplies != null) { - smartReplyView.addRepliesFromRemoteInput( - smartRepliesAndActions.smartReplies, mSmartReplyController, entry); - } - if (smartRepliesAndActions.smartActions != null) { - smartReplyView.addSmartActions( - smartRepliesAndActions.smartActions, mSmartReplyController, entry, - mContainingNotification.getHeadsUpManager()); - } + smartReplyView.addPreInflatedButtons( + inflatedSmartReplyView.getSmartSuggestionButtons()); smartReplyContainer.setVisibility(View.VISIBLE); } return smartReplyView; } + /** + * Set pre-inflated views necessary to display smart replies and actions in the expanded + * notification state. + * + * @param inflatedSmartReplies the pre-inflated state to add to this view. If null the existing + * {@link SmartReplyView} related to the expanded notification state is cleared. + */ + public void setExpandedInflatedSmartReplies( + @Nullable InflatedSmartReplies inflatedSmartReplies) { + mExpandedInflatedSmartReplies = inflatedSmartReplies; + if (inflatedSmartReplies == null) { + mExpandedSmartReplyView = null; + } + } + + /** + * Set pre-inflated views necessary to display smart replies and actions in the heads-up + * notification state. + * + * @param inflatedSmartReplies the pre-inflated state to add to this view. If null the existing + * {@link SmartReplyView} related to the heads-up notification state is cleared. + */ + public void setHeadsUpInflatedSmartReplies( + @Nullable InflatedSmartReplies inflatedSmartReplies) { + mHeadsUpInflatedSmartReplies = inflatedSmartReplies; + if (inflatedSmartReplies == null) { + mHeadsUpSmartReplyView = null; + } + } + public void closeRemoteInput() { if (mHeadsUpRemoteInput != null) { mHeadsUpRemoteInput.close(); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/policy/InflatedSmartReplies.java b/packages/SystemUI/src/com/android/systemui/statusbar/policy/InflatedSmartReplies.java index 754bfb2b30aa..d8ea1f6eef5f 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/policy/InflatedSmartReplies.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/policy/InflatedSmartReplies.java @@ -20,13 +20,17 @@ import android.annotation.NonNull; import android.annotation.Nullable; import android.app.Notification; import android.app.RemoteInput; +import android.content.Context; import android.os.Build; import android.util.Log; import android.util.Pair; +import android.widget.Button; import com.android.internal.util.ArrayUtils; +import com.android.systemui.statusbar.SmartReplyController; import com.android.systemui.statusbar.notification.collection.NotificationEntry; +import java.util.ArrayList; import java.util.List; /** @@ -36,8 +40,63 @@ import java.util.List; public class InflatedSmartReplies { private static final String TAG = "InflatedSmartReplies"; private static final boolean DEBUG = Log.isLoggable(TAG, Log.DEBUG); + @Nullable private final SmartReplyView mSmartReplyView; + @Nullable private final List