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

Commit 9d076f0c authored by Jiaming Liu's avatar Jiaming Liu
Browse files

[Bubbles] Reduce unnecessary TaskView removal to fix flickers

Frequently adding and removing TaskViews seem to be the source of flickers when quickly switching between bubbles. This CL attempts to reduce unnecessary TaskView removals by (1) defer TaskView removal to after all the switch animations are settled and (2) avoid removing and re-adding of TaskView in prepareExpandedView.

Example of flickers before this change:
A->B->C: A->B animation is waiting for B's visibility. Then B->C animation cancels the previous animation, causing A to be removed. But B->C may be waiting for C's visibility, and this causes no bubble to be visible for a short period.

Bug: 403612574
Test: Manual; atest BubbleBarLayerViewTest
Flag: com.android.wm.shell.enable_bubble_anything
Change-Id: If134f951d00c4a38b956bcec559777dec27c0831
parent 965ccff5
Loading
Loading
Loading
Loading
+121 −0
Original line number Diff line number Diff line
/*
 * Copyright (C) 2025 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.wm.shell.bubbles.util

import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.SmallTest
import com.google.common.truth.Truth
import org.junit.Assert
import org.junit.Test
import org.junit.runner.RunWith

/**
 * Unit tests for [ReferenceCounter].
 *
 * Build/Install/Run:
 *  atest WMShellRobolectricTests:ReferenceCounterTest (on host)
 *  atest WMShellMultivalentTestsOnDevice:ReferenceCounterTest (on device)
 */
@SmallTest
@RunWith(AndroidJUnit4::class)
class ReferenceCounterTest {
    @Test
    fun testIncrementDecrement() {
        val referenceCounter = ReferenceCounter<Object>()
        val a = Object()
        val b = Object()

        Truth.assertThat(referenceCounter.hasReferences()).isFalse()

        referenceCounter.increment(a)
        Truth.assertThat(referenceCounter.mReferences.get(a)).isEqualTo(1)
        Truth.assertThat(referenceCounter.hasReferences()).isTrue()

        referenceCounter.increment(a, b)
        Truth.assertThat(referenceCounter.mReferences.get(a)).isEqualTo(2)
        Truth.assertThat(referenceCounter.mReferences.get(b)).isEqualTo(1)
        Truth.assertThat(referenceCounter.hasReferences()).isTrue()

        referenceCounter.decrement(a, b)
        Truth.assertThat(referenceCounter.mReferences.get(a)).isEqualTo(1)
        Truth.assertThat(referenceCounter.mReferences.get(b)).isEqualTo(0)
        Truth.assertThat(referenceCounter.hasReferences()).isTrue()

        referenceCounter.decrement(a)
        Truth.assertThat(referenceCounter.mReferences.get(a)).isEqualTo(0)
        Truth.assertThat(referenceCounter.hasReferences()).isFalse()
    }

    @Test
    fun testDecrementNonExisting() {
        val referenceCounter = ReferenceCounter<Object>()
        val a = Object()
        val b = Object()

        referenceCounter.increment(a)
        Assert.assertThrows(IllegalArgumentException::class.java) {
            referenceCounter.decrement(b)
        }
    }

    @Test
    fun testDecrementZeroCount() {
        val referenceCounter = ReferenceCounter<Object>()
        val a = Object()

        referenceCounter.increment(a)
        referenceCounter.decrement(a)
        Assert.assertThrows(IllegalArgumentException::class.java) {
            referenceCounter.decrement(a)
        }
    }

    @Test
    fun testForEach() {
        val referenceCounter = ReferenceCounter<Object>()
        val a = Object()
        val b = Object()
        val set = HashSet<Object>()

        referenceCounter.forEach { set.add(it) }
        Truth.assertThat(set).isEmpty()

        referenceCounter.increment(a)
        referenceCounter.forEach { set.add(it) }
        Truth.assertThat(set).hasSize(1)
        Truth.assertThat(set).contains(a)

        set.clear()
        referenceCounter.increment(a, b)
        referenceCounter.forEach { set.add(it) }
        Truth.assertThat(set).hasSize(2)
        Truth.assertThat(set).contains(a)
        Truth.assertThat(set).contains(b)

        set.clear()
        referenceCounter.decrement(a, b)
        referenceCounter.forEach { set.add(it) }
        Truth.assertThat(set).hasSize(2)
        Truth.assertThat(set).contains(a)
        Truth.assertThat(set).contains(b) // zero count items are retained.

        set.clear()
        referenceCounter.clear()
        referenceCounter.forEach { set.add(it) }
        Truth.assertThat(set).isEmpty()
    }
}
+46 −8
Original line number Diff line number Diff line
@@ -53,6 +53,7 @@ import com.android.wm.shell.bubbles.BubblePositioner;
import com.android.wm.shell.bubbles.BubbleViewProvider;
import com.android.wm.shell.bubbles.DismissViewUtils;
import com.android.wm.shell.bubbles.bar.BubbleBarExpandedViewDragController.DragListener;
import com.android.wm.shell.bubbles.util.ReferenceCounter;
import com.android.wm.shell.shared.bubbles.BubbleBarLocation;
import com.android.wm.shell.shared.bubbles.DeviceConfig;
import com.android.wm.shell.shared.bubbles.DismissView;
@@ -108,6 +109,13 @@ public class BubbleBarLayerView extends FrameLayout
    private final Rect mHandleTouchBounds = new Rect();
    private Insets mInsets;

    /**
     * Tracks bubbles used in switch animations. Ideally, we should track bubbles used in all types
     * of animations, but so far it only seems helpful for switch animations.
     */
    private final ReferenceCounter<BubbleViewProvider> mSwitchAnimationBubbleTracker =
            new ReferenceCounter();

    public BubbleBarLayerView(Context context, BubbleController controller, BubbleData bubbleData,
            BubbleLogger bubbleLogger) {
        super(context);
@@ -339,11 +347,22 @@ public class BubbleBarLayerView extends FrameLayout
            mExpandedView = null;
        }
        if (mExpandedView == null) {
            boolean expandedViewAlreadyAdded = false;
            if (expandedView.getParent() != null) {
                // Expanded view might be animating collapse and is still attached
                // Cancel current animations and remove from parent
                // Expanded view might be animating collapse and is still attached. Cancel current
                // animations.
                // Add temporary references to the previous and the current bubbles to prevent
                // them from being removed when canceling ongoing animations. References will be
                // added again when starting the switch animation.
                mSwitchAnimationBubbleTracker.increment(previousBubble, b);
                mAnimationHelper.cancelAnimations();
                removeView(expandedView);
                mSwitchAnimationBubbleTracker.decrement(previousBubble, b);

                // Need to check again because cancelAnimations might remove it from the parent.
                // TODO(b/403612574) use reference tracking for other animations.
                if (expandedView.getParent() != null) {
                    expandedViewAlreadyAdded = true;
                }
            }
            mExpandedBubble = b;
            mExpandedView = expandedView;
@@ -398,7 +417,12 @@ public class BubbleBarLayerView extends FrameLayout
                    mDragZoneFactory,
                    dragListener);

            addView(mExpandedView, new LayoutParams(width, height, Gravity.LEFT));
            final LayoutParams layoutParams = new LayoutParams(width, height, Gravity.LEFT);
            if (expandedViewAlreadyAdded) {
                mExpandedView.setLayoutParams(layoutParams);
            } else {
                addView(mExpandedView, layoutParams);
            }
        }

        if (mEducationViewController.isEducationVisible()) {
@@ -452,10 +476,11 @@ public class BubbleBarLayerView extends FrameLayout
        };

        if (previousBubble != null) {
            final BubbleBarExpandedView previousExpandedView =
                    previousBubble.getBubbleBarExpandedView();
            mAnimationHelper.animateSwitch(previousBubble, mExpandedBubble, () -> {
                removeView(previousExpandedView);
            final BubbleViewProvider expandedBubble = mExpandedBubble;
            mSwitchAnimationBubbleTracker.increment(previousBubble, expandedBubble);
            mAnimationHelper.animateSwitch(previousBubble, expandedBubble, () -> {
                mSwitchAnimationBubbleTracker.decrement(previousBubble, expandedBubble);
                ensureSwitchAnimationEndingState();
                afterAnimation.run();
            });
        } else {
@@ -694,4 +719,17 @@ public class BubbleBarLayerView extends FrameLayout
        }
        setupDragZoneFactory();
    }

    /** Ensures that only the expanded bubble is added at the end of all switch animations. */
    private void ensureSwitchAnimationEndingState() {
        if (!mSwitchAnimationBubbleTracker.hasReferences()) {
            // All switch animations are done, so we remove bubbles except for the expanded one.
            mSwitchAnimationBubbleTracker.forEach(bubble -> {
                if (bubble != mExpandedBubble) {
                    removeView(bubble.getBubbleBarExpandedView());
                }
            });
            mSwitchAnimationBubbleTracker.clear();
        }
    }
}
+91 −0
Original line number Diff line number Diff line
/*
 * Copyright (C) 2025 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.wm.shell.bubbles.util;

import android.util.ArrayMap;

import androidx.annotation.NonNull;
import androidx.annotation.VisibleForTesting;

import java.util.Map;
import java.util.function.Consumer;

/**
 * A simple utility class to track reference count on items. Primarily used to track bubbles used
 * in animations.
 *
 * @param <T> the type of tracked items.
 */
public class ReferenceCounter<T> {
    @VisibleForTesting
    final Map<T, Integer> mReferences = new ArrayMap<>();

    /** Increments reference count of the {@code items}. */
    @SafeVarargs
    public final void increment(T... items) {
        for (T item : items) {
            if (item == null) {
                continue;
            }
            if (mReferences.containsKey(item)) {
                mReferences.put(item, mReferences.get(item) + 1);
            } else {
                mReferences.put(item, 1);
            }
        }
    }

    /**
     * Decrements reference count of the {@code items}. Note that this does NOT remove any items
     * even if they have reached zero reference.
     *
     * @throws IllegalArgumentException if the item is not tracked or already has zero reference
     *                                  count.
     */
    @SafeVarargs
    public final void decrement(T... items) {
        for (T item : items) {
            if (item == null) {
                continue;
            }
            if (!mReferences.containsKey(item)) {
                throw new IllegalArgumentException("Decrement non-existing item=" + item);
            } else if (mReferences.get(item) == 0) {
                throw new IllegalArgumentException("Decrement zero reference item=" + item);
            } else {
                mReferences.put(item, mReferences.get(item) - 1);
            }
        }
    }

    /** Returns whether any tracked item has a non-zero reference count. */
    public boolean hasReferences() {
        return mReferences.values().stream().anyMatch(count -> count != 0);
    }

    /** Executes the {@code callback} on each item, including items with zero reference count. */
    public void forEach(@NonNull Consumer<T> callback) {
        for (T item : mReferences.keySet()) {
            callback.accept(item);
        }
    }

    /** Clears all the reference counts and removes tracked items. */
    public void clear() {
        mReferences.clear();
    }
}