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
This commit is contained in:
Nikita Dubrovsky
2021-02-17 17:21:34 -08:00
parent 3ca8fa18f9
commit 910746fe96
3 changed files with 79 additions and 18 deletions

View File

@@ -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 @@ import com.android.internal.view.IDragAndDropPermissions;
*/
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 @@ public final class DragAndDropPermissions implements Parcelable {
}
/**
* 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 final class DragAndDropPermissions implements Parcelable {
public void release() {
try {
mDragAndDropPermissions.release();
mTransientToken = null;
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
}
}
@@ -142,11 +185,9 @@ public final class DragAndDropPermissions implements Parcelable {
@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();
}
}

View File

@@ -2898,9 +2898,6 @@ public class Editor {
} finally {
mTextView.endBatchEdit();
mUndoInputFilter.freezeLastEdit();
if (permissions != null) {
permissions.release();
}
}
}

View File

@@ -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 @@ import java.util.ArrayList;
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 @@ class DragAndDropPermissionsHandler extends IDragAndDropPermissions.Stub
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 @@ class DragAndDropPermissionsHandler extends IDragAndDropPermissions.Stub
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 @@ class DragAndDropPermissionsHandler extends IDragAndDropPermissions.Stub
}
@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 @@ class DragAndDropPermissionsHandler extends IDragAndDropPermissions.Stub
} 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 @@ class DragAndDropPermissionsHandler extends IDragAndDropPermissions.Stub
@Override
public void binderDied() {
if (DEBUG) {
Log.d(TAG, this + ": app process died: " + toHexString(mAppToken.hashCode()));
}
try {
release();
} catch (RemoteException e) {