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

Commit 0f3f78fc authored by Cosmin Băieș's avatar Cosmin Băieș
Browse files

Add extra checks in isReadyToShowIme

This adds some additional checks to isReadyToShowIme() in
ImeInsetsSourceProvider, so we don't actually start the mShowImeRunner
too early. Otherwise, we would drop this show request from being
scheduled here, and when we eventually get the required missing state,
won't try to show, which in some edge cases leads to the IME eventually
not showing.

We need to ensure there is no mismatch between the InsetsControlTarget
we get, and the one in DisplayContent. We also need to check that the
client already got the InsetsSourceControl, and that the control has a
leash. This also implies checking the controlTarget is not in the
list of those pending to be sent to the client. We should also check
that the control has a leash, and the leash is ready for dispatching.
We need to make sure that the non-empty source frame was set, which
requires server visibile being true. Lastly, we must check if
the ImeInsetsSourceProvider is set frozen, which blocks setting the
server visibility.

Bug: 327712574
Test: atest
  ImeInsetsSourceProviderTest
  InsetsSourceProviderTest#testGetLeash
  InsetsStateControllerTest#testHasPendingControls
Change-Id: Ia370c554b49790bbfe58f58f96c4e3a0bca9b1aa
parent c3f30078
Loading
Loading
Loading
Loading
+65 −24
Original line number Diff line number Diff line
@@ -68,8 +68,9 @@ final class ImeInsetsSourceProvider extends InsetsSourceProvider {
    /** @see #setServerVisible(boolean) */
    private boolean mServerVisible;

    ImeInsetsSourceProvider(InsetsSource source,
            InsetsStateController stateController, DisplayContent displayContent) {
    ImeInsetsSourceProvider(@NonNull InsetsSource source,
            @NonNull InsetsStateController stateController,
            @NonNull DisplayContent displayContent) {
        super(source, stateController, displayContent);
    }

@@ -226,7 +227,7 @@ final class ImeInsetsSourceProvider extends InsetsSourceProvider {
        if (mImeRequesterStatsToken != null) {
            // Cancel the pre-existing stats token, if any.
            // Log state on pre-existing request cancel.
            logShowImePostLayoutState();
            logShowImePostLayoutState(false /* aborted */);
            ImeTracker.forLogging().onCancelled(
                    mImeRequesterStatsToken, ImeTracker.PHASE_WM_SHOW_IME_RUNNER);
        }
@@ -306,7 +307,7 @@ final class ImeInsetsSourceProvider extends InsetsSourceProvider {
        ProtoLog.d(WM_DEBUG_IME, "abortShowImePostLayout");
        if (mImeRequesterStatsToken != null) {
            // Log state on abort.
            logShowImePostLayoutState();
            logShowImePostLayoutState(true /* aborted */);
            ImeTracker.forLogging().onFailed(
                    mImeRequesterStatsToken, ImeTracker.PHASE_WM_ABORT_SHOW_IME_POST_LAYOUT);
            mImeRequesterStatsToken = null;
@@ -329,11 +330,30 @@ final class ImeInsetsSourceProvider extends InsetsSourceProvider {
        //  actual IME target.
        final InsetsControlTarget dcTarget = mDisplayContent.getImeTarget(IME_TARGET_LAYERING);
        if (dcTarget == null || mImeRequester == null) {
            // Not ready to show if there is no IME layering target, or no IME requester.
            return false;
        }
        // Not ready to show if there is no IME control target.
        final InsetsControlTarget controlTarget = mDisplayContent.getImeTarget(IME_TARGET_CONTROL);
        final InsetsControlTarget controlTarget = getControlTarget();
        if (controlTarget == null) {
            // Not ready to show if there is no IME control target.
            return false;
        }
        if (controlTarget != mDisplayContent.getImeTarget(IME_TARGET_CONTROL)) {
            // Not ready to show if control target does not match the one in DisplayContent.
            return false;
        }
        if (!mServerVisible || mFrozen) {
            // Not ready to show if the window container is not available and considered visible.
            // If frozen, the server visibility is not set until unfrozen.
            return false;
        }
        if (mStateController.hasPendingControls(controlTarget)) {
            // Not ready to show if control target has pending controls.
            return false;
        }
        if (getLeash(controlTarget) == null) {
            // Not ready to show if control target has no source control leash (or leash is not
            // ready for dispatching).
            return false;
        }

@@ -349,35 +369,56 @@ final class ImeInsetsSourceProvider extends InsetsSourceProvider {
    }

    /**
     * Logs the current state required for scheduleShowImePostLayout's runnable to be triggered.
     * Logs the current state required for showImePostLayout to be triggered.
     *
     * @param aborted whether the showImePostLayout was aborted or cancelled.
     */
    private void logShowImePostLayoutState() {
    private void logShowImePostLayoutState(boolean aborted) {
        final var windowState = mWindowContainer != null ? mWindowContainer.asWindowState() : null;
        final var dcTarget = mDisplayContent.getImeTarget(IME_TARGET_LAYERING);
        final var controlTarget = mDisplayContent.getImeTarget(IME_TARGET_CONTROL);
        final var controlTarget = getControlTarget();
        final var sb = new StringBuilder();
        sb.append("mWindowContainer: ").append(mWindowContainer);
        sb.append(" windowState: ").append(windowState);
        sb.append("showImePostLayout ").append(aborted ? "aborted" : "cancelled");
        sb.append(", mWindowContainer is: ");
        sb.append(mWindowContainer != null ? "non-null" : "null");
        sb.append(", windowState: ").append(windowState);
        if (windowState != null) {
            sb.append(" windowState.isDrawn(): ").append(windowState.isDrawn());
            sb.append(" windowState.mGivenInsetsPending: ").append(windowState.mGivenInsetsPending);
        }
        sb.append(" mIsImeLayoutDrawn: ").append(mIsImeLayoutDrawn);
        sb.append(" mShowImeRunner: ").append(mShowImeRunner);
        sb.append(" mImeRequester: ").append(mImeRequester);
        sb.append(" dcTarget: ").append(dcTarget);
        sb.append(" controlTarget: ").append(controlTarget);
            sb.append(", windowState.isDrawn(): ");
            sb.append(windowState.isDrawn());
            sb.append(", windowState.mGivenInsetsPending: ");
            sb.append(windowState.mGivenInsetsPending);
        }
        sb.append(", mIsImeLayoutDrawn: ").append(mIsImeLayoutDrawn);
        sb.append(", mShowImeRunner: ").append(mShowImeRunner);
        sb.append(", mImeRequester: ").append(mImeRequester);
        sb.append(", dcTarget: ").append(dcTarget);
        sb.append(", controlTarget: ").append(controlTarget);
        sb.append("\n");
        sb.append("isReadyToShowIme(): ").append(isReadyToShowIme());
        if (mImeRequester != null && dcTarget != null && controlTarget != null) {
            sb.append(" isImeLayeringTarget: ");
            sb.append(", controlTarget == DisplayContent.controlTarget: ");
            sb.append(controlTarget == mDisplayContent.getImeTarget(IME_TARGET_CONTROL));
            sb.append(", hasPendingControls: ");
            sb.append(mStateController.hasPendingControls(controlTarget));
            sb.append(", serverVisible: ");
            sb.append(mServerVisible);
            sb.append(", frozen: ");
            sb.append(mFrozen);
            sb.append(", leash is: ");
            sb.append(getLeash(controlTarget) != null ? "non-null" : "null");
            sb.append(", control is: ");
            sb.append(mControl != null ? "non-null" : "null");
            sb.append(", mIsLeashReadyForDispatching: ");
            sb.append(mIsLeashReadyForDispatching);
            sb.append(", isImeLayeringTarget: ");
            sb.append(isImeLayeringTarget(mImeRequester, dcTarget));
            sb.append(" isAboveImeLayeringTarget: ");
            sb.append(", isAboveImeLayeringTarget: ");
            sb.append(isAboveImeLayeringTarget(mImeRequester, dcTarget));
            sb.append(" isImeFallbackTarget: ");
            sb.append(", isImeFallbackTarget: ");
            sb.append(isImeFallbackTarget(mImeRequester));
            sb.append(" isImeInputTarget: ");
            sb.append(", isImeInputTarget: ");
            sb.append(isImeInputTarget(mImeRequester));
            sb.append(" sameAsImeControlTarget: ");
            sb.append(", sameAsImeControlTarget: ");
            sb.append(sameAsImeControlTarget());
        }
        Slog.d(TAG, sb.toString());
+32 −8
Original line number Diff line number Diff line
@@ -64,15 +64,16 @@ class InsetsSourceProvider {

    private static final Rect EMPTY_RECT = new Rect();

    protected final DisplayContent mDisplayContent;
    protected final @NonNull InsetsSource mSource;
    protected WindowContainer mWindowContainer;
    protected final @NonNull DisplayContent mDisplayContent;
    protected final @NonNull InsetsStateController mStateController;
    protected @Nullable WindowContainer mWindowContainer;
    protected @Nullable InsetsSourceControl mControl;
    protected boolean mIsLeashReadyForDispatching;

    private final Rect mTmpRect = new Rect();
    private final InsetsStateController mStateController;
    private final InsetsSourceControl mFakeControl;
    private final Consumer<Transaction> mSetLeashPositionConsumer;
    private @Nullable InsetsSourceControl mControl;
    private @Nullable InsetsControlTarget mControlTarget;
    private @Nullable InsetsControlTarget mPendingControlTarget;
    private @Nullable InsetsControlTarget mFakeControlTarget;
@@ -82,7 +83,6 @@ class InsetsSourceProvider {
    private SparseArray<TriFunction<DisplayFrames, WindowContainer, Rect, Integer>>
            mOverrideFrameProviders;
    private final SparseArray<Rect> mOverrideFrames = new SparseArray<Rect>();
    private boolean mIsLeashReadyForDispatching;
    private final Rect mSourceFrame = new Rect();
    private final Rect mLastSourceFrame = new Rect();
    private @NonNull Insets mInsetsHint = Insets.NONE;
@@ -114,8 +114,9 @@ class InsetsSourceProvider {
     */
    private boolean mCropToProvidingInsets = false;

    InsetsSourceProvider(InsetsSource source, InsetsStateController stateController,
            DisplayContent displayContent) {
    InsetsSourceProvider(@NonNull InsetsSource source,
            @NonNull InsetsStateController stateController,
            @NonNull DisplayContent displayContent) {
        mClientVisible = (WindowInsets.Type.defaultVisible() & source.getType()) != 0;
        mSource = source;
        mDisplayContent = displayContent;
@@ -560,7 +561,7 @@ class InsetsSourceProvider {
        mDisplayContent.mWmService.mWindowPlacerLocked.requestTraversal();
    }

    @VisibleForTesting
    @VisibleForTesting(visibility = VisibleForTesting.Visibility.PROTECTED)
    void setServerVisible(boolean serverVisible) {
        mServerVisible = serverVisible;
        updateSourceFrameForServerVisibility();
@@ -575,6 +576,14 @@ class InsetsSourceProvider {
                mServerVisible, mClientVisible);
    }

    /**
     * Gets the source control for the given control target. If this is the provider's control
     * target, but the leash is not ready for dispatching, a new source control object with the
     * leash set to {@code null} is returned.
     *
     * @param target the control target to get the source control for.
     */
    @Nullable
    InsetsSourceControl getControl(InsetsControlTarget target) {
        if (target == mControlTarget) {
            if (!mIsLeashReadyForDispatching && mControl != null) {
@@ -593,10 +602,25 @@ class InsetsSourceProvider {
        return null;
    }

    /**
     * Gets the leash of the source control for the given control target. If this is not the
     * provider's control target, or the leash is not ready for dispatching, this will
     * return {@code null}.
     *
     * @param target the control target to get the source control leash for.
     */
    @Nullable
    protected SurfaceControl getLeash(@NonNull InsetsControlTarget target) {
        return target == mControlTarget && mIsLeashReadyForDispatching && mControl != null
                ? mControl.getLeash() : null;
    }

    @Nullable
    InsetsControlTarget getControlTarget() {
        return mControlTarget;
    }

    @Nullable
    InsetsControlTarget getFakeControlTarget() {
        return mFakeControlTarget;
    }
+12 −0
Original line number Diff line number Diff line
@@ -388,6 +388,9 @@ class InsetsStateController {
                onRequestedVisibleTypesChanged(newControlTargets.valueAt(i));
            }
            newControlTargets.clear();
            // Check for and try to run the scheduled show IME request (if it exists), as we
            // now applied the surface transaction and notified the target of the new control.
            getImeSourceProvider().checkShowImePostLayout();
        });
    }

@@ -395,6 +398,15 @@ class InsetsStateController {
        mDisplayContent.notifyInsetsChanged(mDispatchInsetsChanged);
    }

    /**
     * Checks if the control target has pending controls.
     *
     * @param target the control target to check.
     */
    boolean hasPendingControls(@NonNull InsetsControlTarget target) {
        return mPendingControlChanged.contains(target);
    }

    void dump(String prefix, PrintWriter pw) {
        pw.println(prefix + "WindowInsetsStateController");
        prefix = prefix + "  ";
+106 −18
Original line number Diff line number Diff line
@@ -16,8 +16,6 @@

package com.android.server.wm;

import static android.view.InsetsSource.ID_IME;
import static android.view.WindowInsets.Type.ime;
import static android.view.WindowManager.LayoutParams.TYPE_APPLICATION;
import static android.view.WindowManager.LayoutParams.TYPE_INPUT_METHOD;

@@ -26,7 +24,6 @@ import static org.junit.Assert.assertTrue;

import android.graphics.PixelFormat;
import android.platform.test.annotations.Presubmit;
import android.view.InsetsSource;
import android.view.inputmethod.ImeTracker;

import androidx.test.filters.SmallTest;
@@ -46,57 +43,148 @@ import org.junit.runner.RunWith;
@RunWith(WindowTestRunner.class)
public class ImeInsetsSourceProviderTest extends WindowTestsBase {

    private InsetsSource mImeSource = new InsetsSource(ID_IME, ime());
    private ImeInsetsSourceProvider mImeProvider;

    @Before
    public void setUp() throws Exception {
        mImeSource.setVisible(true);
        mImeProvider = new ImeInsetsSourceProvider(mImeSource,
                mDisplayContent.getInsetsStateController(), mDisplayContent);
        mImeProvider = mDisplayContent.getInsetsStateController().getImeSourceProvider();
        mImeProvider.getSource().setVisible(true);
    }

    @Test
    public void testTransparentControlTargetWindowCanShowIme() {
        final WindowState ime = createWindow(null, TYPE_INPUT_METHOD, "ime");
        makeWindowVisibleAndDrawn(ime);
        mImeProvider.setWindowContainer(ime, null, null);

        final WindowState appWin = createWindow(null, TYPE_APPLICATION, "app");
        final WindowState popup = createWindow(appWin, TYPE_APPLICATION, "popup");
        mDisplayContent.setImeControlTarget(popup);
        mDisplayContent.setImeLayeringTarget(appWin);
        popup.mAttrs.format = PixelFormat.TRANSPARENT;
        mDisplayContent.setImeLayeringTarget(appWin);
        mDisplayContent.updateImeInputAndControlTarget(popup);
        performSurfacePlacementAndWaitForWindowAnimator();

        mImeProvider.scheduleShowImePostLayout(appWin, ImeTracker.Token.empty());
        assertTrue(mImeProvider.isReadyToShowIme());
    }

    /**
     * Checks that scheduling with all the state set and manually triggering the show does succeed.
     */
    @Test
    public void testInputMethodInputTargetCanShowIme() {
        WindowState target = createWindow(null, TYPE_APPLICATION, "app");
    public void testScheduleShowIme() {
        final WindowState ime = createWindow(null, TYPE_INPUT_METHOD, "ime");
        makeWindowVisibleAndDrawn(ime);
        mImeProvider.setWindowContainer(ime, null, null);

        final WindowState target = createWindow(null, TYPE_APPLICATION, "app");
        mDisplayContent.setImeLayeringTarget(target);
        mDisplayContent.updateImeInputAndControlTarget(target);
        performSurfacePlacementAndWaitForWindowAnimator();

        // Schedule (without triggering) after everything is ready.
        mImeProvider.scheduleShowImePostLayout(target, ImeTracker.Token.empty());
        assertTrue(mImeProvider.isReadyToShowIme());
        assertFalse(mImeProvider.isImeShowing());

        // Manually trigger the show.
        mImeProvider.checkShowImePostLayout();
        // No longer ready as it was already shown.
        assertFalse(mImeProvider.isReadyToShowIme());
        assertTrue(mImeProvider.isImeShowing());
    }

    /**
     * Checks that scheduling to show before any state is set does succeed when
     * all the state becomes available.
     */
    @Test
    public void testScheduleShowIme_noInitialState() {
        final WindowState target = createWindow(null, TYPE_APPLICATION, "app");

        // Schedule before anything is ready.
        mImeProvider.scheduleShowImePostLayout(target, ImeTracker.Token.empty());
        assertFalse(mImeProvider.isReadyToShowIme());
        assertFalse(mImeProvider.isImeShowing());

        final WindowState ime = createWindow(null, TYPE_INPUT_METHOD, "ime");
        makeWindowVisibleAndDrawn(ime);
        mImeProvider.setWindowContainer(ime, null, null);

        mDisplayContent.setImeLayeringTarget(target);
        mDisplayContent.updateImeInputAndControlTarget(target);
        // Performing surface placement picks up the show scheduled above.
        performSurfacePlacementAndWaitForWindowAnimator();
        // No longer ready as it was already shown.
        assertFalse(mImeProvider.isReadyToShowIme());
        assertTrue(mImeProvider.isImeShowing());
    }

    /**
     * Checks that scheduling to show before starting the {@code afterPrepareSurfacesRunnable}
     * from {@link InsetsStateController#notifyPendingInsetsControlChanged}
     * does continue and succeed when the runnable is started.
     */
    @Test
    public void testIsImeShowing() {
        WindowState ime = createWindow(null, TYPE_INPUT_METHOD, "ime");
    public void testScheduleShowIme_delayedAfterPrepareSurfaces() {
        final WindowState ime = createWindow(null, TYPE_INPUT_METHOD, "ime");
        makeWindowVisibleAndDrawn(ime);
        mImeProvider.setWindowContainer(ime, null, null);

        WindowState target = createWindow(null, TYPE_APPLICATION, "app");
        final WindowState target = createWindow(null, TYPE_APPLICATION, "app");
        mDisplayContent.setImeLayeringTarget(target);
        mDisplayContent.setImeControlTarget(target);
        mDisplayContent.updateImeInputAndControlTarget(target);

        // Schedule before starting the afterPrepareSurfacesRunnable.
        mImeProvider.scheduleShowImePostLayout(target, ImeTracker.Token.empty());
        assertFalse(mImeProvider.isReadyToShowIme());
        assertFalse(mImeProvider.isImeShowing());
        mImeProvider.checkShowImePostLayout();

        // This tries to pick up the show scheduled above, but must fail as the
        // afterPrepareSurfacesRunnable was not started yet.
        mDisplayContent.applySurfaceChangesTransaction();
        assertFalse(mImeProvider.isReadyToShowIme());
        assertFalse(mImeProvider.isImeShowing());

        // Starting the afterPrepareSurfacesRunnable picks up the show scheduled above.
        mWm.mAnimator.executeAfterPrepareSurfacesRunnables();
        // No longer ready as it was already shown.
        assertFalse(mImeProvider.isReadyToShowIme());
        assertTrue(mImeProvider.isImeShowing());
        mImeProvider.setImeShowing(false);
    }

    /**
     * Checks that scheduling to show before the surface placement does continue and succeed
     * when the surface placement happens.
     */
    @Test
    public void testScheduleShowIme_delayedSurfacePlacement() {
        final WindowState ime = createWindow(null, TYPE_INPUT_METHOD, "ime");
        makeWindowVisibleAndDrawn(ime);
        mImeProvider.setWindowContainer(ime, null, null);

        final WindowState target = createWindow(null, TYPE_APPLICATION, "app");
        mDisplayContent.setImeLayeringTarget(target);
        mDisplayContent.updateImeInputAndControlTarget(target);

        // Schedule before surface placement.
        mImeProvider.scheduleShowImePostLayout(target, ImeTracker.Token.empty());
        assertFalse(mImeProvider.isReadyToShowIme());
        assertFalse(mImeProvider.isImeShowing());

        // Performing surface placement picks up the show scheduled above, and succeeds.
        // This first executes the afterPrepareSurfacesRunnable, and then
        // applySurfaceChangesTransaction. Both of them try to trigger the show,
        // but only the second one can succeed, as it comes after onPostLayout.
        performSurfacePlacementAndWaitForWindowAnimator();
        // No longer ready as it was already shown.
        assertFalse(mImeProvider.isReadyToShowIme());
        assertTrue(mImeProvider.isImeShowing());
    }

    @Test
    public void testSetFrozen() {
        WindowState ime = createWindow(null, TYPE_INPUT_METHOD, "ime");
        final WindowState ime = createWindow(null, TYPE_INPUT_METHOD, "ime");
        makeWindowVisibleAndDrawn(ime);
        mImeProvider.setWindowContainer(ime, null, null);
        mImeProvider.setServerVisible(true);
+43 −0
Original line number Diff line number Diff line
@@ -24,6 +24,7 @@ import static android.view.WindowManager.LayoutParams.TYPE_INPUT_METHOD;

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;
import static org.junit.Assert.assertTrue;
@@ -162,6 +163,48 @@ public class InsetsSourceProviderTest extends WindowTestsBase {
        assertNull(mProvider.getControl(target));
    }

    @Test
    public void testGetLeash() {
        final WindowState statusBar = createWindow(null, TYPE_APPLICATION, "statusBar");
        final WindowState target = createWindow(null, TYPE_APPLICATION, "target");
        final WindowState fakeTarget = createWindow(null, TYPE_APPLICATION, "fakeTarget");
        final WindowState otherTarget = createWindow(null, TYPE_APPLICATION, "otherTarget");
        statusBar.getFrame().set(0, 0, 500, 100);

        // We must not have control or control target before we have the insets source window,
        // so also no leash.
        mProvider.updateControlForTarget(target, true /* force */);
        assertNull(mProvider.getControl(target));
        assertNull(mProvider.getControlTarget());
        assertNull(mProvider.getLeash(target));

        // We can have the control or the control target after we have the insets source window,
        // but no leash as this is not yet ready for dispatching.
        mProvider.setWindowContainer(statusBar, null, null);
        mProvider.updateControlForTarget(target, false /* force */);
        assertNotNull(mProvider.getControl(target));
        assertNotNull(mProvider.getControlTarget());
        assertEquals(mProvider.getControlTarget(), target);
        assertNull(mProvider.getLeash(target));

        // After surface transactions are applied, the leash is ready for dispatching.
        mProvider.onSurfaceTransactionApplied();
        assertNotNull(mProvider.getLeash(target));

        // We do have fake control for the fake control target, but that has no leash.
        mProvider.updateFakeControlTarget(fakeTarget);
        assertNotNull(mProvider.getControl(fakeTarget));
        assertNotNull(mProvider.getFakeControlTarget());
        assertNotEquals(mProvider.getControlTarget(), fakeTarget);
        assertNull(mProvider.getLeash(fakeTarget));

        // We don't have any control for a different (non-fake control target), so also no leash.
        assertNull(mProvider.getControl(otherTarget));
        assertNotNull(mProvider.getControlTarget());
        assertNotEquals(mProvider.getControlTarget(), otherTarget);
        assertNull(mProvider.getLeash(otherTarget));
    }

    @Test
    public void testUpdateSourceFrame() {
        final WindowState statusBar = createWindow(null, TYPE_APPLICATION, "statusBar");
Loading