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

Commit 2d44a29f authored by Tiger's avatar Tiger
Browse files

Refine equals of both InsetsState and WindowInsets 

InsetsState:
It had logic errors in InsetsState#equals while excludesCaptionBar or
excludesInvisibleIme is true. For example, if the last source in one
InsetsState.mSources is captionBar() or ime(), but the other one doesn't
have a source of the same type, the type of source won't be excluded,
which make it always returns false, regardless of excludesCaptionBar or
excludesInvisibleIme.

WindowInsets:
Two WindowInsets could be considered as non-equal while they provide the
same insets of each type from WindowInsets#getInsets if one WindowInsets
has Insets.NONE for a type and the other one has null for the same type.

This CL fixes both problems.

Fix: 284406283
Test: OnApplyWindowInsetsListenerTest
Test: atest InsetsStateTest
Change-Id: I31b078737def461bd51eab9bb1114307a8ab994f
parent bb33d7cb

core/java/android/view/InsetsController.java

+6 −6
Original line number Diff line number Diff line
@@ -850,12 +850,12 @@ public class InsetsController implements WindowInsetsController, InsetsAnimation 
    public boolean onStateChanged(InsetsState state) {

        boolean stateChanged = false;

        if (!CAPTION_ON_SHELL) {

            stateChanged = !mState.equals(state, true /* excludingCaptionInsets */,

                    false /* excludeInvisibleIme */)

            stateChanged = !mState.equals(state, true /* excludesCaptionBar */,

                    false /* excludesInvisibleIme */)

                    || captionInsetsUnchanged();

        } else {

            stateChanged = !mState.equals(state, false /* excludingCaptionInsets */,

                    false /* excludeInvisibleIme */);

            stateChanged = !mState.equals(state, false /* excludesCaptionBar */,

                    false /* excludesInvisibleIme */);

        }

        if (!stateChanged && mLastDispatchedState.equals(state)) {

            return false;
@@ -868,8 +868,8 @@ public class InsetsController implements WindowInsetsController, InsetsAnimation 
        applyLocalVisibilityOverride();

        updateCompatSysUiVisibility();



        if (!mState.equals(lastState, false /* excludingCaptionInsets */,

                true /* excludeInvisibleIme */)) {

        if (!mState.equals(lastState, false /* excludesCaptionBar */,

                true /* excludesInvisibleIme */)) {

            if (DEBUG) Log.d(TAG, "onStateChanged, notifyInsetsChanged");

            mHost.notifyInsetsChanged();

            if (lastState.getDisplayFrame().equals(mState.getDisplayFrame())) {

core/java/android/view/InsetsState.java

+31 −20
Original line number Diff line number Diff line
@@ -380,12 +380,18 @@ public class InsetsState implements Parcelable { 
            @InternalInsetsSide @Nullable SparseIntArray idSideMap,

            @Nullable boolean[] typeVisibilityMap, Insets insets, int type) {

        int index = indexOf(type);



        // Don't put Insets.NONE into typeInsetsMap. Otherwise, two WindowInsets can be considered

        // as non-equal while they provide the same insets of each type from WindowInsets#getInsets

        // if one WindowInsets has Insets.NONE for a type and the other has null for the same type.

        if (!Insets.NONE.equals(insets)) {

            Insets existing = typeInsetsMap[index];

            if (existing == null) {

                typeInsetsMap[index] = insets;

            } else {

                typeInsetsMap[index] = Insets.max(existing, insets);

            }

        }



        if (typeVisibilityMap != null) {

            typeVisibilityMap[index] = source.isVisible();
@@ -696,15 +702,14 @@ public class InsetsState implements Parcelable { 
     * An equals method can exclude the caption insets. This is useful because we assemble the

     * caption insets information on the client side, and when we communicate with server, it's

     * excluded.

     * @param excludingCaptionInsets {@code true} if we want to compare two InsetsState objects but

     *                                           ignore the caption insets source value.

     * @param excludeInvisibleImeFrames If {@link WindowInsets.Type#ime()} frames should be ignored

     *                                  when IME is not visible.

     * @param excludesCaptionBar If {@link Type#captionBar()}} should be ignored.

     * @param excludesInvisibleIme If {@link WindowInsets.Type#ime()} should be ignored when IME is

     *                             not visible.

     * @return {@code true} if the two InsetsState objects are equal, {@code false} otherwise.

     */

    @VisibleForTesting

    public boolean equals(@Nullable Object o, boolean excludingCaptionInsets,

            boolean excludeInvisibleImeFrames) {

    public boolean equals(@Nullable Object o, boolean excludesCaptionBar,

            boolean excludesInvisibleIme) {

        if (this == o) { return true; }

        if (o == null || getClass() != o.getClass()) { return false; }
@@ -721,29 +726,35 @@ public class InsetsState implements Parcelable { 


        final SparseArray<InsetsSource> thisSources = mSources;

        final SparseArray<InsetsSource> thatSources = state.mSources;

        if (!excludingCaptionInsets && !excludeInvisibleImeFrames) {

        if (!excludesCaptionBar && !excludesInvisibleIme) {

            return thisSources.contentEquals(thatSources);

        } else {

            final int thisSize = thisSources.size();

            final int thatSize = thatSources.size();

            int thisIndex = 0;

            int thatIndex = 0;

            while (thisIndex < thisSize && thatIndex < thatSize) {

            while (thisIndex < thisSize || thatIndex < thatSize) {

                InsetsSource thisSource = thisIndex < thisSize

                        ? thisSources.valueAt(thisIndex)

                        : null;



                // Seek to the next non-excluding source of ours.

                InsetsSource thisSource = thisSources.valueAt(thisIndex);

                while (thisSource != null

                        && (excludingCaptionInsets && thisSource.getType() == captionBar()

                                || excludeInvisibleImeFrames && thisSource.getType() == ime()

                        && (excludesCaptionBar && thisSource.getType() == captionBar()

                                || excludesInvisibleIme && thisSource.getType() == ime()

                                        && !thisSource.isVisible())) {

                    thisIndex++;

                    thisSource = thisIndex < thisSize ? thisSources.valueAt(thisIndex) : null;

                }



                InsetsSource thatSource = thatIndex < thatSize

                        ? thatSources.valueAt(thatIndex)

                        : null;



                // Seek to the next non-excluding source of theirs.

                InsetsSource thatSource = thatSources.valueAt(thatIndex);

                while (thatSource != null

                        && (excludingCaptionInsets && thatSource.getType() == captionBar()

                                || excludeInvisibleImeFrames && thatSource.getType() == ime()

                        && (excludesCaptionBar && thatSource.getType() == captionBar()

                                || excludesInvisibleIme && thatSource.getType() == ime()

                                        && !thatSource.isVisible())) {

                    thatIndex++;

                    thatSource = thatIndex < thatSize ? thatSources.valueAt(thatIndex) : null;
@@ -756,7 +767,7 @@ public class InsetsState implements Parcelable { 
                thisIndex++;

                thatIndex++;

            }

            return thisIndex >= thisSize && thatIndex >= thatSize;

            return true;

        }

    }

core/tests/coretests/src/android/view/InsetsStateTest.java

+80 −11
Original line number Diff line number Diff line
@@ -43,6 +43,7 @@ import static android.view.WindowManager.LayoutParams.TYPE_SYSTEM_ERROR; 
import static android.view.WindowManager.LayoutParams.TYPE_WALLPAPER;



import static org.junit.Assert.assertEquals;

import static org.junit.Assert.assertFalse;

import static org.junit.Assert.assertNotEquals;

import static org.junit.Assert.assertNotNull;

import static org.junit.Assert.assertNull;
@@ -289,6 +290,18 @@ public class InsetsStateTest { 
        assertEquals(Insets.of(0, 0, 20, 0), insets.getInsets(navigationBars()));

    }



    @Test

    public void testCalculateInsets_emptyIme() {

        WindowInsets insets1 = mState.calculateInsets(new Rect(), null, false, false,

                SOFT_INPUT_ADJUST_NOTHING, 0, 0, TYPE_APPLICATION, WINDOWING_MODE_UNDEFINED, null);

        mState.getOrCreateSource(ID_IME, ime());

        WindowInsets insets2 = mState.calculateInsets(new Rect(), null, false, false,

                SOFT_INPUT_ADJUST_NOTHING, 0, 0, TYPE_APPLICATION, WINDOWING_MODE_UNDEFINED, null);

        assertEquals(Insets.NONE, insets1.getInsets(ime()));

        assertEquals(Insets.NONE, insets2.getInsets(ime()));

        assertEquals(insets1, insets2);

    }



    @Test

    public void testStripForDispatch() {

        mState.getOrCreateSource(ID_STATUS_BAR, statusBars())
@@ -303,6 +316,73 @@ public class InsetsStateTest { 
        assertEquals(0, insets.getSystemWindowInsetBottom());

    }



    @Test

    public void testEquals() {

        final InsetsState state1 = new InsetsState();

        final InsetsState state2 = new InsetsState();

        assertTrue(state1.equals(state2));



        state1.addSource(new InsetsSource(ID_STATUS_BAR, statusBars()));

        assertFalse(state1.equals(state2));



        state2.addSource(new InsetsSource(ID_STATUS_BAR, statusBars()));

        assertTrue(state1.equals(state2));



        state2.addSource(new InsetsSource(ID_NAVIGATION_BAR, navigationBars()));

        assertFalse(state1.equals(state2));

    }



    @Test

    public void testEquals_excludesCaptionBar() {

        final InsetsState state1 = new InsetsState();

        final InsetsState state2 = new InsetsState();



        state1.addSource(new InsetsSource(ID_CAPTION_BAR, captionBar()).setFrame(0, 0, 0, 5));

        assertFalse(state1.equals(

                state2, false /* excludesCaptionBar */, false /* excludesInvisibleIme */));

        assertTrue(state1.equals(

                state2, true /* excludesCaptionBar */, false /* excludesInvisibleIme */));



        state2.addSource(new InsetsSource(ID_CAPTION_BAR, captionBar()).setFrame(0, 0, 0, 10));

        assertFalse(state1.equals(

                state2, false /* excludesCaptionBar */, false /* excludesInvisibleIme */));

        assertTrue(state1.equals(

                state2, true /* excludesCaptionBar */, false /* excludesInvisibleIme */));



        state1.addSource(new InsetsSource(ID_STATUS_BAR, statusBars()));

        state2.addSource(new InsetsSource(ID_STATUS_BAR, statusBars()));

        assertFalse(state1.equals(

                state2, false /* excludesCaptionBar */, false /* excludesInvisibleIme */));

        assertTrue(state1.equals(

                state2, true /* excludesCaptionBar */, false /* excludesInvisibleIme */));

    }



    @Test

    public void testEquals_excludesInvisibleIme() {

        final InsetsState state1 = new InsetsState();

        final InsetsState state2 = new InsetsState();



        final InsetsSource imeSource1 = new InsetsSource(ID_IME, ime()).setVisible(true);

        state1.addSource(imeSource1);

        assertFalse(state1.equals(

                state2, false /* excludesCaptionBar */, false /* excludesInvisibleIme */));

        assertFalse(state1.equals(

                state2, false /* excludesCaptionBar */, true /* excludesInvisibleIme */));



        imeSource1.setVisible(false);

        assertFalse(state1.equals(

                state2, false /* excludesCaptionBar */, false /* excludesInvisibleIme */));

        assertTrue(state1.equals(

                state2, false /* excludesCaptionBar */, true /* excludesInvisibleIme */));



        final InsetsSource imeSource2 = new InsetsSource(ID_IME, ime()).setFrame(0, 0, 0, 10);

        state2.addSource(imeSource2);

        assertFalse(state1.equals(

                state2, false /* excludesCaptionBar */, false /* excludesInvisibleIme */));

        assertTrue(state1.equals(

                state2, false /* excludesCaptionBar */, true /* excludesInvisibleIme */));

    }



    @Test

    public void testEquals_differentRect() {

        mState.getOrCreateSource(ID_STATUS_BAR, statusBars())
@@ -403,17 +483,6 @@ public class InsetsStateTest { 
        assertNotEqualsAndHashCode();

    }



    @Test

    public void testEquals_excludeInvisibleIme() {

        mState.getOrCreateSource(ID_IME, ime())

                .setFrame(new Rect(0, 0, 100, 100))

                .setVisible(false);

        mState2.getOrCreateSource(ID_IME, ime())

                .setFrame(new Rect(0, 0, 100, 200))

                .setVisible(false);

        assertTrue(mState2.equals(mState, true, true /* excludeInvisibleIme */));

    }



    @Test

    public void testParcelUnparcel() {

        mState.getOrCreateSource(ID_IME, ime())