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

Commit 929a9d1f authored by Ats Jenk's avatar Ats Jenk
Browse files

Initialize bubble expanded view on shell main thread 

Move bubble expanded view initialization from background thread to main
thread.
Expanded view initialization updates the view hierarchy by adding the
bubble TaskView to the expanded view container. If the TaskView is
already added to a container, it is removed from the previous container.

This fixes a bug related to bubble view inflation. Previously it was
possible for a TaskView to be removed from bubble expanded view
container if the expanded view is inflated and initialized twice. The
second initialization removes the TaskView from the first container. But
we only keep using the first container. Which results in an expanded view
with no TaskView set.

The following order of events had to occur for this bug to manifest.
Bubble gets inflated with TASK1. TASK1 starts and inflates and
initializes expanded view on background thread. TASK1 schedules work on
the main thread to update the bubble and set it to inflated state.
An update to bubble comes in, since main thread work is still pending,
we see that the bubble is not inflated. TASK2 is created to inflate the
bubble. TASK2 starts and inflates and initializes expanded view on
background thread. TASK2 will not recreate the TaskView, but reuses the
existing one created by TASK1. TASK2 will create a new instance of the
expanded view. During init it will move the TaskView to the expanded
view it created. TASK2 schedules work on the main thread to update
bubble with the new expanded view it created.
Now TASK1 work, that it scheduled on the main thread, starts running. It
will see that the bubble is not inflated and proceeds to update the
bubble expanded view with the view it inflated. But this view no longer
has the TaskView since TASK2 removed it.
Work that TASK2 scheduled on main thread starts running now. Since
bubble views were set by TASK1, the views from TASK2 are discarded. This
also includes the TaskView that TASK2 added to the expanded view it
created as part of init.

The fix is to ensure that expanded views are only initialized on main
thread. Removing the chance for two background tasks updating the view
hierarchy independently. And also making sure that we only init only if
views are not set already.

Bug: 353894869
Test: run ShowMultipleBubblesAndSwitchTest 100 times with ABTD before
  and after applying changes
Flag: com.android.wm.shell.bubble_view_info_executors

Change-Id: Iccf1fb3700af87393ec22b8416a1df5c431d6a2b
parent 64dc0466

libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/Bubble.java

+1 −0
Original line number Diff line number Diff line
@@ -568,6 +568,7 @@ public class Bubble implements BubbleViewProvider { 
            @Nullable BubbleBarLayerView layerView,

            BubbleIconFactory iconFactory,

            boolean skipInflation) {

        ProtoLog.v(WM_SHELL_BUBBLES, "Inflate bubble key=%s", getKey());

        if (Flags.bubbleViewInfoExecutors()) {

            if (mInflationTask != null && !mInflationTask.isFinished()) {

                mInflationTask.cancel();

libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/BubbleViewInfoTask.java

+29 −16
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ import static com.android.wm.shell.bubbles.BadgedImageView.DEFAULT_PATH_SIZE; 
import static com.android.wm.shell.bubbles.BadgedImageView.WHITE_SCRIM_ALPHA;

import static com.android.wm.shell.bubbles.BubbleDebugConfig.TAG_BUBBLES;

import static com.android.wm.shell.bubbles.BubbleDebugConfig.TAG_WITH_CLASS_NAME;

import static com.android.wm.shell.protolog.ShellProtoLogGroup.WM_SHELL_BUBBLES;



import android.annotation.NonNull;

import android.annotation.Nullable;
@@ -40,6 +41,7 @@ import android.view.LayoutInflater; 


import com.android.internal.annotations.VisibleForTesting;

import com.android.internal.graphics.ColorUtils;

import com.android.internal.protolog.ProtoLog;

import com.android.launcher3.icons.BitmapInfo;

import com.android.launcher3.icons.BubbleIconFactory;

import com.android.wm.shell.R;
@@ -195,14 +197,13 @@ public class BubbleViewInfoTask { 
            // If we're in an inconsistent state, then switched modes and should just bail now.

            return null;

        }

        ProtoLog.v(WM_SHELL_BUBBLES, "Task loading bubble view info key=%s", mBubble.getKey());

        if (mLayerView.get() != null) {

            return BubbleViewInfo.populateForBubbleBar(mContext.get(), mExpandedViewManager.get(),

                    mTaskViewFactory.get(), mPositioner.get(), mLayerView.get(), mIconFactory,

                    mBubble, mSkipInflation);

            return BubbleViewInfo.populateForBubbleBar(mContext.get(), mTaskViewFactory.get(),

                    mLayerView.get(), mIconFactory, mBubble, mSkipInflation);

        } else {

            return BubbleViewInfo.populate(mContext.get(), mExpandedViewManager.get(),

                    mTaskViewFactory.get(), mPositioner.get(), mStackView.get(), mIconFactory,

                    mBubble, mSkipInflation);

            return BubbleViewInfo.populate(mContext.get(), mTaskViewFactory.get(),

                    mPositioner.get(), mStackView.get(), mIconFactory, mBubble, mSkipInflation);

        }

    }
@@ -210,6 +211,21 @@ public class BubbleViewInfoTask { 
        if (viewInfo == null || !verifyState()) {

            return;

        }

        ProtoLog.v(WM_SHELL_BUBBLES, "Task updating bubble view info key=%s", mBubble.getKey());

        if (!mBubble.isInflated()) {

            if (viewInfo.expandedView != null) {

                ProtoLog.v(WM_SHELL_BUBBLES, "Task initializing expanded view key=%s",

                        mBubble.getKey());

                viewInfo.expandedView.initialize(mExpandedViewManager.get(), mStackView.get(),

                        mPositioner.get(), false /* isOverflow */, viewInfo.taskView);

            } else if (viewInfo.bubbleBarExpandedView != null) {

                ProtoLog.v(WM_SHELL_BUBBLES, "Task initializing bubble bar expanded view key=%s",

                        mBubble.getKey());

                viewInfo.bubbleBarExpandedView.initialize(mExpandedViewManager.get(),

                        mPositioner.get(), false /* isOverflow */, viewInfo.taskView);

            }

        }



        mBubble.setViewInfo(viewInfo);

        if (mCallback != null) {

            mCallback.onBubbleViewsReady(mBubble);
@@ -231,6 +247,9 @@ public class BubbleViewInfoTask { 
    public static class BubbleViewInfo {

        // TODO(b/273312602): for foldables it might make sense to populate all of the views



        // Only set if views where inflated as part of the task

        @Nullable BubbleTaskView taskView;



        // Always populated

        ShortcutInfo shortcutInfo;

        String appName;
@@ -250,9 +269,7 @@ public class BubbleViewInfoTask { 


        @Nullable

        public static BubbleViewInfo populateForBubbleBar(Context c,

                BubbleExpandedViewManager expandedViewManager,

                BubbleTaskViewFactory taskViewFactory,

                BubblePositioner positioner,

                BubbleBarLayerView layerView,

                BubbleIconFactory iconFactory,

                Bubble b,
@@ -260,12 +277,11 @@ public class BubbleViewInfoTask { 
            BubbleViewInfo info = new BubbleViewInfo();



            if (!skipInflation && !b.isInflated()) {

                BubbleTaskView bubbleTaskView = b.getOrCreateBubbleTaskView(taskViewFactory);

                ProtoLog.v(WM_SHELL_BUBBLES, "Task inflating bubble bar views key=%s", b.getKey());

                info.taskView = b.getOrCreateBubbleTaskView(taskViewFactory);

                LayoutInflater inflater = LayoutInflater.from(c);

                info.bubbleBarExpandedView = (BubbleBarExpandedView) inflater.inflate(

                        R.layout.bubble_bar_expanded_view, layerView, false /* attachToRoot */);

                info.bubbleBarExpandedView.initialize(

                        expandedViewManager, positioner, false /* isOverflow */, bubbleTaskView);

            }



            if (!populateCommonInfo(info, c, b, iconFactory)) {
@@ -279,7 +295,6 @@ public class BubbleViewInfoTask { 
        @VisibleForTesting

        @Nullable

        public static BubbleViewInfo populate(Context c,

                BubbleExpandedViewManager expandedViewManager,

                BubbleTaskViewFactory taskViewFactory,

                BubblePositioner positioner,

                BubbleStackView stackView,
@@ -290,17 +305,15 @@ public class BubbleViewInfoTask { 


            // View inflation: only should do this once per bubble

            if (!skipInflation && !b.isInflated()) {

                ProtoLog.v(WM_SHELL_BUBBLES, "Task inflating bubble views key=%s", b.getKey());

                LayoutInflater inflater = LayoutInflater.from(c);

                info.imageView = (BadgedImageView) inflater.inflate(

                        R.layout.bubble_view, stackView, false /* attachToRoot */);

                info.imageView.initialize(positioner);



                BubbleTaskView bubbleTaskView = b.getOrCreateBubbleTaskView(taskViewFactory);

                info.taskView = b.getOrCreateBubbleTaskView(taskViewFactory);

                info.expandedView = (BubbleExpandedView) inflater.inflate(

                        R.layout.bubble_expanded_view, stackView, false /* attachToRoot */);

                info.expandedView.initialize(

                        expandedViewManager, stackView, positioner, false /* isOverflow */,

                        bubbleTaskView);

            }



            if (!populateCommonInfo(info, c, b, iconFactory)) {

libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/bubbles/BubbleViewInfoTest.kt

+0 −7
Original line number Diff line number Diff line
@@ -74,7 +74,6 @@ class BubbleViewInfoTest : ShellTestCase() { 
    private lateinit var bubbleStackView: BubbleStackView

    private lateinit var bubbleBarLayerView: BubbleBarLayerView

    private lateinit var bubblePositioner: BubblePositioner

    private lateinit var expandedViewManager: BubbleExpandedViewManager



    private val bubbleTaskViewFactory = BubbleTaskViewFactory {

        BubbleTaskView(mock<TaskView>(), mock<Executor>())
@@ -155,7 +154,6 @@ class BubbleViewInfoTest : ShellTestCase() { 
                bubbleController,

                mainExecutor

            )

        expandedViewManager = BubbleExpandedViewManager.fromBubbleController(bubbleController)

        bubbleBarLayerView = BubbleBarLayerView(context, bubbleController, bubbleData)

    }
@@ -165,7 +163,6 @@ class BubbleViewInfoTest : ShellTestCase() { 
        val info =

            BubbleViewInfoTask.BubbleViewInfo.populate(

                context,

                expandedViewManager,

                bubbleTaskViewFactory,

                bubblePositioner,

                bubbleStackView,
@@ -193,9 +190,7 @@ class BubbleViewInfoTest : ShellTestCase() { 
        val info =

            BubbleViewInfoTask.BubbleViewInfo.populateForBubbleBar(

                context,

                expandedViewManager,

                bubbleTaskViewFactory,

                bubblePositioner,

                bubbleBarLayerView,

                iconFactory,

                bubble,
@@ -229,9 +224,7 @@ class BubbleViewInfoTest : ShellTestCase() { 
        val info =

            BubbleViewInfoTask.BubbleViewInfo.populateForBubbleBar(

                context,

                expandedViewManager,

                bubbleTaskViewFactory,

                bubblePositioner,

                bubbleBarLayerView,

                iconFactory,

                bubble,

packages/SystemUI/tests/src/com/android/systemui/wmshell/BubblesTest.java

+0 −2
Original line number Diff line number Diff line
@@ -168,7 +168,6 @@ import com.android.wm.shell.bubbles.BubbleData; 
import com.android.wm.shell.bubbles.BubbleDataRepository;

import com.android.wm.shell.bubbles.BubbleEducationController;

import com.android.wm.shell.bubbles.BubbleEntry;

import com.android.wm.shell.bubbles.BubbleExpandedViewManager;

import com.android.wm.shell.bubbles.BubbleLogger;

import com.android.wm.shell.bubbles.BubbleOverflow;

import com.android.wm.shell.bubbles.BubbleStackView;
@@ -1404,7 +1403,6 @@ public class BubblesTest extends SysuiTestCase { 
                .thenReturn(userContext);



        BubbleViewInfoTask.BubbleViewInfo info = BubbleViewInfoTask.BubbleViewInfo.populate(context,

                BubbleExpandedViewManager.fromBubbleController(mBubbleController),

                () -> new BubbleTaskView(mock(TaskView.class), mock(Executor.class)),

                mPositioner,

                mBubbleController.getStackView(),