Fix NPEs when display is added or removed

- In WindowContainer set parent of the child after it is actually
  added. This way if the child class depends on this in overridden
  methods it will be in correct state.
- Reconfigure display only if it is attached. Otherwise there is no
  corresponding DisplayContent record with configuration.

Change-Id: I20c51522d82f9d0ca98f098070585e6472a23e98
Test: Updated WindowContainerTests.

diff --git a/services/core/java/com/android/server/am/ConfigurationContainer.java b/services/core/java/com/android/server/am/ConfigurationContainer.java
index 30f5309..a3e95b8 100644 (file)
--- a/services/core/java/com/android/server/am/ConfigurationContainer.java
+++ b/services/core/java/com/android/server/am/ConfigurationContainer.java
@@ -114,10 +114,14 @@ abstract class ConfigurationContainer<E extends ConfigurationContainer> {
      */
     void onParentChanged() {
         final ConfigurationContainer parent = getParent();
-        // Update full configuration of this container and all its children.
-        onConfigurationChanged(parent != null ? parent.mFullConfiguration : Configuration.EMPTY);
-        // Update merged override configuration of this container and all its children.
-        onMergedOverrideConfigurationChanged();
+        // Removing parent usually means that we've detached this entity to destroy it or to attach
+        // to another parent. In both cases we don't need to update the configuration now.
+        if (parent != null) {
+            // Update full configuration of this container and all its children.
+            onConfigurationChanged(parent.mFullConfiguration);
+            // Update merged override configuration of this container and all its children.
+            onMergedOverrideConfigurationChanged();
+        }
     }
 
     abstract protected int getChildCount();

diff --git a/services/core/java/com/android/server/wm/WindowContainer.java b/services/core/java/com/android/server/wm/WindowContainer.java
index 285c40b..db61c3e 100644 (file)
--- a/services/core/java/com/android/server/wm/WindowContainer.java
+++ b/services/core/java/com/android/server/wm/WindowContainer.java
@@ -71,10 +71,14 @@ class WindowContainer<E extends WindowContainer> implements Comparable<WindowCon
 
     final protected void setParent(WindowContainer parent) {
         mParent = parent;
-        // Update full configuration of this container and all its children.
-        onConfigurationChanged(mParent != null ? mParent.mFullConfiguration : Configuration.EMPTY);
-        // Update merged override configuration of this container and all its children.
-        onMergedOverrideConfigurationChanged();
+        // Removing parent usually means that we've detached this entity to destroy it or to attach
+        // to another parent. In both cases we don't need to update the configuration now.
+        if (mParent != null) {
+            // Update full configuration of this container and all its children.
+            onConfigurationChanged(mParent.mFullConfiguration);
+            // Update merged override configuration of this container and all its children.
+            onMergedOverrideConfigurationChanged();
+        }
     }
 
     // Temp. holders for a chain of containers we are currently processing.
@@ -95,22 +99,25 @@ class WindowContainer<E extends WindowContainer> implements Comparable<WindowCon
                     + " is already a child of container=" + child.getParent().getName()
                     + " can't add to container=" + getName());
         }
-        child.setParent(this);
-
-        if (mChildren.isEmpty() || comparator == null) {
-            mChildren.add(child);
-            return;
-        }
 
-        final int count = mChildren.size();
-        for (int i = 0; i < count; i++) {
-            if (comparator.compare(child, mChildren.get(i)) < 0) {
-                mChildren.add(i, child);
-                return;
+        int positionToAdd = -1;
+        if (comparator != null) {
+            final int count = mChildren.size();
+            for (int i = 0; i < count; i++) {
+                if (comparator.compare(child, mChildren.get(i)) < 0) {
+                    positionToAdd = i;
+                    break;
+                }
             }
         }
 
-        mChildren.add(child);
+        if (positionToAdd == -1) {
+            mChildren.add(child);
+        } else {
+            mChildren.add(positionToAdd, child);
+        }
+        // Set the parent after we've actually added a child in case a subclass depends on this.
+        child.setParent(this);
     }
 
     /** Adds the input window container has a child of this container at the input index. */
@@ -121,8 +128,9 @@ class WindowContainer<E extends WindowContainer> implements Comparable<WindowCon
                     + " is already a child of container=" + child.getParent().getName()
                     + " can't add to container=" + getName());
         }
-        child.setParent(this);
         mChildren.add(index, child);
+        // Set the parent after we've actually added a child in case a subclass depends on this.
+        child.setParent(this);
     }
 
     /**

diff --git a/services/tests/servicestests/src/com/android/server/am/ConfigurationContainerTests.java b/services/tests/servicestests/src/com/android/server/am/ConfigurationContainerTests.java
index 92c442e..fd2fb4b 100644 (file)
--- a/services/tests/servicestests/src/com/android/server/am/ConfigurationContainerTests.java
+++ b/services/tests/servicestests/src/com/android/server/am/ConfigurationContainerTests.java
@@ -97,22 +97,30 @@ public class ConfigurationContainerTests {
         final Configuration childOverrideConfig = new Configuration();
         childOverrideConfig.densityDpi = 320;
         child.onOverrideConfigurationChanged(childOverrideConfig);
+        final Configuration mergedOverrideConfig = new Configuration(root.getConfiguration());
+        mergedOverrideConfig.updateFrom(childOverrideConfig);
 
         // Check configuration update when child is removed from parent.
         root.removeChild(child);
         assertEquals(childOverrideConfig, child.getOverrideConfiguration());
-        assertEquals(childOverrideConfig, child.getMergedOverrideConfiguration());
-        assertEquals(childOverrideConfig, child.getConfiguration());
+        assertEquals(mergedOverrideConfig, child.getMergedOverrideConfiguration());
+        assertEquals(mergedOverrideConfig, child.getConfiguration());
 
         // It may be paranoia... but let's check if parent's config didn't change after removal.
         assertEquals(rootOverrideConfig, root.getOverrideConfiguration());
         assertEquals(rootOverrideConfig, root.getMergedOverrideConfiguration());
         assertEquals(rootOverrideConfig, root.getConfiguration());
 
-        // Check configuration update when child is added to parent.
-        final Configuration mergedOverrideConfig = new Configuration(root.getConfiguration());
+        // Init different root
+        final TestConfigurationContainer root2 = new TestConfigurationContainer();
+        final Configuration rootOverrideConfig2 = new Configuration();
+        rootOverrideConfig2.fontScale = 1.1f;
+        root2.onOverrideConfigurationChanged(rootOverrideConfig2);
+
+        // Check configuration update when child is added to different parent.
+        mergedOverrideConfig.setTo(rootOverrideConfig2);
         mergedOverrideConfig.updateFrom(childOverrideConfig);
-        root.addChild(child);
+        root2.addChild(child);
         assertEquals(childOverrideConfig, child.getOverrideConfiguration());
         assertEquals(mergedOverrideConfig, child.getMergedOverrideConfiguration());
         assertEquals(mergedOverrideConfig, child.getConfiguration());

diff --git a/services/tests/servicestests/src/com/android/server/wm/WindowContainerTests.java b/services/tests/servicestests/src/com/android/server/wm/WindowContainerTests.java
index 6eb347b..128317c 100644 (file)
--- a/services/tests/servicestests/src/com/android/server/wm/WindowContainerTests.java
+++ b/services/tests/servicestests/src/com/android/server/wm/WindowContainerTests.java
@@ -434,22 +434,30 @@ public class WindowContainerTests {
         final Configuration childOverrideConfig = new Configuration();
         childOverrideConfig.densityDpi = 320;
         child.onOverrideConfigurationChanged(childOverrideConfig);
+        final Configuration mergedOverrideConfig = new Configuration(root.getConfiguration());
+        mergedOverrideConfig.updateFrom(childOverrideConfig);
 
-        // Check configuration update when child is removed from parent.
+        // Check configuration update when child is removed from parent - it should remain same.
         root.removeChild(child);
         assertEquals(childOverrideConfig, child.getOverrideConfiguration());
-        assertEquals(childOverrideConfig, child.getMergedOverrideConfiguration());
-        assertEquals(childOverrideConfig, child.getConfiguration());
+        assertEquals(mergedOverrideConfig, child.getMergedOverrideConfiguration());
+        assertEquals(mergedOverrideConfig, child.getConfiguration());
 
         // It may be paranoia... but let's check if parent's config didn't change after removal.
         assertEquals(rootOverrideConfig, root.getOverrideConfiguration());
         assertEquals(rootOverrideConfig, root.getMergedOverrideConfiguration());
         assertEquals(rootOverrideConfig, root.getConfiguration());
 
-        // Check configuration update when child is added to parent.
-        final Configuration mergedOverrideConfig = new Configuration(root.getConfiguration());
+        // Init different root
+        final TestWindowContainer root2 = builder.setLayer(0).build();
+        final Configuration rootOverrideConfig2 = new Configuration();
+        rootOverrideConfig2.fontScale = 1.1f;
+        root2.onOverrideConfigurationChanged(rootOverrideConfig2);
+
+        // Check configuration update when child is added to different parent.
+        mergedOverrideConfig.setTo(rootOverrideConfig2);
         mergedOverrideConfig.updateFrom(childOverrideConfig);
-        root.addChildWindow(child);
+        root2.addChildWindow(child);
         assertEquals(childOverrideConfig, child.getOverrideConfiguration());
         assertEquals(mergedOverrideConfig, child.getMergedOverrideConfiguration());
         assertEquals(mergedOverrideConfig, child.getConfiguration());