From b5d7e0ed37bb4bcbbe3c29873ba4dcaef8b94c16 Mon Sep 17 00:00:00 2001 From: Chavi Weingarten Date: Thu, 23 Mar 2023 19:03:26 +0000 Subject: [PATCH 1/2] Ensure each SCVH child window has a unique focus token. Currently, all windows created via WindowlessWindowManager for the same SCVH will have the same focus token. This will break if there are child windows that also get focus since WMS will not have a way to uniquely identify the tokens. Change-Id: I3c3de947d2e03f3a0b82aa3dde374c3851a64ca5 Test: SurfaceControlViewHostTests Bug: 230340812 --- .../android/view/WindowlessWindowManager.java | 62 ++++++++++--------- 1 file changed, 34 insertions(+), 28 deletions(-) diff --git a/core/java/android/view/WindowlessWindowManager.java b/core/java/android/view/WindowlessWindowManager.java index 0560cafe3e52f..98681446446b2 100644 --- a/core/java/android/view/WindowlessWindowManager.java +++ b/core/java/android/view/WindowlessWindowManager.java @@ -57,18 +57,16 @@ public class WindowlessWindowManager implements IWindowSession { SurfaceControl mLeash; Rect mFrame; Rect mAttachedFrame; + IBinder mFocusGrantToken; - State(SurfaceControl sc, WindowManager.LayoutParams p, - int displayId, IBinder inputChannelToken, IWindow client, SurfaceControl leash, - Rect frame, Rect attachedFrame) { + State(SurfaceControl sc, WindowManager.LayoutParams p, int displayId, IWindow client, + SurfaceControl leash, Rect frame) { mSurfaceControl = sc; mParams.copyFrom(p); mDisplayId = displayId; - mInputChannelToken = inputChannelToken; mClient = client; mLeash = leash; mFrame = frame; - mAttachedFrame = attachedFrame; } }; @@ -182,45 +180,53 @@ public class WindowlessWindowManager implements IWindowSession { .setParent(leash) .build(); + final State state = new State(sc, attrs, displayId, window, leash, /* frame= */ new Rect()); + synchronized (this) { + State parentState = mStateForWindow.get(attrs.token); + if (parentState != null) { + state.mAttachedFrame = parentState.mFrame; + } + + // Give the first window the mFocusGrantToken since that's the token the host can use + // to give focus to the embedded. + if (mStateForWindow.isEmpty()) { + state.mFocusGrantToken = mFocusGrantToken; + } else { + state.mFocusGrantToken = new Binder(); + } + + mStateForWindow.put(window.asBinder(), state); + } + + if (state.mAttachedFrame == null) { + outAttachedFrame.set(0, 0, -1, -1); + } else { + outAttachedFrame.set(state.mAttachedFrame); + } + outSizeCompatScale[0] = 1f; + if (((attrs.inputFeatures & WindowManager.LayoutParams.INPUT_FEATURE_NO_INPUT_CHANNEL) == 0)) { try { if (mRealWm instanceof IWindowSession.Stub) { mRealWm.grantInputChannel(displayId, new SurfaceControl(sc, "WindowlessWindowManager.addToDisplay"), - window, mHostInputToken, - attrs.flags, attrs.privateFlags, attrs.inputFeatures, attrs.type, - attrs.token, mFocusGrantToken, attrs.getTitle().toString(), + window, mHostInputToken, attrs.flags, attrs.privateFlags, + attrs.inputFeatures, attrs.type, + attrs.token, state.mFocusGrantToken, attrs.getTitle().toString(), outInputChannel); } else { mRealWm.grantInputChannel(displayId, sc, window, mHostInputToken, attrs.flags, attrs.privateFlags, attrs.inputFeatures, attrs.type, attrs.token, - mFocusGrantToken, attrs.getTitle().toString(), outInputChannel); + state.mFocusGrantToken, attrs.getTitle().toString(), outInputChannel); } + state.mInputChannelToken = + outInputChannel != null ? outInputChannel.getToken() : null; } catch (RemoteException e) { Log.e(TAG, "Failed to grant input to surface: ", e); } } - final State state = new State(sc, attrs, displayId, - outInputChannel != null ? outInputChannel.getToken() : null, window, - leash, /* frame= */ new Rect(), /* attachedFrame= */ null); - Rect parentFrame = null; - synchronized (this) { - State parentState = mStateForWindow.get(attrs.token); - if (parentState != null) { - parentFrame = parentState.mFrame; - } - mStateForWindow.put(window.asBinder(), state); - } - state.mAttachedFrame = parentFrame; - if (parentFrame == null) { - outAttachedFrame.set(0, 0, -1, -1); - } else { - outAttachedFrame.set(parentFrame); - } - outSizeCompatScale[0] = 1f; - final int res = WindowManagerGlobal.ADD_OKAY | WindowManagerGlobal.ADD_FLAG_APP_VISIBLE | WindowManagerGlobal.ADD_FLAG_USE_BLAST; From 1a3b5b1078e2e0b5eb3dd31751244ee636d94a20 Mon Sep 17 00:00:00 2001 From: Chavi Weingarten Date: Wed, 29 Mar 2023 00:37:27 +0000 Subject: [PATCH 2/2] Add focusTransferTarget in WindowInfo A transfer focus request was originally added on FocusRequest object. This meant when a host wanted to provide focus to an embedded window, it would request to transfer focus, but provide its own token as the focusedToken in the request. In FocusResolver, it would ensure that the focusedToken was current focus before allow the request to proceed. This worked in some cases, but created races where clients had to understand timing in order to properly transfer focus. Instead, use a persistent focusTransferTarget value set on the host to transfer focus to the embedded when the host would have gained focus. This solves a few issues with embedded windows. 1. Apps can request focus to the embedded by requesting focus to the SV that hosts the embedded. However, if they request focus too early, it will drop the request since the host has not yet gained focus. With the current code, it will transfer focus to the embedded once the host could have gained focus. If the host wants to revoke, the focusTransferTarget can be set to null, which will give the host focus again. 2. This fixes the issue if another window becomes focus. When WM gives focus back to the host window, it should automatically give focus to the embedded if it was last requested. The app shouldn't be required to maintain the last state of the embedded focus to see if it needs to transfer it again. It's actually not even possible once focus can be given to embedded via touch since only WM and Input know about this. Additionally, ensure that tapping on the embedded window gives it focus. This was added only for overlay layers, but is also needed for all embedded windows. There's no way for the host to transfer focus when the embedded is tapped since the events will go directly to the embedded window and not to the host. If the embedded window is tapped, but the host is not focused, we need to ensure WMS will bring that window to the top and request focus to inputflinger. When the host focus request is processed by FocusResolver, it will then give focus directly to the embedded because of the focusTransferTarget set on the host WindowInfo. Test: SurfaceControlViewHostTests Bug: 230340812 Change-Id: If610d50e903fe8c437436e6efcb750a809274726 --- core/java/android/view/InputWindowHandle.java | 9 +++ core/java/android/view/SurfaceControl.java | 25 +------ ...droid_hardware_input_InputWindowHandle.cpp | 15 ++++ core/jni/android_view_SurfaceControl.cpp | 15 +--- .../server/wm/EmbeddedWindowController.java | 68 ++++++++----------- .../server/wm/InputWindowHandleWrapper.java | 8 +++ .../android/server/wm/TrustedOverlayHost.java | 2 - .../server/wm/WindowManagerService.java | 21 +++--- 8 files changed, 77 insertions(+), 86 deletions(-) diff --git a/core/java/android/view/InputWindowHandle.java b/core/java/android/view/InputWindowHandle.java index 24a0355dd10eb..d35aff9a72b7c 100644 --- a/core/java/android/view/InputWindowHandle.java +++ b/core/java/android/view/InputWindowHandle.java @@ -158,6 +158,14 @@ public final class InputWindowHandle { */ public Matrix transform; + /** + * The input token for the window to which focus should be transferred when this input window + * can be successfully focused. If null, this input window will not transfer its focus to + * any other window. + */ + @Nullable + public IBinder focusTransferTarget; + private native void nativeDispose(); public InputWindowHandle(InputApplicationHandle inputApplicationHandle, int displayId) { @@ -195,6 +203,7 @@ public final class InputWindowHandle { transform = new Matrix(); transform.set(other.transform); } + focusTransferTarget = other.focusTransferTarget; } @Override diff --git a/core/java/android/view/SurfaceControl.java b/core/java/android/view/SurfaceControl.java index 0db52aaa8b3d7..bc6a3b540ce7a 100644 --- a/core/java/android/view/SurfaceControl.java +++ b/core/java/android/view/SurfaceControl.java @@ -265,7 +265,7 @@ public final class SurfaceControl implements Parcelable { int transformHint); private static native void nativeRemoveCurrentInputFocus(long nativeObject, int displayId); private static native void nativeSetFocusedWindow(long transactionObj, IBinder toToken, - String windowName, IBinder focusedToken, String focusedWindowName, int displayId); + String windowName, int displayId); private static native void nativeSetFrameTimelineVsync(long transactionObj, long frameTimelineVsyncId); private static native void nativeAddJankDataListener(long nativeListener, @@ -3604,28 +3604,7 @@ public final class SurfaceControl implements Parcelable { */ public Transaction setFocusedWindow(@NonNull IBinder token, String windowName, int displayId) { - nativeSetFocusedWindow(mNativeObject, token, windowName, - null /* focusedToken */, null /* focusedWindowName */, displayId); - return this; - } - - /** - * Set focus on the window identified by the input {@code token} if the window identified by - * the input {@code focusedToken} is currently focused. If the {@code focusedToken} does not - * have focus, the request is dropped. - * - * This is used by forward focus transfer requests from clients that host embedded windows, - * and want to transfer focus to/from them. - * - * @hide - */ - public Transaction requestFocusTransfer(@NonNull IBinder token, - String windowName, - @NonNull IBinder focusedToken, - String focusedWindowName, - int displayId) { - nativeSetFocusedWindow(mNativeObject, token, windowName, focusedToken, - focusedWindowName, displayId); + nativeSetFocusedWindow(mNativeObject, token, windowName, displayId); return this; } diff --git a/core/jni/android_hardware_input_InputWindowHandle.cpp b/core/jni/android_hardware_input_InputWindowHandle.cpp index 241320f317486..416d991bd64ce 100644 --- a/core/jni/android_hardware_input_InputWindowHandle.cpp +++ b/core/jni/android_hardware_input_InputWindowHandle.cpp @@ -74,6 +74,7 @@ static struct { WeakRefHandleField touchableRegionSurfaceControl; jfieldID transform; jfieldID windowToken; + jfieldID focusTransferTarget; } gInputWindowHandleClassInfo; static struct { @@ -216,6 +217,17 @@ bool NativeInputWindowHandle::updateInfo() { mInfo.windowToken.clear(); } + ScopedLocalRef + focusTransferTargetObj(env, + env->GetObjectField(obj, + gInputWindowHandleClassInfo + .focusTransferTarget)); + if (focusTransferTargetObj.get()) { + mInfo.focusTransferTarget = ibinderForJavaObject(env, focusTransferTargetObj.get()); + } else { + mInfo.focusTransferTarget.clear(); + } + env->DeleteLocalRef(obj); return true; } @@ -433,6 +445,9 @@ int register_android_view_InputWindowHandle(JNIEnv* env) { GET_FIELD_ID(gInputWindowHandleClassInfo.windowToken, clazz, "windowToken", "Landroid/os/IBinder;"); + GET_FIELD_ID(gInputWindowHandleClassInfo.focusTransferTarget, clazz, "focusTransferTarget", + "Landroid/os/IBinder;"); + jclass weakRefClazz; FIND_CLASS(weakRefClazz, "java/lang/ref/Reference"); diff --git a/core/jni/android_view_SurfaceControl.cpp b/core/jni/android_view_SurfaceControl.cpp index 03d6eece61e63..e42c6f107e6dd 100644 --- a/core/jni/android_view_SurfaceControl.cpp +++ b/core/jni/android_view_SurfaceControl.cpp @@ -1820,17 +1820,11 @@ static void nativeRemoveCurrentInputFocus(JNIEnv* env, jclass clazz, jlong trans } static void nativeSetFocusedWindow(JNIEnv* env, jclass clazz, jlong transactionObj, - jobject toTokenObj, jstring windowNameJstr, - jobject focusedTokenObj, jstring focusedWindowNameJstr, - jint displayId) { + jobject toTokenObj, jstring windowNameJstr, jint displayId) { auto transaction = reinterpret_cast(transactionObj); if (toTokenObj == NULL) return; sp toToken(ibinderForJavaObject(env, toTokenObj)); - sp focusedToken; - if (focusedTokenObj != NULL) { - focusedToken = ibinderForJavaObject(env, focusedTokenObj); - } FocusRequest request; request.token = toToken; @@ -1839,11 +1833,6 @@ static void nativeSetFocusedWindow(JNIEnv* env, jclass clazz, jlong transactionO request.windowName = windowName.c_str(); } - request.focusedToken = focusedToken; - if (focusedWindowNameJstr != NULL) { - ScopedUtfChars focusedWindowName(env, focusedWindowNameJstr); - request.focusedWindowName = focusedWindowName.c_str(); - } request.timestamp = systemTime(SYSTEM_TIME_MONOTONIC); request.displayId = displayId; transaction->setFocusedWindow(request); @@ -2236,7 +2225,7 @@ static const JNINativeMethod sSurfaceControlMethods[] = { (void*)nativeGetHandle }, {"nativeSetFixedTransformHint", "(JJI)V", (void*)nativeSetFixedTransformHint}, - {"nativeSetFocusedWindow", "(JLandroid/os/IBinder;Ljava/lang/String;Landroid/os/IBinder;Ljava/lang/String;I)V", + {"nativeSetFocusedWindow", "(JLandroid/os/IBinder;Ljava/lang/String;I)V", (void*)nativeSetFocusedWindow}, {"nativeRemoveCurrentInputFocus", "(JI)V", (void*)nativeRemoveCurrentInputFocus}, diff --git a/services/core/java/com/android/server/wm/EmbeddedWindowController.java b/services/core/java/com/android/server/wm/EmbeddedWindowController.java index 052c09a0e0ebf..d65f464590c17 100644 --- a/services/core/java/com/android/server/wm/EmbeddedWindowController.java +++ b/services/core/java/com/android/server/wm/EmbeddedWindowController.java @@ -99,23 +99,6 @@ class EmbeddedWindowController { } } - WindowState getHostWindow(IBinder inputToken) { - EmbeddedWindow embeddedWindow = mWindows.get(inputToken); - return embeddedWindow != null ? embeddedWindow.mHostWindowState : null; - } - - boolean isOverlay(IBinder inputToken) { - EmbeddedWindow embeddedWindow = mWindows.get(inputToken); - return embeddedWindow != null ? embeddedWindow.getIsOverlay() : false; - } - - void setIsOverlay(IBinder focusGrantToken) { - EmbeddedWindow embeddedWindow = mWindowsByFocusToken.get(focusGrantToken); - if (embeddedWindow != null) { - embeddedWindow.setIsOverlay(); - } - } - void remove(IWindow client) { for (int i = mWindows.size() - 1; i >= 0; i--) { EmbeddedWindow ew = mWindows.valueAt(i); @@ -176,14 +159,15 @@ class EmbeddedWindowController { public Session mSession; InputChannel mInputChannel; final int mWindowType; - // Track whether the EmbeddedWindow is a system hosted overlay via - // {@link OverlayHost}. In the case of client hosted overlays, the client - // view hierarchy will take care of invoking requestEmbeddedWindowFocus - // but for system hosted overlays we have to do this via tapOutsideDetection - // and this variable is mostly used for tracking that. - boolean mIsOverlay = false; - private IBinder mFocusGrantToken; + /** + * A unique token associated with the embedded window that can be used by the host window + * to request focus transfer to the embedded. This is not the input token since we don't + * want to give clients access to each others input token. + */ + private final IBinder mFocusGrantToken; + + private boolean mIsFocusable; /** * @param session calling session to check ownership of the window @@ -199,7 +183,8 @@ class EmbeddedWindowController { */ EmbeddedWindow(Session session, WindowManagerService service, IWindow clientToken, WindowState hostWindowState, int ownerUid, int ownerPid, int windowType, - int displayId, IBinder focusGrantToken, String inputHandleName) { + int displayId, IBinder focusGrantToken, String inputHandleName, + boolean isFocusable) { mSession = session; mWmService = service; mClient = clientToken; @@ -214,6 +199,7 @@ class EmbeddedWindowController { final String hostWindowName = (mHostWindowState != null) ? "-" + mHostWindowState.getWindowTag().toString() : ""; + mIsFocusable = isFocusable; mName = "Embedded{" + inputHandleName + hostWindowName + "}"; } @@ -279,13 +265,6 @@ class EmbeddedWindowController { return mOwnerUid; } - void setIsOverlay() { - mIsOverlay = true; - } - boolean getIsOverlay() { - return mIsOverlay; - } - IBinder getFocusGrantToken() { return mFocusGrantToken; } @@ -297,20 +276,33 @@ class EmbeddedWindowController { return null; } + void setIsFocusable(boolean isFocusable) { + mIsFocusable = isFocusable; + } + /** - * System hosted overlays need the WM to invoke grantEmbeddedWindowFocus and - * so we need to participate inside handlePointerDownOutsideFocus logic - * however client hosted overlays will rely on the hosting view hierarchy - * to grant and revoke focus, and so the server side logic is not needed. + * When an embedded window is touched when it's not currently focus, we need to switch + * focus to that embedded window unless the embedded window was marked as not focusable. */ @Override public boolean receiveFocusFromTapOutside() { - return mIsOverlay; + return mIsFocusable; } private void handleTap(boolean grantFocus) { if (mInputChannel != null) { - mWmService.grantEmbeddedWindowFocus(mSession, mFocusGrantToken, grantFocus); + if (mHostWindowState != null) { + mWmService.grantEmbeddedWindowFocus(mSession, mHostWindowState.mClient, + mFocusGrantToken, grantFocus); + if (grantFocus) { + // If granting focus to the embedded when tapped, we need to ensure the host + // gains focus as well or the transfer won't take effect since it requires + // the host to transfer the focus to the embedded. + mHostWindowState.handleTapOutsideFocusInsideSelf(); + } + } else { + mWmService.grantEmbeddedWindowFocus(mSession, mFocusGrantToken, grantFocus); + } } } diff --git a/services/core/java/com/android/server/wm/InputWindowHandleWrapper.java b/services/core/java/com/android/server/wm/InputWindowHandleWrapper.java index 301c1846249ff..3d4e0ebac258c 100644 --- a/services/core/java/com/android/server/wm/InputWindowHandleWrapper.java +++ b/services/core/java/com/android/server/wm/InputWindowHandleWrapper.java @@ -289,6 +289,14 @@ class InputWindowHandleWrapper { mChanged = true; } + void setFocusTransferTarget(IBinder toToken) { + if (mHandle.focusTransferTarget == toToken) { + return; + } + mHandle.focusTransferTarget = toToken; + mChanged = true; + } + @Override public String toString() { return mHandle + ", changed=" + mChanged; diff --git a/services/core/java/com/android/server/wm/TrustedOverlayHost.java b/services/core/java/com/android/server/wm/TrustedOverlayHost.java index 88c410b263ca9..f8edc2b871be8 100644 --- a/services/core/java/com/android/server/wm/TrustedOverlayHost.java +++ b/services/core/java/com/android/server/wm/TrustedOverlayHost.java @@ -90,8 +90,6 @@ class TrustedOverlayHost { requireOverlaySurfaceControl(); mOverlays.add(p); - mWmService.mEmbeddedWindowController.setIsOverlay(p.getInputToken()); - SurfaceControl.Transaction t = mWmService.mTransactionFactory.get(); t.reparent(p.getSurfaceControl(), mSurfaceControl) .show(p.getSurfaceControl()); diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java index a7a90604f2286..dde87b1c35e8c 100644 --- a/services/core/java/com/android/server/wm/WindowManagerService.java +++ b/services/core/java/com/android/server/wm/WindowManagerService.java @@ -8624,7 +8624,8 @@ public class WindowManagerService extends IWindowManager.Stub EmbeddedWindowController.EmbeddedWindow win = new EmbeddedWindowController.EmbeddedWindow(session, this, window, mInputToWindowMap.get(hostInputToken), callingUid, callingPid, - sanitizedType, displayId, focusGrantToken, inputHandleName); + sanitizedType, displayId, focusGrantToken, inputHandleName, + (flags & FLAG_NOT_FOCUSABLE) == 0); clientChannel = win.openInputChannel(); mEmbeddedWindowController.add(clientChannel.getToken(), win); applicationHandle = win.getApplicationHandle(); @@ -8745,6 +8746,7 @@ public class WindowManagerService extends IWindowManager.Stub } name = win.toString(); applicationHandle = win.getApplicationHandle(); + win.setIsFocusable((flags & FLAG_NOT_FOCUSABLE) == 0); } updateInputChannel(channelToken, win.mOwnerUid, win.mOwnerPid, displayId, surface, name, @@ -9022,24 +9024,23 @@ public class WindowManagerService extends IWindowManager.Stub Slog.e(TAG, "Embedded window does not belong to the host"); return; } - SurfaceControl.Transaction t = mTransactionFactory.get(); if (grantFocus) { - t.requestFocusTransfer(embeddedWindow.getInputChannelToken(), embeddedWindow.toString(), - hostWindow.mInputChannel.getToken(), - hostWindow.getName(), - hostWindow.getDisplayId()).apply(); + hostWindow.mInputWindowHandle.setFocusTransferTarget( + embeddedWindow.getInputChannelToken()); EventLog.writeEvent(LOGTAG_INPUT_FOCUS, "Transfer focus request " + embeddedWindow, "reason=grantEmbeddedWindowFocus(true)"); } else { - t.requestFocusTransfer(hostWindow.mInputChannel.getToken(), hostWindow.getName(), - embeddedWindow.getInputChannelToken(), - embeddedWindow.toString(), - hostWindow.getDisplayId()).apply(); + hostWindow.mInputWindowHandle.setFocusTransferTarget(null); EventLog.writeEvent(LOGTAG_INPUT_FOCUS, "Transfer focus request " + hostWindow, "reason=grantEmbeddedWindowFocus(false)"); } + DisplayContent dc = mRoot.getDisplayContent(hostWindow.getDisplayId()); + if (dc != null) { + dc.getInputMonitor().updateInputWindowsLw(true); + } + ProtoLog.v(WM_DEBUG_FOCUS, "grantEmbeddedWindowFocus win=%s grantFocus=%s", embeddedWindow, grantFocus); }