Update the activity current config only if it is reported

The current configuration in the client activity could be
updated via the other flow (from ViewRootImpl). So, it is
possible that the client activity configuration is different
from the last reported configuration that cached in the system.

If the configuration changed again in that case, the
ActivityConfigurationChangeItem could end up without reporting
Activity#onConfigurationChanged if the differences between
the new configuration and the current client activity
configuration contains the configuration changes that the
activity cannot handle.

Update the activity current config only when reported, so
the configuration differences can be correctly evaluated from
the new configuration and the configuration last reported.

Bug: 231312158
Test: repro steps on the bug
Test: atest ActivityThreadClientTest
Change-Id: Ifa27d222bb0905053cf12e08c423bf7bfad904cd

2 files changed, 21 insertions, 23 deletions

diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java
index 9e9e985d27de..802458b1793c 100644
--- a/core/java/android/app/ActivityThread.java
+++ b/core/java/android/app/ActivityThread.java
@@ -5894,16 +5894,16 @@ public final class ActivityThread extends ClientTransactionHandler
 
         final boolean movedToDifferentDisplay = isDifferentDisplay(activity.getDisplayId(),
                 displayId);
-        final Configuration currentConfig = activity.mCurrentConfig;
-        final int diff = currentConfig.diffPublicOnly(newConfig);
-        final boolean hasPublicConfigChange = diff != 0;
+        final Configuration currentResConfig = activity.getResources().getConfiguration();
+        final int diff = currentResConfig.diffPublicOnly(newConfig);
+        final boolean hasPublicResConfigChange = diff != 0;
         final ActivityClientRecord r = getActivityClient(activityToken);
         // TODO(b/173090263): Use diff instead after the improvement of AssetManager and
         // ResourcesImpl constructions.
-        final boolean shouldUpdateResources = hasPublicConfigChange
-                || shouldUpdateResources(activityToken, currentConfig, newConfig, amOverrideConfig,
-                movedToDifferentDisplay, hasPublicConfigChange);
-        final boolean shouldReportChange = shouldReportChange(diff, currentConfig, newConfig,
+        final boolean shouldUpdateResources = hasPublicResConfigChange
+                || shouldUpdateResources(activityToken, currentResConfig, newConfig,
+                amOverrideConfig, movedToDifferentDisplay, hasPublicResConfigChange);
+        final boolean shouldReportChange = shouldReportChange(activity.mCurrentConfig, newConfig,
                 r != null ? r.mSizeConfigurations : null,
                 activity.mActivityInfo.getRealConfigChanged());
         // Nothing significant, don't proceed with updating and reporting.
@@ -5927,9 +5927,6 @@ public final class ActivityThread extends ClientTransactionHandler
                 amOverrideConfig, contextThemeWrapperOverrideConfig);
         mResourcesManager.updateResourcesForActivity(activityToken, finalOverrideConfig, displayId);
 
-        activity.mConfigChangeFlags = 0;
-        activity.mCurrentConfig = new Configuration(newConfig);
-
         // Apply the ContextThemeWrapper override if necessary.
         // NOTE: Make sure the configurations are not modified, as they are treated as immutable
         // in many places.
@@ -5940,8 +5937,10 @@ public final class ActivityThread extends ClientTransactionHandler
             activity.dispatchMovedToDisplay(displayId, configToReport);
         }
 
+        activity.mConfigChangeFlags = 0;
         if (shouldReportChange) {
             activity.mCalled = false;
+            activity.mCurrentConfig = new Configuration(newConfig);
             activity.onConfigurationChanged(configToReport);
             if (!activity.mCalled) {
                 throw new SuperNotCalledException("Activity " + activity.getLocalClassName() +
@@ -5956,8 +5955,6 @@ public final class ActivityThread extends ClientTransactionHandler
      * Returns {@code true} if {@link Activity#onConfigurationChanged(Configuration)} should be
      * dispatched.
      *
-     * @param publicDiff Usually computed by {@link Configuration#diffPublicOnly(Configuration)}.
-     *                   This parameter is to prevent we compute it again.
      * @param currentConfig The current configuration cached in {@link Activity#mCurrentConfig}.
      *                      It is {@code null} before the first config update from the server side.
      * @param newConfig The updated {@link Configuration}
@@ -5966,9 +5963,10 @@ public final class ActivityThread extends ClientTransactionHandler
      * @return {@code true} if the config change should be reported to the Activity
      */
     @VisibleForTesting
-    public static boolean shouldReportChange(int publicDiff, @Nullable Configuration currentConfig,
+    public static boolean shouldReportChange(@Nullable Configuration currentConfig,
             @NonNull Configuration newConfig, @Nullable SizeConfigurationBuckets sizeBuckets,
             int handledConfigChanges) {
+        final int publicDiff = currentConfig.diffPublicOnly(newConfig);
         // Don't report the change if there's no public diff between current and new config.
         if (publicDiff == 0) {
             return false;
diff --git a/core/tests/mockingcoretests/src/android/app/activity/ActivityThreadClientTest.java b/core/tests/mockingcoretests/src/android/app/activity/ActivityThreadClientTest.java
index 8d3751e6ad6c..47f70ddf2d42 100644
--- a/core/tests/mockingcoretests/src/android/app/activity/ActivityThreadClientTest.java
+++ b/core/tests/mockingcoretests/src/android/app/activity/ActivityThreadClientTest.java
@@ -207,8 +207,8 @@ public class ActivityThreadClientTest {
         final Configuration currentConfig = new Configuration();
 
         assertFalse("Must not report change if no public diff",
-                shouldReportChange(0 /* publicDiff */, currentConfig, newConfig,
-                null /* sizeBuckets */, 0 /* handledConfigChanges */));
+                shouldReportChange(currentConfig, newConfig, null /* sizeBuckets */,
+                        0 /* handledConfigChanges */));
 
         final int[] verticalThresholds = {100, 400};
         final SizeConfigurationBuckets buckets = new SizeConfigurationBuckets(
@@ -221,25 +221,25 @@ public class ActivityThreadClientTest {
         newConfig.screenHeightDp = 300;
 
         assertFalse("Must not report changes if the diff is small and not handled",
-                shouldReportChange(CONFIG_SCREEN_SIZE /* publicDiff */, currentConfig,
-                newConfig, buckets, CONFIG_FONT_SCALE /* handledConfigChanges */));
+                shouldReportChange(currentConfig, newConfig, buckets,
+                        CONFIG_FONT_SCALE /* handledConfigChanges */));
 
         assertTrue("Must report changes if the small diff is handled",
-                shouldReportChange(CONFIG_SCREEN_SIZE /* publicDiff */, currentConfig, newConfig,
-                buckets, CONFIG_SCREEN_SIZE /* handledConfigChanges */));
+                shouldReportChange(currentConfig, newConfig, buckets,
+                        CONFIG_SCREEN_SIZE /* handledConfigChanges */));
 
         currentConfig.fontScale = 0.8f;
         newConfig.fontScale = 1.2f;
 
         assertTrue("Must report handled changes regardless of small unhandled change",
-                shouldReportChange(CONFIG_SCREEN_SIZE | CONFIG_FONT_SCALE /* publicDiff */,
-                currentConfig, newConfig, buckets, CONFIG_FONT_SCALE /* handledConfigChanges */));
+                shouldReportChange(currentConfig, newConfig, buckets,
+                        CONFIG_FONT_SCALE /* handledConfigChanges */));
 
         newConfig.screenHeightDp = 500;
 
         assertFalse("Must not report changes if there's unhandled big changes",
-                shouldReportChange(CONFIG_SCREEN_SIZE | CONFIG_FONT_SCALE /* publicDiff */,
-                currentConfig, newConfig, buckets, CONFIG_FONT_SCALE /* handledConfigChanges */));
+                shouldReportChange(currentConfig, newConfig, buckets,
+                        CONFIG_FONT_SCALE /* handledConfigChanges */));
     }
 
     private void recreateAndVerifyNoRelaunch(ActivityThread activityThread, TestActivity activity) {