From 658d984f048e846b9039b7a44dc44fd22756c830 Mon Sep 17 00:00:00 2001
From: Bryce Lee <brycelee@google.com>
Date: Fri, 28 Jul 2017 08:33:36 -0700
Subject: [PATCH] Do not call onConfigurationChanged for appBound position
 changes.

Since appBounds encodes both dimensions and positions, movement will
cause a diff change. This happens in situations where the dimensions
stay constant, such as dragging a PiP window around.

To avoid flooding the client side with configuration changes, this CL
checks whether the new configuration is equivalent to the existing
configuration with the exception of the position of the appBounds
before sending to the registered callbacks.

Change-Id: I8fbc94458fd9ed3b39494c3587f25e704ec02a7d
Fixes: 63927944
Test: bit FrameworksServicesTests:com.android.server.wm.AppBoundsTests
Test: go/wm-smoke
---
 core/java/android/app/ActivityThread.java     | 13 ++++++++++--
 core/java/android/app/ResourcesManager.java   |  9 ++++++---
 .../android/content/res/Configuration.java    | 20 ++++++++++++++++---
 .../com/android/server/wm/AppBoundsTests.java |  1 +
 4 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java
index 35cb2f15a0a36..895b448ae127e 100644
--- a/core/java/android/app/ActivityThread.java
+++ b/core/java/android/app/ActivityThread.java
@@ -4954,7 +4954,8 @@ public final class ActivityThread {
             // If the new config is the same as the config this Activity is already running with and
             // the override config also didn't change, then don't bother calling
             // onConfigurationChanged.
-            int diff = activity.mCurrentConfig.diff(newConfig);
+            final int diff = activity.mCurrentConfig.diffPublicOnly(newConfig);
+
             if (diff != 0 || !mResourcesManager.isSameResourcesOverrideConfig(activityToken,
                     amOverrideConfig)) {
                 // Always send the task-level config changes. For system-level configuration, if
@@ -5042,6 +5043,14 @@ public final class ActivityThread {
 
         int configDiff = 0;
 
+        // This flag tracks whether the new configuration is fundamentally equivalent to the
+        // existing configuration. This is necessary to determine whether non-activity
+        // callbacks should receive notice when the only changes are related to non-public fields.
+        // We do not gate calling {@link #performActivityConfigurationChanged} based on this flag
+        // as that method uses the same check on the activity config override as well.
+        final boolean equivalent = config != null && mConfiguration != null
+                && (0 == mConfiguration.diffPublicOnly(config));
+
         synchronized (mResourcesManager) {
             if (mPendingConfiguration != null) {
                 if (!mPendingConfiguration.isOtherSeqNewer(config)) {
@@ -5098,7 +5107,7 @@ public final class ActivityThread {
                     Activity a = (Activity) cb;
                     performConfigurationChangedForActivity(mActivities.get(a.getActivityToken()),
                             config);
-                } else {
+                } else if (!equivalent) {
                     performConfigurationChanged(cb, config);
                 }
             }
diff --git a/core/java/android/app/ResourcesManager.java b/core/java/android/app/ResourcesManager.java
index 6f326de76150b..636dea7ddaa11 100644
--- a/core/java/android/app/ResourcesManager.java
+++ b/core/java/android/app/ResourcesManager.java
@@ -44,8 +44,6 @@ import com.android.internal.util.ArrayUtils;
 
 import java.lang.ref.WeakReference;
 import java.util.ArrayList;
-import java.util.Iterator;
-import java.util.Map;
 import java.util.Objects;
 import java.util.WeakHashMap;
 import java.util.function.Predicate;
@@ -417,7 +415,12 @@ public class ResourcesManager {
             if (activityResources == null) {
                 return overrideConfig == null;
             } else {
-                return Objects.equals(activityResources.overrideConfig, overrideConfig);
+                // The two configurations must either be equal or publicly equivalent to be
+                // considered the same.
+                return Objects.equals(activityResources.overrideConfig, overrideConfig)
+                        || (overrideConfig != null && activityResources.overrideConfig != null
+                                && 0 == overrideConfig.diffPublicOnly(
+                                        activityResources.overrideConfig));
             }
         }
     }
diff --git a/core/java/android/content/res/Configuration.java b/core/java/android/content/res/Configuration.java
index af2713199950b..f7cccd56f0795 100644
--- a/core/java/android/content/res/Configuration.java
+++ b/core/java/android/content/res/Configuration.java
@@ -1318,7 +1318,19 @@ public final class Configuration implements Parcelable, Comparable<Configuration
      * PackageManager.ActivityInfo.CONFIG_LAYOUT_DIRECTION}.
      */
     public int diff(Configuration delta) {
-        return diff(delta, false /* compareUndefined */);
+        return diff(delta, false /* compareUndefined */, false /* publicOnly */);
+    }
+
+    /**
+     * Returns the diff against the provided {@link Configuration} excluding values that would
+     * publicly be equivalent, such as appBounds.
+     * @param delta {@link Configuration} to compare to.
+     *
+     * TODO(b/36812336): Remove once appBounds has been moved out of Configuration.
+     * {@hide}
+     */
+    public int diffPublicOnly(Configuration delta) {
+        return diff(delta, false /* compareUndefined */, true /* publicOnly */);
     }
 
     /**
@@ -1326,7 +1338,7 @@ public final class Configuration implements Parcelable, Comparable<Configuration
      *
      * @hide
      */
-    public int diff(Configuration delta, boolean compareUndefined) {
+    public int diff(Configuration delta, boolean compareUndefined, boolean publicOnly) {
         int changed = 0;
         if ((compareUndefined || delta.fontScale > 0) && fontScale != delta.fontScale) {
             changed |= ActivityInfo.CONFIG_FONT_SCALE;
@@ -1424,7 +1436,9 @@ public final class Configuration implements Parcelable, Comparable<Configuration
         // Make sure that one of the values is not null and that they are not equal.
         if ((compareUndefined || delta.appBounds != null)
                 && appBounds != delta.appBounds
-                && (appBounds == null || !appBounds.equals(delta.appBounds))) {
+                && (appBounds == null || (!publicOnly && !appBounds.equals(delta.appBounds))
+                        || (publicOnly && (appBounds.width() != delta.appBounds.width()
+                                || appBounds.height() != delta.appBounds.height())))) {
             changed |= ActivityInfo.CONFIG_SCREEN_SIZE;
         }
 
diff --git a/services/tests/servicestests/src/com/android/server/wm/AppBoundsTests.java b/services/tests/servicestests/src/com/android/server/wm/AppBoundsTests.java
index 6c95bd5146999..432cfc7a22eee 100644
--- a/services/tests/servicestests/src/com/android/server/wm/AppBoundsTests.java
+++ b/services/tests/servicestests/src/com/android/server/wm/AppBoundsTests.java
@@ -60,6 +60,7 @@ public class AppBoundsTests extends WindowTestsBase {
         config2.appBounds = new Rect(1, 2, 2, 1);
 
         assertEquals(ActivityInfo.CONFIG_SCREEN_SIZE, config.diff(config2));
+        assertEquals(0, config.diffPublicOnly(config2));
     }
 
     /**