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

Commit 8945b5bc authored by Ming-Shin Lu's avatar Ming-Shin Lu
Browse files

Fix IME jumpcut when playing user IME animation after IME restarted

There are 2 cases that will seeing IME jumpcut when IME insets animation
is controlled by the app during IME prcocess being killed & restarted:

1) When IME process been killed for some reasons
 (e.g. force-stop package ), during IME restarting,
 DC#computeImeTarget will select the remote insets target as the control
 target and deliver IME leash to WM shell. If the focused window is
 not in multi-windowing mode, it may have a timing that IME surface
 may unexpectedly shown when starting insets animation from the remote.

2) Previously CL[1] fixed an IME surface visiblity issue when enabling
 shell-transition that related to somehow IME insets source visibility
 out of sync with the actual IME surface visiblity. The CL fix is
 setting IME surface alpha value according to the requested visiblity in
 InsetsSourceConsumer#setControl as a workaround. However, this may
 cause a side-effect that after IME restarted and leash updated,
 when the app requests showing IME animation, system calls setControl to
 the client to set the same leash after IME onPostLayout, then
 applyRequestedVisibilityToControl will be called before the
 insets controller calls setInsetsAndAlpha for the animation, if the
 IME requested visibility has visible, IME jumpcut will happen.

To fix this issue:
For 1), returning null control target in DC#computeImeTarget for
the above special case.

For 2), with CL[2] can fully fixed IME visiblity issue by letting the
client know the initial visibility of an InsetsSourceControl and
correctly animate insets when delivering the new leash or the
visibility change of the InsetsSourceControl. for delivering same
leash to request animation case, adding a check to call
applyRequestedVisibilityToControl only when there is no running
animation.

[1]: Iaacdf5f57e68b928e2a19036cbd8a137cf320497
[2]: I2c02e97e191ebd83238c0c54908e861d200d4c8d

Fix: 239808087
Bug: 209064170

Test: atest DisplayContentTests InsetsSourceConsumerTest
Test: manual test steps in b/239808087#comment2
Test: manual test as steps in b/209064170
   0) Enabling shell-transition
   1) Launching camera app
   2) Swiping camera app to launcher
   3) Verify status bar appear animation works

Change-Id: I242517a9214b36049d94e89d1ee63ffe505b91ac
parent 16ad7a88
Loading
Loading
Loading
Loading
+3 −1
Original line number Diff line number Diff line
@@ -176,7 +176,9 @@ public class InsetsSourceConsumer {

                // If we have a new leash, make sure visibility is up-to-date, even though we
                // didn't want to run an animation above.
                if (mController.getAnimationType(control.getType()) == ANIMATION_TYPE_NONE) {
                    applyRequestedVisibilityToControl();
                }

                // Remove the surface that owned by last control when it lost.
                if (!requestedVisible && lastControl == null) {
+43 −4
Original line number Diff line number Diff line
@@ -18,8 +18,10 @@ package android.view;

import static android.view.InsetsController.ANIMATION_TYPE_NONE;
import static android.view.InsetsController.ANIMATION_TYPE_USER;
import static android.view.InsetsSourceConsumer.ShowResult.SHOW_IMMEDIATELY;
import static android.view.InsetsState.ITYPE_IME;
import static android.view.InsetsState.ITYPE_STATUS_BAR;
import static android.view.WindowInsets.Type.ime;
import static android.view.WindowInsets.Type.statusBars;

import static junit.framework.Assert.assertEquals;
@@ -28,6 +30,7 @@ import static junit.framework.TestCase.assertTrue;

import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyZeroInteractions;
@@ -75,6 +78,7 @@ public class InsetsSourceConsumerTest {
    private boolean mRemoveSurfaceCalled = false;
    private InsetsController mController;
    private InsetsState mState;
    private ViewRootImpl mViewRoot;

    @Before
    public void setup() {
@@ -86,10 +90,9 @@ public class InsetsSourceConsumerTest {
        instrumentation.runOnMainSync(() -> {
            final Context context = instrumentation.getTargetContext();
            // cannot mock ViewRootImpl since it's final.
            final ViewRootImpl viewRootImpl = new ViewRootImpl(context,
                    context.getDisplayNoVerify());
            mViewRoot = new ViewRootImpl(context, context.getDisplayNoVerify());
            try {
                viewRootImpl.setView(new TextView(context), new LayoutParams(), null);
                mViewRoot.setView(new TextView(context), new LayoutParams(), null);
            } catch (BadTokenException e) {
                // activity isn't running, lets ignore BadTokenException.
            }
@@ -97,7 +100,7 @@ public class InsetsSourceConsumerTest {
            mSpyInsetsSource = Mockito.spy(new InsetsSource(ITYPE_STATUS_BAR));
            mState.addSource(mSpyInsetsSource);

            mController = new InsetsController(new ViewRootInsetsControllerHost(viewRootImpl));
            mController = new InsetsController(new ViewRootInsetsControllerHost(mViewRoot));
            mConsumer = new InsetsSourceConsumer(ITYPE_STATUS_BAR, mState,
                    () -> mMockTransaction, mController) {
                @Override
@@ -207,4 +210,40 @@ public class InsetsSourceConsumerTest {
        });

    }

    @Test
    public void testWontUpdateImeLeashVisibility_whenAnimation() {
        InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> {
            InsetsState state = new InsetsState();
            ViewRootInsetsControllerHost host = new ViewRootInsetsControllerHost(mViewRoot);
            InsetsController insetsController = new InsetsController(host, (controller, type) -> {
                if (type == ITYPE_IME) {
                    return new InsetsSourceConsumer(ITYPE_IME, state,
                            () -> mMockTransaction, controller) {
                        @Override
                        public int requestShow(boolean fromController) {
                            return SHOW_IMMEDIATELY;
                        }
                    };
                }
                return new InsetsSourceConsumer(type, controller.getState(), Transaction::new,
                        controller);
            }, host.getHandler());
            InsetsSourceConsumer imeConsumer = insetsController.getSourceConsumer(ITYPE_IME);

            // Initial IME insets source control with its leash.
            imeConsumer.setControl(new InsetsSourceControl(ITYPE_IME, mLeash,
                    false /* initialVisible */, new Point(), Insets.NONE), new int[1], new int[1]);
            reset(mMockTransaction);

            // Verify when the app requests controlling show IME animation, the IME leash
            // visibility won't be updated when the consumer received the same leash in setControl.
            insetsController.controlWindowInsetsAnimation(ime(), 0L,
                    null /* interpolator */, null /* cancellationSignal */, null /* listener */);
            assertTrue(insetsController.getAnimationType(ITYPE_IME) == ANIMATION_TYPE_USER);
            imeConsumer.setControl(new InsetsSourceControl(ITYPE_IME, mLeash,
                    true /* initialVisible */, new Point(), Insets.NONE), new int[1], new int[1]);
            verify(mMockTransaction, never()).show(mLeash);
        });
    }
}
+11 −4
Original line number Diff line number Diff line
@@ -4416,13 +4416,20 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp
     */
    @VisibleForTesting
    InsetsControlTarget computeImeControlTarget() {
        if (mImeInputTarget == null) {
            // A special case that if there is no IME input target while the IME is being killed,
            // in case seeing unexpected IME surface visibility change when delivering the IME leash
            // to the remote insets target during the IME restarting, but the focus window is not in
            // multi-windowing mode, return null target until the next input target updated.
            return null;
        }

        final WindowState imeInputTarget = mImeInputTarget.getWindowState();
        if (!isImeControlledByApp() && mRemoteInsetsControlTarget != null
                || (mImeInputTarget != null
                        && getImeHostOrFallback(mImeInputTarget.getWindowState())
                               == mRemoteInsetsControlTarget)) {
                || getImeHostOrFallback(imeInputTarget) == mRemoteInsetsControlTarget) {
            return mRemoteInsetsControlTarget;
        } else {
            return mImeInputTarget != null ? mImeInputTarget.getWindowState() : null;
            return imeInputTarget;
        }
    }

+9 −1
Original line number Diff line number Diff line
@@ -1251,7 +1251,15 @@ public class DisplayContentTests extends WindowTestsBase {
    public void testComputeImeControlTarget() throws Exception {
        final DisplayContent dc = createNewDisplay();
        dc.setRemoteInsetsController(createDisplayWindowInsetsController());
        dc.setImeInputTarget(createWindow(null, TYPE_BASE_APPLICATION, "app"));
        dc.mCurrentFocus = createWindow(null, TYPE_BASE_APPLICATION, "app");

        // Expect returning null IME control target when the focus window has not yet been the
        // IME input target (e.g. IME is restarting) in fullscreen windowing mode.
        dc.setImeInputTarget(null);
        assertFalse(dc.mCurrentFocus.inMultiWindowMode());
        assertNull(dc.computeImeControlTarget());

        dc.setImeInputTarget(dc.mCurrentFocus);
        dc.setImeLayeringTarget(dc.getImeInputTarget().getWindowState());
        assertEquals(dc.getImeInputTarget().getWindowState(), dc.computeImeControlTarget());
    }