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

Commit b6478edb authored by Tiger Huang's avatar Tiger Huang
Browse files

Dispatch insets to client if mState is changed 

Previous logic in onStateChanged notifies insetsChanged based on the
change of mLastDispatchedState, which can make us dispatch redundant
insets changes to the app.

In this CL, we only notifies insetsChanged if mState is really changed
in onStateChanged -- we use the final mState (after updateState and
applyLocalVisibilityOverride) to compare with the one before changing.

Fix: 161924448
Test: atest InsetsControllerTest WindowInsetsControllerTests
Test: Swipe up to home while IME open and see if there is any jank
Change-Id: Ia536cdf76805caa56ca1b6eaf2b3db83b6ecd94e
parent 40a85e28

core/java/android/view/InsetsController.java

+10 −6
Original line number Original line Diff line number Diff line
@@ -618,16 +618,20 @@ public class InsetsController implements WindowInsetsController, InsetsAnimation 
            return false;

            return false;

        }

        }

        if (DEBUG) Log.d(TAG, "onStateChanged: " + state);

        if (DEBUG) Log.d(TAG, "onStateChanged: " + state);

        updateState(state);



        boolean localStateChanged = !mState.equals(mLastDispatchedState,

                true /* excludingCaptionInsets */, true /* excludeInvisibleIme */);

        mLastDispatchedState.set(state, true /* copySources */);

        mLastDispatchedState.set(state, true /* copySources */);





        final InsetsState lastState = new InsetsState(mState, true /* copySources */);

        updateState(state);

        applyLocalVisibilityOverride();

        applyLocalVisibilityOverride();

        if (localStateChanged) {



            if (DEBUG) Log.d(TAG, "onStateChanged, notifyInsetsChanged, send state to WM: " + mState);

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

                true /* excludeInvisibleIme */)) {

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

            mHost.notifyInsetsChanged();

            mHost.notifyInsetsChanged();

        }

        if (!mState.equals(state, true /* excludingCaptionInsets */,

                true /* excludeInvisibleIme */)) {

            if (DEBUG) Log.d(TAG, "onStateChanged, send state to WM: " + mState);

            updateRequestedState();

            updateRequestedState();

        }

        }

        return true;

        return true;

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

+99 −2
Original line number Original line Diff line number Diff line
@@ -27,6 +27,7 @@ import static android.view.InsetsState.ITYPE_NAVIGATION_BAR; 
import static android.view.InsetsState.ITYPE_STATUS_BAR;

import static android.view.InsetsState.ITYPE_STATUS_BAR;

import static android.view.ViewRootImpl.NEW_INSETS_MODE_FULL;

import static android.view.ViewRootImpl.NEW_INSETS_MODE_FULL;

import static android.view.WindowInsets.Type.ime;

import static android.view.WindowInsets.Type.ime;

import static android.view.WindowInsets.Type.navigationBars;

import static android.view.WindowInsets.Type.statusBars;

import static android.view.WindowInsets.Type.statusBars;

import static android.view.WindowManager.LayoutParams.SOFT_INPUT_ADJUST_RESIZE;

import static android.view.WindowManager.LayoutParams.SOFT_INPUT_ADJUST_RESIZE;
@@ -40,8 +41,11 @@ import static org.mockito.ArgumentMatchers.any; 
import static org.mockito.ArgumentMatchers.anyInt;

import static org.mockito.ArgumentMatchers.anyInt;

import static org.mockito.ArgumentMatchers.eq;

import static org.mockito.ArgumentMatchers.eq;

import static org.mockito.ArgumentMatchers.notNull;

import static org.mockito.ArgumentMatchers.notNull;

import static org.mockito.Mockito.clearInvocations;

import static org.mockito.Mockito.mock;

import static org.mockito.Mockito.mock;

import static org.mockito.Mockito.never;

import static org.mockito.Mockito.never;

import static org.mockito.Mockito.spy;

import static org.mockito.Mockito.times;

import static org.mockito.Mockito.verify;

import static org.mockito.Mockito.verify;





import android.content.Context;

import android.content.Context;
@@ -124,7 +128,7 @@ public class InsetsControllerTest { 
            }

            }

            mTestClock = new OffsettableClock();

            mTestClock = new OffsettableClock();

            mTestHandler = new TestHandler(null, mTestClock);

            mTestHandler = new TestHandler(null, mTestClock);

            mTestHost = new TestHost(mViewRoot);

            mTestHost = spy(new TestHost(mViewRoot));

            mController = new InsetsController(mTestHost, (controller, type) -> {

            mController = new InsetsController(mTestHost, (controller, type) -> {

                if (type == ITYPE_IME) {

                if (type == ITYPE_IME) {

                    return new InsetsSourceConsumer(type, controller.getState(),

                    return new InsetsSourceConsumer(type, controller.getState(),
@@ -745,6 +749,99 @@ public class InsetsControllerTest { 
        });

        });

    }

    }





    @Test

    public void testInsetsChangedCount_controlSystemBars() {

        InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> {

            prepareControls();



            // Hiding visible system bars should only causes insets change once for each bar.

            clearInvocations(mTestHost);

            mController.hide(statusBars() | navigationBars());

            verify(mTestHost, times(2)).notifyInsetsChanged();



            // Sending the same insets state should not cause insets change.

            // This simulates the callback from server after hiding system bars.

            clearInvocations(mTestHost);

            mController.onStateChanged(mController.getState());

            verify(mTestHost, never()).notifyInsetsChanged();



            // Showing invisible system bars should only causes insets change once for each bar.

            clearInvocations(mTestHost);

            mController.show(statusBars() | navigationBars());

            verify(mTestHost, times(2)).notifyInsetsChanged();



            // Sending the same insets state should not cause insets change.

            // This simulates the callback from server after showing system bars.

            clearInvocations(mTestHost);

            mController.onStateChanged(mController.getState());

            verify(mTestHost, never()).notifyInsetsChanged();

        });

    }



    @Test

    public void testInsetsChangedCount_controlIme() {

        InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> {

            prepareControls();



            // Showing invisible ime should only causes insets change once.

            clearInvocations(mTestHost);

            mController.show(ime(), true /* fromIme */);

            verify(mTestHost, times(1)).notifyInsetsChanged();



            // Sending the same insets state should not cause insets change.

            // This simulates the callback from server after showing ime.

            clearInvocations(mTestHost);

            mController.onStateChanged(mController.getState());

            verify(mTestHost, never()).notifyInsetsChanged();



            // Hiding visible ime should only causes insets change once.

            clearInvocations(mTestHost);

            mController.hide(ime());

            verify(mTestHost, times(1)).notifyInsetsChanged();



            // Sending the same insets state should not cause insets change.

            // This simulates the callback from server after hiding ime.

            clearInvocations(mTestHost);

            mController.onStateChanged(mController.getState());

            verify(mTestHost, never()).notifyInsetsChanged();

        });

    }



    @Test

    public void testInsetsChangedCount_onStateChanged() {

        InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> {

            final InsetsState localState = mController.getState();



            // Changing status bar frame should cause notifyInsetsChanged.

            clearInvocations(mTestHost);

            InsetsState newState = new InsetsState(localState, true /* copySources */);

            newState.getSource(ITYPE_STATUS_BAR).getFrame().bottom++;

            mController.onStateChanged(newState);

            verify(mTestHost, times(1)).notifyInsetsChanged();



            // Changing status bar visibility should cause notifyInsetsChanged.

            clearInvocations(mTestHost);

            newState = new InsetsState(localState, true /* copySources */);

            newState.getSource(ITYPE_STATUS_BAR).setVisible(false);

            mController.onStateChanged(newState);

            verify(mTestHost, times(1)).notifyInsetsChanged();



            // Changing invisible IME frame should not cause notifyInsetsChanged.

            clearInvocations(mTestHost);

            newState = new InsetsState(localState, true /* copySources */);

            newState.getSource(ITYPE_IME).getFrame().top--;

            mController.onStateChanged(newState);

            verify(mTestHost, never()).notifyInsetsChanged();



            // Changing IME visibility should cause notifyInsetsChanged.

            clearInvocations(mTestHost);

            newState = new InsetsState(localState, true /* copySources */);

            newState.getSource(ITYPE_IME).setVisible(true);

            mController.onStateChanged(newState);

            verify(mTestHost, times(1)).notifyInsetsChanged();

        });

    }



    private void waitUntilNextFrame() throws Exception {

    private void waitUntilNextFrame() throws Exception {

        final CountDownLatch latch = new CountDownLatch(1);

        final CountDownLatch latch = new CountDownLatch(1);

        Choreographer.getMainThreadInstance().postCallback(Choreographer.CALLBACK_COMMIT,

        Choreographer.getMainThreadInstance().postCallback(Choreographer.CALLBACK_COMMIT,
@@ -777,7 +874,7 @@ public class InsetsControllerTest { 
        return controls;

        return controls;

    }

    }





    private static class TestHost extends ViewRootInsetsControllerHost {

    public static class TestHost extends ViewRootInsetsControllerHost {





        private InsetsState mModifiedState = new InsetsState();

        private InsetsState mModifiedState = new InsetsState();