Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Add pre/post callback to IDragDropCallback

Browse Source 
Previoulsy we uses mWriteLock to avoid race condition between
DragDropController and IDragDropCallback. However it turns out we have
dead lock here.

1. DragDropController invokes performDrag for drag operation B with
   holding write lock.
2. The Callback initiates other drag operation A before processing B.
3. A does not complete because DragDropController holding write lock for
   B. B does not complete because the callback try to process request A
   before B.

The CL removes write lock and separates the existing callback methods
into two: pre and post callbacks.

1. DragDropController invokes prePerfromDrag for drag operaiton B, which
   blocks.
2. The callback initiates other drag operation A before processing B.
3. DragDropController invokes prePerfromDrag for drag operaiton A, which
   returns immediately. DragDropController processes A and invokes
   postPefromCallback for A.
4. The Callback unblocks prePerfromDrag for B.

Bug: 65564090
Test: android.server.wm.CrossAppDragAndDropTests, manually check the
      drag and drop behavior on test app.
Change-Id: I393a031cb276d6fa7ea56909959d0f2c5d621125
This commit is contained in:
Daichi Hirono
2017-11-21 15:11:01 +09:00
parent 3952e25663
commit bb28efb631
2 changed files with 105 additions and 127 deletions
Download Patch File Download Diff File Expand all files Collapse all files

205
services/core/java/com/android/server/wm/DragDropController.java
View File

@@ -40,6 +40,7 @@ import android.view.View;
import com.android.internal.util.Preconditions;
import com.android.server.input.InputWindowHandle;
import com.android.server.wm.WindowManagerInternal.IDragDropCallback;
import java.util.concurrent.atomic.AtomicReference;
/**
* Managing drag and drop operations initiated by View#startDragAndDrop.
@@ -66,43 +67,11 @@ 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();
/**
* Callback which is used to sync drag state with the vendor-specific code.
*/
@NonNull private IDragDropCallback mCallback = new IDragDropCallback() {};
@NonNull private AtomicReference<IDragDropCallback> mCallback = new AtomicReference<>(
new IDragDropCallback() {});
boolean dragDropActiveLocked() {
return mDragState != null;
@@ -114,9 +83,7 @@ class DragDropController {
void registerCallback(IDragDropCallback callback) {
Preconditions.checkNotNull(callback);
synchronized (mWriteLock) {
mCallback = callback;
}
mCallback.set(callback);
}
DragDropController(WindowManagerService service, Looper looper) {
@@ -136,45 +103,43 @@ class DragDropController {
+ " asbinder=" + window.asBinder());
}
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;
}
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;
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;
}
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;
}
}
@@ -185,11 +150,11 @@ class DragDropController {
Slog.d(TAG_WM, "perform drag: win=" + window + " data=" + data);
}
synchronized (mWriteLock) {
if (!mCallback.performDrag(window, dragToken, touchSource, touchX, touchY, thumbCenterX,
thumbCenterY, data)) {
return false;
}
if (!mCallback.get().prePerformDrag(window, dragToken, touchSource, touchX, touchY, thumbCenterX,
thumbCenterY, data)) {
return false;
}
try {
synchronized (mService.mWindowMap) {
if (mDragState == null) {
Slog.w(TAG_WM, "No drag prepared");
@@ -260,9 +225,10 @@ class DragDropController {
mDragState.notifyLocationLocked(touchX, touchY);
}
return true; // success!
} finally {
mCallback.get().postPerformDrag();
}
return true; // success!
}
void reportDropResult(IWindow window, boolean consumed) {
@@ -271,8 +237,8 @@ class DragDropController {
Slog.d(TAG_WM, "Drop result=" + consumed + " reported by " + token);
}
synchronized (mWriteLock) {
mCallback.reportDropResult(window, consumed);
mCallback.get().preReportDropResult(window, consumed);
try {
synchronized (mService.mWindowMap) {
if (mDragState == null) {
// Most likely the drop recipient ANRed and we ended the drag
@@ -297,10 +263,11 @@ class DragDropController {
return; // !!! TODO: throw here?
}
mDragState.mDragResult = consumed;
mDragState.endDragLocked();
}
} finally {
mCallback.get().postReportDropResult();
}
}
@@ -309,8 +276,8 @@ class DragDropController {
Slog.d(TAG_WM, "cancelDragAndDrop");
}
synchronized (mWriteLock) {
mCallback.cancelDragAndDrop(dragToken);
mCallback.get().preCancelDragAndDrop(dragToken);
try {
synchronized (mService.mWindowMap) {
if (mDragState == null) {
Slog.w(TAG_WM, "cancelDragAndDrop() without prepareDrag()");
@@ -327,6 +294,8 @@ class DragDropController {
mDragState.mDragResult = false;
mDragState.cancelDragLocked();
}
} finally {
mCallback.get().postCancelDragAndDrop();
}
}
@@ -338,20 +307,18 @@ class DragDropController {
* @param newY Y coordinate value in dp in the screen coordinate
*/
void handleMotionEvent(boolean keepHandling, float newX, float newY) {
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;
}
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);
}
}
}
@@ -414,12 +381,11 @@ class DragDropController {
if (DEBUG_DRAG) {
Slog.w(TAG_WM, "Timeout starting drag by win " + win);
}
synchronized (mWriteLock) {
synchronized (mService.mWindowMap) {
// !!! TODO: ANR the app that has failed to start the drag in time
if (mDragState != null) {
mDragState.closeLocked();
}
synchronized (mService.mWindowMap) {
// !!! TODO: ANR the app that has failed to start the drag in time
if (mDragState != null) {
mDragState.closeLocked();
}
}
break;
@@ -430,13 +396,12 @@ class DragDropController {
if (DEBUG_DRAG) {
Slog.w(TAG_WM, "Timeout ending drag to win " + win);
}
synchronized (mWriteLock) {
synchronized (mService.mWindowMap) {
// !!! TODO: ANR the drag-receiving app
if (mDragState != null) {
mDragState.mDragResult = false;
mDragState.endDragLocked();
}
synchronized (mService.mWindowMap) {
// !!! TODO: ANR the drag-receiving app
if (mDragState != null) {
mDragState.mDragResult = false;
mDragState.endDragLocked();
}
}
break;
@@ -455,15 +420,13 @@ class DragDropController {
}
case MSG_ANIMATION_END: {
synchronized (mWriteLock) {
synchronized (mService.mWindowMap) {
if (mDragState == null) {
Slog.wtf(TAG_WM, "mDragState unexpectedly became null while " +
"plyaing animation");
return;
}
mDragState.closeLocked();
synchronized (mService.mWindowMap) {
if (mDragState == null) {
Slog.wtf(TAG_WM, "mDragState unexpectedly became null while " +
"plyaing animation");
return;
}
mDragState.closeLocked();
}
break;
}

27
services/core/java/com/android/server/wm/WindowManagerInternal.java
View File

@@ -151,23 +151,38 @@ public abstract class WindowManagerInternal {
*/
public interface IDragDropCallback {
/**
* Called when drag operation is started.
* Called when drag operation is starting.
*/
default boolean performDrag(IWindow window, IBinder dragToken,
default boolean prePerformDrag(IWindow window, IBinder dragToken,
int touchSource, float touchX, float touchY, float thumbCenterX, float thumbCenterY,
ClipData data) {
return true;
}
/**
* Called when drop result is reported.
* Called when drag operation is started.
*/
default void reportDropResult(IWindow window, boolean consumed) {}
default void postPerformDrag() {}
/**
* Called when drag operation is cancelled.
* Called when drop result is being reported.
*/
default void cancelDragAndDrop(IBinder dragToken) {}
default void preReportDropResult(IWindow window, boolean consumed) {}
/**
* Called when drop result was reported.
*/
default void postReportDropResult() {}
/**
* Called when drag operation is being cancelled.
*/
default void preCancelDragAndDrop(IBinder dragToken) {}
/**
* Called when drag operation was cancelled.
*/
default void postCancelDragAndDrop() {}
}
/**