Commit 35e7a8d3 authored by Anita Woodruff's avatar Anita Woodruff Committed by Commit Bot

[Android Permissions] Return bool from onRequestPermissionsResult

- Renamed onRequestPermissionsResult -> handlePermissionResult in the
AndroidPermissionDelegate interface; this avoids a name-clash with the
method on FragmentActivity, hopefully making it easier to reason about
what is calling what.

- Made handlePermissionResult return a boolean indicating whether the
result was handled.

- *Actually return false sometimes instead of always true from
ActivityAndroidPermissionDelegate/ActivityWindowAndroid* <-- this is
the only bit that actually might change behaviour.

- Made WindowAndroid an AndroidPermissionDelegate itself so it's now
a proper decorator pattern - it had most of the methods already and
this will allow easier unit testing.

R=tedchoc@chromium.org

Change-Id: I3787529184ce1acafd3d71d6f511941478026ecc
Reviewed-on: https://chromium-review.googlesource.com/1105042
Commit-Queue: Anita Woodruff <awdf@chromium.org>
Reviewed-by: default avatarTed Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568623}
parent a2574688
...@@ -109,6 +109,6 @@ public class DownloadActivity extends SnackbarActivity { ...@@ -109,6 +109,6 @@ public class DownloadActivity extends SnackbarActivity {
@Override @Override
public void onRequestPermissionsResult( public void onRequestPermissionsResult(
int requestCode, String[] permissions, int[] grantResults) { int requestCode, String[] permissions, int[] grantResults) {
mPermissionDelegate.onRequestPermissionsResult(requestCode, permissions, grantResults); mPermissionDelegate.handlePermissionResult(requestCode, permissions, grantResults);
} }
} }
...@@ -165,7 +165,7 @@ public class DownloadController { ...@@ -165,7 +165,7 @@ public class DownloadController {
if (activity instanceof ChromeActivity) { if (activity instanceof ChromeActivity) {
WindowAndroid windowAndroid = ((ChromeActivity) activity).getWindowAndroid(); WindowAndroid windowAndroid = ((ChromeActivity) activity).getWindowAndroid();
if (windowAndroid != null) { if (windowAndroid != null) {
delegate = windowAndroid.getAndroidPermissionDelegate(); delegate = windowAndroid;
} }
} else if (activity instanceof DownloadActivity) { } else if (activity instanceof DownloadActivity) {
delegate = ((DownloadActivity) activity).getAndroidPermissionDelegate(); delegate = ((DownloadActivity) activity).getAndroidPermissionDelegate();
......
...@@ -577,7 +577,7 @@ public abstract class AsyncInitializationActivity extends AppCompatActivity impl ...@@ -577,7 +577,7 @@ public abstract class AsyncInitializationActivity extends AppCompatActivity impl
public void onRequestPermissionsResult( public void onRequestPermissionsResult(
int requestCode, String[] permissions, int[] grantResults) { int requestCode, String[] permissions, int[] grantResults) {
if (mWindowAndroid != null) { if (mWindowAndroid != null) {
if (mWindowAndroid.onRequestPermissionsResult(requestCode, permissions, grantResults)) { if (mWindowAndroid.handlePermissionResult(requestCode, permissions, grantResults)) {
return; return;
} }
} }
......
...@@ -104,7 +104,9 @@ public class VrWindowAndroid extends WindowAndroid ...@@ -104,7 +104,9 @@ public class VrWindowAndroid extends WindowAndroid
} }
@Override @Override
public void onRequestPermissionsResult( public boolean handlePermissionResult(
int requestCode, String[] permissions, int[] grantResults) {} int requestCode, String[] permissions, int[] grantResults) {
return false;
}
} }
} }
...@@ -448,8 +448,10 @@ public class BluetoothChooserDialogTest { ...@@ -448,8 +448,10 @@ public class BluetoothChooserDialogTest {
} }
@Override @Override
public void onRequestPermissionsResult( public boolean handlePermissionResult(
int requestCode, String[] permissions, int[] grantResults) {} int requestCode, String[] permissions, int[] grantResults) {
return false;
}
} }
private static class FakeLocationUtils extends LocationUtils { private static class FakeLocationUtils extends LocationUtils {
......
...@@ -197,8 +197,10 @@ public class PermissionUpdateInfobarTest { ...@@ -197,8 +197,10 @@ public class PermissionUpdateInfobarTest {
} }
@Override @Override
public void onRequestPermissionsResult( public boolean handlePermissionResult(
int requestCode, String[] permissions, int[] grantResults) {} int requestCode, String[] permissions, int[] grantResults) {
return false;
}
} }
} }
...@@ -394,8 +394,10 @@ public class LocationBarVoiceRecognitionHandlerTest { ...@@ -394,8 +394,10 @@ public class LocationBarVoiceRecognitionHandlerTest {
} }
@Override @Override
public void onRequestPermissionsResult( public boolean handlePermissionResult(
int requestCode, String[] permissions, int[] grantResults) {} int requestCode, String[] permissions, int[] grantResults) {
return false;
}
} }
@Before @Before
......
...@@ -107,7 +107,7 @@ public class ActivityAndroidPermissionDelegate implements AndroidPermissionDeleg ...@@ -107,7 +107,7 @@ public class ActivityAndroidPermissionDelegate implements AndroidPermissionDeleg
} }
@Override @Override
public void onRequestPermissionsResult( public boolean handlePermissionResult(
int requestCode, String[] permissions, int[] grantResults) { int requestCode, String[] permissions, int[] grantResults) {
Activity activity = mActivity.get(); Activity activity = mActivity.get();
assert activity != null; assert activity != null;
...@@ -120,8 +120,9 @@ public class ActivityAndroidPermissionDelegate implements AndroidPermissionDeleg ...@@ -120,8 +120,9 @@ public class ActivityAndroidPermissionDelegate implements AndroidPermissionDeleg
PermissionCallback callback = mOutstandingPermissionRequests.get(requestCode); PermissionCallback callback = mOutstandingPermissionRequests.get(requestCode);
mOutstandingPermissionRequests.delete(requestCode); mOutstandingPermissionRequests.delete(requestCode);
if (callback == null) return; if (callback == null) return false;
callback.onRequestPermissionsResult(permissions, grantResults); callback.onRequestPermissionsResult(permissions, grantResults);
return true;
} }
protected void logUMAOnRequestPermissionDenied(String permission) {} protected void logUMAOnRequestPermissionDenied(String permission) {}
......
...@@ -162,20 +162,6 @@ public class ActivityWindowAndroid ...@@ -162,20 +162,6 @@ public class ActivityWindowAndroid
return false; return false;
} }
/**
* Responds to a pending permission result.
* @param requestCode The unique code for the permission request.
* @param permissions The list of permissions in the result.
* @param grantResults Whether the permissions were granted.
* @return Whether the permission request corresponding to a pending permission request.
*/
public boolean onRequestPermissionsResult(int requestCode, String[] permissions,
int[] grantResults) {
getAndroidPermissionDelegate().onRequestPermissionsResult(
requestCode, permissions, grantResults);
return true;
}
@Override @Override
public WeakReference<Activity> getActivity() { public WeakReference<Activity> getActivity() {
return new WeakReference<Activity>(activityFromContext(getContext().get())); return new WeakReference<Activity>(activityFromContext(getContext().get()));
......
...@@ -45,10 +45,11 @@ public interface AndroidPermissionDelegate { ...@@ -45,10 +45,11 @@ public interface AndroidPermissionDelegate {
void requestPermissions(String[] permissions, PermissionCallback callback); void requestPermissions(String[] permissions, PermissionCallback callback);
/** /**
* Callback for the result from requesting permissions. * Handle the result from requesting permissions.
* @param requestCode The request code passed in requestPermissions. * @param requestCode The request code passed in requestPermissions.
* @param permissions The list of requested permissions. * @param permissions The list of requested permissions.
* @param grantResults The grant results for the corresponding permissions. * @param grantResults The grant results for the corresponding permissions.
* @return True if the result was handled.
*/ */
void onRequestPermissionsResult(int requestCode, String[] permissions, int[] grantResults); boolean handlePermissionResult(int requestCode, String[] permissions, int[] grantResults);
} }
...@@ -46,7 +46,7 @@ import java.util.HashSet; ...@@ -46,7 +46,7 @@ import java.util.HashSet;
* The window base class that has the minimum functionality. * The window base class that has the minimum functionality.
*/ */
@JNINamespace("ui") @JNINamespace("ui")
public class WindowAndroid { public class WindowAndroid implements AndroidPermissionDelegate {
private static final String TAG = "WindowAndroid"; private static final String TAG = "WindowAndroid";
@TargetApi(Build.VERSION_CODES.KITKAT) @TargetApi(Build.VERSION_CODES.KITKAT)
...@@ -264,14 +264,6 @@ public class WindowAndroid { ...@@ -264,14 +264,6 @@ public class WindowAndroid {
mNativeWindowAndroid = 0; mNativeWindowAndroid = 0;
} }
/**
* @return the delegate to interact with the android permissions system, or null if not
* available
*/
public AndroidPermissionDelegate getAndroidPermissionDelegate() {
return mPermissionDelegate;
}
/** /**
* Set the delegate that will handle android permissions requests. * Set the delegate that will handle android permissions requests.
*/ */
...@@ -377,6 +369,7 @@ public class WindowAndroid { ...@@ -377,6 +369,7 @@ public class WindowAndroid {
* @return Whether access to the permission is granted. * @return Whether access to the permission is granted.
*/ */
@CalledByNative @CalledByNative
@Override
public final boolean hasPermission(String permission) { public final boolean hasPermission(String permission) {
if (mPermissionDelegate != null) return mPermissionDelegate.hasPermission(permission); if (mPermissionDelegate != null) return mPermissionDelegate.hasPermission(permission);
...@@ -398,6 +391,7 @@ public class WindowAndroid { ...@@ -398,6 +391,7 @@ public class WindowAndroid {
* @return Whether the requesting the permission is allowed. * @return Whether the requesting the permission is allowed.
*/ */
@CalledByNative @CalledByNative
@Override
public final boolean canRequestPermission(String permission) { public final boolean canRequestPermission(String permission) {
if (mPermissionDelegate != null) { if (mPermissionDelegate != null) {
return mPermissionDelegate.canRequestPermission(permission); return mPermissionDelegate.canRequestPermission(permission);
...@@ -416,6 +410,7 @@ public class WindowAndroid { ...@@ -416,6 +410,7 @@ public class WindowAndroid {
* @param permission The permission name. * @param permission The permission name.
* @return Whether the permission is revoked by policy and the user has no ability to change it. * @return Whether the permission is revoked by policy and the user has no ability to change it.
*/ */
@Override
public final boolean isPermissionRevokedByPolicy(String permission) { public final boolean isPermissionRevokedByPolicy(String permission) {
if (mPermissionDelegate != null) { if (mPermissionDelegate != null) {
return mPermissionDelegate.isPermissionRevokedByPolicy(permission); return mPermissionDelegate.isPermissionRevokedByPolicy(permission);
...@@ -433,6 +428,7 @@ public class WindowAndroid { ...@@ -433,6 +428,7 @@ public class WindowAndroid {
* @param permissions The list of permissions to request access to. * @param permissions The list of permissions to request access to.
* @param callback The callback to be notified whether the permissions were granted. * @param callback The callback to be notified whether the permissions were granted.
*/ */
@Override
public final void requestPermissions(String[] permissions, PermissionCallback callback) { public final void requestPermissions(String[] permissions, PermissionCallback callback) {
if (mPermissionDelegate != null) { if (mPermissionDelegate != null) {
mPermissionDelegate.requestPermissions(permissions, callback); mPermissionDelegate.requestPermissions(permissions, callback);
...@@ -443,6 +439,16 @@ public class WindowAndroid { ...@@ -443,6 +439,16 @@ public class WindowAndroid {
assert false : "Failed to request permissions using a WindowAndroid without an Activity"; assert false : "Failed to request permissions using a WindowAndroid without an Activity";
} }
@Override
public boolean handlePermissionResult(
int requestCode, String[] permissions, int[] grantResults) {
if (mPermissionDelegate != null) {
return mPermissionDelegate.handlePermissionResult(
requestCode, permissions, grantResults);
}
return false;
}
/** /**
* Displays an error message with a provided error message string. * Displays an error message with a provided error message string.
* @param error The error message string to be displayed. * @param error The error message string to be displayed.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment