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
Merged-In: Ia536cdf76805caa56ca1b6eaf2b3db83b6ecd94e
Change-Id: Ia536cdf76805caa56ca1b6eaf2b3db83b6ecd94e
This commit is contained in:
Tiger Huang
2020-07-27 23:46:30 +08:00
parent 6128b1ba0f
commit f8175d1a5b
2 changed files with 109 additions and 8 deletions

View File

@@ -618,16 +618,20 @@ public class InsetsController implements WindowInsetsController, InsetsAnimation
return false;
}
if (DEBUG) Log.d(TAG, "onStateChanged: " + state);
updateState(state);
boolean localStateChanged = !mState.equals(mLastDispatchedState,
true /* excludingCaptionInsets */, true /* excludeInvisibleIme */);
mLastDispatchedState.set(state, true /* copySources */);
final InsetsState lastState = new InsetsState(mState, true /* copySources */);
updateState(state);
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();
}
if (!mState.equals(mLastDispatchedState, true /* excludingCaptionInsets */,
true /* excludeInvisibleIme */)) {
if (DEBUG) Log.d(TAG, "onStateChanged, send state to WM: " + mState);
updateRequestedState();
}
return true;

View File

@@ -27,6 +27,7 @@ import static android.view.InsetsState.ITYPE_NAVIGATION_BAR;
import static android.view.InsetsState.ITYPE_STATUS_BAR;
import static android.view.ViewRootImpl.NEW_INSETS_MODE_FULL;
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.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.eq;
import static org.mockito.ArgumentMatchers.notNull;
import static org.mockito.Mockito.clearInvocations;
import static org.mockito.Mockito.mock;
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 android.content.Context;
@@ -124,7 +128,7 @@ public class InsetsControllerTest {
}
mTestClock = new OffsettableClock();
mTestHandler = new TestHandler(null, mTestClock);
mTestHost = new TestHost(mViewRoot);
mTestHost = spy(new TestHost(mViewRoot));
mController = new InsetsController(mTestHost, (controller, type) -> {
if (type == ITYPE_IME) {
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 {
final CountDownLatch latch = new CountDownLatch(1);
Choreographer.getMainThreadInstance().postCallback(Choreographer.CALLBACK_COMMIT,
@@ -777,7 +874,7 @@ public class InsetsControllerTest {
return controls;
}
private static class TestHost extends ViewRootInsetsControllerHost {
public static class TestHost extends ViewRootInsetsControllerHost {
private InsetsState mModifiedState = new InsetsState();