From 3bae0b027feb873cbb430ee1c3e0408e565b69f2 Mon Sep 17 00:00:00 2001 From: Daichi Hirono Date: Fri, 27 Oct 2017 13:05:13 +0900 Subject: [PATCH 1/2] Sort out various cleanup methods of DragState DragState has mainly three states. * just created * after input intercepters is registered * after drag start broadcast is sent out * after drag end/cancel animation is completed Previously DragState has multiple cleanup methods for each state and callers need to choose the correct one. The CL merges the cleanup methods into one 'closeLocked' method so that callers can always invoke closeLocked, and DragState can take the responsibility to relelase its relevant resources depends on its internal state. Bug: 65564090 Test: android.server.wm.CrossAppDragAndDropTests, manually check the drag and drop behavior on test app. Change-Id: I257231073b350726bfcebaf1cdf6a02dc0d4ca39 --- .../android/server/wm/DragDropController.java | 26 ++- .../java/com/android/server/wm/DragState.java | 159 +++++++++--------- 2 files changed, 95 insertions(+), 90 deletions(-) diff --git a/services/core/java/com/android/server/wm/DragDropController.java b/services/core/java/com/android/server/wm/DragDropController.java index 860ff387d6681..b6f0f6a134543 100644 --- a/services/core/java/com/android/server/wm/DragDropController.java +++ b/services/core/java/com/android/server/wm/DragDropController.java @@ -49,6 +49,11 @@ class DragDropController { static final int MSG_TEAR_DOWN_DRAG_AND_DROP_INPUT = 2; static final int MSG_ANIMATION_END = 3; + /** + * Drag state per operation. + * The variable is cleared by {@code #onDragStateClosedLocked} which is invoked by DragState + * itself, thus the variable can be null after calling DragState's methods. + */ DragState mDragState; private WindowManagerService mService; @@ -158,9 +163,7 @@ class DragDropController { if (!mService.mInputManager.transferTouchFocus(callingWin.mInputChannel, mDragState.getInputChannel())) { Slog.e(TAG_WM, "Unable to transfer touch focus"); - mDragState.unregister(); - mDragState.reset(); - mDragState = null; + mDragState.closeLocked(); return false; } @@ -282,6 +285,17 @@ class DragDropController { mHandler.sendMessageDelayed(msg, DRAG_TIMEOUT_MS); } + /** + * Notifies the current drag state is closed. + */ + void onDragStateClosedLocked(DragState dragState) { + if (mDragState != dragState) { + Slog.wtf(TAG_WM, "Unknown drag state is closed"); + return; + } + mDragState = null; + } + private class DragHandler extends Handler { /** * Lock for window manager. @@ -304,9 +318,7 @@ class DragDropController { synchronized (mService.mWindowMap) { // !!! TODO: ANR the app that has failed to start the drag in time if (mDragState != null) { - mDragState.unregister(); - mDragState.reset(); - mDragState = null; + mDragState.closeLocked(); } } break; @@ -346,7 +358,7 @@ class DragDropController { "plyaing animation"); return; } - mDragState.onAnimationEndLocked(); + mDragState.closeLocked(); } break; } diff --git a/services/core/java/com/android/server/wm/DragState.java b/services/core/java/com/android/server/wm/DragState.java index 9a955de9706cc..da0394b6e294d 100644 --- a/services/core/java/com/android/server/wm/DragState.java +++ b/services/core/java/com/android/server/wm/DragState.java @@ -105,6 +105,11 @@ class DragState { WindowState mTargetWindow; ArrayList mNotifiedWindows; boolean mDragInProgress; + /** + * Whether if animation is completed. Needs to be volatile to update from the animation thread + * without having a WM lock. + */ + volatile boolean mAnimationCompleted = false; DisplayContent mDisplayContent; @Nullable private ValueAnimator mAnimator; @@ -122,21 +127,79 @@ class DragState { mNotifiedWindows = new ArrayList(); } - void reset() { - if (mAnimator != null) { + /** + * After calling this, DragDropController#onDragStateClosedLocked is invoked, which causes + * DragDropController#mDragState becomes null. + */ + void closeLocked() { + // Unregister the input interceptor. + if (mInputInterceptor != null) { + if (DEBUG_DRAG) + Slog.d(TAG_WM, "unregistering drag input channel"); + + // Input channel should be disposed on the thread where the input is being handled. + mDragDropController.sendHandlerMessage( + MSG_TEAR_DOWN_DRAG_AND_DROP_INPUT, mInputInterceptor); + mInputInterceptor = null; + mService.mInputMonitor.updateInputWindowsLw(true /*force*/); + } + + // Send drag end broadcast if drag start has been sent. + if (mDragInProgress) { + final int myPid = Process.myPid(); + + if (DEBUG_DRAG) { + Slog.d(TAG_WM, "broadcasting DRAG_ENDED"); + } + for (WindowState ws : mNotifiedWindows) { + float x = 0; + float y = 0; + if (!mDragResult && (ws.mSession.mPid == mPid)) { + // Report unconsumed drop location back to the app that started the drag. + x = mCurrentX; + y = mCurrentY; + } + DragEvent evt = DragEvent.obtain(DragEvent.ACTION_DRAG_ENDED, + x, y, null, null, null, null, mDragResult); + try { + ws.mClient.dispatchDragEvent(evt); + } catch (RemoteException e) { + Slog.w(TAG_WM, "Unable to drag-end window " + ws); + } + // if the current window is in the same process, + // the dispatch has already recycled the event + if (myPid != ws.mSession.mPid) { + evt.recycle(); + } + } + mNotifiedWindows.clear(); + mDragInProgress = false; + } + + // Take the cursor back if it has been changed. + if (isFromSource(InputDevice.SOURCE_MOUSE)) { + mService.restorePointerIconLocked(mDisplayContent, mCurrentX, mCurrentY); + mTouchSource = 0; + } + + // Clear the internal variables. + if (mSurfaceControl != null) { + mSurfaceControl.destroy(); + mSurfaceControl = null; + } + if (mAnimator != null && !mAnimationCompleted) { Slog.wtf(TAG_WM, "Unexpectedly destroying mSurfaceControl while animation is running"); } - if (mSurfaceControl != null) { - mSurfaceControl.destroy(); - } - mSurfaceControl = null; mFlags = 0; mLocalWin = null; mToken = null; mData = null; mThumbOffsetX = mThumbOffsetY = 0; mNotifiedWindows = null; + + // Notifies the controller that the drag state is closed. + mDragDropController.onDragStateClosedLocked(this); } class InputInterceptor { @@ -235,19 +298,6 @@ class DragState { } } - void unregister() { - if (DEBUG_DRAG) Slog.d(TAG_WM, "unregistering drag input channel"); - if (mInputInterceptor == null) { - Slog.e(TAG_WM, "Unregister of nonexistent drag input channel"); - } else { - // Input channel should be disposed on the thread where the input is being handled. - mDragDropController.sendHandlerMessage( - MSG_TEAR_DOWN_DRAG_AND_DROP_INPUT, mInputInterceptor); - mInputInterceptor = null; - mService.mInputMonitor.updateInputWindowsLw(true /*force*/); - } - } - int getDragLayerLocked() { return mService.mPolicy.getWindowLayerFromTypeLw(WindowManager.LayoutParams.TYPE_DRAG) * WindowManagerService.TYPE_LAYER_MULTIPLIER @@ -366,46 +416,15 @@ class DragState { return false; } - private void broadcastDragEndedLocked() { - final int myPid = Process.myPid(); - - if (DEBUG_DRAG) { - Slog.d(TAG_WM, "broadcasting DRAG_ENDED"); - } - for (WindowState ws : mNotifiedWindows) { - float x = 0; - float y = 0; - if (!mDragResult && (ws.mSession.mPid == mPid)) { - // Report unconsumed drop location back to the app that started the drag. - x = mCurrentX; - y = mCurrentY; - } - DragEvent evt = DragEvent.obtain(DragEvent.ACTION_DRAG_ENDED, - x, y, null, null, null, null, mDragResult); - try { - ws.mClient.dispatchDragEvent(evt); - } catch (RemoteException e) { - Slog.w(TAG_WM, "Unable to drag-end window " + ws); - } - // if the current window is in the same process, - // the dispatch has already recycled the event - if (myPid != ws.mSession.mPid) { - evt.recycle(); - } - } - mNotifiedWindows.clear(); - mDragInProgress = false; - } - void endDragLocked() { if (mAnimator != null) { return; } if (!mDragResult) { mAnimator = createReturnAnimationLocked(); - return; // Will call cleanUpDragLw when the animation is done. + return; // Will call closeLocked() when the animation is done. } - cleanUpDragLocked(); + closeLocked(); } void cancelDragLocked() { @@ -414,33 +433,15 @@ class DragState { } if (!mDragInProgress) { // This can happen if an app invokes Session#cancelDragAndDrop before - // Session#performDrag. Reset the drag state: - // 1. without sending the end broadcast because the start broadcast has not been sent, - // and - // 2. without playing the cancel animation because H.DRAG_START_TIMEOUT may be sent to - // WindowManagerService, which will cause DragState#reset() while playing the - // cancel animation. - reset(); - mDragDropController.mDragState = null; + // Session#performDrag. Reset the drag state without playing the cancel animation + // because H.DRAG_START_TIMEOUT may be sent to WindowManagerService, which will cause + // DragState#reset() while playing the cancel animation. + closeLocked(); return; } mAnimator = createCancelAnimationLocked(); } - private void cleanUpDragLocked() { - broadcastDragEndedLocked(); - if (isFromSource(InputDevice.SOURCE_MOUSE)) { - mService.restorePointerIconLocked(mDisplayContent, mCurrentX, mCurrentY); - } - - // stop intercepting input - unregister(); - - // free our resources and drop all the object references - reset(); - mDragDropController.mDragState = null; - } - void notifyMoveLocked(float x, float y) { if (mAnimator != null) { return; @@ -567,15 +568,6 @@ class DragState { return false; } - void onAnimationEndLocked() { - if (mAnimator == null) { - Slog.wtf(TAG_WM, "Unexpected null mAnimator"); - return; - } - mAnimator = null; - cleanUpDragLocked(); - } - private static DragEvent obtainDragEvent(WindowState win, int action, float x, float y, Object localState, ClipDescription description, ClipData data, @@ -677,6 +669,7 @@ class DragState { @Override public void onAnimationEnd(Animator animator) { + mAnimationCompleted = true; // Updating mDragState requires the WM lock so continues it on the out of // AnimationThread. mDragDropController.sendHandlerMessage(MSG_ANIMATION_END, null); From 768012e4c18dd8650bf1b067f7e0b37a3c5db462 Mon Sep 17 00:00:00 2001 From: Daichi Hirono Date: Mon, 30 Oct 2017 10:05:37 +0900 Subject: [PATCH 2/2] Make DragDropController#mDragState private Before introducing the drag and drop callback, we need a write lock to preserve the order of drag state update. The CL changes the visibility of mDragState to private so that we can ensure the upcoming write lock guards all updates for mDragState. Bug: 65564090 Test: android.server.wm.CrossAppDragAndDropTests, manually check the drag and drop behavior on test app. Change-Id: I3afc06221e4a9688b417768365e87b19706f9372 --- .../android/server/wm/DragDropController.java | 35 ++++- .../server/wm/DragInputEventReceiver.java | 134 +++++++----------- .../java/com/android/server/wm/DragState.java | 34 ++--- .../com/android/server/wm/InputMonitor.java | 4 +- .../server/wm/WindowManagerService.java | 2 +- 5 files changed, 102 insertions(+), 107 deletions(-) diff --git a/services/core/java/com/android/server/wm/DragDropController.java b/services/core/java/com/android/server/wm/DragDropController.java index b6f0f6a134543..70908337dfd38 100644 --- a/services/core/java/com/android/server/wm/DragDropController.java +++ b/services/core/java/com/android/server/wm/DragDropController.java @@ -35,6 +35,7 @@ import android.view.Surface; import android.view.SurfaceControl; import android.view.SurfaceSession; import android.view.View; +import com.android.server.input.InputWindowHandle; /** * Managing drag and drop operations initiated by View#startDragAndDrop. @@ -54,7 +55,7 @@ class DragDropController { * The variable is cleared by {@code #onDragStateClosedLocked} which is invoked by DragState * itself, thus the variable can be null after calling DragState's methods. */ - DragState mDragState; + private DragState mDragState; private WindowManagerService mService; private final Handler mHandler; @@ -63,11 +64,19 @@ class DragDropController { return mDragState != null; } + InputWindowHandle getInputWindowHandleLocked() { + return mDragState.getInputWindowHandle(); + } + DragDropController(WindowManagerService service, Looper looper) { mService = service; mHandler = new DragHandler(service, looper); } + void sendDragStartedIfNeededLocked(WindowState window) { + mDragState.sendDragStartedIfNeededLocked(window); + } + IBinder prepareDrag(SurfaceSession session, int callerPid, int callerUid, IWindow window, int flags, int width, int height, Surface outSurface) { if (DEBUG_DRAG) { @@ -257,6 +266,30 @@ class DragDropController { } } + /** + * Handles motion events. + * @param keepHandling Whether if the drag operation is continuing or this is the last motion + * event. + * @param newX X coordinate value in dp in the screen coordinate + * @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; + } + + if (keepHandling) { + mDragState.notifyMoveLocked(newX, newY); + } else { + mDragState.notifyDropLocked(newX, newY); + } + } + } + void dragRecipientEntered(IWindow window) { if (DEBUG_DRAG) { Slog.d(TAG_WM, "Drag into new candidate view @ " + window.asBinder()); diff --git a/services/core/java/com/android/server/wm/DragInputEventReceiver.java b/services/core/java/com/android/server/wm/DragInputEventReceiver.java index b4bbc9055631d..bee2bacf83d2e 100644 --- a/services/core/java/com/android/server/wm/DragInputEventReceiver.java +++ b/services/core/java/com/android/server/wm/DragInputEventReceiver.java @@ -37,7 +37,6 @@ import android.view.MotionEvent; * Input receiver for drag and drop */ class DragInputEventReceiver extends InputEventReceiver { - private final WindowManagerService mService; private final DragDropController mDragDropController; // Set, if stylus button was down at the start of the drag. @@ -48,100 +47,63 @@ class DragInputEventReceiver extends InputEventReceiver { // are still being dispatched. private boolean mMuteInput = false; - public DragInputEventReceiver(InputChannel inputChannel, Looper looper, - DragDropController controller, WindowManagerService service) { + DragInputEventReceiver(InputChannel inputChannel, Looper looper, + DragDropController controller) { super(inputChannel, looper); mDragDropController = controller; - mService = service; } @Override public void onInputEvent(InputEvent event, int displayId) { boolean handled = false; try { - synchronized (mService.mWindowMap) { - if (!mDragDropController.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. - handled = true; - return; - } - if (!(event instanceof MotionEvent) - || (event.getSource() & SOURCE_CLASS_POINTER) == 0 - || mMuteInput) { - return; - } - final MotionEvent motionEvent = (MotionEvent) event; - boolean endDrag = false; - final float newX = motionEvent.getRawX(); - final float newY = motionEvent.getRawY(); - final boolean isStylusButtonDown = - (motionEvent.getButtonState() & BUTTON_STYLUS_PRIMARY) != 0; - - if (mIsStartEvent) { - if (isStylusButtonDown) { - // First event and the button was down, check for the button being - // lifted in the future, if that happens we'll drop the item. - mStylusButtonDownAtStart = true; - } - mIsStartEvent = false; - } - - switch (motionEvent.getAction()) { - case ACTION_DOWN: { - if (DEBUG_DRAG) Slog.w(TAG_WM, "Unexpected ACTION_DOWN in drag layer"); - } - break; - - case ACTION_MOVE: { - if (mStylusButtonDownAtStart && !isStylusButtonDown) { - if (DEBUG_DRAG) { - Slog.d(TAG_WM, "Button no longer pressed; dropping at " - + newX + "," + newY); - } - mMuteInput = true; - endDrag = mDragDropController.mDragState - .notifyDropLocked(newX, newY); - } else { - // move the surface and tell the involved window(s) where we are - mDragDropController.mDragState.notifyMoveLocked(newX, newY); - } - } - break; - - case ACTION_UP: { - if (DEBUG_DRAG) { - Slog.d(TAG_WM, "Got UP on move channel; dropping at " - + newX + "," + newY); - } - mMuteInput = true; - endDrag = mDragDropController.mDragState - .notifyDropLocked(newX, newY); - } - break; - - case ACTION_CANCEL: { - if (DEBUG_DRAG) Slog.d(TAG_WM, "Drag cancelled!"); - mMuteInput = true; - endDrag = true; - } - break; - } - - if (endDrag) { - if (DEBUG_DRAG) - Slog.d(TAG_WM, "Drag ended; tearing down state"); - // tell all the windows that the drag has ended - // endDragLocked will post back to looper to dispose the receiver - // since we still need the receiver for the last finishInputEvent. - mDragDropController.mDragState.endDragLocked(); - mStylusButtonDownAtStart = false; - mIsStartEvent = true; - } - - handled = true; + if (!(event instanceof MotionEvent) + || (event.getSource() & SOURCE_CLASS_POINTER) == 0 + || mMuteInput) { + return; } + final MotionEvent motionEvent = (MotionEvent) event; + final float newX = motionEvent.getRawX(); + final float newY = motionEvent.getRawY(); + final boolean isStylusButtonDown = + (motionEvent.getButtonState() & BUTTON_STYLUS_PRIMARY) != 0; + + if (mIsStartEvent) { + // First event and the button was down, check for the button being + // lifted in the future, if that happens we'll drop the item. + mStylusButtonDownAtStart = isStylusButtonDown; + mIsStartEvent = false; + } + + switch (motionEvent.getAction()) { + case ACTION_DOWN: + if (DEBUG_DRAG) Slog.w(TAG_WM, "Unexpected ACTION_DOWN in drag layer"); + return; + case ACTION_MOVE: + if (mStylusButtonDownAtStart && !isStylusButtonDown) { + if (DEBUG_DRAG) { + Slog.d(TAG_WM, "Button no longer pressed; dropping at " + newX + "," + + newY); + } + mMuteInput = true; + } + break; + case ACTION_UP: + if (DEBUG_DRAG) { + Slog.d(TAG_WM, "Got UP on move channel; dropping at " + newX + "," + newY); + } + mMuteInput = true; + break; + case ACTION_CANCEL: + if (DEBUG_DRAG) Slog.d(TAG_WM, "Drag cancelled!"); + mMuteInput = true; + break; + default: + return; + } + + mDragDropController.handleMotionEvent(!mMuteInput /* keepHandling */, newX, newY); + handled = true; } catch (Exception e) { Slog.e(TAG_WM, "Exception caught by drag handleMotion", e); } finally { diff --git a/services/core/java/com/android/server/wm/DragState.java b/services/core/java/com/android/server/wm/DragState.java index da0394b6e294d..0727107cd4528 100644 --- a/services/core/java/com/android/server/wm/DragState.java +++ b/services/core/java/com/android/server/wm/DragState.java @@ -214,7 +214,7 @@ class DragState { mClientChannel = channels[1]; mService.mInputManager.registerInputChannel(mServerChannel, null); mInputEventReceiver = new DragInputEventReceiver(mClientChannel, - mService.mH.getLooper(), mDragDropController, mService); + mService.mH.getLooper(), mDragDropController); mDragApplicationHandle = new InputApplicationHandle(null); mDragApplicationHandle.name = "drag"; @@ -508,34 +508,33 @@ class DragState { mTargetWindow = touchedWin; } - // Find the drop target and tell it about the data. Returns 'true' if we can immediately - // dispatch the global drag-ended message, 'false' if we need to wait for a - // result from the recipient. - boolean notifyDropLocked(float x, float y) { + /** + * Finds the drop target and tells it about the data. If the drop event is not sent to the + * target, invokes {@code endDragLocked} immediately. + */ + void notifyDropLocked(float x, float y) { if (mAnimator != null) { - return false; + return; } mCurrentX = x; mCurrentY = y; - WindowState touchedWin = mDisplayContent.getTouchableWinAtPointLocked(x, y); + final WindowState touchedWin = mDisplayContent.getTouchableWinAtPointLocked(x, y); if (!isWindowNotified(touchedWin)) { // "drop" outside a valid window -- no recipient to apply a // timeout to, and we can send the drag-ended message immediately. mDragResult = false; - return true; + endDragLocked(); + return; } - if (DEBUG_DRAG) { - Slog.d(TAG_WM, "sending DROP to " + touchedWin); - } + if (DEBUG_DRAG) Slog.d(TAG_WM, "sending DROP to " + touchedWin); final int targetUserId = UserHandle.getUserId(touchedWin.getOwningUid()); - DragAndDropPermissionsHandler dragAndDropPermissions = null; - if ((mFlags & View.DRAG_FLAG_GLOBAL) != 0 && - (mFlags & DRAG_FLAGS_URI_ACCESS) != 0) { + final DragAndDropPermissionsHandler dragAndDropPermissions; + if ((mFlags & View.DRAG_FLAG_GLOBAL) != 0 && (mFlags & DRAG_FLAGS_URI_ACCESS) != 0) { dragAndDropPermissions = new DragAndDropPermissionsHandler( mData, mUid, @@ -543,13 +542,15 @@ class DragState { mFlags & DRAG_FLAGS_URI_PERMISSIONS, mSourceUserId, targetUserId); + } else { + dragAndDropPermissions = null; } if (mSourceUserId != targetUserId){ mData.fixUris(mSourceUserId); } final int myPid = Process.myPid(); final IBinder token = touchedWin.mClient.asBinder(); - DragEvent evt = obtainDragEvent(touchedWin, DragEvent.ACTION_DROP, x, y, + final DragEvent evt = obtainDragEvent(touchedWin, DragEvent.ACTION_DROP, x, y, null, null, mData, dragAndDropPermissions, false); try { touchedWin.mClient.dispatchDragEvent(evt); @@ -558,14 +559,13 @@ class DragState { mDragDropController.sendTimeoutMessage(MSG_DRAG_END_TIMEOUT, token); } catch (RemoteException e) { Slog.w(TAG_WM, "can't send drop notification to win " + touchedWin); - return true; + endDragLocked(); } finally { if (myPid != touchedWin.mSession.mPid) { evt.recycle(); } } mToken = token; - return false; } private static DragEvent obtainDragEvent(WindowState win, int action, diff --git a/services/core/java/com/android/server/wm/InputMonitor.java b/services/core/java/com/android/server/wm/InputMonitor.java index d23369efd2971..a4080cacf4305 100644 --- a/services/core/java/com/android/server/wm/InputMonitor.java +++ b/services/core/java/com/android/server/wm/InputMonitor.java @@ -373,7 +373,7 @@ final class InputMonitor implements InputManagerService.WindowManagerCallbacks { Log.d(TAG_WM, "Inserting drag window"); } final InputWindowHandle dragWindowHandle = - mService.mDragDropController.mDragState.getInputWindowHandle(); + mService.mDragDropController.getInputWindowHandleLocked(); if (dragWindowHandle != null) { addInputWindowHandle(dragWindowHandle); } else { @@ -690,7 +690,7 @@ final class InputMonitor implements InputManagerService.WindowManagerCallbacks { // If there's a drag in progress and 'child' is a potential drop target, // make sure it's been told about the drag if (inDrag && isVisible && w.getDisplayContent().isDefaultDisplay) { - mService.mDragDropController.mDragState.sendDragStartedIfNeededLocked(w); + mService.mDragDropController.sendDragStartedIfNeededLocked(w); } addInputWindowHandle( diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java index c982d1576d466..ac2977c58f794 100644 --- a/services/core/java/com/android/server/wm/WindowManagerService.java +++ b/services/core/java/com/android/server/wm/WindowManagerService.java @@ -7054,7 +7054,7 @@ public class WindowManagerService extends IWindowManager.Stub } synchronized (mWindowMap) { - if (mDragDropController.mDragState != null) { + if (mDragDropController.dragDropActiveLocked()) { // Drag cursor overrides the app cursor. return; }