From 0f1bf53ca660813d4939c4fc397fc0ffe29d2945 Mon Sep 17 00:00:00 2001 From: Jorge Gil Date: Tue, 16 Apr 2024 03:22:52 +0000 Subject: Do not assume display is available when a task launches The resize veil creates a SCVH as soon as a task launches to draw the icon drawable early and once to be ready for any resize operation. However, it cannot be assumed that the DisplayController will return a non-null display if the task and display were both launched roughly at the same time. To avoid a NPE, this change checks whether the display is available and if not attaches a listener to do the resize veil set up when it actually appears. Bug: 333442423 Test: atest WMShellUnitTests:ResizeVeil; resize veil works as usual; atest CtsInputTestCases doesn't cause a crash Change-Id: Id8efb265e1910144f92fee1144aa07afb9327056 --- .../windowdecor/DesktopModeWindowDecoration.java | 4 +- .../android/wm/shell/windowdecor/ResizeVeil.java | 134 +++++++++++-- .../wm/shell/windowdecor/WindowDecoration.java | 4 + .../android/wm/shell/windowdecor/ResizeVeilTest.kt | 216 +++++++++++++++++++++ 4 files changed, 340 insertions(+), 18 deletions(-) create mode 100644 libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/windowdecor/ResizeVeilTest.kt diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/DesktopModeWindowDecoration.java b/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/DesktopModeWindowDecoration.java index 963b1303c379..385e8cc5257a 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/DesktopModeWindowDecoration.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/DesktopModeWindowDecoration.java @@ -466,8 +466,8 @@ public class DesktopModeWindowDecoration extends WindowDecoration mSurfaceControlBuilderSupplier; + private final DisplayController mDisplayController; private final Supplier mSurfaceControlTransactionSupplier; + private final SurfaceControlBuilderFactory mSurfaceControlBuilderFactory; + private final WindowDecoration.SurfaceControlViewHostFactory mSurfaceControlViewHostFactory; private final SurfaceSession mSurfaceSession = new SurfaceSession(); private final Drawable mAppIcon; private ImageView mIconView; @@ -74,41 +78,82 @@ public class ResizeVeil { private final RunningTaskInfo mTaskInfo; private SurfaceControlViewHost mViewHost; - private final Display mDisplay; + private Display mDisplay; private ValueAnimator mVeilAnimator; - public ResizeVeil(Context context, Drawable appIcon, RunningTaskInfo taskInfo, + private boolean mIsShowing = false; + + private final DisplayController.OnDisplaysChangedListener mOnDisplaysChangedListener = + new DisplayController.OnDisplaysChangedListener() { + @Override + public void onDisplayAdded(int displayId) { + if (mTaskInfo.displayId != displayId) { + return; + } + mDisplayController.removeDisplayWindowListener(this); + setupResizeVeil(); + } + }; + + public ResizeVeil(Context context, + @NonNull DisplayController displayController, + Drawable appIcon, RunningTaskInfo taskInfo, SurfaceControl taskSurface, - Supplier surfaceControlBuilderSupplier, Display display, Supplier surfaceControlTransactionSupplier) { + this(context, + displayController, + appIcon, + taskInfo, + taskSurface, + surfaceControlTransactionSupplier, + new SurfaceControlBuilderFactory() {}, + new WindowDecoration.SurfaceControlViewHostFactory() {}); + } + + public ResizeVeil(Context context, + @NonNull DisplayController displayController, + Drawable appIcon, RunningTaskInfo taskInfo, + SurfaceControl taskSurface, + Supplier surfaceControlTransactionSupplier, + SurfaceControlBuilderFactory surfaceControlBuilderFactory, + WindowDecoration.SurfaceControlViewHostFactory surfaceControlViewHostFactory) { mContext = context; + mDisplayController = displayController; mAppIcon = appIcon; - mSurfaceControlBuilderSupplier = surfaceControlBuilderSupplier; mSurfaceControlTransactionSupplier = surfaceControlTransactionSupplier; mTaskInfo = taskInfo; mParentSurface = taskSurface; - mDisplay = display; + mSurfaceControlBuilderFactory = surfaceControlBuilderFactory; + mSurfaceControlViewHostFactory = surfaceControlViewHostFactory; setupResizeVeil(); } - /** * Create the veil in its default invisible state. */ private void setupResizeVeil() { - mVeilSurface = mSurfaceControlBuilderSupplier.get() + if (!obtainDisplayOrRegisterListener()) { + // Display may not be available yet, skip this until then. + return; + } + mVeilSurface = mSurfaceControlBuilderFactory + .create("Resize veil of Task=" + mTaskInfo.taskId) .setContainerLayer() - .setName("Resize veil of Task=" + mTaskInfo.taskId) .setHidden(true) .setParent(mParentSurface) .setCallsite("ResizeVeil#setupResizeVeil") .build(); - mBackgroundSurface = SurfaceUtils.makeColorLayer(mVeilSurface, - "Resize veil background of Task=" + mTaskInfo.taskId, mSurfaceSession); - mIconSurface = mSurfaceControlBuilderSupplier.get() - .setName("Resize veil icon of Task= " + mTaskInfo.taskId) - .setContainerLayer() + mBackgroundSurface = mSurfaceControlBuilderFactory + .create("Resize veil background of Task=" + mTaskInfo.taskId, mSurfaceSession) + .setColorLayer() + .setHidden(true) .setParent(mVeilSurface) + .setCallsite("ResizeVeil#setupResizeVeil") + .build(); + mIconSurface = mSurfaceControlBuilderFactory + .create("Resize veil icon of Task=" + mTaskInfo.taskId) + .setContainerLayer() .setHidden(true) + .setParent(mVeilSurface) .setCallsite("ResizeVeil#setupResizeVeil") .build(); @@ -131,10 +176,20 @@ public class ResizeVeil { final WindowlessWindowManager wwm = new WindowlessWindowManager(mTaskInfo.configuration, mIconSurface, null /* hostInputToken */); - mViewHost = new SurfaceControlViewHost(mContext, mDisplay, wwm, "ResizeVeil"); + + mViewHost = mSurfaceControlViewHostFactory.create(mContext, mDisplay, wwm, "ResizeVeil"); mViewHost.setView(root, lp); } + private boolean obtainDisplayOrRegisterListener() { + mDisplay = mDisplayController.getDisplay(mTaskInfo.displayId); + if (mDisplay == null) { + mDisplayController.addDisplayWindowListener(mOnDisplaysChangedListener); + return false; + } + return true; + } + /** * Shows the veil surface/view. * @@ -146,6 +201,12 @@ public class ResizeVeil { */ public void showVeil(SurfaceControl.Transaction t, SurfaceControl parentSurface, Rect taskBounds, boolean fadeIn) { + if (!isReady() || isVisible()) { + t.apply(); + return; + } + mIsShowing = true; + // Parent surface can change, ensure it is up to date. if (!parentSurface.equals(mParentSurface)) { t.reparent(mVeilSurface, parentSurface); @@ -226,6 +287,9 @@ public class ResizeVeil { * Animate veil's alpha to 1, fading it in. */ public void showVeil(SurfaceControl parentSurface, Rect taskBounds) { + if (!isReady() || isVisible()) { + return; + } SurfaceControl.Transaction t = mSurfaceControlTransactionSupplier.get(); showVeil(t, parentSurface, taskBounds, true /* fadeIn */); } @@ -247,6 +311,9 @@ public class ResizeVeil { * @param newBounds bounds to update veil to. */ public void updateResizeVeil(Rect newBounds) { + if (!isVisible()) { + return; + } SurfaceControl.Transaction t = mSurfaceControlTransactionSupplier.get(); updateResizeVeil(t, newBounds); } @@ -260,6 +327,10 @@ public class ResizeVeil { * @param newBounds bounds to update veil to. */ public void updateResizeVeil(SurfaceControl.Transaction t, Rect newBounds) { + if (!isVisible()) { + t.apply(); + return; + } if (mVeilAnimator != null && mVeilAnimator.isStarted()) { mVeilAnimator.removeAllUpdateListeners(); mVeilAnimator.end(); @@ -272,6 +343,9 @@ public class ResizeVeil { * Animate veil's alpha to 0, fading it out. */ public void hideVeil() { + if (!isVisible()) { + return; + } cancelAnimation(); mVeilAnimator = new ValueAnimator(); mVeilAnimator.setFloatValues(1, 0); @@ -292,6 +366,7 @@ public class ResizeVeil { } }); mVeilAnimator.start(); + mIsShowing = false; } @ColorRes @@ -317,11 +392,27 @@ public class ResizeVeil { } } + /** + * Whether the resize veil is currently visible. + * + * Note: when animating a {@link ResizeVeil#hideVeil()}, the veil is considered visible as soon + * as the animation starts. + */ + private boolean isVisible() { + return mIsShowing; + } + + /** Whether the resize veil is ready to be shown. */ + private boolean isReady() { + return mViewHost != null; + } + /** * Dispose of veil when it is no longer needed, likely on close of its container decor. */ void dispose() { cancelAnimation(); + mIsShowing = false; mVeilAnimator = null; if (mViewHost != null) { @@ -342,5 +433,16 @@ public class ResizeVeil { mVeilSurface = null; } t.apply(); + mDisplayController.removeDisplayWindowListener(mOnDisplaysChangedListener); + } + + interface SurfaceControlBuilderFactory { + default SurfaceControl.Builder create(@NonNull String name) { + return new SurfaceControl.Builder().setName(name); + } + default SurfaceControl.Builder create(@NonNull String name, + @NonNull SurfaceSession surfaceSession) { + return new SurfaceControl.Builder(surfaceSession).setName(name); + } } } diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/WindowDecoration.java b/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/WindowDecoration.java index 51b0a246f3e9..36da1ace8408 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/WindowDecoration.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/WindowDecoration.java @@ -668,6 +668,10 @@ public abstract class WindowDecoration default SurfaceControlViewHost create(Context c, Display d, WindowlessWindowManager wmm) { return new SurfaceControlViewHost(c, d, wmm, "WindowDecoration"); } + default SurfaceControlViewHost create(Context c, Display d, + WindowlessWindowManager wmm, String callsite) { + return new SurfaceControlViewHost(c, d, wmm, callsite); + } } /** diff --git a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/windowdecor/ResizeVeilTest.kt b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/windowdecor/ResizeVeilTest.kt new file mode 100644 index 000000000000..847c2dd77d0a --- /dev/null +++ b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/windowdecor/ResizeVeilTest.kt @@ -0,0 +1,216 @@ +/* + * Copyright (C) 2024 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.windowdecor + +import android.graphics.Rect +import android.graphics.drawable.Drawable +import android.testing.AndroidTestingRunner +import android.testing.TestableLooper +import android.view.Display +import android.view.SurfaceControl +import android.view.SurfaceControlViewHost +import android.view.WindowlessWindowManager +import androidx.test.filters.SmallTest +import com.android.wm.shell.ShellTestCase +import com.android.wm.shell.TestRunningTaskInfoBuilder +import com.android.wm.shell.common.DisplayController +import com.android.wm.shell.common.DisplayController.OnDisplaysChangedListener +import com.android.wm.shell.windowdecor.WindowDecoration.SurfaceControlViewHostFactory +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.ArgumentCaptor +import org.mockito.Mock +import org.mockito.Spy +import org.mockito.kotlin.any +import org.mockito.kotlin.doReturn +import org.mockito.kotlin.eq +import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.times +import org.mockito.kotlin.verify +import org.mockito.kotlin.verifyZeroInteractions +import org.mockito.kotlin.whenever + + +/** + * Tests for [ResizeVeil]. + * + * Build/Install/Run: + * atest WMShellUnitTests:ResizeVeilTest + */ +@SmallTest +@RunWith(AndroidTestingRunner::class) +@TestableLooper.RunWithLooper +class ResizeVeilTest : ShellTestCase() { + + @Mock + private lateinit var mockDisplayController: DisplayController + @Mock + private lateinit var mockAppIcon: Drawable + @Mock + private lateinit var mockDisplay: Display + @Mock + private lateinit var mockSurfaceControlViewHost: SurfaceControlViewHost + @Mock + private lateinit var mockSurfaceControlBuilderFactory: ResizeVeil.SurfaceControlBuilderFactory + @Mock + private lateinit var mockSurfaceControlViewHostFactory: SurfaceControlViewHostFactory + @Spy + private val spyResizeVeilSurfaceBuilder = SurfaceControl.Builder() + @Mock + private lateinit var mockResizeVeilSurface: SurfaceControl + @Spy + private val spyBackgroundSurfaceBuilder = SurfaceControl.Builder() + @Mock + private lateinit var mockBackgroundSurface: SurfaceControl + @Spy + private val spyIconSurfaceBuilder = SurfaceControl.Builder() + @Mock + private lateinit var mockIconSurface: SurfaceControl + @Mock + private lateinit var mockTransaction: SurfaceControl.Transaction + + private val taskInfo = TestRunningTaskInfoBuilder().build() + + @Before + fun setUp() { + whenever(mockSurfaceControlViewHostFactory.create(any(), any(), any(), any())) + .thenReturn(mockSurfaceControlViewHost) + whenever(mockSurfaceControlBuilderFactory + .create("Resize veil of Task=" + taskInfo.taskId)) + .thenReturn(spyResizeVeilSurfaceBuilder) + doReturn(mockResizeVeilSurface).whenever(spyResizeVeilSurfaceBuilder).build() + whenever(mockSurfaceControlBuilderFactory + .create(eq("Resize veil background of Task=" + taskInfo.taskId), any())) + .thenReturn(spyBackgroundSurfaceBuilder) + doReturn(mockBackgroundSurface).whenever(spyBackgroundSurfaceBuilder).build() + whenever(mockSurfaceControlBuilderFactory + .create("Resize veil icon of Task=" + taskInfo.taskId)) + .thenReturn(spyIconSurfaceBuilder) + doReturn(mockIconSurface).whenever(spyIconSurfaceBuilder).build() + } + + @Test + fun init_displayAvailable_viewHostCreated() { + createResizeVeil(withDisplayAvailable = true) + + verify(mockSurfaceControlViewHostFactory) + .create(any(), eq(mockDisplay), any(), eq("ResizeVeil")) + } + + @Test + fun init_displayUnavailable_viewHostNotCreatedUntilDisplayAppears() { + createResizeVeil(withDisplayAvailable = false) + + verify(mockSurfaceControlViewHostFactory, never()) + .create(any(), eq(mockDisplay), any(), eq("ResizeVeil")) + val captor = ArgumentCaptor.forClass(OnDisplaysChangedListener::class.java) + verify(mockDisplayController).addDisplayWindowListener(captor.capture()) + + whenever(mockDisplayController.getDisplay(taskInfo.displayId)).thenReturn(mockDisplay) + captor.value.onDisplayAdded(taskInfo.displayId) + + verify(mockSurfaceControlViewHostFactory) + .create(any(), eq(mockDisplay), any(), eq("ResizeVeil")) + verify(mockDisplayController).removeDisplayWindowListener(any()) + } + + @Test + fun dispose_removesDisplayWindowListener() { + createResizeVeil().dispose() + + verify(mockDisplayController).removeDisplayWindowListener(any()) + } + + @Test + fun showVeil() { + val veil = createResizeVeil() + val tx = mock() + + veil.showVeil(tx, mock(), Rect(0, 0, 100, 100), false /* fadeIn */) + + verify(tx).show(mockResizeVeilSurface) + verify(tx).show(mockBackgroundSurface) + verify(tx).show(mockIconSurface) + verify(tx).apply() + } + + @Test + fun showVeil_displayUnavailable_doesNotShow() { + val veil = createResizeVeil(withDisplayAvailable = false) + val tx = mock() + + veil.showVeil(tx, mock(), Rect(0, 0, 100, 100), false /* fadeIn */) + + verify(tx, never()).show(mockResizeVeilSurface) + verify(tx, never()).show(mockBackgroundSurface) + verify(tx, never()).show(mockIconSurface) + verify(tx).apply() + } + + @Test + fun showVeil_alreadyVisible_doesNotShowAgain() { + val veil = createResizeVeil() + val tx = mock() + + veil.showVeil(tx, mock(), Rect(0, 0, 100, 100), false /* fadeIn */) + veil.showVeil(tx, mock(), Rect(0, 0, 100, 100), false /* fadeIn */) + + verify(tx, times(1)).show(mockResizeVeilSurface) + verify(tx, times(1)).show(mockBackgroundSurface) + verify(tx, times(1)).show(mockIconSurface) + verify(tx, times(2)).apply() + } + + @Test + fun showVeil_reparentsVeilToNewParent() { + val veil = createResizeVeil(parent = mock()) + val tx = mock() + + val newParent = mock() + veil.showVeil(tx, newParent, Rect(0, 0, 100, 100), false /* fadeIn */) + + verify(tx).reparent(mockResizeVeilSurface, newParent) + } + + @Test + fun hideVeil_alreadyHidden_doesNothing() { + val veil = createResizeVeil() + + veil.hideVeil() + + verifyZeroInteractions(mockTransaction) + } + + private fun createResizeVeil( + withDisplayAvailable: Boolean = true, + parent: SurfaceControl = mock() + ): ResizeVeil { + whenever(mockDisplayController.getDisplay(taskInfo.displayId)) + .thenReturn(if (withDisplayAvailable) mockDisplay else null) + return ResizeVeil( + context, + mockDisplayController, + mockAppIcon, + taskInfo, + parent, + { mockTransaction }, + mockSurfaceControlBuilderFactory, + mockSurfaceControlViewHostFactory + ) + } +} -- cgit v1.2.3-59-g8ed1b