From 173480f29de378076256d6b0e2f37cb42e861fd5 Mon Sep 17 00:00:00 2001 From: Tiger Huang Date: Wed, 29 Apr 2020 01:05:42 +0800 Subject: [PATCH] Ensure the requested insets state is up to date Previously, the client won't send the modified insets state to window manager if the dispatched state and the local state are the same. The following case can make the requested insets state at the server side stale: 1. Window A requests an insets source to be invisible while having the control. 2. Window A loses the control. 3. The new control target requests the insets source to be visible. 4. Window A receives the new insets state from server. 5. Window A also requests the insets source to be visible while not having the control. 6. Window A gain the control. 7. Window A won't send the new requested state (visible) to window manager because the local state and the dispatched state are the same. 8. Window manager keeps assuming that window A is requesting the insets source to be invisible which is incorrect. This CL stores what insets state is sent to server, and updates/sends the state to server while gaining the control if the requested state is changed. Fix: 146964271 Test: Manual. Change-Id: I6ee533d9316c769faef539bfb980197c351ee8d1 --- core/java/android/view/InsetsController.java | 56 +++++++++++++------ .../android/view/InsetsControllerTest.java | 42 +++++++++++++- 2 files changed, 78 insertions(+), 20 deletions(-) diff --git a/core/java/android/view/InsetsController.java b/core/java/android/view/InsetsController.java index 887607972bbc6..a135b0ca148b7 100644 --- a/core/java/android/view/InsetsController.java +++ b/core/java/android/view/InsetsController.java @@ -22,8 +22,6 @@ import static android.view.InsetsState.toInternalType; import static android.view.InsetsState.toPublicType; import static android.view.WindowInsets.Type.all; import static android.view.WindowInsets.Type.ime; -import static android.view.WindowManager.LayoutParams.PRIVATE_FLAG_APPEARANCE_CONTROLLED; -import static android.view.WindowManager.LayoutParams.PRIVATE_FLAG_BEHAVIOR_CONTROLLED; import android.animation.AnimationHandler; import android.animation.Animator; @@ -43,7 +41,6 @@ import android.util.SparseArray; import android.view.InsetsSourceConsumer.ShowResult; import android.view.InsetsState.InternalInsetsType; import android.view.SurfaceControl.Transaction; -import android.view.ViewTreeObserver.OnPreDrawListener; import android.view.WindowInsets.Type; import android.view.WindowInsets.Type.InsetsType; import android.view.WindowInsetsAnimation.Bounds; @@ -424,8 +421,14 @@ public class InsetsController implements WindowInsetsController, InsetsAnimation private final String TAG = "InsetsControllerImpl"; + /** The local state */ private final InsetsState mState = new InsetsState(); - private final InsetsState mLastDispachedState = new InsetsState(); + + /** The state dispatched from server */ + private final InsetsState mLastDispatchedState = new InsetsState(); + + /** The state sent to server */ + private final InsetsState mRequestedState = new InsetsState(); private final Rect mFrame = new Rect(); private final BiFunction mConsumerCreator; @@ -539,24 +542,24 @@ public class InsetsController implements WindowInsetsController, InsetsAnimation } public InsetsState getLastDispatchedState() { - return mLastDispachedState; + return mLastDispatchedState; } @VisibleForTesting public boolean onStateChanged(InsetsState state) { boolean localStateChanged = !mState.equals(state, true /* excludingCaptionInsets */) || !captionInsetsUnchanged(); - if (!localStateChanged && mLastDispachedState.equals(state)) { + if (!localStateChanged && mLastDispatchedState.equals(state)) { return false; } updateState(state); - mLastDispachedState.set(state, true /* copySources */); + mLastDispatchedState.set(state, true /* copySources */); applyLocalVisibilityOverride(); if (localStateChanged) { mHost.notifyInsetsChanged(); } - if (!mState.equals(mLastDispachedState, true /* excludingCaptionInsets */)) { - sendStateToWindowManager(); + if (!mState.equals(mLastDispatchedState, true /* excludingCaptionInsets */)) { + updateRequestedState(); } return true; } @@ -629,8 +632,9 @@ public class InsetsController implements WindowInsetsController, InsetsAnimation } } - int[] showTypes = new int[1]; - int[] hideTypes = new int[1]; + final boolean hasControl = mTmpControlArray.size() > 0; + final int[] showTypes = new int[1]; + final int[] hideTypes = new int[1]; // Ensure to update all existing source consumers for (int i = mSourceConsumers.size() - 1; i >= 0; i--) { @@ -663,6 +667,12 @@ public class InsetsController implements WindowInsetsController, InsetsAnimation if (hideTypes[0] != 0) { applyAnimation(hideTypes[0], false /* show */, false /* fromIme */); } + if (hasControl) { + // We might have changed our requested visibilities while we don't have the control, + // so we need to update our requested state once we have control. Otherwise, our + // requested state at the server side might be incorrect. + updateRequestedState(); + } } @Override @@ -992,7 +1002,7 @@ public class InsetsController implements WindowInsetsController, InsetsAnimation @VisibleForTesting public void notifyVisibilityChanged() { mHost.notifyInsetsChanged(); - sendStateToWindowManager(); + updateRequestedState(); } /** @@ -1042,18 +1052,28 @@ public class InsetsController implements WindowInsetsController, InsetsAnimation } /** - * Sends the local visibility state back to window manager. + * Sends the local visibility state back to window manager if it is changed. */ - private void sendStateToWindowManager() { - InsetsState tmpState = new InsetsState(); + private void updateRequestedState() { + boolean changed = false; for (int i = mSourceConsumers.size() - 1; i >= 0; i--) { final InsetsSourceConsumer consumer = mSourceConsumers.valueAt(i); - if (consumer.getType() == ITYPE_CAPTION_BAR) continue; + final @InternalInsetsType int type = consumer.getType(); + if (type == ITYPE_CAPTION_BAR) { + continue; + } if (consumer.getControl() != null) { - tmpState.addSource(mState.getSource(consumer.getType())); + final InsetsSource localSource = mState.getSource(type); + if (!localSource.equals(mRequestedState.peekSource(type))) { + mRequestedState.addSource(new InsetsSource(localSource)); + changed = true; + } } } - mHost.onInsetsModified(tmpState); + if (!changed) { + return; + } + mHost.onInsetsModified(mRequestedState); } @VisibleForTesting diff --git a/core/tests/coretests/src/android/view/InsetsControllerTest.java b/core/tests/coretests/src/android/view/InsetsControllerTest.java index ccf4192ac2f48..cc85332590bae 100644 --- a/core/tests/coretests/src/android/view/InsetsControllerTest.java +++ b/core/tests/coretests/src/android/view/InsetsControllerTest.java @@ -93,6 +93,7 @@ public class InsetsControllerTest { private SurfaceSession mSession = new SurfaceSession(); private SurfaceControl mLeash; private ViewRootImpl mViewRoot; + private TestHost mTestHost; private TestHandler mTestHandler; private OffsettableClock mTestClock; private static InsetsModeSession sInsetsModeSession; @@ -123,8 +124,8 @@ public class InsetsControllerTest { } mTestClock = new OffsettableClock(); mTestHandler = new TestHandler(null, mTestClock); - mController = new InsetsController(new ViewRootInsetsControllerHost(mViewRoot), - (controller, type) -> { + mTestHost = new TestHost(mViewRoot); + mController = new InsetsController(mTestHost, (controller, type) -> { if (type == ITYPE_IME) { return new InsetsSourceConsumer(type, controller.getState(), Transaction::new, controller) { @@ -686,6 +687,24 @@ public class InsetsControllerTest { }); } + @Test + public void testRequestedState() { + InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> { + mController.onControlsChanged(createSingletonControl(ITYPE_STATUS_BAR)); + mController.hide(statusBars()); + assertFalse(mTestHost.getModifiedState().peekSource(ITYPE_STATUS_BAR).isVisible()); + mController.onControlsChanged(new InsetsSourceControl[0]); + assertFalse(mTestHost.getModifiedState().peekSource(ITYPE_STATUS_BAR).isVisible()); + InsetsState newState = new InsetsState(mController.getState(), true /* copySource */); + mController.onStateChanged(newState); + assertFalse(mTestHost.getModifiedState().peekSource(ITYPE_STATUS_BAR).isVisible()); + mController.show(statusBars()); + assertFalse(mTestHost.getModifiedState().peekSource(ITYPE_STATUS_BAR).isVisible()); + mController.onControlsChanged(createSingletonControl(ITYPE_STATUS_BAR)); + assertTrue(mTestHost.getModifiedState().peekSource(ITYPE_STATUS_BAR).isVisible()); + }); + } + private void waitUntilNextFrame() throws Exception { final CountDownLatch latch = new CountDownLatch(1); Choreographer.getMainThreadInstance().postCallback(Choreographer.CALLBACK_COMMIT, @@ -717,4 +736,23 @@ public class InsetsControllerTest { mController.onControlsChanged(controls); return controls; } + + private static class TestHost extends ViewRootInsetsControllerHost { + + private InsetsState mModifiedState = new InsetsState(); + + TestHost(ViewRootImpl viewRoot) { + super(viewRoot); + } + + @Override + public void onInsetsModified(InsetsState insetsState) { + mModifiedState = new InsetsState(insetsState, true /* copySource */); + super.onInsetsModified(insetsState); + } + + public InsetsState getModifiedState() { + return mModifiedState; + } + } }