From 5a36e6bc9ec40d8e335c59551a0223c5c485d4b5 Mon Sep 17 00:00:00 2001 From: Louis Chang Date: Tue, 21 Jun 2022 04:19:33 +0000 Subject: [PATCH] Prevent waiting if no new activity instance launched An existing task can be brought to front while starting an Intent. In that case, the launching TaskFragment will remain empty because no new activity instance created. Monitor the activity launch result and prevent waiting if the activity instance was not created and cleared the pending appeared intent. Bug: 233989810 Test: atest TaskFragmentContainerTest Test: atest SplitControllerTest Change-Id: I1bdf516673b71892921da14bdcdb9d1c23ce0a9a --- core/java/android/app/Instrumentation.java | 39 +++++++++++++++++++ .../extensions/embedding/SplitController.java | 33 +++++++++++++++- .../embedding/TaskFragmentContainer.java | 27 +++++++++++-- .../embedding/SplitControllerTest.java | 21 ++++++++++ .../embedding/TaskFragmentContainerTest.java | 17 ++++---- 5 files changed, 124 insertions(+), 13 deletions(-) diff --git a/core/java/android/app/Instrumentation.java b/core/java/android/app/Instrumentation.java index 8984c4292023b..556058b567f99 100644 --- a/core/java/android/app/Instrumentation.java +++ b/core/java/android/app/Instrumentation.java @@ -783,6 +783,17 @@ public class Instrumentation { return null; } + /** + * This is called after starting an Activity and provides the result code that defined in + * {@link ActivityManager}, like {@link ActivityManager#START_SUCCESS}. + * + * @param result the result code that returns after starting an Activity. + * @param bOptions the bundle generated from {@link ActivityOptions} that originally + * being used to start the Activity. + * @hide + */ + public void onStartActivityResult(int result, @NonNull Bundle bOptions) {} + final boolean match(Context who, Activity activity, Intent intent) { @@ -1344,6 +1355,28 @@ public class Instrumentation { return apk.getAppFactory(); } + /** + * This should be called before {@link #checkStartActivityResult(int, Object)}, because + * exceptions might be thrown while checking the results. + */ + private void notifyStartActivityResult(int result, @Nullable Bundle options) { + if (mActivityMonitors == null) { + return; + } + synchronized (mSync) { + final int size = mActivityMonitors.size(); + for (int i = 0; i < size; i++) { + final ActivityMonitor am = mActivityMonitors.get(i); + if (am.ignoreMatchingSpecificIntents()) { + if (options == null) { + options = ActivityOptions.makeBasic().toBundle(); + } + am.onStartActivityResult(result, options); + } + } + } + } + private void prePerformCreate(Activity activity) { if (mWaitingActivities != null) { synchronized (mSync) { @@ -1802,6 +1835,7 @@ public class Instrumentation { who.getOpPackageName(), who.getAttributionTag(), intent, intent.resolveTypeIfNeeded(who.getContentResolver()), token, target != null ? target.mEmbeddedID : null, requestCode, 0, null, options); + notifyStartActivityResult(result, options); checkStartActivityResult(result, intent); } catch (RemoteException e) { throw new RuntimeException("Failure from system", e); @@ -1876,6 +1910,7 @@ public class Instrumentation { int result = ActivityTaskManager.getService().startActivities(whoThread, who.getOpPackageName(), who.getAttributionTag(), intents, resolvedTypes, token, options, userId); + notifyStartActivityResult(result, options); checkStartActivityResult(result, intents[0]); return result; } catch (RemoteException e) { @@ -1947,6 +1982,7 @@ public class Instrumentation { who.getOpPackageName(), who.getAttributionTag(), intent, intent.resolveTypeIfNeeded(who.getContentResolver()), token, target, requestCode, 0, null, options); + notifyStartActivityResult(result, options); checkStartActivityResult(result, intent); } catch (RemoteException e) { throw new RuntimeException("Failure from system", e); @@ -2017,6 +2053,7 @@ public class Instrumentation { who.getOpPackageName(), who.getAttributionTag(), intent, intent.resolveTypeIfNeeded(who.getContentResolver()), token, resultWho, requestCode, 0, null, options, user.getIdentifier()); + notifyStartActivityResult(result, options); checkStartActivityResult(result, intent); } catch (RemoteException e) { throw new RuntimeException("Failure from system", e); @@ -2068,6 +2105,7 @@ public class Instrumentation { token, target != null ? target.mEmbeddedID : null, requestCode, 0, null, options, ignoreTargetSecurity, userId); + notifyStartActivityResult(result, options); checkStartActivityResult(result, intent); } catch (RemoteException e) { throw new RuntimeException("Failure from system", e); @@ -2115,6 +2153,7 @@ public class Instrumentation { int result = appTask.startActivity(whoThread.asBinder(), who.getOpPackageName(), who.getAttributionTag(), intent, intent.resolveTypeIfNeeded(who.getContentResolver()), options); + notifyStartActivityResult(result, options); checkStartActivityResult(result, intent); } catch (RemoteException e) { throw new RuntimeException("Failure from system", e); diff --git a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitController.java b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitController.java index 2f79caeec7ba0..da9fd0c2d96f0 100644 --- a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitController.java +++ b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitController.java @@ -16,6 +16,7 @@ package androidx.window.extensions.embedding; +import static android.app.ActivityManager.START_SUCCESS; import static android.app.WindowConfiguration.WINDOWING_MODE_PINNED; import static android.app.WindowConfiguration.WINDOWING_MODE_UNDEFINED; @@ -97,6 +98,7 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen private final List mLastReportedSplitStates = new ArrayList<>(); private final Handler mHandler; private final Object mLock = new Object(); + private final ActivityStartMonitor mActivityStartMonitor; public SplitController() { final MainThreadExecutor executor = new MainThreadExecutor(); @@ -108,7 +110,8 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen new LifecycleCallbacks()); // Intercept activity starts to route activities to new containers if necessary. Instrumentation instrumentation = activityThread.getInstrumentation(); - instrumentation.addMonitor(new ActivityStartMonitor()); + mActivityStartMonitor = new ActivityStartMonitor(); + instrumentation.addMonitor(mActivityStartMonitor); } /** Updates the embedding rules applied to future activity launches. */ @@ -1385,6 +1388,11 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen return ActivityThread.currentActivityThread().getActivity(activityToken); } + @VisibleForTesting + ActivityStartMonitor getActivityStartMonitor() { + return mActivityStartMonitor; + } + /** * Gets the token of the initial TaskFragment that embedded this activity. Do not rely on it * after creation because the activity could be reparented. @@ -1536,7 +1544,10 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen * A monitor that intercepts all activity start requests originating in the client process and * can amend them to target a specific task fragment to form a split. */ - private class ActivityStartMonitor extends Instrumentation.ActivityMonitor { + @VisibleForTesting + class ActivityStartMonitor extends Instrumentation.ActivityMonitor { + @VisibleForTesting + Intent mCurrentIntent; @Override public Instrumentation.ActivityResult onStartActivity(@NonNull Context who, @@ -1564,11 +1575,29 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen // the dedicated container. options.putBinder(ActivityOptions.KEY_LAUNCH_TASK_FRAGMENT_TOKEN, launchedInTaskFragment.getTaskFragmentToken()); + mCurrentIntent = intent; } } return super.onStartActivity(who, intent, options); } + + @Override + public void onStartActivityResult(int result, @NonNull Bundle bOptions) { + super.onStartActivityResult(result, bOptions); + if (mCurrentIntent != null && result != START_SUCCESS) { + // Clear the pending appeared intent if the activity was not started successfully. + final IBinder token = bOptions.getBinder( + ActivityOptions.KEY_LAUNCH_TASK_FRAGMENT_TOKEN); + if (token != null) { + final TaskFragmentContainer container = getContainer(token); + if (container != null) { + container.clearPendingAppearedIntentIfNeeded(mCurrentIntent); + } + } + } + mCurrentIntent = null; + } } /** diff --git a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskFragmentContainer.java b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskFragmentContainer.java index abf32a26efa2d..a188e2bf49851 100644 --- a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskFragmentContainer.java +++ b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskFragmentContainer.java @@ -198,6 +198,22 @@ class TaskFragmentContainer { return mPendingAppearedIntent; } + void setPendingAppearedIntent(@Nullable Intent intent) { + mPendingAppearedIntent = intent; + } + + /** + * Clears the pending appeared Intent if it is the same as given Intent. Otherwise, the + * pending appeared Intent is cleared when TaskFragmentInfo is set and is not empty (has + * running activities). + */ + void clearPendingAppearedIntentIfNeeded(@NonNull Intent intent) { + if (mPendingAppearedIntent == null || mPendingAppearedIntent != intent) { + return; + } + mPendingAppearedIntent = null; + } + boolean hasActivity(@NonNull IBinder token) { if (mInfo != null && mInfo.getActivities().contains(token)) { return true; @@ -230,13 +246,18 @@ class TaskFragmentContainer { void setInfo(@NonNull TaskFragmentInfo info) { if (!mIsFinished && mInfo == null && info.isEmpty()) { - // onTaskFragmentAppeared with empty info. We will remove the TaskFragment if it is - // still empty after timeout. + // onTaskFragmentAppeared with empty info. We will remove the TaskFragment if no + // pending appeared intent/activities. Otherwise, wait and removing the TaskFragment if + // it is still empty after timeout. mAppearEmptyTimeout = () -> { mAppearEmptyTimeout = null; mController.onTaskFragmentAppearEmptyTimeout(this); }; - mController.getHandler().postDelayed(mAppearEmptyTimeout, APPEAR_EMPTY_TIMEOUT_MS); + if (mPendingAppearedIntent != null || !mPendingAppearedActivities.isEmpty()) { + mController.getHandler().postDelayed(mAppearEmptyTimeout, APPEAR_EMPTY_TIMEOUT_MS); + } else { + mAppearEmptyTimeout.run(); + } } else if (mAppearEmptyTimeout != null && !info.isEmpty()) { mController.getHandler().removeCallbacks(mAppearEmptyTimeout); mAppearEmptyTimeout = null; diff --git a/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/SplitControllerTest.java b/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/SplitControllerTest.java index 042547fd30f21..4bc503369d0e9 100644 --- a/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/SplitControllerTest.java +++ b/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/SplitControllerTest.java @@ -16,6 +16,7 @@ package androidx.window.extensions.embedding; +import static android.app.ActivityManager.START_CANCELED; import static android.app.WindowConfiguration.WINDOWING_MODE_MULTI_WINDOW; import static android.app.WindowConfiguration.WINDOWING_MODE_UNDEFINED; @@ -292,6 +293,26 @@ public class SplitControllerTest { verify(mSplitPresenter).updateSplitContainer(splitContainer, tf, mTransaction); } + @Test + public void testOnStartActivityResultError() { + final Intent intent = new Intent(); + final TaskContainer taskContainer = new TaskContainer(TASK_ID); + final TaskFragmentContainer container = new TaskFragmentContainer(null /* activity */, + intent, taskContainer, mSplitController); + final SplitController.ActivityStartMonitor monitor = + mSplitController.getActivityStartMonitor(); + + container.setPendingAppearedIntent(intent); + final Bundle bundle = new Bundle(); + bundle.putBinder(ActivityOptions.KEY_LAUNCH_TASK_FRAGMENT_TOKEN, + container.getTaskFragmentToken()); + monitor.mCurrentIntent = intent; + doReturn(container).when(mSplitController).getContainer(any()); + + monitor.onStartActivityResult(START_CANCELED, bundle); + assertNull(container.getPendingAppearedIntent()); + } + @Test public void testOnActivityCreated() { mSplitController.onActivityCreated(mActivity); diff --git a/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/TaskFragmentContainerTest.java b/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/TaskFragmentContainerTest.java index 28c2773e25cb1..44c7e6c611dee 100644 --- a/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/TaskFragmentContainerTest.java +++ b/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/TaskFragmentContainerTest.java @@ -209,21 +209,21 @@ public class TaskFragmentContainerTest { assertNull(container.mAppearEmptyTimeout); - // Not set if it is not appeared empty. - final TaskFragmentInfo info = mock(TaskFragmentInfo.class); - doReturn(new ArrayList<>()).when(info).getActivities(); - doReturn(false).when(info).isEmpty(); - container.setInfo(info); - - assertNull(container.mAppearEmptyTimeout); - // Set timeout if the first info set is empty. + final TaskFragmentInfo info = mock(TaskFragmentInfo.class); container.mInfo = null; doReturn(true).when(info).isEmpty(); container.setInfo(info); assertNotNull(container.mAppearEmptyTimeout); + // Not set if it is not appeared empty. + doReturn(new ArrayList<>()).when(info).getActivities(); + doReturn(false).when(info).isEmpty(); + container.setInfo(info); + + assertNull(container.mAppearEmptyTimeout); + // Remove timeout after the container becomes non-empty. doReturn(false).when(info).isEmpty(); container.setInfo(info); @@ -232,6 +232,7 @@ public class TaskFragmentContainerTest { // Running the timeout will call into SplitController.onTaskFragmentAppearEmptyTimeout. container.mInfo = null; + container.setPendingAppearedIntent(mIntent); doReturn(true).when(info).isEmpty(); container.setInfo(info); container.mAppearEmptyTimeout.run();