From d427c6283cda37676ea1328c33598c110b4d7677 Mon Sep 17 00:00:00 2001 From: Garfield Tan Date: Fri, 30 Nov 2018 13:00:04 -0800 Subject: [PATCH] Fix a bug that effectively disallows hiding surfaces. InputMonitor checks DragDropController and TaskPositioningController's state to decide if it should show or hide respective surface. However when InputMonitor checks the status when stopping drag-resizing/drag-and-drop, the status is not yet changed because TaskPositioner/DragState instance isn't cleared yet. Therefore this CL clears TaskPositioning before updating InputWindow, and flags the closing state in DragState to avoid surface leak. Bug: 120289807 Test: Drag-resize works with a workaround to another race condition. Change-Id: I743d0a97c937b9d6a06c70d42da34cc77822cd58 --- .../android/server/wm/DragDropController.java | 5 ++-- .../java/com/android/server/wm/DragState.java | 22 ++++++++++------ .../server/wm/TaskPositioningController.java | 25 ++++++++++++------- 3 files changed, 33 insertions(+), 19 deletions(-) diff --git a/services/core/java/com/android/server/wm/DragDropController.java b/services/core/java/com/android/server/wm/DragDropController.java index e3ddadc7f70ec..a667d679ee947 100644 --- a/services/core/java/com/android/server/wm/DragDropController.java +++ b/services/core/java/com/android/server/wm/DragDropController.java @@ -31,13 +31,12 @@ import android.util.Slog; import android.view.Display; import android.view.IWindow; import android.view.SurfaceControl; -import android.view.SurfaceControl.Transaction; import android.view.SurfaceSession; import android.view.View; import com.android.internal.util.Preconditions; -import android.view.InputWindowHandle; import com.android.server.wm.WindowManagerInternal.IDragDropCallback; + import java.util.concurrent.atomic.AtomicReference; /** @@ -71,7 +70,7 @@ class DragDropController { new IDragDropCallback() {}); boolean dragDropActiveLocked() { - return mDragState != null; + return mDragState != null && !mDragState.isClosing(); } void showInputSurface(SurfaceControl.Transaction t, int displayId) { diff --git a/services/core/java/com/android/server/wm/DragState.java b/services/core/java/com/android/server/wm/DragState.java index 8f6ed85d122d1..607ee767869f1 100644 --- a/services/core/java/com/android/server/wm/DragState.java +++ b/services/core/java/com/android/server/wm/DragState.java @@ -31,25 +31,24 @@ import android.animation.ValueAnimator; import android.annotation.Nullable; import android.content.ClipData; import android.content.ClipDescription; -import android.content.Context; -import android.graphics.Rect; import android.graphics.Point; +import android.graphics.Rect; import android.hardware.input.InputManager; -import android.os.Build; import android.os.Binder; +import android.os.Build; import android.os.IBinder; import android.os.Process; import android.os.RemoteException; -import android.os.ServiceManager; import android.os.UserHandle; import android.os.UserManager; -import android.os.IUserManager; import android.os.UserManagerInternal; import android.util.Slog; import android.view.Display; import android.view.DragEvent; +import android.view.InputApplicationHandle; import android.view.InputChannel; import android.view.InputDevice; +import android.view.InputWindowHandle; import android.view.PointerIcon; import android.view.SurfaceControl; import android.view.View; @@ -59,8 +58,6 @@ import android.view.animation.Interpolator; import com.android.internal.view.IDragAndDropPermissions; import com.android.server.LocalServices; -import android.view.InputApplicationHandle; -import android.view.InputWindowHandle; import java.util.ArrayList; @@ -125,6 +122,12 @@ class DragState { private final Rect mTmpClipRect = new Rect(); + /** + * Whether we are finishing this drag and drop. This starts with {@code false}, and is set to + * {@code true} when {@link #closeLocked()} is called. + */ + private boolean mIsClosing; + DragState(WindowManagerService service, DragDropController controller, IBinder token, SurfaceControl surface, int flags, IBinder localWin) { mService = service; @@ -137,6 +140,10 @@ class DragState { } + boolean isClosing() { + return mIsClosing; + } + void hideInputSurface(SurfaceControl.Transaction t, int displayId) { if (displayId != mDisplayContent.getDisplayId()) { return; @@ -177,6 +184,7 @@ class DragState { * DragDropController#mDragState becomes null. */ void closeLocked() { + mIsClosing = true; // Unregister the input interceptor. if (mInputInterceptor != null) { if (DEBUG_DRAG) diff --git a/services/core/java/com/android/server/wm/TaskPositioningController.java b/services/core/java/com/android/server/wm/TaskPositioningController.java index 5a70325fbd877..e15bf5b9a6f09 100644 --- a/services/core/java/com/android/server/wm/TaskPositioningController.java +++ b/services/core/java/com/android/server/wm/TaskPositioningController.java @@ -28,12 +28,12 @@ import android.os.Looper; import android.os.RemoteException; import android.util.Slog; import android.view.Display; -import android.view.SurfaceControl; import android.view.IWindow; +import android.view.InputWindowHandle; +import android.view.SurfaceControl; import com.android.internal.annotations.GuardedBy; import com.android.server.input.InputManagerService; -import android.view.InputWindowHandle; /** * Controller for task positioning by drag. @@ -184,9 +184,7 @@ class TaskPositioningController { if (!mInputManager.transferTouchFocus( transferFocusFromWin.mInputChannel, mTaskPositioner.mServerChannel)) { Slog.e(TAG_WM, "startPositioningLocked: Unable to transfer touch focus"); - mTaskPositioner.unregister(); - mTaskPositioner = null; - displayContent.getInputMonitor().updateInputWindowsLw(true /*force*/); + cleanUpTaskPositioner(); return false; } @@ -199,12 +197,21 @@ class TaskPositioningController { if (DEBUG_TASK_POSITIONING) Slog.d(TAG_WM, "finishPositioning"); synchronized (mService.mGlobalLock) { - if (mTaskPositioner != null) { - mTaskPositioner.unregister(); - mTaskPositioner = null; - } + cleanUpTaskPositioner(); mPositioningDisplay = null; } }); } + + private void cleanUpTaskPositioner() { + final TaskPositioner positioner = mTaskPositioner; + if (positioner == null) { + return; + } + + // We need to assign task positioner to null first to indicate that we're finishing task + // positioning. + mTaskPositioner = null; + positioner.unregister(); + } }