Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 5300f144 authored by Kevin Han's avatar Kevin Han
Browse files

Change free API to free when the content is safe

Rather than have the NotifBindPipeline API free immediately and have the
caller know when it is safe to free, the default behavior is now to
allow the callers to "free" immediately but not actually free until the
view is safe to remove. This aligns more closely with how the API is
actually used.

This does mean that rather than immediately free, we're really just
marking the view for freeing / putting it in a transient state. We
update the documentation to reflect this and deprecate
row.freeContentViewWhenSafe (full removal will happen in further CLs)

Bug: 150719232
Test: atest SystemUITests
Test: heap dump for group of 25 children, views are properly freed
Test: Post heads up notification, see that freeing happens after
animation
Change-Id: Icebbda1519dc00fd1f90611af30b1516d1307f3d

Change-Id: I0c841bec463329801855dfccdb2cd1551e7a959e
parent 88c870e6
Loading
Loading
Loading
Loading
+2 −2
Original line number Diff line number Diff line
@@ -95,8 +95,8 @@ public class DynamicChildBindController {

    private void freeChildContent(NotificationEntry entry) {
        RowContentBindParams params = mStage.getStageParams(entry);
        params.freeContentViews(FLAG_CONTENT_VIEW_CONTRACTED);
        params.freeContentViews(FLAG_CONTENT_VIEW_EXPANDED);
        params.markContentViewsFreeable(FLAG_CONTENT_VIEW_CONTRACTED);
        params.markContentViewsFreeable(FLAG_CONTENT_VIEW_EXPANDED);
        mStage.requestRebind(entry, null);
    }

+8 −38
Original line number Diff line number Diff line
@@ -17,12 +17,7 @@
package com.android.systemui.statusbar.notification.row;

import static com.android.systemui.statusbar.notification.ActivityLaunchAnimator.ExpandAnimationParameters;
import static com.android.systemui.statusbar.notification.row.NotificationContentView.VISIBLE_TYPE_CONTRACTED;
import static com.android.systemui.statusbar.notification.row.NotificationContentView.VISIBLE_TYPE_EXPANDED;
import static com.android.systemui.statusbar.notification.row.NotificationContentView.VISIBLE_TYPE_HEADSUP;
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 com.android.systemui.statusbar.notification.row.NotificationRowContentBinder.FLAG_CONTENT_VIEW_HEADS_UP;
import static com.android.systemui.statusbar.notification.row.NotificationRowContentBinder.FLAG_CONTENT_VIEW_PUBLIC;

import android.animation.Animator;
@@ -451,42 +446,17 @@ public class ExpandableNotificationRow extends ActivatableNotificationView
     * Marks a content view as freeable, setting it so that future inflations do not reinflate
     * and ensuring that the view is freed when it is safe to remove.
     *
     * TODO: This should be moved to the respective coordinator and call
     * {@link RowContentBindParams#freeContentViews} directly after disappear animation
     * finishes instead of depending on binding API to know when it's "safe".
     *
     * @param inflationFlag flag corresponding to the content view to be freed
     * @deprecated By default, {@link NotificationContentInflater#unbindContent} will tell the
     * view hierarchy to only free when the view is safe to remove so this method is no longer
     * needed. Will remove when all uses are gone.
     */
    @Deprecated
    public void freeContentViewWhenSafe(@InflationFlag int inflationFlag) {
        // View should not be reinflated in the future
        Runnable freeViewRunnable = () -> {
            // Possible for notification to be removed after free request.
            if (!isRemoved()) {
        RowContentBindParams params = mRowContentBindStage.getStageParams(mEntry);
                params.freeContentViews(inflationFlag);
        params.markContentViewsFreeable(inflationFlag);
        mRowContentBindStage.requestRebind(mEntry, null /* callback */);
    }
        };
        switch (inflationFlag) {
            case FLAG_CONTENT_VIEW_CONTRACTED:
                getPrivateLayout().performWhenContentInactive(VISIBLE_TYPE_CONTRACTED,
                        freeViewRunnable);
                break;
            case FLAG_CONTENT_VIEW_EXPANDED:
                getPrivateLayout().performWhenContentInactive(VISIBLE_TYPE_EXPANDED,
                        freeViewRunnable);
                break;
            case FLAG_CONTENT_VIEW_HEADS_UP:
                getPrivateLayout().performWhenContentInactive(VISIBLE_TYPE_HEADSUP,
                        freeViewRunnable);
                break;
            case FLAG_CONTENT_VIEW_PUBLIC:
                getPublicLayout().performWhenContentInactive(VISIBLE_TYPE_CONTRACTED,
                        freeViewRunnable);
            default:
                break;
        }
    }

    /**
     * Caches whether or not this row contains a system notification. Note, this is only cached
@@ -1559,7 +1529,7 @@ public class ExpandableNotificationRow extends ActivatableNotificationView
                if (needsRedaction) {
                    params.requireContentViews(FLAG_CONTENT_VIEW_PUBLIC);
                } else {
                    params.freeContentViews(FLAG_CONTENT_VIEW_PUBLIC);
                    params.markContentViewsFreeable(FLAG_CONTENT_VIEW_PUBLIC);
                }
                mRowContentBindStage.requestRebind(mEntry, null /* callback */);
            }
+39 −11
Original line number Diff line number Diff line
@@ -118,6 +118,9 @@ public class NotificationContentInflater implements NotificationRowContentBinder
            mRemoteViewCache.clearCache(entry);
        }

        // Cancel any pending frees on any view we're trying to bind since we should be bound after.
        cancelContentViewFrees(row, contentToBind);

        AsyncInflationTask task = new AsyncInflationTask(
                mBgExecutor,
                mInflateSynchronously,
@@ -198,44 +201,69 @@ public class NotificationContentInflater implements NotificationRowContentBinder
    }

    /**
     * Frees the content view associated with the inflation flag.  Will only succeed if the
     * view is safe to remove.
     * Frees the content view associated with the inflation flag as soon as the view is not showing.
     *
     * @param inflateFlag the flag corresponding to the content view which should be freed
     */
    private void freeNotificationView(NotificationEntry entry, ExpandableNotificationRow row,
    private void freeNotificationView(
            NotificationEntry entry,
            ExpandableNotificationRow row,
            @InflationFlag int inflateFlag) {
        switch (inflateFlag) {
            case FLAG_CONTENT_VIEW_CONTRACTED:
                if (row.getPrivateLayout().isContentViewInactive(VISIBLE_TYPE_CONTRACTED)) {
                row.getPrivateLayout().performWhenContentInactive(VISIBLE_TYPE_CONTRACTED, () -> {
                    row.getPrivateLayout().setContractedChild(null);
                    mRemoteViewCache.removeCachedView(entry, FLAG_CONTENT_VIEW_CONTRACTED);
                }
                });
                break;
            case FLAG_CONTENT_VIEW_EXPANDED:
                if (row.getPrivateLayout().isContentViewInactive(VISIBLE_TYPE_EXPANDED)) {
                row.getPrivateLayout().performWhenContentInactive(VISIBLE_TYPE_EXPANDED, () -> {
                    row.getPrivateLayout().setExpandedChild(null);
                    mRemoteViewCache.removeCachedView(entry, FLAG_CONTENT_VIEW_EXPANDED);
                }
                });
                break;
            case FLAG_CONTENT_VIEW_HEADS_UP:
                if (row.getPrivateLayout().isContentViewInactive(VISIBLE_TYPE_HEADSUP)) {
                row.getPrivateLayout().performWhenContentInactive(VISIBLE_TYPE_HEADSUP, () -> {
                    row.getPrivateLayout().setHeadsUpChild(null);
                    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().performWhenContentInactive(VISIBLE_TYPE_CONTRACTED, () -> {
                    row.getPublicLayout().setContractedChild(null);
                    mRemoteViewCache.removeCachedView(entry, FLAG_CONTENT_VIEW_PUBLIC);
                }
                });
                break;
            default:
                break;
        }
    }

    /**
     * Cancel any pending content view frees from {@link #freeNotificationView} for the provided
     * content views.
     *
     * @param row top level notification row containing the content views
     * @param contentViews content views to cancel pending frees on
     */
    private void cancelContentViewFrees(
            ExpandableNotificationRow row,
            @InflationFlag int contentViews) {
        if ((contentViews & FLAG_CONTENT_VIEW_CONTRACTED) != 0) {
            row.getPrivateLayout().removeContentInactiveRunnable(VISIBLE_TYPE_CONTRACTED);
        }
        if ((contentViews & FLAG_CONTENT_VIEW_EXPANDED) != 0) {
            row.getPrivateLayout().removeContentInactiveRunnable(VISIBLE_TYPE_EXPANDED);
        }
        if ((contentViews & FLAG_CONTENT_VIEW_HEADS_UP) != 0) {
            row.getPrivateLayout().removeContentInactiveRunnable(VISIBLE_TYPE_HEADSUP);
        }
        if ((contentViews & FLAG_CONTENT_VIEW_PUBLIC) != 0) {
            row.getPublicLayout().removeContentInactiveRunnable(VISIBLE_TYPE_CONTRACTED);
        }
    }

    private static InflationProgress inflateSmartReplyViews(InflationProgress result,
            @InflationFlag int reInflateFlags, NotificationEntry entry, Context context,
            Context packageContext, HeadsUpManager headsUpManager,
+20 −2
Original line number Diff line number Diff line
@@ -385,6 +385,7 @@ public class NotificationContentView extends FrameLayout {
     */
    public void setContractedChild(@Nullable View child) {
        if (mContractedChild != null) {
            mOnContentViewInactiveListeners.remove(mContractedChild);
            mContractedChild.animate().cancel();
            removeView(mContractedChild);
        }
@@ -432,6 +433,7 @@ public class NotificationContentView extends FrameLayout {
                    ((ViewGroup)mExpandedRemoteInput.getParent()).removeView(mExpandedRemoteInput);
                }
            }
            mOnContentViewInactiveListeners.remove(mExpandedChild);
            mExpandedChild.animate().cancel();
            removeView(mExpandedChild);
            mExpandedRemoteInput = null;
@@ -470,6 +472,7 @@ public class NotificationContentView extends FrameLayout {
                    ((ViewGroup)mHeadsUpRemoteInput.getParent()).removeView(mHeadsUpRemoteInput);
                }
            }
            mOnContentViewInactiveListeners.remove(mHeadsUpChild);
            mHeadsUpChild.animate().cancel();
            removeView(mHeadsUpChild);
            mHeadsUpRemoteInput = null;
@@ -1108,7 +1111,6 @@ public class NotificationContentView extends FrameLayout {

    public void onNotificationUpdated(NotificationEntry entry) {
        mStatusBarNotification = entry.getSbn();
        mOnContentViewInactiveListeners.clear();
        mBeforeN = entry.targetSdk < Build.VERSION_CODES.N;
        updateAllSingleLineViews();
        ExpandableNotificationRow row = entry.getRow();
@@ -1623,7 +1625,7 @@ public class NotificationContentView extends FrameLayout {
     * @param visibleType visible type corresponding to the content view to listen
     * @param listener runnable to run once when the content view becomes inactive
     */
    public void performWhenContentInactive(int visibleType, Runnable listener) {
    void performWhenContentInactive(int visibleType, Runnable listener) {
        View view = getViewForVisibleType(visibleType);
        // View is already inactive
        if (view == null || isContentViewInactive(visibleType)) {
@@ -1633,6 +1635,22 @@ public class NotificationContentView extends FrameLayout {
        mOnContentViewInactiveListeners.put(view, listener);
    }

    /**
     * Remove content inactive listeners for a given content view . See
     * {@link #performWhenContentInactive}.
     *
     * @param visibleType visible type corresponding to the content type
     */
    void removeContentInactiveRunnable(int visibleType) {
        View view = getViewForVisibleType(visibleType);
        // View is already inactive
        if (view == null) {
            return;
        }

        mOnContentViewInactiveListeners.remove(view);
    }

    /**
     * Whether or not the content view is inactive.  This means it should not be visible
     * or the showing content as removing it would cause visual jank.
+5 −2
Original line number Diff line number Diff line
@@ -109,11 +109,14 @@ public final class RowContentBindParams {
    }

    /**
     * Free the content view so that it will no longer be bound after the rebind request.
     * Mark the content view to be freed. The view may not be immediately freeable since it may
     * be visible and animating out but this lets the binder know to free the view when safe.
     * Note that the callback passed into {@link RowContentBindStage#requestRebind}
     * may return before the view is actually freed since the view is considered up-to-date.
     *
     * @see InflationFlag
     */
    public void freeContentViews(@InflationFlag int contentViews) {
    public void markContentViewsFreeable(@InflationFlag int contentViews) {
        mContentViews &= ~contentViews;
        mDirtyContentViews &= ~contentViews;
    }
Loading