From da0748d00c645225317ed2656b8b3b0561aaaf4c Mon Sep 17 00:00:00 2001 From: Daichi Hirono Date: Wed, 13 Dec 2017 12:48:59 +0900 Subject: [PATCH] Fix NPE occured when null ClipData is passed to startDrag. Previously we have NPE for the following cases: * when specifying both DRAG_FLAG_GLOBAL and DRAG_FLAGS_URI_ACCESS, OR * when the userIds of source and target apps are different Bug: 70477060 Test: com.android.server.wm.DragDropControllerTests, android.server.wm.CrossAppDragAndDropTests, manually check the drag and drop behavior on test app. Change-Id: I454b434e4466cb159ce23e0982012cdb21c9d002 --- .../android/server/wm/DragDropController.java | 2 +- .../java/com/android/server/wm/DragState.java | 13 +- .../server/wm/DragDropControllerTests.java | 130 +++++++++++++++--- .../android/server/wm/WindowTestsBase.java | 10 +- 4 files changed, 128 insertions(+), 27 deletions(-) diff --git a/services/core/java/com/android/server/wm/DragDropController.java b/services/core/java/com/android/server/wm/DragDropController.java index 28b4c1dbeb8ef..0171b56ffc47e 100644 --- a/services/core/java/com/android/server/wm/DragDropController.java +++ b/services/core/java/com/android/server/wm/DragDropController.java @@ -136,7 +136,7 @@ class DragDropController { outSurface.copyFrom(surface); final IBinder winBinder = window.asBinder(); IBinder token = new Binder(); - mDragState = new DragState(mService, token, surface, flags, winBinder); + mDragState = new DragState(mService, this, token, surface, flags, winBinder); mDragState.mPid = callerPid; mDragState.mUid = callerUid; mDragState.mOriginalAlpha = alpha; diff --git a/services/core/java/com/android/server/wm/DragState.java b/services/core/java/com/android/server/wm/DragState.java index f0fffb95d03c6..1ac9b88749a78 100644 --- a/services/core/java/com/android/server/wm/DragState.java +++ b/services/core/java/com/android/server/wm/DragState.java @@ -118,10 +118,10 @@ class DragState { private final Interpolator mCubicEaseOutInterpolator = new DecelerateInterpolator(1.5f); private Point mDisplaySize = new Point(); - DragState(WindowManagerService service, IBinder token, SurfaceControl surface, - int flags, IBinder localWin) { + DragState(WindowManagerService service, DragDropController controller, IBinder token, + SurfaceControl surface, int flags, IBinder localWin) { mService = service; - mDragDropController = service.mDragDropController; + mDragDropController = controller; mToken = token; mSurfaceControl = surface; mFlags = flags; @@ -530,7 +530,8 @@ class DragState { final int targetUserId = UserHandle.getUserId(touchedWin.getOwningUid()); final DragAndDropPermissionsHandler dragAndDropPermissions; - if ((mFlags & View.DRAG_FLAG_GLOBAL) != 0 && (mFlags & DRAG_FLAGS_URI_ACCESS) != 0) { + if ((mFlags & View.DRAG_FLAG_GLOBAL) != 0 && (mFlags & DRAG_FLAGS_URI_ACCESS) != 0 + && mData != null) { dragAndDropPermissions = new DragAndDropPermissionsHandler( mData, mUid, @@ -542,7 +543,9 @@ class DragState { dragAndDropPermissions = null; } if (mSourceUserId != targetUserId){ - mData.fixUris(mSourceUserId); + if (mData != null) { + mData.fixUris(mSourceUserId); + } } final int myPid = Process.myPid(); final IBinder token = touchedWin.mClient.asBinder(); diff --git a/services/tests/servicestests/src/com/android/server/wm/DragDropControllerTests.java b/services/tests/servicestests/src/com/android/server/wm/DragDropControllerTests.java index ce76a223ef203..ac291632c8772 100644 --- a/services/tests/servicestests/src/com/android/server/wm/DragDropControllerTests.java +++ b/services/tests/servicestests/src/com/android/server/wm/DragDropControllerTests.java @@ -16,27 +16,38 @@ package com.android.server.wm; +import static android.app.WindowConfiguration.ACTIVITY_TYPE_STANDARD; +import static android.app.WindowConfiguration.WINDOWING_MODE_FULLSCREEN; import static android.view.WindowManager.LayoutParams.TYPE_BASE_APPLICATION; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; -import static org.mockito.Matchers.anyInt; import static org.mockito.Mockito.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; +import android.content.ClipData; import android.os.IBinder; +import android.os.Looper; +import android.os.UserHandle; +import android.os.UserManagerInternal; import android.platform.test.annotations.Presubmit; import android.support.test.filters.SmallTest; import android.support.test.runner.AndroidJUnit4; import android.view.InputChannel; import android.view.Surface; import android.view.SurfaceSession; +import android.view.View; +import com.android.internal.annotations.GuardedBy; +import com.android.server.LocalServices; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; + /** * Tests for the {@link DragDropController} class. * @@ -46,36 +57,92 @@ import org.junit.runner.RunWith; @RunWith(AndroidJUnit4.class) @Presubmit public class DragDropControllerTests extends WindowTestsBase { - private static final int TIMEOUT_MS = 1000; - private DragDropController mTarget; + private static final int TIMEOUT_MS = 3000; + private TestDragDropController mTarget; private WindowState mWindow; private IBinder mToken; + static class TestDragDropController extends DragDropController { + @GuardedBy("sWm.mWindowMap") + private Runnable mCloseCallback; + + TestDragDropController(WindowManagerService service, Looper looper) { + super(service, looper); + } + + void setOnClosedCallbackLocked(Runnable runnable) { + assertTrue(dragDropActiveLocked()); + mCloseCallback = runnable; + } + + @Override + void onDragStateClosedLocked(DragState dragState) { + super.onDragStateClosedLocked(dragState); + if (mCloseCallback != null) { + mCloseCallback.run(); + mCloseCallback = null; + } + } + } + + /** + * Creates a window state which can be used as a drop target. + */ + private WindowState createDropTargetWindow(String name, int ownerId) { + final WindowTestUtils.TestAppWindowToken token = new WindowTestUtils.TestAppWindowToken( + mDisplayContent); + final TaskStack stack = createStackControllerOnStackOnDisplay( + WINDOWING_MODE_FULLSCREEN, ACTIVITY_TYPE_STANDARD, mDisplayContent).mContainer; + final Task task = createTaskInStack(stack, ownerId); + task.addChild(token, 0); + + final WindowState window = createWindow( + null, TYPE_BASE_APPLICATION, token, name, ownerId, false); + window.mInputChannel = new InputChannel(); + window.mHasSurface = true; + return window; + } + @Before public void setUp() throws Exception { + final UserManagerInternal userManager = mock(UserManagerInternal.class); + LocalServices.addService(UserManagerInternal.class, userManager); + super.setUp(); - assertNotNull(sWm.mDragDropController); - mTarget = sWm.mDragDropController; - mWindow = createWindow(null, TYPE_BASE_APPLICATION, "window"); + + mTarget = new TestDragDropController(sWm, sWm.mH.getLooper()); + mDisplayContent = spy(mDisplayContent); + mWindow = createDropTargetWindow("Drag test window", 0); + when(mDisplayContent.getTouchableWinAtPointLocked(0, 0)).thenReturn(mWindow); + when(sWm.mInputManager.transferTouchFocus(any(), any())).thenReturn(true); + synchronized (sWm.mWindowMap) { - // Because sWm is a static object, the previous operation may remain. - assertFalse(mTarget.dragDropActiveLocked()); + sWm.mWindowMap.put(mWindow.mClient.asBinder(), mWindow); } } @After - public void tearDown() { - if (mToken != null) { - mTarget.cancelDragAndDrop(mToken); + public void tearDown() throws Exception { + LocalServices.removeServiceForTest(UserManagerInternal.class); + final CountDownLatch latch; + synchronized (sWm.mWindowMap) { + if (!mTarget.dragDropActiveLocked()) { + return; + } + if (mToken != null) { + mTarget.cancelDragAndDrop(mToken); + } + latch = new CountDownLatch(1); + mTarget.setOnClosedCallbackLocked(() -> { + latch.countDown(); + }); } + assertTrue(latch.await(TIMEOUT_MS, TimeUnit.MILLISECONDS)); } @Test - public void testPrepareDrag() throws Exception { - final Surface surface = new Surface(); - mToken = mTarget.prepareDrag( - new SurfaceSession(), 0, 0, mWindow.mClient, 0, 100, 100, surface); - assertNotNull(mToken); + public void testDragFlow() throws Exception { + dragFlow(0, ClipData.newPlainText("label", "Test"), 0, 0); } @Test @@ -85,4 +152,33 @@ public class DragDropControllerTests extends WindowTestsBase { new SurfaceSession(), 0, 0, mWindow.mClient, 0, 0, 0, surface); assertNull(mToken); } + + @Test + public void testPerformDrag_NullDataWithGrantUri() throws Exception { + dragFlow(View.DRAG_FLAG_GLOBAL | View.DRAG_FLAG_GLOBAL_URI_READ, null, 0, 0); + } + + @Test + public void testPerformDrag_NullDataToOtherUser() throws Exception { + final WindowState otherUsersWindow = + createDropTargetWindow("Other user's window", 1 * UserHandle.PER_USER_RANGE); + when(mDisplayContent.getTouchableWinAtPointLocked(10, 10)) + .thenReturn(otherUsersWindow); + + dragFlow(0, null, 10, 10); + } + + private void dragFlow(int flag, ClipData data, float dropX, float dropY) { + final Surface surface = new Surface(); + mToken = mTarget.prepareDrag( + new SurfaceSession(), 0, 0, mWindow.mClient, flag, 100, 100, surface); + assertNotNull(mToken); + + assertTrue(sWm.mInputManager.transferTouchFocus(null, null)); + assertTrue(mTarget.performDrag( + mWindow.mClient, mToken, 0, 0, 0, 0, 0, data)); + + mTarget.handleMotionEvent(false, dropX, dropY); + mToken = mWindow.mClient.asBinder(); + } } diff --git a/services/tests/servicestests/src/com/android/server/wm/WindowTestsBase.java b/services/tests/servicestests/src/com/android/server/wm/WindowTestsBase.java index c699a94db279b..69b13787ef93a 100644 --- a/services/tests/servicestests/src/com/android/server/wm/WindowTestsBase.java +++ b/services/tests/servicestests/src/com/android/server/wm/WindowTestsBase.java @@ -230,20 +230,22 @@ class WindowTestsBase { boolean ownerCanAddInternalSystemWindow) { final WindowToken token = createWindowToken( dc, WINDOWING_MODE_FULLSCREEN, ACTIVITY_TYPE_STANDARD, type); - return createWindow(parent, type, token, name, ownerCanAddInternalSystemWindow); + return createWindow(parent, type, token, name, 0 /* ownerId */, + ownerCanAddInternalSystemWindow); } static WindowState createWindow(WindowState parent, int type, WindowToken token, String name) { - return createWindow(parent, type, token, name, false /* ownerCanAddInternalSystemWindow */); + return createWindow(parent, type, token, name, 0 /* ownerId */, + false /* ownerCanAddInternalSystemWindow */); } static WindowState createWindow(WindowState parent, int type, WindowToken token, String name, - boolean ownerCanAddInternalSystemWindow) { + int ownerId, boolean ownerCanAddInternalSystemWindow) { final WindowManager.LayoutParams attrs = new WindowManager.LayoutParams(type); attrs.setTitle(name); final WindowState w = new WindowState(sWm, sMockSession, sIWindow, token, parent, OP_NONE, - 0, attrs, VISIBLE, 0, ownerCanAddInternalSystemWindow); + 0, attrs, VISIBLE, ownerId, ownerCanAddInternalSystemWindow); // TODO: Probably better to make this call in the WindowState ctor to avoid errors with // adding it to the token... token.addWindow(w);