From b9faa03b905abe3a630491ce0c768328757910cd Mon Sep 17 00:00:00 2001 From: Andrii Kulian Date: Thu, 17 Oct 2019 23:11:54 -0700 Subject: [PATCH] Use START/STOP messages to update visibility Activity visibility messages simply move the activity to STOPPED or STARTED state. We can use the lifecycle messages to do the same and simplify the logic/remove duplicated code. This CL also removes the option to send STOP message without making the client invisible and actually calling onStop(). This option caused a mismatch of the state between server (STOPPED) and client (PAUSED). Also, in cases when the device was going to sleep, STOP message was always followed by SLEEP message, which called onStop() anyway. Bug: 137329632 Bug: 129750406 Test: AM/WM CTS and unit tests Change-Id: I487575520ce301bb2f65519f0c0a30b6b9edac0c --- core/java/android/app/ActivityThread.java | 66 +++------ .../android/app/ClientTransactionHandler.java | 8 +- .../android/app/LocalActivityManager.java | 2 +- ...bilityItem.java => StartActivityItem.java} | 61 ++++---- .../servertransaction/StopActivityItem.java | 20 +-- .../TransactionExecutor.java | 6 +- .../TransactionExecutorHelper.java | 3 +- .../app/activity/ActivityThreadTest.java | 3 +- .../servertransaction/ObjectPoolTests.java | 21 +-- .../TransactionParcelTests.java | 52 +++---- .../activity/ActivityThreadClientTest.java | 140 +----------------- .../com/android/server/wm/ActivityRecord.java | 68 ++++----- .../wm/EnsureActivitiesVisibleHelper.java | 8 +- .../server/wm/ActivityRecordTests.java | 5 +- 14 files changed, 128 insertions(+), 335 deletions(-) rename core/java/android/app/servertransaction/{WindowVisibilityItem.java => StartActivityItem.java} (54%) diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java index a4f6f57c097e9..b82a67556fc08 100644 --- a/core/java/android/app/ActivityThread.java +++ b/core/java/android/app/ActivityThread.java @@ -3349,8 +3349,8 @@ public final class ActivityThread extends ClientTransactionHandler { } @Override - public void handleStartActivity(ActivityClientRecord r, - PendingTransactionActions pendingActions) { + public void handleStartActivity(IBinder token, PendingTransactionActions pendingActions) { + final ActivityClientRecord r = mActivities.get(token); final Activity activity = r.activity; if (r.activity == null) { // TODO(lifecycler): What do we do in this case? @@ -3364,6 +3364,8 @@ public final class ActivityThread extends ClientTransactionHandler { return; } + unscheduleGcIdler(); + // Start activity.performStart("handleStartActivity"); r.setState(ON_START); @@ -3400,6 +3402,9 @@ public final class ActivityThread extends ClientTransactionHandler { + " did not call through to super.onPostCreate()"); } } + + updateVisibility(r, true /* show */); + mSomeActivitiesChanged = true; } /** @@ -4660,8 +4665,8 @@ public final class ActivityThread extends ClientTransactionHandler { @UnsupportedAppUsage final void performStopActivity(IBinder token, boolean saveState, String reason) { ActivityClientRecord r = mActivities.get(token); - performStopActivityInner(r, null /* stopInfo */, false /* keepShown */, saveState, - false /* finalStateRequest */, reason); + performStopActivityInner(r, null /* stopInfo */, saveState, false /* finalStateRequest */, + reason); } private static final class ProviderRefCount { @@ -4687,25 +4692,19 @@ public final class ActivityThread extends ClientTransactionHandler { } /** - * Core implementation of stopping an activity. Note this is a little - * tricky because the server's meaning of stop is slightly different - * than our client -- for the server, stop means to save state and give - * it the result when it is done, but the window may still be visible. - * For the client, we want to call onStop()/onStart() to indicate when - * the activity's UI visibility changes. + * Core implementation of stopping an activity. * @param r Target activity client record. * @param info Action that will report activity stop to server. - * @param keepShown Flag indicating whether the activity is still shown. * @param saveState Flag indicating whether the activity state should be saved. * @param finalStateRequest Flag indicating if this call is handling final lifecycle state * request for a transaction. * @param reason Reason for performing this operation. */ - private void performStopActivityInner(ActivityClientRecord r, StopInfo info, boolean keepShown, + private void performStopActivityInner(ActivityClientRecord r, StopInfo info, boolean saveState, boolean finalStateRequest, String reason) { if (localLOGV) Slog.v(TAG, "Performing stop of " + r); if (r != null) { - if (!keepShown && r.stopped) { + if (r.stopped) { if (r.activity.mFinished) { // If we are finishing, we won't call onResume() in certain // cases. So here we likewise don't want to call onStop() @@ -4740,9 +4739,7 @@ public final class ActivityThread extends ClientTransactionHandler { } } - if (!keepShown) { - callActivityOnStop(r, saveState, reason); - } + callActivityOnStop(r, saveState, reason); } } @@ -4810,20 +4807,19 @@ public final class ActivityThread extends ClientTransactionHandler { } @Override - public void handleStopActivity(IBinder token, boolean show, int configChanges, + public void handleStopActivity(IBinder token, int configChanges, PendingTransactionActions pendingActions, boolean finalStateRequest, String reason) { final ActivityClientRecord r = mActivities.get(token); r.activity.mConfigChangeFlags |= configChanges; final StopInfo stopInfo = new StopInfo(); - performStopActivityInner(r, stopInfo, show, true /* saveState */, finalStateRequest, + performStopActivityInner(r, stopInfo, true /* saveState */, finalStateRequest, reason); if (localLOGV) Slog.v( - TAG, "Finishing stop of " + r + ": show=" + show - + " win=" + r.window); + TAG, "Finishing stop of " + r + ": win=" + r.window); - updateVisibility(r, show); + updateVisibility(r, false); // Make sure any pending writes are now committed. if (!r.isPreHoneycomb()) { @@ -4859,34 +4855,6 @@ public final class ActivityThread extends ClientTransactionHandler { } } - @Override - public void handleWindowVisibility(IBinder token, boolean show) { - ActivityClientRecord r = mActivities.get(token); - - if (r == null) { - Log.w(TAG, "handleWindowVisibility: no activity for token " + token); - return; - } - - if (!show && !r.stopped) { - performStopActivityInner(r, null /* stopInfo */, show, false /* saveState */, - false /* finalStateRequest */, "handleWindowVisibility"); - } else if (show && r.getLifecycleState() == ON_STOP) { - // If we are getting ready to gc after going to the background, well - // we are back active so skip it. - unscheduleGcIdler(); - - r.activity.performRestart(true /* start */, "handleWindowVisibility"); - r.setState(ON_START); - } - if (r.activity.mDecor != null) { - if (false) Slog.v( - TAG, "Handle window " + r + " visibility: " + show); - updateVisibility(r, show); - } - mSomeActivitiesChanged = true; - } - // TODO: This method should be changed to use {@link #performStopActivityInner} to perform to // stop operation on the activity to reduce code duplication and the chance of fixing a bug in // one place and missing the other. diff --git a/core/java/android/app/ClientTransactionHandler.java b/core/java/android/app/ClientTransactionHandler.java index f9a689a7e1dea..d2235f10da99e 100644 --- a/core/java/android/app/ClientTransactionHandler.java +++ b/core/java/android/app/ClientTransactionHandler.java @@ -119,7 +119,6 @@ public abstract class ClientTransactionHandler { /** * Stop the activity. * @param token Target activity token. - * @param show Flag indicating whether activity is still shown. * @param configChanges Activity configuration changes. * @param pendingActions Pending actions to be used on this or later stages of activity * transaction. @@ -127,7 +126,7 @@ public abstract class ClientTransactionHandler { * request for a transaction. * @param reason Reason for performing this operation. */ - public abstract void handleStopActivity(IBinder token, boolean show, int configChanges, + public abstract void handleStopActivity(IBinder token, int configChanges, PendingTransactionActions pendingActions, boolean finalStateRequest, String reason); /** Report that activity was stopped to server. */ @@ -161,15 +160,12 @@ public abstract class ClientTransactionHandler { /** Request that an activity enter picture-in-picture. */ public abstract void handlePictureInPictureRequested(IBinder token); - /** Update window visibility. */ - public abstract void handleWindowVisibility(IBinder token, boolean show); - /** Perform activity launch. */ public abstract Activity handleLaunchActivity(ActivityThread.ActivityClientRecord r, PendingTransactionActions pendingActions, Intent customIntent); /** Perform activity start. */ - public abstract void handleStartActivity(ActivityThread.ActivityClientRecord r, + public abstract void handleStartActivity(IBinder token, PendingTransactionActions pendingActions); /** Get package info. */ diff --git a/core/java/android/app/LocalActivityManager.java b/core/java/android/app/LocalActivityManager.java index 4033aea32b556..7cdf85e0a6b89 100644 --- a/core/java/android/app/LocalActivityManager.java +++ b/core/java/android/app/LocalActivityManager.java @@ -177,7 +177,7 @@ public class LocalActivityManager { pendingActions = null; } - mActivityThread.handleStartActivity(clientRecord, pendingActions); + mActivityThread.handleStartActivity(r, pendingActions); r.curState = STARTED; if (desiredState == RESUMED) { diff --git a/core/java/android/app/servertransaction/WindowVisibilityItem.java b/core/java/android/app/servertransaction/StartActivityItem.java similarity index 54% rename from core/java/android/app/servertransaction/WindowVisibilityItem.java rename to core/java/android/app/servertransaction/StartActivityItem.java index 115d1ececc0e5..4fbe02b9cf763 100644 --- a/core/java/android/app/servertransaction/WindowVisibilityItem.java +++ b/core/java/android/app/servertransaction/StartActivityItem.java @@ -1,5 +1,5 @@ /* - * Copyright 2017 The Android Open Source Project + * Copyright 2019 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. @@ -24,41 +24,44 @@ import android.os.Parcel; import android.os.Trace; /** - * Window visibility change message. + * Request to move an activity to started and visible state. * @hide */ -public class WindowVisibilityItem extends ClientTransactionItem { +public class StartActivityItem extends ActivityLifecycleItem { - private boolean mShowWindow; + private static final String TAG = "StartActivityItem"; @Override public void execute(ClientTransactionHandler client, IBinder token, PendingTransactionActions pendingActions) { - Trace.traceBegin(TRACE_TAG_ACTIVITY_MANAGER, - mShowWindow ? "activityShowWindow" : "activityHideWindow"); - client.handleWindowVisibility(token, mShowWindow); + Trace.traceBegin(TRACE_TAG_ACTIVITY_MANAGER, "startActivityItem"); + client.handleStartActivity(token, pendingActions); Trace.traceEnd(TRACE_TAG_ACTIVITY_MANAGER); } + @Override + public int getTargetState() { + return ON_START; + } + // ObjectPoolItem implementation - private WindowVisibilityItem() {} + private StartActivityItem() {} /** Obtain an instance initialized with provided params. */ - public static WindowVisibilityItem obtain(boolean showWindow) { - WindowVisibilityItem instance = ObjectPool.obtain(WindowVisibilityItem.class); + public static StartActivityItem obtain() { + StartActivityItem instance = ObjectPool.obtain(StartActivityItem.class); if (instance == null) { - instance = new WindowVisibilityItem(); + instance = new StartActivityItem(); } - instance.mShowWindow = showWindow; return instance; } @Override public void recycle() { - mShowWindow = false; + super.recycle(); ObjectPool.recycle(this); } @@ -68,24 +71,24 @@ public class WindowVisibilityItem extends ClientTransactionItem { /** Write to Parcel. */ @Override public void writeToParcel(Parcel dest, int flags) { - dest.writeBoolean(mShowWindow); + // Empty } /** Read from Parcel. */ - private WindowVisibilityItem(Parcel in) { - mShowWindow = in.readBoolean(); + private StartActivityItem(Parcel in) { + // Empty } - public static final @android.annotation.NonNull Creator CREATOR = - new Creator() { - public WindowVisibilityItem createFromParcel(Parcel in) { - return new WindowVisibilityItem(in); - } + public static final @android.annotation.NonNull Creator CREATOR = + new Creator() { + public StartActivityItem createFromParcel(Parcel in) { + return new StartActivityItem(in); + } - public WindowVisibilityItem[] newArray(int size) { - return new WindowVisibilityItem[size]; - } - }; + public StartActivityItem[] newArray(int size) { + return new StartActivityItem[size]; + } + }; @Override public boolean equals(Object o) { @@ -95,17 +98,17 @@ public class WindowVisibilityItem extends ClientTransactionItem { if (o == null || getClass() != o.getClass()) { return false; } - final WindowVisibilityItem other = (WindowVisibilityItem) o; - return mShowWindow == other.mShowWindow; + return true; } @Override public int hashCode() { - return 17 + 31 * (mShowWindow ? 1 : 0); + return 17; } @Override public String toString() { - return "WindowVisibilityItem{showWindow=" + mShowWindow + "}"; + return "StartActivityItem{}"; } } + diff --git a/core/java/android/app/servertransaction/StopActivityItem.java b/core/java/android/app/servertransaction/StopActivityItem.java index 63efa6fe7c17c..8668bd49c8f50 100644 --- a/core/java/android/app/servertransaction/StopActivityItem.java +++ b/core/java/android/app/servertransaction/StopActivityItem.java @@ -31,14 +31,13 @@ public class StopActivityItem extends ActivityLifecycleItem { private static final String TAG = "StopActivityItem"; - private boolean mShowWindow; private int mConfigChanges; @Override public void execute(ClientTransactionHandler client, IBinder token, PendingTransactionActions pendingActions) { Trace.traceBegin(TRACE_TAG_ACTIVITY_MANAGER, "activityStop"); - client.handleStopActivity(token, mShowWindow, mConfigChanges, pendingActions, + client.handleStopActivity(token, mConfigChanges, pendingActions, true /* finalStateRequest */, "STOP_ACTIVITY_ITEM"); Trace.traceEnd(TRACE_TAG_ACTIVITY_MANAGER); } @@ -59,13 +58,15 @@ public class StopActivityItem extends ActivityLifecycleItem { private StopActivityItem() {} - /** Obtain an instance initialized with provided params. */ - public static StopActivityItem obtain(boolean showWindow, int configChanges) { + /** + * Obtain an instance initialized with provided params. + * @param configChanges Configuration pieces that changed. + */ + public static StopActivityItem obtain(int configChanges) { StopActivityItem instance = ObjectPool.obtain(StopActivityItem.class); if (instance == null) { instance = new StopActivityItem(); } - instance.mShowWindow = showWindow; instance.mConfigChanges = configChanges; return instance; @@ -74,7 +75,6 @@ public class StopActivityItem extends ActivityLifecycleItem { @Override public void recycle() { super.recycle(); - mShowWindow = false; mConfigChanges = 0; ObjectPool.recycle(this); } @@ -85,13 +85,11 @@ public class StopActivityItem extends ActivityLifecycleItem { /** Write to Parcel. */ @Override public void writeToParcel(Parcel dest, int flags) { - dest.writeBoolean(mShowWindow); dest.writeInt(mConfigChanges); } /** Read from Parcel. */ private StopActivityItem(Parcel in) { - mShowWindow = in.readBoolean(); mConfigChanges = in.readInt(); } @@ -115,20 +113,18 @@ public class StopActivityItem extends ActivityLifecycleItem { return false; } final StopActivityItem other = (StopActivityItem) o; - return mShowWindow == other.mShowWindow && mConfigChanges == other.mConfigChanges; + return mConfigChanges == other.mConfigChanges; } @Override public int hashCode() { int result = 17; - result = 31 * result + (mShowWindow ? 1 : 0); result = 31 * result + mConfigChanges; return result; } @Override public String toString() { - return "StopActivityItem{showWindow=" + mShowWindow + ",configChanges=" + mConfigChanges - + "}"; + return "StopActivityItem{configChanges=" + mConfigChanges + "}"; } } diff --git a/core/java/android/app/servertransaction/TransactionExecutor.java b/core/java/android/app/servertransaction/TransactionExecutor.java index 20e0da3ead8a2..17fcda587322f 100644 --- a/core/java/android/app/servertransaction/TransactionExecutor.java +++ b/core/java/android/app/servertransaction/TransactionExecutor.java @@ -218,7 +218,7 @@ public class TransactionExecutor { null /* customIntent */); break; case ON_START: - mTransactionHandler.handleStartActivity(r, mPendingActions); + mTransactionHandler.handleStartActivity(r.token, mPendingActions); break; case ON_RESUME: mTransactionHandler.handleResumeActivity(r.token, false /* finalStateRequest */, @@ -230,8 +230,8 @@ public class TransactionExecutor { "LIFECYCLER_PAUSE_ACTIVITY"); break; case ON_STOP: - mTransactionHandler.handleStopActivity(r.token, false /* show */, - 0 /* configChanges */, mPendingActions, false /* finalStateRequest */, + mTransactionHandler.handleStopActivity(r.token, 0 /* configChanges */, + mPendingActions, false /* finalStateRequest */, "LIFECYCLER_STOP_ACTIVITY"); break; case ON_DESTROY: diff --git a/core/java/android/app/servertransaction/TransactionExecutorHelper.java b/core/java/android/app/servertransaction/TransactionExecutorHelper.java index 0ea8c3c159fa3..6df92a78cc9fd 100644 --- a/core/java/android/app/servertransaction/TransactionExecutorHelper.java +++ b/core/java/android/app/servertransaction/TransactionExecutorHelper.java @@ -183,8 +183,7 @@ public class TransactionExecutorHelper { lifecycleItem = PauseActivityItem.obtain(); break; case ON_STOP: - lifecycleItem = StopActivityItem.obtain(r.isVisibleFromServer(), - 0 /* configChanges */); + lifecycleItem = StopActivityItem.obtain(0 /* configChanges */); break; default: lifecycleItem = ResumeActivityItem.obtain(false /* isForward */); diff --git a/core/tests/coretests/src/android/app/activity/ActivityThreadTest.java b/core/tests/coretests/src/android/app/activity/ActivityThreadTest.java index beaaa373b1a04..d8b527c8a11ae 100644 --- a/core/tests/coretests/src/android/app/activity/ActivityThreadTest.java +++ b/core/tests/coretests/src/android/app/activity/ActivityThreadTest.java @@ -438,8 +438,7 @@ public class ActivityThreadTest { } private static ClientTransaction newStopTransaction(Activity activity) { - final StopActivityItem stopStateRequest = - StopActivityItem.obtain(false /* showWindow */, 0 /* configChanges */); + final StopActivityItem stopStateRequest = StopActivityItem.obtain(0 /* configChanges */); final ClientTransaction transaction = newTransaction(activity); transaction.setLifecycleStateRequest(stopStateRequest); diff --git a/core/tests/coretests/src/android/app/servertransaction/ObjectPoolTests.java b/core/tests/coretests/src/android/app/servertransaction/ObjectPoolTests.java index 37d21f0928bea..4b29d59de332d 100644 --- a/core/tests/coretests/src/android/app/servertransaction/ObjectPoolTests.java +++ b/core/tests/coretests/src/android/app/servertransaction/ObjectPoolTests.java @@ -274,30 +274,15 @@ public class ObjectPoolTests { @Test public void testRecycleStopItem() { - StopActivityItem emptyItem = StopActivityItem.obtain(false, 0); - StopActivityItem item = StopActivityItem.obtain(true, 4); + StopActivityItem emptyItem = StopActivityItem.obtain(0); + StopActivityItem item = StopActivityItem.obtain(4); assertNotSame(item, emptyItem); assertFalse(item.equals(emptyItem)); item.recycle(); assertEquals(item, emptyItem); - StopActivityItem item2 = StopActivityItem.obtain(true, 3); - assertSame(item, item2); - assertFalse(item2.equals(emptyItem)); - } - - @Test - public void testRecycleWindowVisibleItem() { - WindowVisibilityItem emptyItem = WindowVisibilityItem.obtain(false); - WindowVisibilityItem item = WindowVisibilityItem.obtain(true); - assertNotSame(item, emptyItem); - assertFalse(item.equals(emptyItem)); - - item.recycle(); - assertEquals(item, emptyItem); - - WindowVisibilityItem item2 = WindowVisibilityItem.obtain(true); + StopActivityItem item2 = StopActivityItem.obtain(3); assertSame(item, item2); assertFalse(item2.equals(emptyItem)); } diff --git a/core/tests/coretests/src/android/app/servertransaction/TransactionParcelTests.java b/core/tests/coretests/src/android/app/servertransaction/TransactionParcelTests.java index 39bf7421b15ea..ecea9011e7043 100644 --- a/core/tests/coretests/src/android/app/servertransaction/TransactionParcelTests.java +++ b/core/tests/coretests/src/android/app/servertransaction/TransactionParcelTests.java @@ -61,6 +61,7 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -179,31 +180,6 @@ public class TransactionParcelTests { assertTrue(item.equals(result)); } - @Test - public void testWindowVisibilityChange() { - // Write to parcel - WindowVisibilityItem item = WindowVisibilityItem.obtain(true /* showWindow */); - writeAndPrepareForReading(item); - - // Read from parcel and assert - WindowVisibilityItem result = WindowVisibilityItem.CREATOR.createFromParcel(mParcel); - - assertEquals(item.hashCode(), result.hashCode()); - assertTrue(item.equals(result)); - - // Check different value - item = WindowVisibilityItem.obtain(false); - - mParcel = Parcel.obtain(); - writeAndPrepareForReading(item); - - // Read from parcel and assert - result = WindowVisibilityItem.CREATOR.createFromParcel(mParcel); - - assertEquals(item.hashCode(), result.hashCode()); - assertTrue(item.equals(result)); - } - @Test public void testDestroy() { DestroyActivityItem item = DestroyActivityItem.obtain(true /* finished */, @@ -299,8 +275,7 @@ public class TransactionParcelTests { @Test public void testStop() { // Write to parcel - StopActivityItem item = StopActivityItem.obtain(true /* showWindow */, - 14 /* configChanges */); + StopActivityItem item = StopActivityItem.obtain(14 /* configChanges */); writeAndPrepareForReading(item); // Read from parcel and assert @@ -310,15 +285,27 @@ public class TransactionParcelTests { assertTrue(item.equals(result)); } + @Test + public void testStart() { + // Write to parcel + StartActivityItem item = StartActivityItem.obtain(); + writeAndPrepareForReading(item); + + // Read from parcel and assert + StartActivityItem result = StartActivityItem.CREATOR.createFromParcel(mParcel); + + assertEquals(item.hashCode(), result.hashCode()); + assertEquals(item, result); + } + @Test public void testClientTransaction() { // Write to parcel - WindowVisibilityItem callback1 = WindowVisibilityItem.obtain(true); + NewIntentItem callback1 = NewIntentItem.obtain(new ArrayList<>(), true); ActivityConfigurationChangeItem callback2 = ActivityConfigurationChangeItem.obtain( config()); - StopActivityItem lifecycleRequest = StopActivityItem.obtain(true /* showWindow */, - 78 /* configChanges */); + StopActivityItem lifecycleRequest = StopActivityItem.obtain(78 /* configChanges */); IApplicationThread appThread = new StubAppThread(); Binder activityToken = new Binder(); @@ -340,7 +327,7 @@ public class TransactionParcelTests { @Test public void testClientTransactionCallbacksOnly() { // Write to parcel - WindowVisibilityItem callback1 = WindowVisibilityItem.obtain(true); + NewIntentItem callback1 = NewIntentItem.obtain(new ArrayList<>(), true); ActivityConfigurationChangeItem callback2 = ActivityConfigurationChangeItem.obtain( config()); @@ -363,8 +350,7 @@ public class TransactionParcelTests { @Test public void testClientTransactionLifecycleOnly() { // Write to parcel - StopActivityItem lifecycleRequest = StopActivityItem.obtain(true /* showWindow */, - 78 /* configChanges */); + StopActivityItem lifecycleRequest = StopActivityItem.obtain(78 /* configChanges */); IApplicationThread appThread = new StubAppThread(); Binder activityToken = new Binder(); diff --git a/core/tests/mockingcoretests/src/android/app/activity/ActivityThreadClientTest.java b/core/tests/mockingcoretests/src/android/app/activity/ActivityThreadClientTest.java index 66d84aaf35cd1..9018320e479c5 100644 --- a/core/tests/mockingcoretests/src/android/app/activity/ActivityThreadClientTest.java +++ b/core/tests/mockingcoretests/src/android/app/activity/ActivityThreadClientTest.java @@ -77,138 +77,6 @@ import org.mockito.quality.Strictness; @Presubmit public class ActivityThreadClientTest { - @Test - @UiThreadTest - public void testWindowVisibilityChange_OnCreate() throws Exception { - try (ClientMockSession clientSession = new ClientMockSession()) { - ActivityClientRecord r = clientSession.stubActivityRecord(); - - clientSession.launchActivity(r); - assertEquals(ON_CREATE, r.getLifecycleState()); - - clientSession.changeVisibility(r, true); - assertEquals(ON_CREATE, r.getLifecycleState()); - - clientSession.changeVisibility(r, false); - assertEquals(ON_CREATE, r.getLifecycleState()); - } - } - - @Test - @UiThreadTest - public void testWindowVisibilityChange_OnCreate_Finished() throws Exception { - try (ClientMockSession clientSession = new ClientMockSession()) { - ActivityClientRecord r = clientSession.stubActivityRecord(); - - Activity activity = clientSession.launchActivity(r); - activity.finish(); - assertEquals(ON_CREATE, r.getLifecycleState()); - - clientSession.changeVisibility(r, true); - assertEquals(ON_CREATE, r.getLifecycleState()); - - clientSession.changeVisibility(r, false); - assertEquals(ON_CREATE, r.getLifecycleState()); - } - } - - @Test - @UiThreadTest - public void testWindowVisibilityChange_OnStart() throws Exception { - try (ClientMockSession clientSession = new ClientMockSession()) { - ActivityClientRecord r = clientSession.stubActivityRecord(); - - clientSession.launchActivity(r); - clientSession.startActivity(r); - assertEquals(ON_START, r.getLifecycleState()); - - clientSession.changeVisibility(r, false); - assertEquals(ON_STOP, r.getLifecycleState()); - - clientSession.changeVisibility(r, true); - assertEquals(ON_START, r.getLifecycleState()); - } - } - - @Test - @UiThreadTest - public void testWindowVisibilityChange_OnStart_Finished() throws Exception { - try (ClientMockSession clientSession = new ClientMockSession()) { - ActivityClientRecord r = clientSession.stubActivityRecord(); - - Activity activity = clientSession.launchActivity(r); - clientSession.startActivity(r); - activity.finish(); - assertEquals(ON_START, r.getLifecycleState()); - - clientSession.changeVisibility(r, false); - assertEquals(ON_STOP, r.getLifecycleState()); - - clientSession.changeVisibility(r, true); - assertEquals(ON_START, r.getLifecycleState()); - } - } - - @Test - @UiThreadTest - public void testWindowVisibilityChange_OnResume() throws Exception { - try (ClientMockSession clientSession = new ClientMockSession()) { - ActivityClientRecord r = clientSession.stubActivityRecord(); - - clientSession.launchActivity(r); - clientSession.startActivity(r); - clientSession.resumeActivity(r); - assertEquals(ON_RESUME, r.getLifecycleState()); - - clientSession.changeVisibility(r, false); - assertEquals(ON_STOP, r.getLifecycleState()); - - clientSession.changeVisibility(r, true); - assertEquals(ON_START, r.getLifecycleState()); - } - } - - @Test - @UiThreadTest - public void testWindowVisibilityChange_OnPause() throws Exception { - try (ClientMockSession clientSession = new ClientMockSession()) { - ActivityClientRecord r = clientSession.stubActivityRecord(); - - clientSession.launchActivity(r); - clientSession.startActivity(r); - clientSession.resumeActivity(r); - clientSession.pauseActivity(r); - assertEquals(ON_PAUSE, r.getLifecycleState()); - - clientSession.changeVisibility(r, false); - assertEquals(ON_STOP, r.getLifecycleState()); - - clientSession.changeVisibility(r, true); - assertEquals(ON_START, r.getLifecycleState()); - } - } - - @Test - @UiThreadTest - public void testWindowVisibilityChange_OnStop() throws Exception { - try (ClientMockSession clientSession = new ClientMockSession()) { - ActivityClientRecord r = clientSession.stubActivityRecord(); - - clientSession.launchActivity(r); - clientSession.startActivity(r); - clientSession.resumeActivity(r); - clientSession.pauseActivity(r); - clientSession.stopActivity(r); - assertEquals(ON_STOP, r.getLifecycleState()); - - clientSession.changeVisibility(r, true); - assertEquals(ON_START, r.getLifecycleState()); - - clientSession.changeVisibility(r, false); - assertEquals(ON_STOP, r.getLifecycleState()); - } - } - @Test @UiThreadTest public void testLifecycleAfterFinished_OnCreate() throws Exception { @@ -308,7 +176,7 @@ public class ActivityThreadClientTest { } private void startActivity(ActivityClientRecord r) { - mThread.handleStartActivity(r, null /* pendingActions */); + mThread.handleStartActivity(r.token, null /* pendingActions */); } private void resumeActivity(ActivityClientRecord r) { @@ -323,7 +191,7 @@ public class ActivityThreadClientTest { } private void stopActivity(ActivityClientRecord r) { - mThread.handleStopActivity(r.token, false /* show */, 0 /* configChanges */, + mThread.handleStopActivity(r.token, 0 /* configChanges */, new PendingTransactionActions(), false /* finalStateRequest */, "test"); } @@ -332,10 +200,6 @@ public class ActivityThreadClientTest { false /* getNonConfigInstance */, "test"); } - private void changeVisibility(ActivityClientRecord r, boolean show) { - mThread.handleWindowVisibility(r.token, show); - } - private ActivityClientRecord stubActivityRecord() { ComponentName component = new ComponentName( InstrumentationRegistry.getInstrumentation().getContext(), TestActivity.class); diff --git a/services/core/java/com/android/server/wm/ActivityRecord.java b/services/core/java/com/android/server/wm/ActivityRecord.java index d49270d7f5578..252a691879760 100644 --- a/services/core/java/com/android/server/wm/ActivityRecord.java +++ b/services/core/java/com/android/server/wm/ActivityRecord.java @@ -243,9 +243,9 @@ import android.app.servertransaction.NewIntentItem; import android.app.servertransaction.PauseActivityItem; import android.app.servertransaction.PipModeChangeItem; import android.app.servertransaction.ResumeActivityItem; +import android.app.servertransaction.StartActivityItem; import android.app.servertransaction.StopActivityItem; import android.app.servertransaction.TopResumedActivityChangeItem; -import android.app.servertransaction.WindowVisibilityItem; import android.app.usage.UsageEvents.Event; import android.content.ComponentName; import android.content.Intent; @@ -4507,7 +4507,8 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A sleeping = false; app.postPendingUiCleanMsg(true); if (reportToClient) { - makeClientVisible(); + mClientVisibilityDeferred = false; + makeActiveIfNeeded(starting); } else { mClientVisibilityDeferred = true; } @@ -4521,23 +4522,6 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A handleAlreadyVisible(); } - /** Send visibility change message to the client and pause if needed. */ - void makeClientVisible() { - mClientVisibilityDeferred = false; - try { - mAtmService.getLifecycleManager().scheduleTransaction(app.getThread(), appToken, - WindowVisibilityItem.obtain(true /* showWindow */)); - makeActiveIfNeeded(null /* activeActivity*/); - if (isState(STOPPING, STOPPED)) { - // Set state to STARTED in order to have consistent state with client while - // making an non-active activity visible from stopped. - setState(STARTED, "makeClientVisible"); - } - } catch (Exception e) { - Slog.w(TAG, "Exception thrown sending visibility update: " + intent.getComponent(), e); - } - } - void makeInvisible() { if (!mVisibleRequested) { if (DEBUG_VISIBILITY) Slog.v(TAG_VISIBILITY, "Already invisible: " + this); @@ -4566,14 +4550,6 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A switch (getState()) { case STOPPING: case STOPPED: - if (attachedToProcess()) { - if (DEBUG_VISIBILITY) { - Slog.v(TAG_VISIBILITY, "Scheduling invisibility: " + this); - } - mAtmService.getLifecycleManager().scheduleTransaction(app.getThread(), - appToken, WindowVisibilityItem.obtain(false /* showWindow */)); - } - // Reset the flag indicating that an app can enter picture-in-picture once the // activity is hidden supportsEnterPipOnTaskSwitch = false; @@ -4605,17 +4581,17 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A boolean makeActiveIfNeeded(ActivityRecord activeActivity) { if (shouldResumeActivity(activeActivity)) { if (DEBUG_VISIBILITY) { - Slog.v("TAG_VISIBILITY", "Resume visible activity, " + this); + Slog.v(TAG_VISIBILITY, "Resume visible activity, " + this); } return getActivityStack().resumeTopActivityUncheckedLocked(activeActivity /* prev */, null /* options */); } else if (shouldPauseActivity(activeActivity)) { if (DEBUG_VISIBILITY) { - Slog.v("TAG_VISIBILITY", "Pause visible activity, " + this); + Slog.v(TAG_VISIBILITY, "Pause visible activity, " + this); } // An activity must be in the {@link PAUSING} state for the system to validate // the move to {@link PAUSED}. - setState(PAUSING, "makeVisibleIfNeeded"); + setState(PAUSING, "makeActiveIfNeeded"); try { mAtmService.getLifecycleManager().scheduleTransaction(app.getThread(), appToken, PauseActivityItem.obtain(finishing, false /* userLeaving */, @@ -4623,6 +4599,17 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A } catch (Exception e) { Slog.w(TAG, "Exception thrown sending pause: " + intent.getComponent(), e); } + } else if (shouldStartActivity()) { + if (DEBUG_VISIBILITY) { + Slog.v(TAG_VISIBILITY, "Start visible activity, " + this); + } + setState(STARTED, "makeActiveIfNeeded"); + try { + mAtmService.getLifecycleManager().scheduleTransaction(app.getThread(), appToken, + StartActivityItem.obtain()); + } catch (Exception e) { + Slog.w(TAG, "Exception thrown sending start: " + intent.getComponent(), e); + } } return false; } @@ -4665,6 +4652,16 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A && canResumeByCompat(); } + /** + * Check if activity should be moved to STARTED state. + * NOTE: This will not check if activity should be made paused or resumed first, so it must only + * be called after checking with {@link #shouldResumeActivity(ActivityRecord)} + * and {@link #shouldPauseActivity(ActivityRecord)}. + */ + private boolean shouldStartActivity() { + return mVisibleRequested && isState(STOPPED); + } + /** * Check if activity is eligible to be made active (resumed of paused). The activity: * - should be paused, stopped or stopping @@ -4901,16 +4898,13 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A } setState(STOPPING, "stopIfPossible"); if (DEBUG_VISIBILITY) { - Slog.v(TAG_VISIBILITY, "Stopping visibleRequested=" - + mVisibleRequested + " for " + this); - } - if (!mVisibleRequested) { - setVisibility(false); + Slog.v(TAG_VISIBILITY, "Stopping:" + this); } EventLogTags.writeWmStopActivity( mUserId, System.identityHashCode(this), shortComponentName); mAtmService.getLifecycleManager().scheduleTransaction(app.getThread(), appToken, - StopActivityItem.obtain(mVisibleRequested, configChangeFlags)); + StopActivityItem.obtain(configChangeFlags)); + if (stack.shouldSleepOrShutDownActivities()) { setSleeping(true); } @@ -7211,7 +7205,7 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A // {@link ActivityTaskManagerService.activityStopped}). try { mAtmService.getLifecycleManager().scheduleTransaction(app.getThread(), appToken, - StopActivityItem.obtain(false /* showWindow */, 0 /* configChanges */)); + StopActivityItem.obtain(0 /* configChanges */)); } catch (RemoteException e) { Slog.w(TAG, "Exception thrown during restart " + this, e); } diff --git a/services/core/java/com/android/server/wm/EnsureActivitiesVisibleHelper.java b/services/core/java/com/android/server/wm/EnsureActivitiesVisibleHelper.java index e74f61d53fea0..dbee310bc34dd 100644 --- a/services/core/java/com/android/server/wm/EnsureActivitiesVisibleHelper.java +++ b/services/core/java/com/android/server/wm/EnsureActivitiesVisibleHelper.java @@ -79,12 +79,13 @@ class EnsureActivitiesVisibleHelper { final PooledConsumer f = PooledLambda.obtainConsumer( EnsureActivitiesVisibleHelper::setActivityVisibilityState, this, - PooledLambda.__(ActivityRecord.class), resumeTopActivity); + PooledLambda.__(ActivityRecord.class), starting, resumeTopActivity); mContiner.forAllActivities(f); f.recycle(); } - private void setActivityVisibilityState(ActivityRecord r, final boolean resumeTopActivity) { + private void setActivityVisibilityState(ActivityRecord r, ActivityRecord starting, + final boolean resumeTopActivity) { final boolean isTop = r == mTop; if (mAboveTop && !isTop) { return; @@ -129,7 +130,8 @@ class EnsureActivitiesVisibleHelper { "Skipping: already visible at " + r); if (r.mClientVisibilityDeferred && mNotifyClients) { - r.makeClientVisible(); + r.makeActiveIfNeeded(r.mClientVisibilityDeferred ? null : starting); + r.mClientVisibilityDeferred = false; } r.handleAlreadyVisible(); diff --git a/services/tests/wmtests/src/com/android/server/wm/ActivityRecordTests.java b/services/tests/wmtests/src/com/android/server/wm/ActivityRecordTests.java index d6bd1d07a7860..5d5943bdda5fd 100644 --- a/services/tests/wmtests/src/com/android/server/wm/ActivityRecordTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/ActivityRecordTests.java @@ -521,11 +521,12 @@ public class ActivityRecordTests extends ActivityTestsBase { } @Test - public void testShouldPauseWhenMakeClientVisible() { + public void testShouldStartWhenMakeClientActive() { ActivityRecord topActivity = new ActivityBuilder(mService).setTask(mTask).build(); topActivity.setOccludesParent(false); mActivity.setState(ActivityStack.ActivityState.STOPPED, "Testing"); - mActivity.makeClientVisible(); + mActivity.setVisibility(true); + mActivity.makeActiveIfNeeded(null /* activeActivity */); assertEquals(STARTED, mActivity.getState()); }