remove reliance on webcontents when requesting storage permission

remove reliance on webcontents when requesting storage permission

When requesting storage permission, we don't need WebContents most of the time.
WebContents is only needed when we need to show the permission infobar.
This CL removes all the WebContents check when requesting storage permission.
Caller can pass an empty WebContentsGetter callback if they don't need infobar.

BUG=717077
Review-Url: https://codereview.chromium.org/2850223002
Cr-Commit-Position: refs/heads/master@{#469861}
Committed: https://chromium.googlesource.com/chromium/src/+/05fa41d9751a443a76dde0fe53cbfa927bb5d001

[qinmin, 2017-05-01 17:46:40]

qinmin@chromium.org changed reviewers:
+ dtrainor@chromium.org

[qinmin, 2017-05-01 17:46:41]

PTAL

[David Trainor- moved to gerrit, 2017-05-03 20:43:47]

https://codereview.chromium.org/2850223002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadController.java
(right):

https://codereview.chromium.org/2850223002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadController.java:146:
if (windowAndroid.canRequestPermission(permission.WRITE_EXTERNAL_STORAGE)) {
if (!windowAndroid.canRequestPermission(...)) {
  nativeOnAcquire...();
  return;
}

https://codereview.chromium.org/2850223002/diff/1/chrome/browser/android/down...
File chrome/browser/android/download/download_controller.cc (right):

https://codereview.chromium.org/2850223002/diff/1/chrome/browser/android/down...
chrome/browser/android/download/download_controller.cc:141: if (!granted &&
!permission_to_update.empty() && web_contents_getter.Run()) {
Will this show a dialog and then, if the user says no, show an infobar?  Should
we just have a uniform UI here or only show the infobar if we have a
WebContents?

[qinmin, 2017-05-04 17:36:16]

https://codereview.chromium.org/2850223002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadController.java
(right):

https://codereview.chromium.org/2850223002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadController.java:146:
if (windowAndroid.canRequestPermission(permission.WRITE_EXTERNAL_STORAGE)) {
On 2017/05/03 20:43:47, David Trainor-ping if over 24h wrote:
> if (!windowAndroid.canRequestPermission(...)) {
>   nativeOnAcquire...();
>   return;
> }

Done.

https://codereview.chromium.org/2850223002/diff/1/chrome/browser/android/down...
File chrome/browser/android/download/download_controller.cc (right):

https://codereview.chromium.org/2850223002/diff/1/chrome/browser/android/down...
chrome/browser/android/download/download_controller.cc:141: if (!granted &&
!permission_to_update.empty() && web_contents_getter.Run()) {
On 2017/05/03 20:43:47, David Trainor-ping if over 24h wrote:
> Will this show a dialog and then, if the user says no, show an infobar? 
Should
> we just have a uniform UI here or only show the infobar if we have a
> WebContents?

If we show the dialog, there will be no infobar.
So if dialog is shown and granted is false, permission_to_update must be empty.

[David Trainor- moved to gerrit, 2017-05-05 20:08:53]

lgtm

[qinmin, 2017-05-05 21:43:55]

The CQ bit was checked by qinmin@chromium.org

[commit-bot: I haz the power, 2017-05-05 21:45:20]

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, 2017-05-05 22:16:13]

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

[commit-bot: I haz the power, 2017-05-05 22:16:13]

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...)

[qinmin, 2017-05-06 00:02:39]

The CQ bit was checked by qinmin@chromium.org

[qinmin, 2017-05-06 00:02:39]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtrainor@chromium.org
Link to the patchset: https://codereview.chromium.org/2850223002/#ps40001
(title: "fix mock class")

[commit-bot: I haz the power, 2017-05-06 00:03:21]

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, 2017-05-06 01:03:31]

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

[commit-bot: I haz the power, 2017-05-06 01:03:32]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[qinmin, 2017-05-06 13:44:48]

The CQ bit was checked by qinmin@chromium.org

[commit-bot: I haz the power, 2017-05-06 13:45:22]

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, 2017-05-06 19:03:17]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1494078288290630,
"parent_rev": "a641d5e814782a12dd261b234b761be85b734175", "commit_rev":
"05fa41d9751a443a76dde0fe53cbfa927bb5d001"}

[commit-bot: I haz the power, 2017-05-06 19:03:29]

Description was changed from

==========
remove reliance on webcontents when requesting storage permission

When requesting storage permission, we don't need WebContents most of the time.
WebContents is only needed when we need to show the permission infobar.
This CL removes all the WebContents check when requesting storage permission.
Caller can pass an empty WebContentsGetter callback if they don't need infobar.

BUG=717077
==========

to

==========
remove reliance on webcontents when requesting storage permission

When requesting storage permission, we don't need WebContents most of the time.
WebContents is only needed when we need to show the permission infobar.
This CL removes all the WebContents check when requesting storage permission.
Caller can pass an empty WebContentsGetter callback if they don't need infobar.

BUG=717077

Review-Url: https://codereview.chromium.org/2850223002
Cr-Commit-Position: refs/heads/master@{#469861}
Committed:
https://chromium.googlesource.com/chromium/src/+/05fa41d9751a443a76dde0fe53cb...
==========

[commit-bot: I haz the power, 2017-05-06 19:03:30]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/05fa41d9751a443a76dde0fe53cb...