Reland “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
Bug: 236415038
Test: repro steps on the bug
Test: atest ActivityThreadClientTest
Test: atest OverlayHostTest
Change-Id: I56a871fb30809f5f75f0007601a7629f9435e03a

1 files changed, 12 insertions, 14 deletions

diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java
index d19ecd450f0a..f384fa9e6a0b 100644
--- a/core/java/android/app/ActivityThread.java
+++ b/core/java/android/app/ActivityThread.java
@@ -5863,20 +5863,20 @@ 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.
-        if (!shouldUpdateResources) {
+        if (!shouldUpdateResources && !shouldReportChange) {
             return null;
         }
 
@@ -5896,9 +5896,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.
@@ -5909,8 +5906,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() +
@@ -5925,8 +5924,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}
@@ -5935,9 +5932,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;