Update logic for drag-and-drop URI permissions for editable TextViews
Previously, Editor.onDrop used a try/finally block to request and
release URI permissions. This doesn't work for the case when an app
sets an OnReceiveContentListener (or subclasses TextView and overrides
the onReceiveContent() method). If using a background thread to
process the content in the custom impl for receiving content,
permissions will be revoked in the finally block before the app has
had a chance to process the URI.
With this change, when Editor requests permissions, they will be bound
to the app process and released automatically when the app process
terminates.
Bug: 181178998
Test: atest CtsWindowManagerDeviceTestCases:CrossAppDragAndDropTests
Test: Manually verified using development/samples/ReceiveContentDemo
Change-Id: I7ae38069fb741925211b42c7ff28784eb9722ce3
diff --git a/core/java/android/view/DragAndDropPermissions.java b/core/java/android/view/DragAndDropPermissions.java
index d47604d..16204d8 100644
--- a/core/java/android/view/DragAndDropPermissions.java
+++ b/core/java/android/view/DragAndDropPermissions.java
@@ -16,12 +16,15 @@
package android.view;
+import static java.lang.Integer.toHexString;
+
import android.app.Activity;
import android.os.Binder;
import android.os.IBinder;
import android.os.Parcel;
import android.os.Parcelable;
import android.os.RemoteException;
+import android.util.Log;
import com.android.internal.view.IDragAndDropPermissions;
@@ -56,9 +59,28 @@
*/
public final class DragAndDropPermissions implements Parcelable {
- private final IDragAndDropPermissions mDragAndDropPermissions;
+ private static final String TAG = "DragAndDrop";
+ private static final boolean DEBUG = false;
- private IBinder mTransientToken;
+ /**
+ * Permissions for a drop can be granted in one of two ways:
+ * <ol>
+ * <li>An app can explicitly request permissions using
+ * {@link Activity#requestDragAndDropPermissions(DragEvent)}. In this case permissions are
+ * revoked automatically when then activity is destroyed. See {@link #take(IBinder)}.
+ * <li>The platform can request permissions on behalf of the app (e.g. in
+ * {@link android.widget.Editor}). In this case permissions are revoked automatically when
+ * the app process terminates. See {@link #takeTransient()}.
+ * </ol>
+ *
+ * <p>In order to implement the second case above, we create a static token object here. This
+ * ensures that the token stays alive for the lifetime of the app process, allowing us to
+ * revoke permissions when the app process terminates using {@link IBinder#linkToDeath} in
+ * {@code DragAndDropPermissionsHandler}.
+ */
+ private static IBinder sAppToken;
+
+ private final IDragAndDropPermissions mDragAndDropPermissions;
/**
* Create a new {@link DragAndDropPermissions} object to control the access permissions for
@@ -81,30 +103,51 @@
}
/**
- * Take the permissions and bind their lifetime to the activity.
+ * Take permissions, binding their lifetime to the activity.
+ *
+ * <p>Note: This API is exposed to apps via
+ * {@link Activity#requestDragAndDropPermissions(DragEvent)}.
+ *
* @param activityToken Binder pointing to an Activity instance to bind the lifetime to.
* @return True if permissions are successfully taken.
+ *
* @hide
*/
public boolean take(IBinder activityToken) {
try {
+ if (DEBUG) {
+ Log.d(TAG, this + ": calling take() with activity-bound token: "
+ + toHexString(activityToken.hashCode()));
+ }
mDragAndDropPermissions.take(activityToken);
} catch (RemoteException e) {
+ Log.w(TAG, this + ": take() failed with a RemoteException", e);
return false;
}
return true;
}
/**
- * Take the permissions. Must call {@link #release} explicitly.
+ * Take permissions transiently. Permissions will be revoked when the app process terminates.
+ *
+ * <p>Note: This API is not exposed to apps.
+ *
* @return True if permissions are successfully taken.
+ *
* @hide
*/
public boolean takeTransient() {
try {
- mTransientToken = new Binder();
- mDragAndDropPermissions.takeTransient(mTransientToken);
+ if (sAppToken == null) {
+ sAppToken = new Binder();
+ }
+ if (DEBUG) {
+ Log.d(TAG, this + ": calling takeTransient() with process-bound token: "
+ + toHexString(sAppToken.hashCode()));
+ }
+ mDragAndDropPermissions.takeTransient(sAppToken);
} catch (RemoteException e) {
+ Log.w(TAG, this + ": takeTransient() failed with a RemoteException", e);
return false;
}
return true;
@@ -116,8 +159,8 @@
public void release() {
try {
mDragAndDropPermissions.release();
- mTransientToken = null;
} catch (RemoteException e) {
+ throw e.rethrowFromSystemServer();
}
}
@@ -142,11 +185,9 @@
@Override
public void writeToParcel(Parcel destination, int flags) {
destination.writeStrongInterface(mDragAndDropPermissions);
- destination.writeStrongBinder(mTransientToken);
}
private DragAndDropPermissions(Parcel in) {
mDragAndDropPermissions = IDragAndDropPermissions.Stub.asInterface(in.readStrongBinder());
- mTransientToken = in.readStrongBinder();
}
}
diff --git a/core/java/android/widget/Editor.java b/core/java/android/widget/Editor.java
index 012352d..bcbfe37 100644
--- a/core/java/android/widget/Editor.java
+++ b/core/java/android/widget/Editor.java
@@ -2898,9 +2898,6 @@
} finally {
mTextView.endBatchEdit();
mUndoInputFilter.freezeLastEdit();
- if (permissions != null) {
- permissions.release();
- }
}
}
diff --git a/services/core/java/com/android/server/wm/DragAndDropPermissionsHandler.java b/services/core/java/com/android/server/wm/DragAndDropPermissionsHandler.java
index 999c585..62e4a85 100644
--- a/services/core/java/com/android/server/wm/DragAndDropPermissionsHandler.java
+++ b/services/core/java/com/android/server/wm/DragAndDropPermissionsHandler.java
@@ -16,12 +16,15 @@
package com.android.server.wm;
+import static java.lang.Integer.toHexString;
+
import android.app.UriGrantsManager;
import android.content.ClipData;
import android.net.Uri;
import android.os.Binder;
import android.os.IBinder;
import android.os.RemoteException;
+import android.util.Log;
import com.android.internal.view.IDragAndDropPermissions;
import com.android.server.LocalServices;
@@ -32,6 +35,9 @@
class DragAndDropPermissionsHandler extends IDragAndDropPermissions.Stub
implements IBinder.DeathRecipient {
+ private static final String TAG = "DragAndDrop";
+ private static final boolean DEBUG = false;
+
private final WindowManagerGlobalLock mGlobalLock;
private final int mSourceUid;
private final String mTargetPackage;
@@ -43,7 +49,7 @@
private IBinder mActivityToken = null;
private IBinder mPermissionOwnerToken = null;
- private IBinder mTransientToken = null;
+ private IBinder mAppToken = null;
DragAndDropPermissionsHandler(WindowManagerGlobalLock lock, ClipData clipData, int sourceUid,
String targetPackage, int mode, int sourceUserId, int targetUserId) {
@@ -62,6 +68,10 @@
if (mActivityToken != null || mPermissionOwnerToken != null) {
return;
}
+ if (DEBUG) {
+ Log.d(TAG, this + ": taking permissions bound to activity: "
+ + toHexString(activityToken.hashCode()));
+ }
mActivityToken = activityToken;
// Will throw if Activity is not found.
@@ -84,14 +94,18 @@
}
@Override
- public void takeTransient(IBinder transientToken) throws RemoteException {
+ public void takeTransient(IBinder appToken) throws RemoteException {
if (mActivityToken != null || mPermissionOwnerToken != null) {
return;
}
+ if (DEBUG) {
+ Log.d(TAG, this + ": taking permissions bound to app process: "
+ + toHexString(appToken.hashCode()));
+ }
mPermissionOwnerToken = LocalServices.getService(UriGrantsManagerInternal.class)
.newUriPermissionOwner("drop");
- mTransientToken = transientToken;
- mTransientToken.linkToDeath(this, 0);
+ mAppToken = appToken;
+ mAppToken.linkToDeath(this, 0);
doTake(mPermissionOwnerToken);
}
@@ -112,11 +126,17 @@
} finally {
mActivityToken = null;
}
+ if (DEBUG) {
+ Log.d(TAG, this + ": releasing activity-bound permissions");
+ }
} else {
permissionOwner = mPermissionOwnerToken;
mPermissionOwnerToken = null;
- mTransientToken.unlinkToDeath(this, 0);
- mTransientToken = null;
+ mAppToken.unlinkToDeath(this, 0);
+ mAppToken = null;
+ if (DEBUG) {
+ Log.d(TAG, this + ": releasing process-bound permissions");
+ }
}
UriGrantsManagerInternal ugm = LocalServices.getService(UriGrantsManagerInternal.class);
@@ -139,6 +159,9 @@
@Override
public void binderDied() {
+ if (DEBUG) {
+ Log.d(TAG, this + ": app process died: " + toHexString(mAppToken.hashCode()));
+ }
try {
release();
} catch (RemoteException e) {