From 8b096b2d952938ab386fab60ff25303d683a05bf Mon Sep 17 00:00:00 2001 From: Garfield Tan Date: Tue, 7 Jan 2020 14:55:20 -0800 Subject: [PATCH] Pass activity stack when saving launch params. When setParent() is called on a task with null as a step to remove it mParent in WindowContainer is already set to null before onParentChanged() is called. Therefore getStack() returns null and we'll get an NPE. Thus pass ActivityStack into it to avoid that. Bug: 147301825 Test: NPE disappears when a task is closed. Test: atest TestRecordTest Change-Id: I94e81f7f22d348b73eac70edd0998be721ad2597 --- .../server/wm/LaunchParamsPersister.java | 15 ++-- .../core/java/com/android/server/wm/Task.java | 23 ++++-- .../wm/LaunchParamsControllerTests.java | 2 +- .../android/server/wm/TaskRecordTests.java | 78 +++++++++++++++++++ 4 files changed, 104 insertions(+), 14 deletions(-) diff --git a/services/core/java/com/android/server/wm/LaunchParamsPersister.java b/services/core/java/com/android/server/wm/LaunchParamsPersister.java index f4e608e355dd2..660706e4139bd 100644 --- a/services/core/java/com/android/server/wm/LaunchParamsPersister.java +++ b/services/core/java/com/android/server/wm/LaunchParamsPersister.java @@ -197,6 +197,10 @@ class LaunchParamsPersister { } void saveTask(Task task) { + saveTask(task, task.getDisplayContent()); + } + + void saveTask(Task task, DisplayContent display) { final ComponentName name = task.realActivity; final int userId = task.mUserId; PersistableLaunchParams params; @@ -211,7 +215,7 @@ class LaunchParamsPersister { params = new PersistableLaunchParams(); map.put(name, params); } - final boolean changed = saveTaskToLaunchParam(task, params); + final boolean changed = saveTaskToLaunchParam(task, display, params); if (changed) { mPersisterQueue.updateLastOrAddItem( @@ -220,17 +224,16 @@ class LaunchParamsPersister { } } - private boolean saveTaskToLaunchParam(Task task, PersistableLaunchParams params) { - final ActivityStack stack = task.getStack(); - final DisplayContent display = stack.getDisplayContent(); + private boolean saveTaskToLaunchParam( + Task task, DisplayContent display, PersistableLaunchParams params) { final DisplayInfo info = new DisplayInfo(); display.mDisplay.getDisplayInfo(info); boolean changed = !Objects.equals(params.mDisplayUniqueId, info.uniqueId); params.mDisplayUniqueId = info.uniqueId; - changed |= params.mWindowingMode != stack.getWindowingMode(); - params.mWindowingMode = stack.getWindowingMode(); + changed |= params.mWindowingMode != task.getWindowingMode(); + params.mWindowingMode = task.getWindowingMode(); if (task.mLastNonFullscreenBounds != null) { changed |= !Objects.equals(params.mBounds, task.mLastNonFullscreenBounds); diff --git a/services/core/java/com/android/server/wm/Task.java b/services/core/java/com/android/server/wm/Task.java index 9e6cb6884820c..4fca8bebb2dab 100644 --- a/services/core/java/com/android/server/wm/Task.java +++ b/services/core/java/com/android/server/wm/Task.java @@ -574,13 +574,19 @@ class Task extends WindowContainer { mAtmService.getTaskChangeNotificationController().notifyTaskCreated(_taskId, realActivity); } - private void cleanUpResourcesForDestroy() { + private void cleanUpResourcesForDestroy(ConfigurationContainer oldParent) { if (hasChild()) { return; } - // This task is going away, so save the last state if necessary. - saveLaunchingStateIfNeeded(); + // TODO(xutan): Removed type check after stack and task is merged. + // Before the real merge of stack and task, we need to avoid saving state of stacks. Once + // the merge is finished we can just pass DisplayContent because both windowing mode and + // bounds are set in the merged task. + if (oldParent instanceof ActivityStack) { + // This task is going away, so save the last state if necessary. + saveLaunchingStateIfNeeded(((WindowContainer) oldParent).getDisplayContent()); + } // TODO: VI what about activity? final boolean isVoiceSession = voiceSession != null; @@ -1053,9 +1059,8 @@ class Task extends WindowContainer { mPrevDisplayId = (oldDisplay != null) ? oldDisplay.mDisplayId : INVALID_DISPLAY; - // Task is going to be removed, clean it up before detaching from hierarchy. if (oldParent != null && newParent == null) { - cleanUpResourcesForDestroy(); + cleanUpResourcesForDestroy(oldParent); } if (display != null) { @@ -1884,7 +1889,11 @@ class Task extends WindowContainer { * It only saves state if this task has been shown to user and it's in fullscreen or freeform * mode on freeform displays. */ - void saveLaunchingStateIfNeeded() { + private void saveLaunchingStateIfNeeded() { + saveLaunchingStateIfNeeded(getDisplayContent()); + } + + private void saveLaunchingStateIfNeeded(DisplayContent display) { if (!hasBeenVisible) { // Not ever visible to user. return; @@ -1904,7 +1913,7 @@ class Task extends WindowContainer { } // Saves the new state so that we can launch the activity at the same location. - mStackSupervisor.mLaunchParamsPersister.saveTask(this); + mStackSupervisor.mLaunchParamsPersister.saveTask(this, display); } /** diff --git a/services/tests/wmtests/src/com/android/server/wm/LaunchParamsControllerTests.java b/services/tests/wmtests/src/com/android/server/wm/LaunchParamsControllerTests.java index cf1f0a822232b..1f815ff6893d4 100644 --- a/services/tests/wmtests/src/com/android/server/wm/LaunchParamsControllerTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/LaunchParamsControllerTests.java @@ -421,7 +421,7 @@ public class LaunchParamsControllerTests extends ActivityTestsBase { } @Override - void saveTask(Task task) { + void saveTask(Task task, DisplayContent display) { final int userId = task.mUserId; final ComponentName realActivity = task.realActivity; mTmpParams.mPreferredDisplayId = task.getDisplayId(); diff --git a/services/tests/wmtests/src/com/android/server/wm/TaskRecordTests.java b/services/tests/wmtests/src/com/android/server/wm/TaskRecordTests.java index fa0485c8678b9..ebf14d2c6a191 100644 --- a/services/tests/wmtests/src/com/android/server/wm/TaskRecordTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/TaskRecordTests.java @@ -20,6 +20,8 @@ import static android.app.ActivityTaskManager.INVALID_TASK_ID; import static android.app.WindowConfiguration.ACTIVITY_TYPE_STANDARD; import static android.app.WindowConfiguration.WINDOWING_MODE_FREEFORM; import static android.app.WindowConfiguration.WINDOWING_MODE_FULLSCREEN; +import static android.app.WindowConfiguration.WINDOWING_MODE_PINNED; +import static android.app.WindowConfiguration.WINDOWING_MODE_UNDEFINED; import static android.content.Intent.FLAG_ACTIVITY_NEW_TASK; import static android.content.Intent.FLAG_ACTIVITY_TASK_ON_HOME; import static android.content.pm.ActivityInfo.FLAG_RELINQUISH_TASK_IDENTITY; @@ -34,6 +36,7 @@ import static android.view.Surface.ROTATION_90; import static com.android.dx.mockito.inline.extended.ExtendedMockito.doAnswer; import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn; import static com.android.dx.mockito.inline.extended.ExtendedMockito.eq; +import static com.android.dx.mockito.inline.extended.ExtendedMockito.never; import static com.android.dx.mockito.inline.extended.ExtendedMockito.spyOn; import static com.android.dx.mockito.inline.extended.ExtendedMockito.verify; import static com.android.server.policy.WindowManagerPolicy.USER_ROTATION_FREE; @@ -51,10 +54,13 @@ import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.same; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import android.app.ActivityManager; import android.app.TaskInfo; +import android.app.WindowConfiguration; import android.content.ComponentName; import android.content.Intent; import android.content.pm.ActivityInfo; @@ -859,6 +865,78 @@ public class TaskRecordTests extends ActivityTestsBase { verify(task).setIntent(eq(activity0)); } + @Test + public void testSaveLaunchingStateWhenConfigurationChanged() { + LaunchParamsPersister persister = mService.mStackSupervisor.mLaunchParamsPersister; + spyOn(persister); + + final Task task = getTestTask(); + task.hasBeenVisible = false; + task.getDisplayContent().setDisplayWindowingMode(WINDOWING_MODE_FREEFORM); + task.getStack().setWindowingMode(WINDOWING_MODE_FULLSCREEN); + + task.hasBeenVisible = true; + task.onConfigurationChanged(task.getParent().getConfiguration()); + + verify(persister).saveTask(task, task.getDisplayContent()); + } + + @Test + public void testSaveLaunchingStateWhenClearingParent() { + LaunchParamsPersister persister = mService.mStackSupervisor.mLaunchParamsPersister; + spyOn(persister); + + final Task task = getTestTask(); + task.hasBeenVisible = false; + task.getDisplayContent().setWindowingMode(WindowConfiguration.WINDOWING_MODE_FREEFORM); + task.getStack().setWindowingMode(WINDOWING_MODE_FULLSCREEN); + final DisplayContent oldDisplay = task.getDisplayContent(); + + LaunchParamsController.LaunchParams params = new LaunchParamsController.LaunchParams(); + params.mWindowingMode = WINDOWING_MODE_UNDEFINED; + persister.getLaunchParams(task, null, params); + assertEquals(WINDOWING_MODE_UNDEFINED, params.mWindowingMode); + + task.hasBeenVisible = true; + task.removeImmediately(); + + verify(persister).saveTask(task, oldDisplay); + + persister.getLaunchParams(task, null, params); + assertEquals(WINDOWING_MODE_FULLSCREEN, params.mWindowingMode); + } + + @Test + public void testNotSaveLaunchingStateNonFreeformDisplay() { + LaunchParamsPersister persister = mService.mStackSupervisor.mLaunchParamsPersister; + spyOn(persister); + + final Task task = getTestTask(); + task.hasBeenVisible = false; + task.getStack().setWindowingMode(WINDOWING_MODE_FULLSCREEN); + + task.hasBeenVisible = true; + task.onConfigurationChanged(task.getParent().getConfiguration()); + + verify(persister, never()).saveTask(same(task), any()); + } + + @Test + public void testNotSaveLaunchingStateWhenNotFullscreenOrFreeformWindow() { + LaunchParamsPersister persister = mService.mStackSupervisor.mLaunchParamsPersister; + spyOn(persister); + + final Task task = getTestTask(); + task.hasBeenVisible = false; + task.getDisplayContent().setDisplayWindowingMode(WINDOWING_MODE_FREEFORM); + task.getStack().setWindowingMode(WINDOWING_MODE_PINNED); + + task.hasBeenVisible = true; + task.onConfigurationChanged(task.getParent().getConfiguration()); + + verify(persister, never()).saveTask(same(task), any()); + } + private Task getTestTask() { final ActivityStack stack = new StackBuilder(mRootWindowContainer).build(); return stack.getBottomMostTask();