From c4b8f36de5523366e354fc01b6cba81ad72f6423 Mon Sep 17 00:00:00 2001 From: Yohei Yukawa Date: Thu, 30 Jun 2016 09:32:54 -0700 Subject: [PATCH] Remove InputContentInfo#requestPermission() It turns out that requiring editor authors to call InputContentInfo#requestPermission() as needed is just confusing and can cause compatibility issues, because if an editor author forgot to call that method then there would be no way for IME developers to prevent permission denial except for relaxing the default permission of the ContentProvider just because of such an application. This is not what we want to see. My conclusion is that the system should automatically call InputContentInfo#requestPermission() (or do any equivalent operation) when InputConnection#INPUT_CONTENT_GRANT_READ_URI_PERMISSION is specified, like we have done in Context#startActivity(). With this CL, the system automatically grants a temporary URI permission to the target application when the IME calls InputConnection#commitContent() with InputConnection#INPUT_CONTENT_GRANT_READ_URI_PERMISSION, and the temporary permission will be revoked by any of the following events: - InputContentInfo#releasePermission() is explicitly called by the target application. - The target application returned false in InputConnection#commitContent(). - All the InputContentInfo instances copied from the original one are GC-ed. Bug: 29450031 Bug: 29892936 Change-Id: I37fb744e4d3d1c59177fb0a9be4ef5c325c9a39f --- api/current.txt | 1 - api/system-current.txt | 1 - api/test-current.txt | 1 - .../android/view/inputmethod/InputConnection.java | 9 ++-- .../android/view/inputmethod/InputContentInfo.java | 16 ------ .../inputmethod/IInputContentUriToken.aidl | 1 - .../internal/view/InputConnectionWrapper.java | 10 +++- .../server/InputContentUriTokenHandler.java | 58 ++++++++++------------ .../android/server/InputMethodManagerService.java | 2 +- 9 files changed, 39 insertions(+), 60 deletions(-) diff --git a/api/current.txt b/api/current.txt index c6316271c636..a03041675829 100644 --- a/api/current.txt +++ b/api/current.txt @@ -44921,7 +44921,6 @@ package android.view.inputmethod { method public android.content.ClipDescription getDescription(); method public android.net.Uri getLinkUri(); method public void releasePermission(); - method public void requestPermission(); method public void writeToParcel(android.os.Parcel, int); field public static final android.os.Parcelable.Creator CREATOR; } diff --git a/api/system-current.txt b/api/system-current.txt index 3fa518a43e49..1a2097921a8c 100644 --- a/api/system-current.txt +++ b/api/system-current.txt @@ -48049,7 +48049,6 @@ package android.view.inputmethod { method public android.content.ClipDescription getDescription(); method public android.net.Uri getLinkUri(); method public void releasePermission(); - method public void requestPermission(); method public void writeToParcel(android.os.Parcel, int); field public static final android.os.Parcelable.Creator CREATOR; } diff --git a/api/test-current.txt b/api/test-current.txt index 993f388f15ab..b1e8fae80f74 100644 --- a/api/test-current.txt +++ b/api/test-current.txt @@ -45001,7 +45001,6 @@ package android.view.inputmethod { method public android.content.ClipDescription getDescription(); method public android.net.Uri getLinkUri(); method public void releasePermission(); - method public void requestPermission(); method public void writeToParcel(android.os.Parcel, int); field public static final android.os.Parcelable.Creator CREATOR; } diff --git a/core/java/android/view/inputmethod/InputConnection.java b/core/java/android/view/inputmethod/InputConnection.java index 07910b60df3f..ecfcbc5ee2fa 100644 --- a/core/java/android/view/inputmethod/InputConnection.java +++ b/core/java/android/view/inputmethod/InputConnection.java @@ -840,15 +840,16 @@ public interface InputConnection { public void closeConnection(); /** - * When this flag is used, the editor will be able to request read access to the content URI - * contained in the {@link InputContentInfo} object. + * When this flag is used in {@link #commitContent(InputContentInfo, int, Bundle)}, the editor + * will be able to request read access to the content URI contained in the + * {@link InputContentInfo} object. * *

Make sure that the content provider owning the Uri sets the * {@link android.R.styleable#AndroidManifestProvider_grantUriPermissions * grantUriPermissions} attribute in its manifest or included the * {@link android.R.styleable#AndroidManifestGrantUriPermission - * <grant-uri-permissions>} tag. Otherwise {@link InputContentInfo#requestPermission()} - * can fail.

+ * <grant-uri-permissions>} tag. Otherwise + * {@link #commitContent(InputContentInfo, int, Bundle)} can fail.

* *

Although calling this API is allowed only for the IME that is currently selected, the * client is able to request a temporary read-only access even after the current IME is switched diff --git a/core/java/android/view/inputmethod/InputContentInfo.java b/core/java/android/view/inputmethod/InputContentInfo.java index 9579bbf32835..df206432ed7a 100644 --- a/core/java/android/view/inputmethod/InputContentInfo.java +++ b/core/java/android/view/inputmethod/InputContentInfo.java @@ -163,22 +163,6 @@ public final class InputContentInfo implements Parcelable { } /** - * Requests a temporary read-only access permission for content URI associated with this object. - * - *

Does nothing if the temporary permission is already granted.

- */ - public void requestPermission() { - if (mUriToken == null) { - return; - } - try { - mUriToken.take(); - } catch (RemoteException e) { - e.rethrowFromSystemServer(); - } - } - - /** * Releases a temporary read-only access permission for content URI associated with this object. * *

Does nothing if the temporary permission is not granted.

diff --git a/core/java/com/android/internal/inputmethod/IInputContentUriToken.aidl b/core/java/com/android/internal/inputmethod/IInputContentUriToken.aidl index 8abc8074b5ac..c259348072c0 100644 --- a/core/java/com/android/internal/inputmethod/IInputContentUriToken.aidl +++ b/core/java/com/android/internal/inputmethod/IInputContentUriToken.aidl @@ -22,6 +22,5 @@ import android.os.IBinder; * {@hide} */ interface IInputContentUriToken { - void take(); void release(); } diff --git a/core/java/com/android/internal/view/InputConnectionWrapper.java b/core/java/com/android/internal/view/InputConnectionWrapper.java index 9a09dcccd1a7..15422b64b717 100644 --- a/core/java/com/android/internal/view/InputConnectionWrapper.java +++ b/core/java/com/android/internal/view/InputConnectionWrapper.java @@ -517,20 +517,22 @@ public class InputConnectionWrapper implements InputConnection { public boolean commitContent(InputContentInfo inputContentInfo, int flags, Bundle opts) { boolean result = false; + final boolean grantUriPermission = + (flags & InputConnection.INPUT_CONTENT_GRANT_READ_URI_PERMISSION) != 0; if (isMethodMissing(MissingMethodFlags.COMMIT_CONTENT)) { // This method is not implemented. return false; } try { - if ((flags & InputConnection.INPUT_CONTENT_GRANT_READ_URI_PERMISSION) != 0) { + if (grantUriPermission) { final AbstractInputMethodService inputMethodService = mInputMethodService.get(); if (inputMethodService == null) { // This basically should not happen, because it's the the caller of this method. return false; } + // Temporarily grant URI permission. inputMethodService.exposeContent(inputContentInfo, this); } - InputContextCallback callback = InputContextCallback.getInstance(); mIInputContext.commitContent(inputContentInfo, flags, opts, callback.mSeq, callback); synchronized (callback) { @@ -540,6 +542,10 @@ public class InputConnectionWrapper implements InputConnection { } } callback.dispose(); + // If this request is not handled, then there is no reason to keep the URI permission. + if (grantUriPermission && !result) { + inputContentInfo.releasePermission(); + } } catch (RemoteException e) { return false; } diff --git a/services/core/java/com/android/server/InputContentUriTokenHandler.java b/services/core/java/com/android/server/InputContentUriTokenHandler.java index 3f4972babf6e..12e1d627df1a 100644 --- a/services/core/java/com/android/server/InputContentUriTokenHandler.java +++ b/services/core/java/com/android/server/InputContentUriTokenHandler.java @@ -45,48 +45,40 @@ final class InputContentUriTokenHandler extends IInputContentUriToken.Stub { @GuardedBy("mLock") private IBinder mPermissionOwnerToken = null; - InputContentUriTokenHandler(@NonNull Uri contentUri, int sourceUid, + static InputContentUriTokenHandler create(@NonNull Uri contentUri, int sourceUid, @NonNull String targetPackage, @UserIdInt int sourceUserId, @UserIdInt int targetUserId) { - mUri = contentUri; - mSourceUid = sourceUid; - mTargetPackage = targetPackage; - mSourceUserId = sourceUserId; - mTargetUserId = targetUserId; - } - - @Override - public void take() { - synchronized (mLock) { - if (mPermissionOwnerToken != null) { - // Permission is already granted. - return; - } - - try { - mPermissionOwnerToken = ActivityManagerNative.getDefault() - .newUriPermissionOwner("InputContentUriTokenHandler"); - } catch (RemoteException e) { - e.rethrowFromSystemServer(); - } - - doTakeLocked(mPermissionOwnerToken); + final IBinder permissionOwner; + try { + permissionOwner = ActivityManagerNative.getDefault() + .newUriPermissionOwner("InputContentUriTokenHandler"); + } catch (RemoteException e) { + return null; } - } - private void doTakeLocked(@NonNull IBinder permissionOwner) { long origId = Binder.clearCallingIdentity(); try { - try { - ActivityManagerNative.getDefault().grantUriPermissionFromOwner( - permissionOwner, mSourceUid, mTargetPackage, mUri, - Intent.FLAG_GRANT_READ_URI_PERMISSION, mSourceUserId, mTargetUserId); - } catch (RemoteException e) { - e.rethrowFromSystemServer(); - } + ActivityManagerNative.getDefault().grantUriPermissionFromOwner( + permissionOwner, sourceUserId, targetPackage, contentUri, + Intent.FLAG_GRANT_READ_URI_PERMISSION, sourceUserId, targetUserId); + } catch (RemoteException e) { + return null; } finally { Binder.restoreCallingIdentity(origId); } + return new InputContentUriTokenHandler(contentUri, sourceUid, targetPackage, sourceUserId, + targetUserId, permissionOwner); + } + + private InputContentUriTokenHandler(@NonNull Uri contentUri, int sourceUid, + @NonNull String targetPackage, @UserIdInt int sourceUserId, + @UserIdInt int targetUserId, @NonNull IBinder permissionOwnerToken) { + mUri = contentUri; + mSourceUid = sourceUid; + mTargetPackage = targetPackage; + mSourceUserId = sourceUserId; + mTargetUserId = targetUserId; + mPermissionOwnerToken = permissionOwnerToken; } @Override diff --git a/services/core/java/com/android/server/InputMethodManagerService.java b/services/core/java/com/android/server/InputMethodManagerService.java index e0d89f2fe6d3..2c228291e384 100644 --- a/services/core/java/com/android/server/InputMethodManagerService.java +++ b/services/core/java/com/android/server/InputMethodManagerService.java @@ -3953,7 +3953,7 @@ public class InputMethodManagerService extends IInputMethodManager.Stub } final int imeUserId = UserHandle.getUserId(uid); final int appUserId = UserHandle.getUserId(mCurClient.uid); - return new InputContentUriTokenHandler(contentUri, uid, packageName, imeUserId, + return InputContentUriTokenHandler.create(contentUri, uid, packageName, imeUserId, appUserId); } } -- 2.11.0