summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Daichi Hirono <hirono@google.com> 2017-10-30 10:07:24 +0900
committer Daichi Hirono <hirono@google.com> 2017-11-07 14:24:24 +0900
commit7a5d007aad1bb7b3e11f2165e5280f7662739ee0 (patch)
tree7dbb8b5bcbf7ebf8972d2105c475ae6043e9a998
parent5647b92d234c5ef40a04c917037cf91b266d060d (diff)
Introduce write lock for mDragState
We are going to introduce the drag drop callback however we would not like to keep holding the window manager lock while invoking the callback. The CL introduce separated write lock for mDragState which we can hold while invoking the upcoming drag and drop callback. Bug: 65564090 Test: android.server.wm.CrossAppDragAndDropTests, manually check the drag and drop behavior on test app. Change-Id: Icb6484784097bc93c5aa1154c71324aa5353620c
-rw-r--r--services/core/java/com/android/server/wm/DragDropController.java379
1 files changed, 214 insertions, 165 deletions
diff --git a/services/core/java/com/android/server/wm/DragDropController.java b/services/core/java/com/android/server/wm/DragDropController.java
index cc4e0f8a5004..76e25bae5db2 100644
--- a/services/core/java/com/android/server/wm/DragDropController.java
+++ b/services/core/java/com/android/server/wm/DragDropController.java
@@ -52,6 +52,8 @@ class DragDropController {
/**
* Drag state per operation.
+ * Needs a lock of {@code WindowManagerService#mWindowMap} to read this. Needs both locks of
+ * {@code mWriteLock} and {@code WindowManagerService#mWindowMap} to update this.
* The variable is cleared by {@code #onDragStateClosedLocked} which is invoked by DragState
* itself, thus the variable can be null after calling DragState's methods.
*/
@@ -59,6 +61,38 @@ class DragDropController {
private WindowManagerService mService;
private final Handler mHandler;
+ /**
+ * Lock to preserve the order of state updates.
+ * The lock is used to process drag and drop state updates in order without having the window
+ * manager lock.
+ *
+ * Suppose DragDropController invokes a callback method A, then processes the following update
+ * A'. Same for a callback method B and the following update B'. The callback wants
+ * DragDropController to processes the updates in the order of A' then B'.
+ *
+ * Without mWriteLock: the following race can happen.
+ *
+ * 1. Thread a calls A.
+ * 2. Thread b calls B.
+ * 3. Thread b acquires the window manager lock
+ * 4. thread b processes the update B'
+ * 5. Thread a acquires the window manager lock
+ * 6. thread a processes the update A'
+ *
+ * With mWriteLock we can ensure the order of A' and B'
+ *
+ * 1. Thread a acquire mWriteLock
+ * 2. Thread a calls A
+ * 3. Thread a acquire the window manager lock
+ * 4. Thread a processes A'
+ * 5. Thread b acquire mWriteLock
+ * 6. Thread b calls B
+ * 7. Thread b acquire the window manager lock
+ * 8. Thread b processes B'
+ *
+ * Don't acquire the lock while holding the window manager lock, otherwise it causes a deadlock.
+ */
+ private final Object mWriteLock = new Object();
boolean dragDropActiveLocked() {
return mDragState != null;
@@ -85,46 +119,46 @@ class DragDropController {
+ " asbinder=" + window.asBinder());
}
- IBinder token = null;
-
- synchronized (mService.mWindowMap) {
- if (dragDropActiveLocked()) {
- Slog.w(TAG_WM, "Drag already in progress");
- return null;
- }
+ synchronized (mWriteLock) {
+ synchronized (mService.mWindowMap) {
+ if (dragDropActiveLocked()) {
+ Slog.w(TAG_WM, "Drag already in progress");
+ return null;
+ }
- // TODO(multi-display): support other displays
- final DisplayContent displayContent =
- mService.getDefaultDisplayContentLocked();
- final Display display = displayContent.getDisplay();
-
- final SurfaceControl surface = new SurfaceControl.Builder(session)
- .setName("drag surface")
- .setSize(width, height)
- .setFormat(PixelFormat.TRANSLUCENT)
- .build();
- surface.setLayerStack(display.getLayerStack());
- float alpha = 1;
- if ((flags & View.DRAG_FLAG_OPAQUE) == 0) {
- alpha = DRAG_SHADOW_ALPHA_TRANSPARENT;
+ // TODO(multi-display): support other displays
+ final DisplayContent displayContent =
+ mService.getDefaultDisplayContentLocked();
+ final Display display = displayContent.getDisplay();
+
+ final SurfaceControl surface = new SurfaceControl.Builder(session)
+ .setName("drag surface")
+ .setSize(width, height)
+ .setFormat(PixelFormat.TRANSLUCENT)
+ .build();
+ surface.setLayerStack(display.getLayerStack());
+ float alpha = 1;
+ if ((flags & View.DRAG_FLAG_OPAQUE) == 0) {
+ alpha = DRAG_SHADOW_ALPHA_TRANSPARENT;
+ }
+ surface.setAlpha(alpha);
+
+ if (SHOW_TRANSACTIONS)
+ Slog.i(TAG_WM, " DRAG " + surface + ": CREATE");
+ outSurface.copyFrom(surface);
+ final IBinder winBinder = window.asBinder();
+ IBinder token = new Binder();
+ mDragState = new DragState(mService, token, surface, flags, winBinder);
+ mDragState.mPid = callerPid;
+ mDragState.mUid = callerUid;
+ mDragState.mOriginalAlpha = alpha;
+ token = mDragState.mToken = new Binder();
+
+ // 5 second timeout for this window to actually begin the drag
+ sendTimeoutMessage(MSG_DRAG_START_TIMEOUT, winBinder);
+ return token;
}
- surface.setAlpha(alpha);
-
- if (SHOW_TRANSACTIONS) Slog.i(TAG_WM, " DRAG " + surface + ": CREATE");
- outSurface.copyFrom(surface);
- final IBinder winBinder = window.asBinder();
- token = new Binder();
- mDragState = new DragState(mService, token, surface, flags, winBinder);
- mDragState.mPid = callerPid;
- mDragState.mUid = callerUid;
- mDragState.mOriginalAlpha = alpha;
- token = mDragState.mToken = new Binder();
-
- // 5 second timeout for this window to actually begin the drag
- sendTimeoutMessage(MSG_DRAG_START_TIMEOUT, winBinder);
}
-
- return token;
}
boolean performDrag(IWindow window, IBinder dragToken,
@@ -134,75 +168,77 @@ class DragDropController {
Slog.d(TAG_WM, "perform drag: win=" + window + " data=" + data);
}
- synchronized (mService.mWindowMap) {
- if (mDragState == null) {
- Slog.w(TAG_WM, "No drag prepared");
- throw new IllegalStateException("performDrag() without prepareDrag()");
- }
+ synchronized (mWriteLock) {
+ synchronized (mService.mWindowMap) {
+ if (mDragState == null) {
+ Slog.w(TAG_WM, "No drag prepared");
+ throw new IllegalStateException("performDrag() without prepareDrag()");
+ }
- if (dragToken != mDragState.mToken) {
- Slog.w(TAG_WM, "Performing mismatched drag");
- throw new IllegalStateException("performDrag() does not match prepareDrag()");
- }
+ if (dragToken != mDragState.mToken) {
+ Slog.w(TAG_WM, "Performing mismatched drag");
+ throw new IllegalStateException("performDrag() does not match prepareDrag()");
+ }
- final WindowState callingWin = mService.windowForClientLocked(null, window, false);
- if (callingWin == null) {
- Slog.w(TAG_WM, "Bad requesting window " + window);
- return false; // !!! TODO: throw here?
- }
+ final WindowState callingWin = mService.windowForClientLocked(null, window, false);
+ if (callingWin == null) {
+ Slog.w(TAG_WM, "Bad requesting window " + window);
+ return false; // !!! TODO: throw here?
+ }
- // !!! TODO: if input is not still focused on the initiating window, fail
- // the drag initiation (e.g. an alarm window popped up just as the application
- // called performDrag()
+ // !!! TODO: if input is not still focused on the initiating window, fail
+ // the drag initiation (e.g. an alarm window popped up just as the application
+ // called performDrag()
- mHandler.removeMessages(MSG_DRAG_START_TIMEOUT, window.asBinder());
+ mHandler.removeMessages(MSG_DRAG_START_TIMEOUT, window.asBinder());
- // !!! TODO: extract the current touch (x, y) in screen coordinates. That
- // will let us eliminate the (touchX,touchY) parameters from the API.
+ // !!! TODO: extract the current touch (x, y) in screen coordinates. That
+ // will let us eliminate the (touchX,touchY) parameters from the API.
- // !!! FIXME: put all this heavy stuff onto the mHandler looper, as well as
- // the actual drag event dispatch stuff in the dragstate
+ // !!! FIXME: put all this heavy stuff onto the mHandler looper, as well as
+ // the actual drag event dispatch stuff in the dragstate
- final DisplayContent displayContent = callingWin.getDisplayContent();
- if (displayContent == null) {
- return false;
- }
- Display display = displayContent.getDisplay();
- mDragState.register(display);
- if (!mService.mInputManager.transferTouchFocus(callingWin.mInputChannel,
- mDragState.getInputChannel())) {
- Slog.e(TAG_WM, "Unable to transfer touch focus");
- mDragState.closeLocked();
- return false;
- }
+ final DisplayContent displayContent = callingWin.getDisplayContent();
+ if (displayContent == null) {
+ return false;
+ }
+ Display display = displayContent.getDisplay();
+ mDragState.register(display);
+ if (!mService.mInputManager.transferTouchFocus(callingWin.mInputChannel,
+ mDragState.getInputChannel())) {
+ Slog.e(TAG_WM, "Unable to transfer touch focus");
+ mDragState.closeLocked();
+ return false;
+ }
- mDragState.mDisplayContent = displayContent;
- mDragState.mData = data;
- mDragState.broadcastDragStartedLocked(touchX, touchY);
- mDragState.overridePointerIconLocked(touchSource);
-
- // remember the thumb offsets for later
- mDragState.mThumbOffsetX = thumbCenterX;
- mDragState.mThumbOffsetY = thumbCenterY;
-
- // Make the surface visible at the proper location
- final SurfaceControl surfaceControl = mDragState.mSurfaceControl;
- if (SHOW_LIGHT_TRANSACTIONS) Slog.i(
- TAG_WM, ">>> OPEN TRANSACTION performDrag");
- mService.openSurfaceTransaction();
- try {
- surfaceControl.setPosition(touchX - thumbCenterX,
- touchY - thumbCenterY);
- surfaceControl.setLayer(mDragState.getDragLayerLocked());
- surfaceControl.setLayerStack(display.getLayerStack());
- surfaceControl.show();
- } finally {
- mService.closeSurfaceTransaction("performDrag");
- if (SHOW_LIGHT_TRANSACTIONS) Slog.i(
- TAG_WM, "<<< CLOSE TRANSACTION performDrag");
- }
+ mDragState.mDisplayContent = displayContent;
+ mDragState.mData = data;
+ mDragState.broadcastDragStartedLocked(touchX, touchY);
+ mDragState.overridePointerIconLocked(touchSource);
+
+ // remember the thumb offsets for later
+ mDragState.mThumbOffsetX = thumbCenterX;
+ mDragState.mThumbOffsetY = thumbCenterY;
+
+ // Make the surface visible at the proper location
+ final SurfaceControl surfaceControl = mDragState.mSurfaceControl;
+ if (SHOW_LIGHT_TRANSACTIONS) Slog.i(TAG_WM, ">>> OPEN TRANSACTION performDrag");
+ mService.openSurfaceTransaction();
+ try {
+ surfaceControl.setPosition(touchX - thumbCenterX,
+ touchY - thumbCenterY);
+ surfaceControl.setLayer(mDragState.getDragLayerLocked());
+ surfaceControl.setLayerStack(display.getLayerStack());
+ surfaceControl.show();
+ } finally {
+ mService.closeSurfaceTransaction("performDrag");
+ if (SHOW_LIGHT_TRANSACTIONS) {
+ Slog.i(TAG_WM, "<<< CLOSE TRANSACTION performDrag");
+ }
+ }
- mDragState.notifyLocationLocked(touchX, touchY);
+ mDragState.notifyLocationLocked(touchX, touchY);
+ }
}
return true; // success!
@@ -214,32 +250,35 @@ class DragDropController {
Slog.d(TAG_WM, "Drop result=" + consumed + " reported by " + token);
}
- synchronized (mService.mWindowMap) {
- if (mDragState == null) {
- // Most likely the drop recipient ANRed and we ended the drag
- // out from under it. Log the issue and move on.
- Slog.w(TAG_WM, "Drop result given but no drag in progress");
- return;
- }
+ synchronized (mWriteLock) {
+ synchronized (mService.mWindowMap) {
+ if (mDragState == null) {
+ // Most likely the drop recipient ANRed and we ended the drag
+ // out from under it. Log the issue and move on.
+ Slog.w(TAG_WM, "Drop result given but no drag in progress");
+ return;
+ }
- if (mDragState.mToken != token) {
- // We're in a drag, but the wrong window has responded.
- Slog.w(TAG_WM, "Invalid drop-result claim by " + window);
- throw new IllegalStateException("reportDropResult() by non-recipient");
- }
+ if (mDragState.mToken != token) {
+ // We're in a drag, but the wrong window has responded.
+ Slog.w(TAG_WM, "Invalid drop-result claim by " + window);
+ throw new IllegalStateException("reportDropResult() by non-recipient");
+ }
+
+ // The right window has responded, even if it's no longer around,
+ // so be sure to halt the timeout even if the later WindowState
+ // lookup fails.
+ mHandler.removeMessages(MSG_DRAG_END_TIMEOUT, window.asBinder());
+ WindowState callingWin = mService.windowForClientLocked(null, window, false);
+ if (callingWin == null) {
+ Slog.w(TAG_WM, "Bad result-reporting window " + window);
+ return; // !!! TODO: throw here?
+ }
- // The right window has responded, even if it's no longer around,
- // so be sure to halt the timeout even if the later WindowState
- // lookup fails.
- mHandler.removeMessages(MSG_DRAG_END_TIMEOUT, window.asBinder());
- WindowState callingWin = mService.windowForClientLocked(null, window, false);
- if (callingWin == null) {
- Slog.w(TAG_WM, "Bad result-reporting window " + window);
- return; // !!! TODO: throw here?
- }
- mDragState.mDragResult = consumed;
- mDragState.endDragLocked();
+ mDragState.mDragResult = consumed;
+ mDragState.endDragLocked();
+ }
}
}
@@ -248,21 +287,23 @@ class DragDropController {
Slog.d(TAG_WM, "cancelDragAndDrop");
}
- synchronized (mService.mWindowMap) {
- if (mDragState == null) {
- Slog.w(TAG_WM, "cancelDragAndDrop() without prepareDrag()");
- throw new IllegalStateException("cancelDragAndDrop() without prepareDrag()");
- }
+ synchronized (mWriteLock) {
+ synchronized (mService.mWindowMap) {
+ if (mDragState == null) {
+ Slog.w(TAG_WM, "cancelDragAndDrop() without prepareDrag()");
+ throw new IllegalStateException("cancelDragAndDrop() without prepareDrag()");
+ }
- if (mDragState.mToken != dragToken) {
- Slog.w(TAG_WM,
- "cancelDragAndDrop() does not match prepareDrag()");
- throw new IllegalStateException(
- "cancelDragAndDrop() does not match prepareDrag()");
- }
+ if (mDragState.mToken != dragToken) {
+ Slog.w(TAG_WM,
+ "cancelDragAndDrop() does not match prepareDrag()");
+ throw new IllegalStateException(
+ "cancelDragAndDrop() does not match prepareDrag()");
+ }
- mDragState.mDragResult = false;
- mDragState.cancelDragLocked();
+ mDragState.mDragResult = false;
+ mDragState.cancelDragLocked();
+ }
}
}
@@ -274,18 +315,20 @@ class DragDropController {
* @param newY Y coordinate value in dp in the screen coordinate
*/
void handleMotionEvent(boolean keepHandling, float newX, float newY) {
- synchronized (mService.mWindowMap) {
- if (!dragDropActiveLocked()) {
- // The drag has ended but the clean-up message has not been processed by
- // window manager. Drop events that occur after this until window manager
- // has a chance to clean-up the input handle.
- return;
- }
+ synchronized (mWriteLock) {
+ synchronized (mService.mWindowMap) {
+ if (!dragDropActiveLocked()) {
+ // The drag has ended but the clean-up message has not been processed by
+ // window manager. Drop events that occur after this until window manager
+ // has a chance to clean-up the input handle.
+ return;
+ }
- if (keepHandling) {
- mDragState.notifyMoveLocked(newX, newY);
- } else {
- mDragState.notifyDropLocked(newX, newY);
+ if (keepHandling) {
+ mDragState.notifyMoveLocked(newX, newY);
+ } else {
+ mDragState.notifyDropLocked(newX, newY);
+ }
}
}
}
@@ -348,25 +391,29 @@ class DragDropController {
if (DEBUG_DRAG) {
Slog.w(TAG_WM, "Timeout starting drag by win " + win);
}
- synchronized (mService.mWindowMap) {
- // !!! TODO: ANR the app that has failed to start the drag in time
- if (mDragState != null) {
- mDragState.closeLocked();
+ synchronized (mWriteLock) {
+ synchronized (mService.mWindowMap) {
+ // !!! TODO: ANR the app that has failed to start the drag in time
+ if (mDragState != null) {
+ mDragState.closeLocked();
+ }
}
}
break;
}
case MSG_DRAG_END_TIMEOUT: {
- IBinder win = (IBinder) msg.obj;
+ final IBinder win = (IBinder) msg.obj;
if (DEBUG_DRAG) {
Slog.w(TAG_WM, "Timeout ending drag to win " + win);
}
- synchronized (mService.mWindowMap) {
- // !!! TODO: ANR the drag-receiving app
- if (mDragState != null) {
- mDragState.mDragResult = false;
- mDragState.endDragLocked();
+ synchronized (mWriteLock) {
+ synchronized (mService.mWindowMap) {
+ // !!! TODO: ANR the drag-receiving app
+ if (mDragState != null) {
+ mDragState.mDragResult = false;
+ mDragState.endDragLocked();
+ }
}
}
break;
@@ -375,23 +422,25 @@ class DragDropController {
case MSG_TEAR_DOWN_DRAG_AND_DROP_INPUT: {
if (DEBUG_DRAG)
Slog.d(TAG_WM, "Drag ending; tearing down input channel");
- DragState.InputInterceptor interceptor = (DragState.InputInterceptor) msg.obj;
- if (interceptor != null) {
- synchronized (mService.mWindowMap) {
- interceptor.tearDown();
- }
+ final DragState.InputInterceptor interceptor =
+ (DragState.InputInterceptor) msg.obj;
+ if (interceptor == null) return;
+ synchronized (mService.mWindowMap) {
+ interceptor.tearDown();
}
break;
}
case MSG_ANIMATION_END: {
- synchronized (mService.mWindowMap) {
- if (mDragState == null) {
- Slog.wtf(TAG_WM, "mDragState unexpectedly became null while " +
- "plyaing animation");
- return;
+ synchronized (mWriteLock) {
+ synchronized (mService.mWindowMap) {
+ if (mDragState == null) {
+ Slog.wtf(TAG_WM, "mDragState unexpectedly became null while " +
+ "plyaing animation");
+ return;
+ }
+ mDragState.closeLocked();
}
- mDragState.closeLocked();
}
break;
}