diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java index eea1d69b6326a..ea8b19028beb8 100644 --- a/core/java/android/app/ActivityThread.java +++ b/core/java/android/app/ActivityThread.java @@ -5902,6 +5902,12 @@ public final class ActivityThread extends ClientTransactionHandler { } } + /** + * Sets the supplied {@code overrideConfig} as pending for the {@code activityToken}. Calling + * this method prevents any calls to + * {@link #handleActivityConfigurationChanged(IBinder, Configuration, int, boolean)} from + * processing any configurations older than {@code overrideConfig}. + */ @Override public void updatePendingActivityConfiguration(IBinder activityToken, Configuration overrideConfig) { @@ -5918,13 +5924,22 @@ public final class ActivityThread extends ClientTransactionHandler { } synchronized (r) { + if (r.mPendingOverrideConfig != null + && !r.mPendingOverrideConfig.isOtherSeqNewer(overrideConfig)) { + if (DEBUG_CONFIGURATION) { + Slog.v(TAG, "Activity has newer configuration pending so drop this" + + " transaction. overrideConfig=" + overrideConfig + + " r.mPendingOverrideConfig=" + r.mPendingOverrideConfig); + } + return; + } r.mPendingOverrideConfig = overrideConfig; } } @Override public void handleActivityConfigurationChanged(IBinder activityToken, - Configuration overrideConfig, int displayId) { + @NonNull Configuration overrideConfig, int displayId) { handleActivityConfigurationChanged(activityToken, overrideConfig, displayId, // This is the only place that uses alwaysReportChange=true. The entry point should // be from ActivityConfigurationChangeItem or MoveToDisplayItem, so the server side @@ -5935,15 +5950,18 @@ public final class ActivityThread extends ClientTransactionHandler { } /** - * Handle new activity configuration and/or move to a different display. + * Handle new activity configuration and/or move to a different display. This method is a noop + * if {@link #updatePendingActivityConfiguration(IBinder, Configuration)} has been called with + * a newer config than {@code overrideConfig}. + * * @param activityToken Target activity token. * @param overrideConfig Activity override config. * @param displayId Id of the display where activity was moved to, -1 if there was no move and * value didn't change. * @param alwaysReportChange If the configuration is changed, always report to activity. */ - void handleActivityConfigurationChanged(IBinder activityToken, Configuration overrideConfig, - int displayId, boolean alwaysReportChange) { + void handleActivityConfigurationChanged(IBinder activityToken, + @NonNull Configuration overrideConfig, int displayId, boolean alwaysReportChange) { ActivityClientRecord r = mActivities.get(activityToken); // Check input params. if (r == null || r.activity == null) { @@ -5954,9 +5972,13 @@ public final class ActivityThread extends ClientTransactionHandler { && displayId != r.activity.getDisplayId(); synchronized (r) { - if (r.mPendingOverrideConfig != null - && !r.mPendingOverrideConfig.isOtherSeqNewer(overrideConfig)) { - overrideConfig = r.mPendingOverrideConfig; + if (overrideConfig.isOtherSeqNewer(r.mPendingOverrideConfig)) { + if (DEBUG_CONFIGURATION) { + Slog.v(TAG, "Activity has newer configuration pending so drop this" + + " transaction. overrideConfig=" + overrideConfig + + " r.mPendingOverrideConfig=" + r.mPendingOverrideConfig); + } + return; } r.mPendingOverrideConfig = null; } diff --git a/core/java/android/app/servertransaction/ActivityConfigurationChangeItem.java b/core/java/android/app/servertransaction/ActivityConfigurationChangeItem.java index 0d4e16bbb0f3a..8b52242a6b6ce 100644 --- a/core/java/android/app/servertransaction/ActivityConfigurationChangeItem.java +++ b/core/java/android/app/servertransaction/ActivityConfigurationChangeItem.java @@ -19,6 +19,7 @@ package android.app.servertransaction; import static android.os.Trace.TRACE_TAG_ACTIVITY_MANAGER; import static android.view.Display.INVALID_DISPLAY; +import android.annotation.NonNull; import android.app.ClientTransactionHandler; import android.content.res.Configuration; import android.os.IBinder; @@ -37,6 +38,8 @@ public class ActivityConfigurationChangeItem extends ClientTransactionItem { @Override public void preExecute(android.app.ClientTransactionHandler client, IBinder token) { + // Notify the client of an upcoming change in the token configuration. This ensures that + // batches of config change items only process the newest configuration. client.updatePendingActivityConfiguration(token, mConfiguration); } @@ -55,7 +58,11 @@ public class ActivityConfigurationChangeItem extends ClientTransactionItem { private ActivityConfigurationChangeItem() {} /** Obtain an instance initialized with provided params. */ - public static ActivityConfigurationChangeItem obtain(Configuration config) { + public static ActivityConfigurationChangeItem obtain(@NonNull Configuration config) { + if (config == null) { + throw new IllegalArgumentException("Config must not be null."); + } + ActivityConfigurationChangeItem instance = ObjectPool.obtain(ActivityConfigurationChangeItem.class); if (instance == null) { @@ -68,7 +75,7 @@ public class ActivityConfigurationChangeItem extends ClientTransactionItem { @Override public void recycle() { - mConfiguration = null; + mConfiguration = Configuration.EMPTY; ObjectPool.recycle(this); } diff --git a/core/java/android/app/servertransaction/MoveToDisplayItem.java b/core/java/android/app/servertransaction/MoveToDisplayItem.java index f6d3dbdf55960..9a457a3aad409 100644 --- a/core/java/android/app/servertransaction/MoveToDisplayItem.java +++ b/core/java/android/app/servertransaction/MoveToDisplayItem.java @@ -18,6 +18,7 @@ package android.app.servertransaction; import static android.os.Trace.TRACE_TAG_ACTIVITY_MANAGER; +import android.annotation.NonNull; import android.app.ClientTransactionHandler; import android.content.res.Configuration; import android.os.IBinder; @@ -35,6 +36,13 @@ public class MoveToDisplayItem extends ClientTransactionItem { private int mTargetDisplayId; private Configuration mConfiguration; + @Override + public void preExecute(ClientTransactionHandler client, IBinder token) { + // Notify the client of an upcoming change in the token configuration. This ensures that + // batches of config change items only process the newest configuration. + client.updatePendingActivityConfiguration(token, mConfiguration); + } + @Override public void execute(ClientTransactionHandler client, IBinder token, PendingTransactionActions pendingActions) { @@ -49,7 +57,12 @@ public class MoveToDisplayItem extends ClientTransactionItem { private MoveToDisplayItem() {} /** Obtain an instance initialized with provided params. */ - public static MoveToDisplayItem obtain(int targetDisplayId, Configuration configuration) { + public static MoveToDisplayItem obtain(int targetDisplayId, + @NonNull Configuration configuration) { + if (configuration == null) { + throw new IllegalArgumentException("Configuration must not be null"); + } + MoveToDisplayItem instance = ObjectPool.obtain(MoveToDisplayItem.class); if (instance == null) { instance = new MoveToDisplayItem(); @@ -63,7 +76,7 @@ public class MoveToDisplayItem extends ClientTransactionItem { @Override public void recycle() { mTargetDisplayId = 0; - mConfiguration = null; + mConfiguration = Configuration.EMPTY; ObjectPool.recycle(this); } diff --git a/core/tests/coretests/src/android/app/activity/ActivityThreadTest.java b/core/tests/coretests/src/android/app/activity/ActivityThreadTest.java index a93dacf470504..000e870369dbf 100644 --- a/core/tests/coretests/src/android/app/activity/ActivityThreadTest.java +++ b/core/tests/coretests/src/android/app/activity/ActivityThreadTest.java @@ -20,6 +20,7 @@ import static android.content.Intent.ACTION_EDIT; import static android.content.Intent.ACTION_VIEW; import static android.content.res.Configuration.ORIENTATION_LANDSCAPE; import static android.content.res.Configuration.ORIENTATION_PORTRAIT; +import static android.view.Display.INVALID_DISPLAY; import static com.google.common.truth.Truth.assertThat; @@ -29,6 +30,7 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.testng.Assert.assertFalse; +import android.annotation.Nullable; import android.app.Activity; import android.app.ActivityThread; import android.app.IApplicationThread; @@ -38,6 +40,7 @@ import android.app.servertransaction.ActivityConfigurationChangeItem; import android.app.servertransaction.ActivityRelaunchItem; import android.app.servertransaction.ClientTransaction; import android.app.servertransaction.ClientTransactionItem; +import android.app.servertransaction.ConfigurationChangeItem; import android.app.servertransaction.NewIntentItem; import android.app.servertransaction.ResumeActivityItem; import android.app.servertransaction.StopActivityItem; @@ -225,7 +228,7 @@ public class ActivityThreadTest { } @Test - public void testHandleActivityConfigurationChanged_PickNewerPendingConfiguration() { + public void testHandleActivityConfigurationChanged_SkipWhenNewerConfigurationPending() { final TestActivity activity = mActivityTestRule.launchActivity(new Intent()); InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> { @@ -237,25 +240,90 @@ public class ActivityThreadTest { final ActivityThread activityThread = activity.getActivityThread(); - final Configuration pendingConfig = new Configuration(); - pendingConfig.orientation = orientation == ORIENTATION_LANDSCAPE - ? ORIENTATION_PORTRAIT - : ORIENTATION_LANDSCAPE; - pendingConfig.seq = seq + 2; + final Configuration newerConfig = new Configuration(); + newerConfig.orientation = orientation == ORIENTATION_LANDSCAPE + ? ORIENTATION_PORTRAIT : ORIENTATION_LANDSCAPE; + newerConfig.seq = seq + 2; activityThread.updatePendingActivityConfiguration(activity.getActivityToken(), - pendingConfig); + newerConfig); - final Configuration newConfig = new Configuration(); - newConfig.orientation = orientation; - newConfig.seq = seq + 1; + final Configuration olderConfig = new Configuration(); + olderConfig.orientation = orientation; + olderConfig.seq = seq + 1; activityThread.handleActivityConfigurationChanged(activity.getActivityToken(), - newConfig, Display.INVALID_DISPLAY); + olderConfig, INVALID_DISPLAY); + assertEquals(numOfConfig, activity.mNumOfConfigChanges); + assertEquals(olderConfig.orientation, activity.mConfig.orientation); + + activityThread.handleActivityConfigurationChanged(activity.getActivityToken(), + newerConfig, INVALID_DISPLAY); assertEquals(numOfConfig + 1, activity.mNumOfConfigChanges); - assertEquals(pendingConfig.orientation, activity.mConfig.orientation); + assertEquals(newerConfig.orientation, activity.mConfig.orientation); }); } + @Test + public void testHandleActivityConfigurationChanged_EnsureUpdatesProcessedInOrder() + throws Exception { + final TestActivity activity = mActivityTestRule.launchActivity(new Intent()); + + final ActivityThread activityThread = activity.getActivityThread(); + InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> { + final Configuration config = new Configuration(); + config.seq = BASE_SEQ; + config.orientation = ORIENTATION_PORTRAIT; + + activityThread.handleActivityConfigurationChanged(activity.getActivityToken(), + config, INVALID_DISPLAY); + }); + + final IApplicationThread appThread = activityThread.getApplicationThread(); + final int numOfConfig = activity.mNumOfConfigChanges; + + final Configuration processConfigLandscape = new Configuration(); + processConfigLandscape.windowConfiguration.setBounds(new Rect(0, 0, 100, 60)); + processConfigLandscape.seq = BASE_SEQ + 1; + + final Configuration activityConfigLandscape = new Configuration(); + activityConfigLandscape.windowConfiguration.setBounds(new Rect(0, 0, 100, 50)); + activityConfigLandscape.seq = BASE_SEQ + 2; + + final Configuration processConfigPortrait = new Configuration(); + processConfigPortrait.windowConfiguration.setBounds(new Rect(0, 0, 60, 100)); + processConfigPortrait.seq = BASE_SEQ + 3; + + final Configuration activityConfigPortrait = new Configuration(); + activityConfigPortrait.windowConfiguration.setBounds(new Rect(0, 0, 50, 100)); + activityConfigPortrait.seq = BASE_SEQ + 4; + + activity.mConfigLatch = new CountDownLatch(1); + activity.mTestLatch = new CountDownLatch(1); + + ClientTransaction transaction = newTransaction(activityThread, null); + transaction.addCallback(ConfigurationChangeItem.obtain(processConfigLandscape)); + appThread.scheduleTransaction(transaction); + + transaction = newTransaction(activityThread, activity.getActivityToken()); + transaction.addCallback(ActivityConfigurationChangeItem.obtain(activityConfigLandscape)); + transaction.addCallback(ConfigurationChangeItem.obtain(processConfigPortrait)); + transaction.addCallback(ActivityConfigurationChangeItem.obtain(activityConfigPortrait)); + appThread.scheduleTransaction(transaction); + + activity.mTestLatch.await(); + activity.mConfigLatch.countDown(); + + activity.mConfigLatch = null; + activity.mTestLatch = null; + + // Check display metrics, bounds should match the portrait activity bounds. + final Rect bounds = activity.getWindowManager().getCurrentWindowMetrics().getBounds(); + assertEquals(activityConfigPortrait.windowConfiguration.getBounds(), bounds); + + // Ensure that Activity#onConfigurationChanged() is only called once. + assertEquals(numOfConfig + 1, activity.mNumOfConfigChanges); + } + @Test public void testHandleActivityConfigurationChanged_OnlyAppliesNewestConfiguration() throws Exception { @@ -268,7 +336,7 @@ public class ActivityThreadTest { config.orientation = ORIENTATION_PORTRAIT; activityThread.handleActivityConfigurationChanged(activity.getActivityToken(), - config, Display.INVALID_DISPLAY); + config, INVALID_DISPLAY); }); final int numOfConfig = activity.mNumOfConfigChanges; @@ -504,7 +572,7 @@ public class ActivityThreadTest { config.orientation = ORIENTATION_PORTRAIT; config.seq = seq; activityThread.handleActivityConfigurationChanged(activity.getActivityToken(), config, - Display.INVALID_DISPLAY); + INVALID_DISPLAY); if (activity.mNumOfConfigChanges > numOfConfig) { return config.seq; @@ -514,7 +582,7 @@ public class ActivityThreadTest { config.orientation = ORIENTATION_LANDSCAPE; config.seq = seq + 1; activityThread.handleActivityConfigurationChanged(activity.getActivityToken(), config, - Display.INVALID_DISPLAY); + INVALID_DISPLAY); return config.seq; } @@ -572,8 +640,12 @@ public class ActivityThreadTest { } private static ClientTransaction newTransaction(Activity activity) { - final IApplicationThread appThread = activity.getActivityThread().getApplicationThread(); - return ClientTransaction.obtain(appThread, activity.getActivityToken()); + return newTransaction(activity.getActivityThread(), activity.getActivityToken()); + } + + private static ClientTransaction newTransaction(ActivityThread activityThread, + @Nullable IBinder activityToken) { + return ClientTransaction.obtain(activityThread.getApplicationThread(), activityToken); } // Test activity diff --git a/core/tests/coretests/src/android/app/servertransaction/ObjectPoolTests.java b/core/tests/coretests/src/android/app/servertransaction/ObjectPoolTests.java index 6c23125aaf137..4654f63a2a911 100644 --- a/core/tests/coretests/src/android/app/servertransaction/ObjectPoolTests.java +++ b/core/tests/coretests/src/android/app/servertransaction/ObjectPoolTests.java @@ -63,7 +63,8 @@ public class ObjectPoolTests { @Test public void testRecycleActivityConfigurationChangeItem() { - ActivityConfigurationChangeItem emptyItem = ActivityConfigurationChangeItem.obtain(null); + ActivityConfigurationChangeItem emptyItem = + ActivityConfigurationChangeItem.obtain(Configuration.EMPTY); ActivityConfigurationChangeItem item = ActivityConfigurationChangeItem.obtain(config()); assertNotSame(item, emptyItem); assertFalse(item.equals(emptyItem)); @@ -186,7 +187,7 @@ public class ObjectPoolTests { @Test public void testRecycleMoveToDisplayItem() { - MoveToDisplayItem emptyItem = MoveToDisplayItem.obtain(0, null); + MoveToDisplayItem emptyItem = MoveToDisplayItem.obtain(0, Configuration.EMPTY); MoveToDisplayItem item = MoveToDisplayItem.obtain(4, config()); assertNotSame(item, emptyItem); assertFalse(item.equals(emptyItem));