From 3bae0b027feb873cbb430ee1c3e0408e565b69f2 Mon Sep 17 00:00:00 2001 From: Daichi Hirono Date: Fri, 27 Oct 2017 13:05:13 +0900 Subject: [PATCH] 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);