From bc7b86dc2c871e412bd3dec7e670434075de84a5 Mon Sep 17 00:00:00 2001 From: David Samuelson Date: Fri, 22 Apr 2022 19:29:12 +0000 Subject: Ensure GameSession is not destroyed when restarting game. Previously when restarting the game via `GameSession.restartGame()` the assocaited GameSession would be destroyed, and then quickly recreated as the game's task was reported as removed and created (via TaskStackListener). In addition, any Activities that were in the task stack of the game where not removed, leading to the newly launched game's Activity to be placed above them in the task stack. This could led to lead to strange behavior, as it is expected that the game's Activity to be the lowest in the task stack. For example, closing the game's Activity would not close the task as the user would expect, instead revealing the previously launched Activites below it. To fix this issue, when restarting the game via `GameSession.restartGame` the game's task stack is no longer changed. Instead the game's Activity (and process) is restarted. As an added benifit this solution also gives the game an opportunity to save their instance state. Test: Manually verified Bug: 228842686 Bug: 202414447 Bug: 202417255 Change-Id: Iaf66de05ad6198f4f4ba4f526c171b83fcba234a --- .../GameServiceProviderInstanceFactoryImpl.java | 2 + .../app/GameServiceProviderInstanceImpl.java | 43 +++++++++++++++------- .../server/wm/ActivityTaskManagerInternal.java | 11 ++++++ .../server/wm/ActivityTaskManagerService.java | 26 +++++++++++++ .../app/GameServiceProviderInstanceImplTest.java | 27 +++++++------- 5 files changed, 82 insertions(+), 27 deletions(-) diff --git a/services/core/java/com/android/server/app/GameServiceProviderInstanceFactoryImpl.java b/services/core/java/com/android/server/app/GameServiceProviderInstanceFactoryImpl.java index 90b1b634f2ee..50e529c66e65 100644 --- a/services/core/java/com/android/server/app/GameServiceProviderInstanceFactoryImpl.java +++ b/services/core/java/com/android/server/app/GameServiceProviderInstanceFactoryImpl.java @@ -35,6 +35,7 @@ import com.android.internal.os.BackgroundThread; import com.android.internal.util.ScreenshotHelper; import com.android.server.LocalServices; import com.android.server.app.GameServiceConfiguration.GameServiceComponentConfiguration; +import com.android.server.wm.ActivityTaskManagerInternal; import com.android.server.wm.WindowManagerInternal; import com.android.server.wm.WindowManagerService; @@ -62,6 +63,7 @@ final class GameServiceProviderInstanceFactoryImpl implements GameServiceProvide activityTaskManager, (WindowManagerService) ServiceManager.getService(Context.WINDOW_SERVICE), LocalServices.getService(WindowManagerInternal.class), + LocalServices.getService(ActivityTaskManagerInternal.class), new GameServiceConnector(mContext, configuration), new GameSessionServiceConnector(mContext, configuration), new ScreenshotHelper(mContext)); diff --git a/services/core/java/com/android/server/app/GameServiceProviderInstanceImpl.java b/services/core/java/com/android/server/app/GameServiceProviderInstanceImpl.java index a200067b8f3f..4fd739ca5e3e 100644 --- a/services/core/java/com/android/server/app/GameServiceProviderInstanceImpl.java +++ b/services/core/java/com/android/server/app/GameServiceProviderInstanceImpl.java @@ -29,7 +29,6 @@ import android.app.IProcessObserver; import android.app.TaskStackListener; import android.content.ComponentName; import android.content.Context; -import android.content.Intent; import android.graphics.Bitmap; import android.graphics.Insets; import android.graphics.Rect; @@ -60,6 +59,7 @@ import com.android.internal.infra.ServiceConnector; import com.android.internal.infra.ServiceConnector.ServiceLifecycleCallbacks; import com.android.internal.os.BackgroundThread; import com.android.internal.util.ScreenshotHelper; +import com.android.server.wm.ActivityTaskManagerInternal; import com.android.server.wm.WindowManagerInternal; import com.android.server.wm.WindowManagerInternal.TaskSystemBarsListener; import com.android.server.wm.WindowManagerService; @@ -93,6 +93,9 @@ final class GameServiceProviderInstanceImpl implements GameServiceProviderInstan public void onBinderDied() { mBackgroundExecutor.execute(() -> { synchronized (mLock) { + if (DEBUG) { + Slog.d(TAG, "GameSessionService died. Destroying all sessions"); + } destroyAndClearAllGameSessionsLocked(); } }); @@ -223,6 +226,7 @@ final class GameServiceProviderInstanceImpl implements GameServiceProviderInstan private final IActivityTaskManager mActivityTaskManager; private final WindowManagerService mWindowManagerService; private final WindowManagerInternal mWindowManagerInternal; + private final ActivityTaskManagerInternal mActivityTaskManagerInternal; private final ScreenshotHelper mScreenshotHelper; private final ServiceConnector mGameServiceConnector; private final ServiceConnector mGameSessionServiceConnector; @@ -249,6 +253,7 @@ final class GameServiceProviderInstanceImpl implements GameServiceProviderInstan @NonNull IActivityTaskManager activityTaskManager, @NonNull WindowManagerService windowManagerService, @NonNull WindowManagerInternal windowManagerInternal, + @NonNull ActivityTaskManagerInternal activityTaskManagerInternal, @NonNull ServiceConnector gameServiceConnector, @NonNull ServiceConnector gameSessionServiceConnector, @NonNull ScreenshotHelper screenshotHelper) { @@ -261,6 +266,7 @@ final class GameServiceProviderInstanceImpl implements GameServiceProviderInstan mActivityTaskManager = activityTaskManager; mWindowManagerService = windowManagerService; mWindowManagerInternal = windowManagerInternal; + mActivityTaskManagerInternal = activityTaskManagerInternal; mGameServiceConnector = gameServiceConnector; mGameSessionServiceConnector = gameSessionServiceConnector; mScreenshotHelper = screenshotHelper; @@ -443,6 +449,11 @@ final class GameServiceProviderInstanceImpl implements GameServiceProviderInstan return; } + + if (DEBUG) { + Slog.i(TAG, "onTaskRemoved() id: " + taskId); + } + removeAndDestroyGameSessionIfNecessaryLocked(taskId); } } @@ -775,10 +786,20 @@ final class GameServiceProviderInstanceImpl implements GameServiceProviderInstan if (gameSessionRecord.getGameSession() != null && packageName.equals( gameSessionRecord.getComponentName().getPackageName())) { if (DEBUG) { - Slog.d(TAG, "endGameSessionsForPackageLocked(): No more processes for " + Slog.i(TAG, "endGameSessionsForPackageLocked(): No more processes for " + packageName + ", ending game session with taskId: " + gameSessionRecord.getTaskId()); } + + RunningTaskInfo runningTaskInfo = + mGameTaskInfoProvider.getRunningTaskInfo(gameSessionRecord.getTaskId()); + if (runningTaskInfo != null && (runningTaskInfo.isVisible)) { + if (DEBUG) { + Slog.i(TAG, "Found visible task. Ignoring end game session. taskId:" + + gameSessionRecord.getTaskId()); + } + continue; + } mGameSessions.put(gameSessionRecord.getTaskId(), gameSessionRecord.withGameSessionEndedOnProcessDeath()); destroyGameSessionFromRecordLocked(gameSessionRecord); @@ -867,24 +888,18 @@ final class GameServiceProviderInstanceImpl implements GameServiceProviderInstan private void restartGame(int taskId) { String packageName; - synchronized (mLock) { - boolean isTaskAssociatedWithGameSession = mGameSessions.containsKey(taskId); - if (!isTaskAssociatedWithGameSession) { + GameSessionRecord gameSessionRecord = mGameSessions.get(taskId); + if (gameSessionRecord == null) { return; } - - packageName = mGameSessions.get(taskId).getComponentName().getPackageName(); + packageName = gameSessionRecord.getComponentName().getPackageName(); } - try { - mActivityManager.forceStopPackage(packageName, UserHandle.USER_CURRENT); - } catch (RemoteException e) { - e.rethrowFromSystemServer(); + if (packageName == null) { + return; } - Intent launchIntent = - mContext.getPackageManager().getLaunchIntentForPackage(packageName); - mContext.startActivity(launchIntent); + mActivityTaskManagerInternal.restartTaskActivityProcessIfVisible(taskId, packageName); } } diff --git a/services/core/java/com/android/server/wm/ActivityTaskManagerInternal.java b/services/core/java/com/android/server/wm/ActivityTaskManagerInternal.java index a4b216fc7ff8..f75d73b04476 100644 --- a/services/core/java/com/android/server/wm/ActivityTaskManagerInternal.java +++ b/services/core/java/com/android/server/wm/ActivityTaskManagerInternal.java @@ -693,4 +693,15 @@ public abstract class ActivityTaskManagerInternal { * @return a task ID if a valid task ID is found. Otherwise, return INVALID_TASK_ID */ public abstract int getTaskToShowPermissionDialogOn(String pkgName, int uid); + + /** + * Attempts to restart the process associated with the top most Activity associated with the + * given {@code packageName} in the task associated with the given {@code taskId}. + * + * This will request the process of the activity to restart with its saved state (via + * {@link android.app.Activity#onSaveInstanceState(Bundle)}) if possible. If the activity is in + * background the process will be killed keeping its record. + */ + public abstract void restartTaskActivityProcessIfVisible( + int taskId, @NonNull String packageName); } diff --git a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java index 41b724f2596f..42b5d0325484 100644 --- a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java +++ b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java @@ -6705,5 +6705,31 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { .getTaskToShowPermissionDialogOn(pkgName, uid); } } + + @Override + public void restartTaskActivityProcessIfVisible(int taskId, String packageName) { + synchronized (ActivityTaskManagerService.this.mGlobalLock) { + final Task task = + ActivityTaskManagerService.this.mRootWindowContainer + .anyTaskForId(taskId, MATCH_ATTACHED_TASK_ONLY); + if (task == null) { + Slog.w(TAG, "Failed to restart Activity. No task found for id: " + taskId); + return; + } + + final ActivityRecord activity = task.getActivity(activityRecord -> { + return packageName.equals(activityRecord.packageName) + && !activityRecord.finishing; + }); + + if (activity == null) { + Slog.w(TAG, "Failed to restart Activity. No Activity found for package name: " + + packageName + " in task: " + taskId); + return; + } + + activity.restartProcessIfVisible(); + } + } } } diff --git a/services/tests/mockingservicestests/src/com/android/server/app/GameServiceProviderInstanceImplTest.java b/services/tests/mockingservicestests/src/com/android/server/app/GameServiceProviderInstanceImplTest.java index 5b551b1c183c..a8b340c6a391 100644 --- a/services/tests/mockingservicestests/src/com/android/server/app/GameServiceProviderInstanceImplTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/app/GameServiceProviderInstanceImplTest.java @@ -32,7 +32,9 @@ import static org.junit.Assert.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.never; import android.Manifest; import android.annotation.Nullable; @@ -80,6 +82,7 @@ import com.android.internal.util.ConcurrentUtils; import com.android.internal.util.FunctionalUtils.ThrowingConsumer; import com.android.internal.util.Preconditions; import com.android.internal.util.ScreenshotHelper; +import com.android.server.wm.ActivityTaskManagerInternal; import com.android.server.wm.WindowManagerInternal; import com.android.server.wm.WindowManagerInternal.TaskSystemBarsListener; import com.android.server.wm.WindowManagerService; @@ -125,6 +128,7 @@ public final class GameServiceProviderInstanceImplTest { private static final Bitmap TEST_BITMAP; + static { Picture picture = new Picture(); Canvas canvas = picture.beginRecording(200, 100); @@ -146,6 +150,8 @@ public final class GameServiceProviderInstanceImplTest { @Mock private WindowManagerInternal mMockWindowManagerInternal; @Mock + private ActivityTaskManagerInternal mActivityTaskManagerInternal; + @Mock private IActivityManager mMockActivityManager; @Mock private ScreenshotHelper mMockScreenshotHelper; @@ -227,6 +233,7 @@ public final class GameServiceProviderInstanceImplTest { mMockActivityTaskManager, mMockWindowManagerService, mMockWindowManagerInternal, + mActivityTaskManagerInternal, mFakeGameServiceConnector, mFakeGameSessionServiceConnector, mMockScreenshotHelper); @@ -1134,8 +1141,9 @@ public final class GameServiceProviderInstanceImplTest { mFakeGameSessionService.getCapturedCreateInvocations().get(0) .mGameSessionController.restartGame(10); - verify(mMockActivityManager).forceStopPackage(GAME_A_PACKAGE, UserHandle.USER_CURRENT); - assertThat(mMockContext.getLastStartedIntent()).isEqualTo(launchIntent); + verify(mActivityTaskManagerInternal).restartTaskActivityProcessIfVisible( + 10, + GAME_A_PACKAGE); } @Test @@ -1158,7 +1166,8 @@ public final class GameServiceProviderInstanceImplTest { verify(mMockActivityManager).registerProcessObserver(any()); verifyNoMoreInteractions(mMockActivityManager); - assertThat(mMockContext.getLastStartedIntent()).isNull(); + verify(mActivityTaskManagerInternal, never()) + .restartTaskActivityProcessIfVisible(anyInt(), anyString()); } @Test @@ -1172,6 +1181,8 @@ public final class GameServiceProviderInstanceImplTest { mockPermissionDenied(Manifest.permission.MANAGE_GAME_ACTIVITY); assertThrows(SecurityException.class, () -> gameSessionController.restartGame(10)); + verify(mActivityTaskManagerInternal, never()) + .restartTaskActivityProcessIfVisible(anyInt(), anyString()); } private void startTask(int taskId, ComponentName componentName) { @@ -1398,7 +1409,6 @@ public final class GameServiceProviderInstanceImplTest { } private final class MockContext extends ContextWrapper { - private Intent mLastStartedIntent; // Map of permission name -> PermissionManager.Permission_{GRANTED|DENIED} constant private final HashMap mMockedPermissions = new HashMap<>(); @@ -1424,11 +1434,6 @@ public final class GameServiceProviderInstanceImplTest { return mMockPackageManager; } - @Override - public void startActivity(Intent intent) { - mLastStartedIntent = intent; - } - @Override public void enforceCallingPermission(String permission, @Nullable String message) { final Integer granted = mMockedPermissions.get(permission); @@ -1441,9 +1446,5 @@ public final class GameServiceProviderInstanceImplTest { throw new SecurityException("[Test] permission denied: " + permission); } } - - Intent getLastStartedIntent() { - return mLastStartedIntent; - } } } -- cgit v1.2.3-59-g8ed1b