Allow modal permission prompts on Android to request system permissions.

Allow modal permission prompts on Android to request system permissions.

The current modal permission prompt does not prompt the user to update
Android's system permissions if Chromium does not have that permission.
For instance, a site requests location permission, the user grants it,
but the app does not have the permission. In this case, the permission
dialog needs to prompt the user to update Android's permissions. The
existing infobar implementation already does this.

This CL factors the Android system permissions request code out of
PermissionInfoBar into a new AndroidPermissionRequester class. That
allows the class to be reused for modal permission dialogs in
PermissionDialogController.

BUG=658125
Committed: https://crrev.com/05fa07ae53aa3ce6beeb2b79def845b73eb18939
Cr-Commit-Position: refs/heads/master@{#431459}

[dominickn, 2016-11-10 07:31:27]

The CQ bit was checked by dominickn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-10 07:32:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dominickn, 2016-11-10 07:34:18]

Description was changed from

==========
Allow modal permission prompts on Android to request system permissions.

The current modal permission prompt does not prompt the user to update
Android's system permissions if Chromium does not have that permission.
For instance, a site requests location permission, the user grants it,
but the app does not have the permission. In this case, the permission
dialog needs to prompt the user to update Android's permissions. The
existing infobar implementation already does this.

This CL factors the Android system permissions request code out of
PermissionInfoBar into a new AndroidPermissionRequester class. That
allows the class to be reused for modal permission dialogs in
PermissionDialogController.

BUG=658125
==========

to

==========
Allow modal permission prompts on Android to request system permissions.

The current modal permission prompt does not prompt the user to update
Android's system permissions if Chromium does not have that permission.
For instance, a site requests location permission, the user grants it,
but the app does not have the permission. In this case, the permission
dialog needs to prompt the user to update Android's permissions. The
existing infobar implementation already does this.

This CL factors the Android system permissions request code out of
PermissionInfoBar into a new AndroidPermissionRequester class. That
allows the class to be reused for modal permission dialogs in
PermissionDialogController.

BUG=658125
==========

[dominickn, 2016-11-10 07:34:19]

dominickn@chromium.org changed reviewers:
+ dfalcantara@chromium.org

[dominickn, 2016-11-10 07:35:58]

dominickn@chromium.org changed reviewers:
+ benwells@chromium.org

[commit-bot: I haz the power, 2016-11-10 08:48:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-10 08:48:29]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)

[dominickn, 2016-11-10 09:18:37]

The CQ bit was checked by dominickn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-10 09:18:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dominickn, 2016-11-10 09:19:29]

dfalcantara: PTAL at Android stuff

benwells: PTAL at permissions/

This corrects an oversight where if your device doesn't grant system permission
to Chrome, modal permission prompts wouldn't request it properly.

[commit-bot: I haz the power, 2016-11-10 09:27:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-10 09:27:05]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[gone, 2016-11-10 19:20:22]

https://codereview.chromium.org/2496473003/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/permissions/PermissionDialogDelegate.java
(right):

https://codereview.chromium.org/2496473003/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/permissions/PermissionDialogDelegate.java:27:
private WindowAndroid mWindowAndroid;
It's not safe to cache the WindowAndroid because of Tab reparenting -- when a
tab moves from a CustomTabActivity to the regular ChromeTabbedActivity (e.g.)
the Window changes.  I see that you account for this elsewhere, but it's safest
to hold onto the Tab instead, then just request the WindowAndroid when it's
necessary.

There's a way to pass the Tab in from native by grabbing it from the
WebContents, if you go that route.

[dominickn, 2016-11-10 22:50:28]

The CQ bit was checked by dominickn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-10 22:51:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dominickn, 2016-11-10 22:58:41]

Thanks!

https://codereview.chromium.org/2496473003/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/permissions/PermissionDialogDelegate.java
(right):

https://codereview.chromium.org/2496473003/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/permissions/PermissionDialogDelegate.java:27:
private WindowAndroid mWindowAndroid;
On 2016/11/10 19:20:22, dfalcantara (check my queue) wrote:
> It's not safe to cache the WindowAndroid because of Tab reparenting -- when a
> tab moves from a CustomTabActivity to the regular ChromeTabbedActivity (e.g.)
> the Window changes.  I see that you account for this elsewhere, but it's
safest
> to hold onto the Tab instead, then just request the WindowAndroid when it's
> necessary.
> 
> There's a way to pass the Tab in from native by grabbing it from the
> WebContents, if you go that route.

Done. Changed to pass in a tab.

[gone, 2016-11-10 23:05:47]

https://codereview.chromium.org/2496473003/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/permissions/AndroidPermissionRequester.java
(right):

https://codereview.chromium.org/2496473003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/permissions/AndroidPermissionRequester.java:27:
private WindowAndroid mWindowAndroid;
There a way around storing this WindowAndroid?

https://codereview.chromium.org/2496473003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/permissions/AndroidPermissionRequester.java:43:
void onAndroidPermissionCanceled();
Welp, this is one of those words no one agrees on a standard spelling for:

git grep -i "canceled" | wc -l
3413

git grep -i "cancelled" | wc -l
4060

[commit-bot: I haz the power, 2016-11-10 23:21:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-10 23:21:02]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[gone, 2016-11-10 23:33:36]

lgtm, with a follow up to see if you can get rid of that remaining cached
WindowAndroid.  Should poke Ted to see what can be done on that front.

[benwells, 2016-11-10 23:40:16]

lgtm

https://codereview.chromium.org/2496473003/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/permissions/PermissionDialogDelegate.java
(right):

https://codereview.chromium.org/2496473003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/permissions/PermissionDialogDelegate.java:46:
private int[] mContentSettings;
Nit: Could this be renamed to mContentSettingsType (and the other similar
things)?

[dominickn, 2016-11-10 23:55:02]

The CQ bit was checked by dominickn@chromium.org to run a CQ dry run

[dominickn, 2016-11-10 23:55:13]

Thanks!

https://codereview.chromium.org/2496473003/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/permissions/AndroidPermissionRequester.java
(right):

https://codereview.chromium.org/2496473003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/permissions/AndroidPermissionRequester.java:27:
private WindowAndroid mWindowAndroid;
On 2016/11/10 23:05:47, dfalcantara (check my queue) wrote:
> There a way around storing this WindowAndroid?

It'll require a bunch more refactoring which I'm happy to do in a follow up.

https://codereview.chromium.org/2496473003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/permissions/AndroidPermissionRequester.java:43:
void onAndroidPermissionCanceled();
On 2016/11/10 23:05:47, dfalcantara (check my queue) wrote:
> Welp, this is one of those words no one agrees on a standard spelling for:
> 
> git grep -i "canceled" | wc -l
> 3413
> 
> git grep -i "cancelled" | wc -l
> 4060

I've been told both spellings on reviews before

https://codereview.chromium.org/2496473003/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/permissions/PermissionDialogDelegate.java
(right):

https://codereview.chromium.org/2496473003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/permissions/PermissionDialogDelegate.java:46:
private int[] mContentSettings;
On 2016/11/10 23:40:16, benwells (slow) wrote:
> Nit: Could this be renamed to mContentSettingsType (and the other similar
> things)?

As discussed offline, this would be inconsistent with the infobar delegates and
infobar classes. so deferring to a follow up.

[commit-bot: I haz the power, 2016-11-10 23:55:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-11 00:14:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-11 00:14:08]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[dominickn, 2016-11-11 01:57:32]

The CQ bit was checked by dominickn@chromium.org

[dominickn, 2016-11-11 01:57:33]

The patchset sent to the CQ was uploaded after l-g-t-m from
benwells@chromium.org, dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/2496473003/#ps60001
(title: "-enum")

[commit-bot: I haz the power, 2016-11-11 01:58:00]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-11 02:04:51]

Description was changed from

==========
Allow modal permission prompts on Android to request system permissions.

The current modal permission prompt does not prompt the user to update
Android's system permissions if Chromium does not have that permission.
For instance, a site requests location permission, the user grants it,
but the app does not have the permission. In this case, the permission
dialog needs to prompt the user to update Android's permissions. The
existing infobar implementation already does this.

This CL factors the Android system permissions request code out of
PermissionInfoBar into a new AndroidPermissionRequester class. That
allows the class to be reused for modal permission dialogs in
PermissionDialogController.

BUG=658125
==========

to

==========
Allow modal permission prompts on Android to request system permissions.

The current modal permission prompt does not prompt the user to update
Android's system permissions if Chromium does not have that permission.
For instance, a site requests location permission, the user grants it,
but the app does not have the permission. In this case, the permission
dialog needs to prompt the user to update Android's permissions. The
existing infobar implementation already does this.

This CL factors the Android system permissions request code out of
PermissionInfoBar into a new AndroidPermissionRequester class. That
allows the class to be reused for modal permission dialogs in
PermissionDialogController.

BUG=658125
==========

[commit-bot: I haz the power, 2016-11-11 02:04:52]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-11-11 02:14:25]

Description was changed from

==========
Allow modal permission prompts on Android to request system permissions.

The current modal permission prompt does not prompt the user to update
Android's system permissions if Chromium does not have that permission.
For instance, a site requests location permission, the user grants it,
but the app does not have the permission. In this case, the permission
dialog needs to prompt the user to update Android's permissions. The
existing infobar implementation already does this.

This CL factors the Android system permissions request code out of
PermissionInfoBar into a new AndroidPermissionRequester class. That
allows the class to be reused for modal permission dialogs in
PermissionDialogController.

BUG=658125
==========

to

==========
Allow modal permission prompts on Android to request system permissions.

The current modal permission prompt does not prompt the user to update
Android's system permissions if Chromium does not have that permission.
For instance, a site requests location permission, the user grants it,
but the app does not have the permission. In this case, the permission
dialog needs to prompt the user to update Android's permissions. The
existing infobar implementation already does this.

This CL factors the Android system permissions request code out of
PermissionInfoBar into a new AndroidPermissionRequester class. That
allows the class to be reused for modal permission dialogs in
PermissionDialogController.

BUG=658125

Committed: https://crrev.com/05fa07ae53aa3ce6beeb2b79def845b73eb18939
Cr-Commit-Position: refs/heads/master@{#431459}
==========

[commit-bot: I haz the power, 2016-11-11 02:14:26]

Patchset 4 (id:??) landed as
https://crrev.com/05fa07ae53aa3ce6beeb2b79def845b73eb18939
Cr-Commit-Position: refs/heads/master@{#431459}