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

Commit 0311eab1 authored by Eric Lin's avatar Eric Lin
Browse files

Fix DesktopModeWindowDecor leak on last bubble removal.

This change fixes a performance regression (b/412782113) in the bubble
cleanup process that caused desktop mode window decoration
"leaks" during bubble removal. The issue particularly impacts the
ShowMultipleBubblesAndSwitchMicrobenchmark test, showing regressions
of over 1,700% in missed frames during SPLASHSCREEN_EXIT_ANIM.

The problem occurs during the removal of the last bubble from the stack,
where two window container transitions (WCT) are triggered in sequence:
a TO_BACK transition followed by a CLOSE transition. By the time the
TO_BACK transition occurs, the bubble is already removed from the bubble
data, causing AppHandleAndHeaderVisibilityHelper to incorrectly
allow desktop mode window decorations for the removed bubble task.

This change eliminates the unnecessary TO_BACK transition when removing
the last bubble from an empty stack, allowing the CLOSE transition to
handle the task removal directly. This prevents the accumulation of
desktop decorations and significantly improves performance metrics.

Bug: 416655338
Bug: 408389476
Bug: 388630258
Flag: com.android.wm.shell.enable_create_any_bubble
Flag: com.android.window.flags.exclude_task_from_recents
Test: atest WMShellRobolectricTests:BubbleControllerTest
Test: atest systemui-bubble-1-jank-suite \
   --request-upload-result \
   -- --enable-module-dynamic-download \
   --module-arg systemui-bubble-1-jank-suite:strict-include-metric-filter:'perfetto_ft_systemui-missed_app_frames-mean' \
   --test-arg com.android.tradefed.testtype.AndroidJUnitTest:class:android.platform.test.scenario.sysui.bubble.ShowMultipleBubblesAndSwitchMicrobenchmark
http: //ab/I4940001039473788 (flag enabled, SPLASHSCREEN_EXIT_ANIM-counter_metrics-missed_app_frames-mean:0.04)
Change-Id: Ia37433a8cd6c2cfb7187cb171e791de6e1b77122
parent c009172c
Loading
Loading
Loading
Loading
+27 −1
Original line number Diff line number Diff line
@@ -70,20 +70,28 @@ import com.google.common.truth.Truth.assertThat
import com.google.common.util.concurrent.MoreExecutors.directExecutor
import java.util.Optional
import org.junit.After
import org.junit.Assume.assumeTrue
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.ArgumentMatchers.anyInt
import org.mockito.kotlin.any
import org.mockito.kotlin.argumentCaptor
import org.mockito.kotlin.isA
import org.mockito.kotlin.doReturn
import org.mockito.kotlin.mock
import org.mockito.kotlin.never
import org.mockito.kotlin.verify
import platform.test.runner.parameterized.ParameterizedAndroidJunit4
import platform.test.runner.parameterized.Parameters

/** Tests for [BubbleController] */
/** Tests for [BubbleController].
 *
 * Build/Install/Run:
 *  atest WMShellRobolectricTests:BubbleControllerTest (on host)
 *  atest WMShellMultivalentTestsOnDevice:BubbleControllerTest (on device)
 */
@SmallTest
@RunWith(ParameterizedAndroidJunit4::class)
class BubbleControllerTest(flags: FlagsParameterization) {
@@ -319,6 +327,24 @@ class BubbleControllerTest(flags: FlagsParameterization) {
        }
    }

    @Test
    fun setTaskViewVisible_lastBubbleRemoval_skipsTaskViewHiding() {
        assumeTrue(BubbleAnythingFlagHelper.enableCreateAnyBubbleWithForceExcludedFromRecents())

        val baseTransitions = mock<TaskViewTransitions>()
        val taskView = mock<TaskViewTaskController>()
        val bubbleTaskViewController = bubbleController.BubbleTaskViewController(baseTransitions)

        bubbleTaskViewController.setTaskViewVisible(taskView, false /* visible */)

        verify(baseTransitions, never()).setTaskViewVisible(
            any(), /* taskView */
            any(), /* visible */
            any(), /* reorder */
            any(), /* syncHiddenWithVisibilityOnReorder */
        )
    }

    @Test
    fun hasStableBubbleForTask_whenBubbleIsCollapsed_returnsTrue() {
        val taskId = 777
+10 −0
Original line number Diff line number Diff line
@@ -3604,6 +3604,16 @@ public class BubbleController implements ConfigurationChangeListener,
        @Override
        public void setTaskViewVisible(TaskViewTaskController taskView, boolean visible) {
            if (BubbleAnythingFlagHelper.enableCreateAnyBubbleWithForceExcludedFromRecents()) {
                // When removing the last bubble, BubbleData has already removed the bubble from
                // the stack before this call occurs. Without this check, the TO_BACK transition
                // would trigger DesktopModeWindowDecorViewModel#onTaskChanging, which
                // incorrectly creates desktop mode window decorations for the removed bubble task
                // since AppHandleAndHeaderVisibilityHelper#allowedForTask can't find the task in
                // the bubble stack anymore. These decorations then "leak" because the task will be
                // closed in the subsequent CLOSE transition. See b/416655338 for more details.
                if (!visible && !mBubbleData.hasBubbleInStackWithTaskView(taskView)) {
                    return;
                }
                // Use reorder instead of always-on-top with hidden.
                mBaseTransitions.setTaskViewVisible(taskView, visible, true /* reorder */,
                        false /* toggleHiddenOnReorder */);
+6 −0
Original line number Diff line number Diff line
@@ -46,6 +46,7 @@ import com.android.wm.shell.shared.annotations.ShellMainThread;
import com.android.wm.shell.shared.bubbles.BubbleBarLocation;
import com.android.wm.shell.shared.bubbles.BubbleBarUpdate;
import com.android.wm.shell.shared.bubbles.RemovedBubble;
import com.android.wm.shell.taskview.TaskViewTaskController;

import java.io.PrintWriter;
import java.util.ArrayList;
@@ -308,6 +309,11 @@ public class BubbleData {
        return !mBubbles.isEmpty();
    }

    boolean hasBubbleInStackWithTaskView(@NonNull TaskViewTaskController taskView) {
        return getBubbleWithPredicate(mBubbles,
                b -> b.getTaskView().getController() == taskView) != null;
    }

    public boolean hasOverflowBubbles() {
        return !mOverflowBubbles.isEmpty();
    }