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

Commit 6a074eca authored by Vadim Caen's avatar Vadim Caen
Browse files

Improve startBackNavigation stability

  - Use the focused window instead of the topApp window
    - Instead we now rely on WindowManagerService to get
     the focused window.
    - SystemUI does not have ActivityRecord so we can't rely
     on the top window of the Task to find the correct
      window on which the callback will be called.

  - Introduce a Builder for BackNavigationInfo
    - This reduces the number of variable needed outside the synchonized
    block.
    - It also reduces the number of early return of BackNavigationInfo
    instances

  - Adding log messages to help further debug the method.

Test: BackNavigationControllerTests
Test: Manual dismiss of SystemUi dialog in QS
Bug: 216604581
Fixes: 221458292
Change-Id: I9ba2c7f89956f34d6338824502c210b3e58dc076

Introduce builder for BackNavigationInfo

Change-Id: I14b4a4b3abc8f417998b7b32831cb3d5c4faa491
parent c20089e4
Loading
Loading
Loading
Loading
+7 −0
Original line number Original line Diff line number Diff line
@@ -147,6 +147,7 @@ import android.os.SystemProperties;
import android.os.Trace;
import android.os.Trace;
import android.os.UserHandle;
import android.os.UserHandle;
import android.sysprop.DisplayProperties;
import android.sysprop.DisplayProperties;
import android.text.TextUtils;
import android.util.AndroidRuntimeException;
import android.util.AndroidRuntimeException;
import android.util.DisplayMetrics;
import android.util.DisplayMetrics;
import android.util.EventLog;
import android.util.EventLog;
@@ -10885,6 +10886,12 @@ public final class ViewRootImpl implements ViewParent,
     * {@link OnBackInvokedCallback} to be called to the server.
     * {@link OnBackInvokedCallback} to be called to the server.
     */
     */
    private void registerBackCallbackOnWindow() {
    private void registerBackCallbackOnWindow() {
        if (OnBackInvokedDispatcher.DEBUG) {
            Log.d(OnBackInvokedDispatcher.TAG, TextUtils.formatSimple(
                    "ViewRootImpl.registerBackCallbackOnWindow. Callback:%s Package:%s "
                            + "IWindow:%s Session:%s",
                    mOnBackInvokedDispatcher, mBasePackageName, mWindow, mWindowSession));
        }
        mOnBackInvokedDispatcher.attachToWindow(mWindowSession, mWindow);
        mOnBackInvokedDispatcher.attachToWindow(mWindowSession, mWindow);
    }
    }


+96 −3
Original line number Original line Diff line number Diff line
@@ -81,7 +81,9 @@ public final class BackNavigationInfo implements Parcelable {
            TYPE_DIALOG_CLOSE,
            TYPE_DIALOG_CLOSE,
            TYPE_RETURN_TO_HOME,
            TYPE_RETURN_TO_HOME,
            TYPE_CROSS_ACTIVITY,
            TYPE_CROSS_ACTIVITY,
            TYPE_CROSS_TASK})
            TYPE_CROSS_TASK,
            TYPE_CALLBACK
    })
    @interface BackTargetType {
    @interface BackTargetType {
    }
    }


@@ -121,8 +123,8 @@ public final class BackNavigationInfo implements Parcelable {
            @Nullable SurfaceControl screenshotSurface,
            @Nullable SurfaceControl screenshotSurface,
            @Nullable HardwareBuffer screenshotBuffer,
            @Nullable HardwareBuffer screenshotBuffer,
            @Nullable WindowConfiguration taskWindowConfiguration,
            @Nullable WindowConfiguration taskWindowConfiguration,
            @NonNull RemoteCallback onBackNavigationDone,
            @Nullable RemoteCallback onBackNavigationDone,
            @NonNull IOnBackInvokedCallback onBackInvokedCallback) {
            @Nullable IOnBackInvokedCallback onBackInvokedCallback) {
        mType = type;
        mType = type;
        mDepartingAnimationTarget = departingAnimationTarget;
        mDepartingAnimationTarget = departingAnimationTarget;
        mScreenshotSurface = screenshotSurface;
        mScreenshotSurface = screenshotSurface;
@@ -278,7 +280,98 @@ public final class BackNavigationInfo implements Parcelable {
                return "TYPE_CROSS_ACTIVITY";
                return "TYPE_CROSS_ACTIVITY";
            case TYPE_CROSS_TASK:
            case TYPE_CROSS_TASK:
                return "TYPE_CROSS_TASK";
                return "TYPE_CROSS_TASK";
            case TYPE_CALLBACK:
                return "TYPE_CALLBACK";
        }
        }
        return String.valueOf(type);
        return String.valueOf(type);
    }
    }

    /**
     * @hide
     */
    @SuppressWarnings("UnusedReturnValue") // Builder pattern
    public static class Builder {

        private int mType = TYPE_UNDEFINED;
        @Nullable
        private RemoteAnimationTarget mDepartingAnimationTarget = null;
        @Nullable
        private SurfaceControl mScreenshotSurface = null;
        @Nullable
        private HardwareBuffer mScreenshotBuffer = null;
        @Nullable
        private WindowConfiguration mTaskWindowConfiguration = null;
        @Nullable
        private RemoteCallback mOnBackNavigationDone = null;
        @Nullable
        private IOnBackInvokedCallback mOnBackInvokedCallback = null;

        /**
         * @see BackNavigationInfo#getType()
         */
        public Builder setType(@BackTargetType int type) {
            mType = type;
            return this;
        }

        /**
         * @see BackNavigationInfo#getDepartingAnimationTarget
         */
        public Builder setDepartingAnimationTarget(
                @Nullable RemoteAnimationTarget departingAnimationTarget) {
            mDepartingAnimationTarget = departingAnimationTarget;
            return this;
        }

        /**
         * @see BackNavigationInfo#getScreenshotSurface
         */
        public Builder setScreenshotSurface(@Nullable SurfaceControl screenshotSurface) {
            mScreenshotSurface = screenshotSurface;
            return this;
        }

        /**
         * @see BackNavigationInfo#getScreenshotHardwareBuffer()
         */
        public Builder setScreenshotBuffer(@Nullable HardwareBuffer screenshotBuffer) {
            mScreenshotBuffer = screenshotBuffer;
            return this;
        }

        /**
         * @see BackNavigationInfo#getTaskWindowConfiguration
         */
        public Builder setTaskWindowConfiguration(
                @Nullable WindowConfiguration taskWindowConfiguration) {
            mTaskWindowConfiguration = taskWindowConfiguration;
            return this;
        }

        /**
         * @see BackNavigationInfo#onBackNavigationFinished(boolean)
         */
        public Builder setOnBackNavigationDone(@Nullable RemoteCallback onBackNavigationDone) {
            mOnBackNavigationDone = onBackNavigationDone;
            return this;
        }

        /**
         * @see BackNavigationInfo#getOnBackInvokedCallback
         */
        public Builder setOnBackInvokedCallback(
                @Nullable IOnBackInvokedCallback onBackInvokedCallback) {
            mOnBackInvokedCallback = onBackInvokedCallback;
            return this;
        }

        /**
         * Builds and returns an instance of {@link BackNavigationInfo}
         */
        public BackNavigationInfo build() {
            return new BackNavigationInfo(mType, mDepartingAnimationTarget, mScreenshotSurface,
                    mScreenshotBuffer, mTaskWindowConfiguration, mOnBackNavigationDone,
                    mOnBackInvokedCallback);
        }
    }
}
}
+5 −5
Original line number Original line Diff line number Diff line
@@ -73,7 +73,7 @@ public class ProxyOnBackInvokedDispatcher implements OnBackInvokedDispatcher {
    public void unregisterOnBackInvokedCallback(
    public void unregisterOnBackInvokedCallback(
            @NonNull OnBackInvokedCallback callback) {
            @NonNull OnBackInvokedCallback callback) {
        if (DEBUG) {
        if (DEBUG) {
            Log.v(TAG, String.format("Pending unregister %s. Actual=%s", callback,
            Log.v(TAG, String.format("Proxy unregister %s. Actual=%s", callback,
                    mActualDispatcherOwner));
                    mActualDispatcherOwner));
        }
        }
        synchronized (mLock) {
        synchronized (mLock) {
@@ -109,8 +109,8 @@ public class ProxyOnBackInvokedDispatcher implements OnBackInvokedDispatcher {
        OnBackInvokedDispatcher dispatcher =
        OnBackInvokedDispatcher dispatcher =
                mActualDispatcherOwner.getOnBackInvokedDispatcher();
                mActualDispatcherOwner.getOnBackInvokedDispatcher();
        if (DEBUG) {
        if (DEBUG) {
            Log.v(TAG, String.format("Pending transferring %d callbacks to %s", mCallbacks.size(),
            Log.v(TAG, String.format("Proxy: transferring %d pending callbacks to %s",
                    dispatcher));
                    mCallbacks.size(), dispatcher));
        }
        }
        for (Pair<OnBackInvokedCallback, Integer> callbackPair : mCallbacks) {
        for (Pair<OnBackInvokedCallback, Integer> callbackPair : mCallbacks) {
            int priority = callbackPair.second;
            int priority = callbackPair.second;
@@ -144,7 +144,7 @@ public class ProxyOnBackInvokedDispatcher implements OnBackInvokedDispatcher {
     */
     */
    public void reset() {
    public void reset() {
        if (DEBUG) {
        if (DEBUG) {
            Log.v(TAG, "Pending reset callbacks");
            Log.v(TAG, "Proxy: reset callbacks");
        }
        }
        synchronized (mLock) {
        synchronized (mLock) {
            mCallbacks.clear();
            mCallbacks.clear();
@@ -165,7 +165,7 @@ public class ProxyOnBackInvokedDispatcher implements OnBackInvokedDispatcher {
    public void setActualDispatcherOwner(
    public void setActualDispatcherOwner(
            @Nullable OnBackInvokedDispatcherOwner actualDispatcherOwner) {
            @Nullable OnBackInvokedDispatcherOwner actualDispatcherOwner) {
        if (DEBUG) {
        if (DEBUG) {
            Log.v(TAG, String.format("Pending setActual %s. Current %s",
            Log.v(TAG, String.format("Proxy setActual %s. Current %s",
                            actualDispatcherOwner, mActualDispatcherOwner));
                            actualDispatcherOwner, mActualDispatcherOwner));
        }
        }
        synchronized (mLock) {
        synchronized (mLock) {
+42 −12
Original line number Original line Diff line number Diff line
@@ -397,6 +397,12 @@
      "group": "WM_DEBUG_WINDOW_TRANSITIONS",
      "group": "WM_DEBUG_WINDOW_TRANSITIONS",
      "at": "com\/android\/server\/wm\/Transition.java"
      "at": "com\/android\/server\/wm\/Transition.java"
    },
    },
    "-1717147904": {
      "message": "Current focused window is embeddedWindow. Dispatch KEYCODE_BACK.",
      "level": "DEBUG",
      "group": "WM_DEBUG_BACK_PREVIEW",
      "at": "com\/android\/server\/wm\/BackNavigationController.java"
    },
    "-1715268616": {
    "-1715268616": {
      "message": "Last window, removing starting window %s",
      "message": "Last window, removing starting window %s",
      "level": "VERBOSE",
      "level": "VERBOSE",
@@ -1087,6 +1093,12 @@
      "group": "WM_DEBUG_STATES",
      "group": "WM_DEBUG_STATES",
      "at": "com\/android\/server\/wm\/ActivityRecord.java"
      "at": "com\/android\/server\/wm\/ActivityRecord.java"
    },
    },
    "-1010850753": {
      "message": "No focused window, defaulting to top task's window",
      "level": "WARN",
      "group": "WM_DEBUG_BACK_PREVIEW",
      "at": "com\/android\/server\/wm\/BackNavigationController.java"
    },
    "-1009117329": {
    "-1009117329": {
      "message": "isFetchingAppTransitionSpecs=true",
      "message": "isFetchingAppTransitionSpecs=true",
      "level": "VERBOSE",
      "level": "VERBOSE",
@@ -1105,6 +1117,12 @@
      "group": "WM_DEBUG_STATES",
      "group": "WM_DEBUG_STATES",
      "at": "com\/android\/server\/wm\/ActivityRecord.java"
      "at": "com\/android\/server\/wm\/ActivityRecord.java"
    },
    },
    "-997565097": {
      "message": "Focused window found using getFocusedWindowToken",
      "level": "DEBUG",
      "group": "WM_DEBUG_BACK_PREVIEW",
      "at": "com\/android\/server\/wm\/BackNavigationController.java"
    },
    "-993378225": {
    "-993378225": {
      "message": "finishDrawingLocked: mDrawState=COMMIT_DRAW_PENDING %s in %s",
      "message": "finishDrawingLocked: mDrawState=COMMIT_DRAW_PENDING %s in %s",
      "level": "VERBOSE",
      "level": "VERBOSE",
@@ -1327,6 +1345,12 @@
      "group": "WM_DEBUG_FOCUS_LIGHT",
      "group": "WM_DEBUG_FOCUS_LIGHT",
      "at": "com\/android\/server\/wm\/ActivityRecord.java"
      "at": "com\/android\/server\/wm\/ActivityRecord.java"
    },
    },
    "-767349300": {
      "message": "%s: Setting back callback %s. Client IWindow %s",
      "level": "DEBUG",
      "group": "WM_DEBUG_BACK_PREVIEW",
      "at": "com\/android\/server\/wm\/WindowState.java"
    },
    "-766059044": {
    "-766059044": {
      "message": "Display id=%d selected orientation %s (%d), got rotation %s (%d)",
      "message": "Display id=%d selected orientation %s (%d), got rotation %s (%d)",
      "level": "VERBOSE",
      "level": "VERBOSE",
@@ -1669,12 +1693,6 @@
      "group": "WM_DEBUG_STATES",
      "group": "WM_DEBUG_STATES",
      "at": "com\/android\/server\/wm\/RootWindowContainer.java"
      "at": "com\/android\/server\/wm\/RootWindowContainer.java"
    },
    },
    "-432881038": {
      "message": "startBackNavigation task=%s, topRunningActivity=%s, applicationBackCallback=%s, systemBackCallback=%s",
      "level": "DEBUG",
      "group": "WM_DEBUG_BACK_PREVIEW",
      "at": "com\/android\/server\/wm\/BackNavigationController.java"
    },
    "-415865166": {
    "-415865166": {
      "message": "findFocusedWindow: Found new focus @ %s",
      "message": "findFocusedWindow: Found new focus @ %s",
      "level": "VERBOSE",
      "level": "VERBOSE",
@@ -1885,12 +1903,6 @@
      "group": "WM_DEBUG_SYNC_ENGINE",
      "group": "WM_DEBUG_SYNC_ENGINE",
      "at": "com\/android\/server\/wm\/BLASTSyncEngine.java"
      "at": "com\/android\/server\/wm\/BLASTSyncEngine.java"
    },
    },
    "-228813488": {
      "message": "%s: Setting back callback %s",
      "level": "DEBUG",
      "group": "WM_DEBUG_BACK_PREVIEW",
      "at": "com\/android\/server\/wm\/WindowState.java"
    },
    "-208825711": {
    "-208825711": {
      "message": "shouldWaitAnimatingExit: isWallpaperTarget: %s",
      "message": "shouldWaitAnimatingExit: isWallpaperTarget: %s",
      "level": "DEBUG",
      "level": "DEBUG",
@@ -2359,6 +2371,12 @@
      "group": "WM_DEBUG_REMOTE_ANIMATIONS",
      "group": "WM_DEBUG_REMOTE_ANIMATIONS",
      "at": "com\/android\/server\/wm\/RemoteAnimationController.java"
      "at": "com\/android\/server\/wm\/RemoteAnimationController.java"
    },
    },
    "250620778": {
      "message": "startBackNavigation task=%s, topRunningActivity=%s, applicationBackCallback=%s, systemBackCallback=%s, currentFocus=%s",
      "level": "DEBUG",
      "group": "WM_DEBUG_BACK_PREVIEW",
      "at": "com\/android\/server\/wm\/BackNavigationController.java"
    },
    "251812577": {
    "251812577": {
      "message": "Register display organizer=%s uid=%d",
      "message": "Register display organizer=%s uid=%d",
      "level": "VERBOSE",
      "level": "VERBOSE",
@@ -2677,6 +2695,12 @@
      "group": "WM_SHOW_TRANSACTIONS",
      "group": "WM_SHOW_TRANSACTIONS",
      "at": "com\/android\/server\/wm\/WindowContainerThumbnail.java"
      "at": "com\/android\/server\/wm\/WindowContainerThumbnail.java"
    },
    },
    "531891870": {
      "message": "Previous Destination is Activity:%s Task:%s removedContainer:%s, backType=%s",
      "level": "DEBUG",
      "group": "WM_DEBUG_BACK_PREVIEW",
      "at": "com\/android\/server\/wm\/BackNavigationController.java"
    },
    "535103992": {
    "535103992": {
      "message": "Wallpaper may change!  Adjusting",
      "message": "Wallpaper may change!  Adjusting",
      "level": "VERBOSE",
      "level": "VERBOSE",
@@ -2911,6 +2935,12 @@
      "group": "WM_DEBUG_LOCKTASK",
      "group": "WM_DEBUG_LOCKTASK",
      "at": "com\/android\/server\/wm\/ActivityTaskManagerService.java"
      "at": "com\/android\/server\/wm\/ActivityTaskManagerService.java"
    },
    },
    "716528224": {
      "message": "Focused window found using wmService.getFocusedWindowLocked()",
      "level": "DEBUG",
      "group": "WM_DEBUG_BACK_PREVIEW",
      "at": "com\/android\/server\/wm\/BackNavigationController.java"
    },
    "726205185": {
    "726205185": {
      "message": "Moving to DESTROYED: %s (destroy skipped)",
      "message": "Moving to DESTROYED: %s (destroy skipped)",
      "level": "VERBOSE",
      "level": "VERBOSE",
+2 −0
Original line number Original line Diff line number Diff line
@@ -44,6 +44,7 @@ import com.android.wm.shell.TestShellExecutor;
import com.android.wm.shell.common.ShellExecutor;
import com.android.wm.shell.common.ShellExecutor;


import org.junit.Before;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
import org.mockito.ArgumentCaptor;
@@ -109,6 +110,7 @@ public class BackAnimationControllerTest {
    }
    }


    @Test
    @Test
    @Ignore("b/207481538")
    public void crossActivity_screenshotAttachedAndVisible() {
    public void crossActivity_screenshotAttachedAndVisible() {
        SurfaceControl screenshotSurface = new SurfaceControl();
        SurfaceControl screenshotSurface = new SurfaceControl();
        HardwareBuffer hardwareBuffer = mock(HardwareBuffer.class);
        HardwareBuffer hardwareBuffer = mock(HardwareBuffer.class);
Loading