Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix NPE for animate frozen snapshot

Browse Source 
Before, the snapshot can be null if the window is unfreezed but not
removed from the display changing list, or when it fails to take
screenshot (like because the display is not ready). Now, we make sure to
remove the window from the changing list in such cases.

Fix: 204400505
Test: atest WmTests:WindowContainerTests
Change-Id: I517232fc2ec689680a00b499495a4bad5a0d5f15
This commit is contained in:
Chris Li
2021-11-02 16:36:56 +08:00
parent 8f7d253431
commit e32aa4fd2b
7 changed files with 98 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
services/core/java/com/android/server/wm/SurfaceAnimator.java
View File

@@ -188,6 +188,10 @@ class SurfaceAnimator {
mAnimation.startAnimation(mLeash, t, type, mInnerAnimationFinishedCallback);
if (snapshotAnim != null) {
mSnapshot = freezer.takeSnapshotForAnimation();
if (mSnapshot == null) {
Slog.e(TAG, "No snapshot target to start animation on for " + mAnimatable);
return;
}
mSnapshot.startAnimation(t, snapshotAnim, type);
}
}

39
services/core/java/com/android/server/wm/SurfaceFreezer.java
View File

@@ -26,6 +26,7 @@ import android.graphics.PixelFormat;
import android.graphics.Point;
import android.graphics.Rect;
import android.hardware.HardwareBuffer;
import android.util.Slog;
import android.view.SurfaceControl;
import com.android.internal.annotations.VisibleForTesting;
@@ -49,8 +50,10 @@ import com.android.internal.protolog.common.ProtoLog;
*/
class SurfaceFreezer {
private final Freezable mAnimatable;
private final WindowManagerService mWmService;
private static final String TAG = "SurfaceFreezer";
private final @NonNull Freezable mAnimatable;
private final @NonNull WindowManagerService mWmService;
@VisibleForTesting
SurfaceControl mLeash;
Snapshot mSnapshot = null;
@@ -59,7 +62,7 @@ class SurfaceFreezer {
/**
* @param animatable The object to animate.
*/
SurfaceFreezer(Freezable animatable, WindowManagerService service) {
SurfaceFreezer(@NonNull Freezable animatable, @NonNull WindowManagerService service) {
mAnimatable = animatable;
mWmService = service;
}
@@ -86,11 +89,14 @@ class SurfaceFreezer {
freezeTarget = freezeTarget != null ? freezeTarget : mAnimatable.getFreezeSnapshotTarget();
if (freezeTarget != null) {
SurfaceControl.ScreenshotHardwareBuffer screenshotBuffer = createSnapshotBuffer(
SurfaceControl.ScreenshotHardwareBuffer screenshotBuffer = createSnapshotBufferInner(
freezeTarget, startBounds);
final HardwareBuffer buffer = screenshotBuffer == null ? null
: screenshotBuffer.getHardwareBuffer();
if (buffer == null || buffer.getWidth() <= 1 || buffer.getHeight() <= 1) {
// This can happen when display is not ready.
Slog.w(TAG, "Failed to capture screenshot for " + mAnimatable);
unfreeze(t);
return;
}
mSnapshot = new Snapshot(t, screenshotBuffer, mLeash);
@@ -124,6 +130,11 @@ class SurfaceFreezer {
* snapshot.
*/
void unfreeze(SurfaceControl.Transaction t) {
unfreezeInner(t);
mAnimatable.onUnfrozen();
}
private void unfreezeInner(SurfaceControl.Transaction t) {
if (mSnapshot != null) {
mSnapshot.cancelAnimation(t, false /* restarting */);
mSnapshot = null;
@@ -188,6 +199,18 @@ class SurfaceFreezer {
return SurfaceControl.captureLayers(captureArgs);
}
@VisibleForTesting
SurfaceControl.ScreenshotHardwareBuffer createSnapshotBufferInner(
SurfaceControl target, Rect bounds) {
return createSnapshotBuffer(target, bounds);
}
@VisibleForTesting
GraphicBuffer createFromHardwareBufferInner(
SurfaceControl.ScreenshotHardwareBuffer screenshotBuffer) {
return GraphicBuffer.createFromHardwareBuffer(screenshotBuffer.getHardwareBuffer());
}
class Snapshot {
private SurfaceControl mSurfaceControl;
private AnimationAdapter mAnimation;
@@ -198,10 +221,7 @@ class SurfaceFreezer {
*/
Snapshot(SurfaceControl.Transaction t,
SurfaceControl.ScreenshotHardwareBuffer screenshotBuffer, SurfaceControl parent) {
// We can't use a delegating constructor since we need to
// reference this::onAnimationFinished
GraphicBuffer graphicBuffer = GraphicBuffer.createFromHardwareBuffer(
screenshotBuffer.getHardwareBuffer());
GraphicBuffer graphicBuffer = createFromHardwareBufferInner(screenshotBuffer);
mSurfaceControl = mAnimatable.makeAnimationLeash()
.setName("snapshot anim: " + mAnimatable.toString())
@@ -275,5 +295,8 @@ class SurfaceFreezer {
* will be generated (but the rest of the freezing logic will still happen).
*/
@Nullable SurfaceControl getFreezeSnapshotTarget();
/** Called when the {@link #unfreeze(SurfaceControl.Transaction)} is called. */
void onUnfrozen();
}
}

11
services/core/java/com/android/server/wm/WindowContainer.java
View File

@@ -614,7 +614,6 @@ class WindowContainer<E extends WindowContainer> extends ConfigurationContainer<
final DisplayContent dc = getDisplayContent();
if (dc != null) {
mSurfaceFreezer.unfreeze(getSyncTransaction());
dc.mChangingContainers.remove(this);
}
while (!mChildren.isEmpty()) {
final E child = mChildren.peekLast();
@@ -1067,9 +1066,6 @@ class WindowContainer<E extends WindowContainer> extends ConfigurationContainer<
// part of a change transition.
if (!visible) {
mSurfaceFreezer.unfreeze(getSyncTransaction());
if (mDisplayContent != null) {
mDisplayContent.mChangingContainers.remove(this);
}
}
WindowContainer parent = getParent();
if (parent != null) {
@@ -2647,6 +2643,13 @@ class WindowContainer<E extends WindowContainer> extends ConfigurationContainer<
return getSurfaceControl();
}
@Override
public void onUnfrozen() {
if (mDisplayContent != null) {
mDisplayContent.mChangingContainers.remove(this);
}
}
@Override
public Builder makeAnimationLeash() {
return makeSurface().setContainerLayer();

1
services/tests/wmtests/src/com/android/server/wm/AppTransitionTests.java
View File

@@ -419,6 +419,7 @@ public class AppTransitionTests extends WindowTestsBase {
task.getBounds(taskBounds);
taskFragment.setBounds(0, 0, taskBounds.right / 2, taskBounds.bottom);
spyOn(taskFragment);
mockSurfaceFreezerSnapshot(taskFragment.mSurfaceFreezer);
assertTrue(mDc.mChangingContainers.isEmpty());
assertFalse(mDc.mAppTransition.isTransitionSet());

1
services/tests/wmtests/src/com/android/server/wm/TaskFragmentTest.java
View File

@@ -82,6 +82,7 @@ public class TaskFragmentTest extends WindowTestsBase {
@Test
public void testStartChangeTransition_resetSurface() {
mockSurfaceFreezerSnapshot(mTaskFragment.mSurfaceFreezer);
final Rect startBounds = new Rect(0, 0, 1000, 1000);
final Rect endBounds = new Rect(500, 500, 1000, 1000);
mTaskFragment.setBounds(startBounds);

46
services/tests/wmtests/src/com/android/server/wm/WindowContainerTests.java
View File

@@ -1118,16 +1118,15 @@ public class WindowContainerTests extends WindowTestsBase {
final SurfaceControl.Transaction t = mock(SurfaceControl.Transaction.class);
spyOn(container);
spyOn(surfaceAnimator);
spyOn(surfaceFreezer);
mockSurfaceFreezerSnapshot(surfaceFreezer);
doReturn(t).when(container).getPendingTransaction();
doReturn(t).when(container).getSyncTransaction();
// Leash and snapshot created for change transition.
container.initializeChangeTransition(new Rect(0, 0, 1000, 2000));
// Can't really take a snapshot, manually set one.
surfaceFreezer.mSnapshot = mock(SurfaceFreezer.Snapshot.class);
assertNotNull(surfaceFreezer.mLeash);
assertNotNull(surfaceFreezer.mSnapshot);
assertEquals(surfaceFreezer.mLeash, container.getAnimationLeash());
// Start animation: surfaceAnimator take over the leash and snapshot from surfaceFreezer.
@@ -1144,9 +1143,9 @@ public class WindowContainerTests extends WindowTestsBase {
// Prepare another change transition.
container.initializeChangeTransition(new Rect(0, 0, 1000, 2000));
surfaceFreezer.mSnapshot = mock(SurfaceFreezer.Snapshot.class);
assertNotNull(surfaceFreezer.mLeash);
assertNotNull(surfaceFreezer.mSnapshot);
assertEquals(surfaceFreezer.mLeash, container.getAnimationLeash());
assertNotEquals(prevLeash, container.getAnimationLeash());
@@ -1173,12 +1172,43 @@ public class WindowContainerTests extends WindowTestsBase {
assertNull(surfaceAnimator.mSnapshot);
}
@Test
public void testUnfreezeWindow_removeWindowFromChanging() {
final WindowContainer container = createTaskFragmentWithParentTask(
createTask(mDisplayContent), false);
mockSurfaceFreezerSnapshot(container.mSurfaceFreezer);
final SurfaceControl.Transaction t = mock(SurfaceControl.Transaction.class);
container.initializeChangeTransition(new Rect(0, 0, 1000, 2000));
assertTrue(mDisplayContent.mChangingContainers.contains(container));
container.mSurfaceFreezer.unfreeze(t);
assertFalse(mDisplayContent.mChangingContainers.contains(container));
}
@Test
public void testFailToTaskSnapshot_unfreezeWindow() {
final WindowContainer container = createTaskFragmentWithParentTask(
createTask(mDisplayContent), false);
final SurfaceControl.Transaction t = mock(SurfaceControl.Transaction.class);
spyOn(container.mSurfaceFreezer);
container.initializeChangeTransition(new Rect(0, 0, 1000, 2000));
verify(container.mSurfaceFreezer).freeze(any(), any(), any(), any());
verify(container.mSurfaceFreezer).unfreeze(any());
assertTrue(mDisplayContent.mChangingContainers.isEmpty());
}
@Test
public void testRemoveUnstartedFreezeSurfaceWhenFreezeAgain() {
final WindowContainer container = createTaskFragmentWithParentTask(
createTask(mDisplayContent), false);
container.mSurfaceControl = mock(SurfaceControl.class);
final SurfaceFreezer surfaceFreezer = container.mSurfaceFreezer;
mockSurfaceFreezerSnapshot(surfaceFreezer);
final SurfaceControl.Transaction t = mock(SurfaceControl.Transaction.class);
spyOn(container);
doReturn(t).when(container).getPendingTransaction();
@@ -1188,16 +1218,16 @@ public class WindowContainerTests extends WindowTestsBase {
container.initializeChangeTransition(new Rect(0, 0, 1000, 2000));
assertNotNull(surfaceFreezer.mLeash);
assertNotNull(surfaceFreezer.mSnapshot);
// Can't really take a snapshot, manually set one.
final SurfaceFreezer.Snapshot snapshot = mock(SurfaceFreezer.Snapshot.class);
surfaceFreezer.mSnapshot = snapshot;
final SurfaceControl prevLeash = surfaceFreezer.mLeash;
final SurfaceFreezer.Snapshot prevSnapshot = surfaceFreezer.mSnapshot;
spyOn(prevSnapshot);
container.initializeChangeTransition(new Rect(0, 0, 1500, 2500));
verify(t).remove(prevLeash);
verify(snapshot).destroy(t);
verify(prevSnapshot).destroy(t);
}
/* Used so we can gain access to some protected members of the {@link WindowContainer} class */

16
services/tests/wmtests/src/com/android/server/wm/WindowTestsBase.java
View File

@@ -77,6 +77,7 @@ import android.content.Context;
import android.content.Intent;
import android.content.pm.ActivityInfo;
import android.content.pm.ApplicationInfo;
import android.hardware.HardwareBuffer;
import android.hardware.display.DisplayManager;
import android.os.Build;
import android.os.Bundle;
@@ -878,6 +879,21 @@ class WindowTestsBase extends SystemServiceTestsBase {
mAtm.mRootWindowContainer.mDefaultMinSizeOfResizeableTaskDp = 1;
}
/** Mocks the behavior of taking a snapshot. */
void mockSurfaceFreezerSnapshot(SurfaceFreezer surfaceFreezer) {
final SurfaceControl.ScreenshotHardwareBuffer screenshotBuffer =
mock(SurfaceControl.ScreenshotHardwareBuffer.class);
final HardwareBuffer hardwareBuffer = mock(HardwareBuffer.class);
spyOn(surfaceFreezer);
doReturn(screenshotBuffer).when(surfaceFreezer)
.createSnapshotBufferInner(any(), any());
doReturn(null).when(surfaceFreezer)
.createFromHardwareBufferInner(any());
doReturn(hardwareBuffer).when(screenshotBuffer).getHardwareBuffer();
doReturn(100).when(hardwareBuffer).getWidth();
doReturn(100).when(hardwareBuffer).getHeight();
}
/**
* Builder for creating new activities.
*/