Merge "Make sure config change items are executed in the order dispatched." into rvc-dev am: 0d8822c94a am: 7944d2771e

Change-Id: Ide23e7e73c7eb666c83602b4727b1eb3c9800663

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,
+    void handleActivityConfigurationChanged(IBinder activityToken,
-            int displayId, boolean alwaysReportChange) {
+            @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
+            if (overrideConfig.isOtherSeqNewer(r.mPendingOverrideConfig)) {
-                    && !r.mPendingOverrideConfig.isOtherSeqNewer(overrideConfig)) {
+                if (DEBUG_CONFIGURATION) {
-                overrideConfig = r.mPendingOverrideConfig;
+                    Slog.v(TAG, "Activity has newer configuration pending so drop this"
+                            + " transaction. overrideConfig=" + overrideConfig
+                            + " r.mPendingOverrideConfig=" + r.mPendingOverrideConfig);
+                }
+                return;
             }
             r.mPendingOverrideConfig = null;
         }

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);
     }
 

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);
     }
 

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();
+            final Configuration newerConfig = new Configuration();
-            pendingConfig.orientation = orientation == ORIENTATION_LANDSCAPE
+            newerConfig.orientation = orientation == ORIENTATION_LANDSCAPE
-                    ? ORIENTATION_PORTRAIT
+                    ? ORIENTATION_PORTRAIT : ORIENTATION_LANDSCAPE;
-                    : ORIENTATION_LANDSCAPE;
+            newerConfig.seq = seq + 2;
-            pendingConfig.seq = seq + 2;
             activityThread.updatePendingActivityConfiguration(activity.getActivityToken(),
-                    pendingConfig);
+                    newerConfig);
 
-            final Configuration newConfig = new Configuration();
+            final Configuration olderConfig = new Configuration();
-            newConfig.orientation = orientation;
+            olderConfig.orientation = orientation;
-            newConfig.seq = seq + 1;
+            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 newTransaction(activity.getActivityThread(), activity.getActivityToken());
-        return ClientTransaction.obtain(appThread, activity.getActivityToken());
+    }
+
+    private static ClientTransaction newTransaction(ActivityThread activityThread,
+            @Nullable IBinder activityToken) {
+        return ClientTransaction.obtain(activityThread.getApplicationThread(), activityToken);
     }
 
     // Test activity

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));