Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b94292e5 authored by Andrii Kulian's avatar Andrii Kulian
Browse files

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.
parent c74c449b

services/core/java/com/android/server/am/ConfigurationContainer.java

+8 −4
Original line number Diff line number Diff line
@@ -114,11 +114,15 @@ abstract class ConfigurationContainer<E extends ConfigurationContainer> { 
     */

    void onParentChanged() {

        final ConfigurationContainer parent = getParent();

        // 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 != null ? parent.mFullConfiguration : Configuration.EMPTY);

            onConfigurationChanged(parent.mFullConfiguration);

            // Update merged override configuration of this container and all its children.

            onMergedOverrideConfigurationChanged();

        }

    }



    abstract protected int getChildCount();

services/core/java/com/android/server/wm/WindowContainer.java

+25 −17
Original line number Diff line number Diff line
@@ -71,11 +71,15 @@ class WindowContainer<E extends WindowContainer> implements Comparable<WindowCon 


    final protected void setParent(WindowContainer parent) {

        mParent = parent;

        // 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 != null ? mParent.mFullConfiguration : Configuration.EMPTY);

            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.

    private final LinkedList<WindowContainer> mTmpChain1 = new LinkedList();
@@ -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;

        }



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

                mChildren.add(i, child);

                return;

                    positionToAdd = i;

                    break;

                }

            }

        }



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

    }



    /**

services/tests/servicestests/src/com/android/server/am/ConfigurationContainerTests.java

+13 −5
Original line number Diff line number Diff line
@@ -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());

services/tests/servicestests/src/com/android/server/wm/WindowContainerTests.java

+14 −6
Original line number Diff line number Diff line
@@ -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());