Make dismiss works for PermissionPromptAndroid

Make dismiss works for PermissionPromptAndroid

Dismiss permission prompt will close the current bubble and display any
pending requests. This CL also makes it to collect UMA metrics through
PermissionUmaUtil, which is the same as desktop.

BUG=606138
Committed: https://crrev.com/d8c80d88fe3890f202ab7b0b08a823004b5a97de
Cr-Commit-Position: refs/heads/master@{#435200}

[lshang, 2016-11-29 09:42:12]

Patchset #1 (id:1) has been deleted

[lshang, 2016-11-29 09:45:21]

Description was changed from

==========
Make dismiss works for PermissionPromptAndroid


BUG=
==========

to

==========
Make dismiss works for PermissionPromptAndroid

Dismiss permission prompt will close the current bubble and display any
pending requests. This CL also makes it to collect UMA metrics through
PermissionUmaUtil, which is the same as desktop.

BUG=606138
==========

[lshang, 2016-11-29 09:45:42]

lshang@chromium.org changed reviewers:
+ raymes@chromium.org

[lshang, 2016-11-29 09:45:43]

PTAL thanks!

[raymes, 2016-11-30 02:21:48]

https://codereview.chromium.org/2537083002/diff/20001/chrome/browser/permissi...
File chrome/browser/permissions/permission_context_base.cc (left):

https://codereview.chromium.org/2537083002/diff/20001/chrome/browser/permissi...
chrome/browser/permissions/permission_context_base.cc:227: #if
!defined(OS_ANDROID)
Hmm - I think currently the permission queue controller records these too (for
the old codepath). So we might need to gate this on the new codepath being
enabled (the runtime flag).

https://codereview.chromium.org/2537083002/diff/20001/chrome/browser/permissi...
File chrome/browser/permissions/permission_prompt_android.cc (left):

https://codereview.chromium.org/2537083002/diff/20001/chrome/browser/permissi...
chrome/browser/permissions/permission_prompt_android.cc:73: infobar_ = nullptr;
Hmm, how come this was removed?

[lshang, 2016-11-30 04:58:39]

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

[lshang, 2016-11-30 04:59:00]

https://codereview.chromium.org/2537083002/diff/20001/chrome/browser/permissi...
File chrome/browser/permissions/permission_context_base.cc (left):

https://codereview.chromium.org/2537083002/diff/20001/chrome/browser/permissi...
chrome/browser/permissions/permission_context_base.cc:227: #if
!defined(OS_ANDROID)
On 2016/11/30 02:21:48, raymes wrote:
> Hmm - I think currently the permission queue controller records these too (for
> the old codepath). So we might need to gate this on the new codepath being
> enabled (the runtime flag).

Done.

https://codereview.chromium.org/2537083002/diff/20001/chrome/browser/permissi...
File chrome/browser/permissions/permission_prompt_android.cc (left):

https://codereview.chromium.org/2537083002/diff/20001/chrome/browser/permissi...
chrome/browser/permissions/permission_prompt_android.cc:73: infobar_ = nullptr;
On 2016/11/30 02:21:48, raymes wrote:
> Hmm, how come this was removed?

So the infobar_ should only be set to null after
infobar_service->RemoveInfoBar(infobar_) is called, like in the Hide() method
above. Otherwise it won't remove the infobar for infobar service and pending
requests won't get displayed afterwards(PermissionRequestManager::Closing() will
call PermissionPromptAndroid::Hide() and line 54 will be skipped).

[commit-bot: I haz the power, 2016-11-30 04:59:20]

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

[raymes, 2016-11-30 05:07:13]

lgtm

https://codereview.chromium.org/2537083002/diff/20001/chrome/browser/permissi...
File chrome/browser/permissions/permission_prompt_android.cc (left):

https://codereview.chromium.org/2537083002/diff/20001/chrome/browser/permissi...
chrome/browser/permissions/permission_prompt_android.cc:73: infobar_ = nullptr;
On 2016/11/30 04:59:00, lshang wrote:
> On 2016/11/30 02:21:48, raymes wrote:
> > Hmm, how come this was removed?
> 
> So the infobar_ should only be set to null after
> infobar_service->RemoveInfoBar(infobar_) is called, like in the Hide() method
> above. Otherwise it won't remove the infobar for infobar service and pending
> requests won't get displayed afterwards(PermissionRequestManager::Closing()
will
> call PermissionPromptAndroid::Hide() and line 54 will be skipped).

Ah that makes sense, thanks for explaining!

[commit-bot: I haz the power, 2016-11-30 08:39:30]

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

[commit-bot: I haz the power, 2016-11-30 08:39:31]

Dry run: This issue passed the CQ dry run.

[lshang, 2016-11-30 08:40:59]

The CQ bit was checked by lshang@chromium.org

[commit-bot: I haz the power, 2016-11-30 08:41:32]

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

[commit-bot: I haz the power, 2016-11-30 08:41:33]

This CL has an open dependency (Issue 2522373002 Patch 100001). Please resolve
the dependency and try again.
If you are sure that there is no real dependency, please use one of the options
listed in https://goo.gl/9Es4OR to land the CL.

[lshang, 2016-11-30 08:52:03]

The CQ bit was checked by lshang@chromium.org

[lshang, 2016-11-30 08:52:04]

The patchset sent to the CQ was uploaded after l-g-t-m from raymes@chromium.org
Link to the patchset: https://codereview.chromium.org/2537083002/#ps60001
(title: "remove dependency and land this first")

[commit-bot: I haz the power, 2016-11-30 08:52:31]

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-30 09:44:25]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1480495923357210,
"parent_rev": "9790617d06b81bcfbb62f90deaa5ff5a83387e73", "commit_rev":
"51bca13cecc9f74bce4eac7657430beccf618558"}

[commit-bot: I haz the power, 2016-11-30 09:44:51]

Description was changed from

==========
Make dismiss works for PermissionPromptAndroid

Dismiss permission prompt will close the current bubble and display any
pending requests. This CL also makes it to collect UMA metrics through
PermissionUmaUtil, which is the same as desktop.

BUG=606138
==========

to

==========
Make dismiss works for PermissionPromptAndroid

Dismiss permission prompt will close the current bubble and display any
pending requests. This CL also makes it to collect UMA metrics through
PermissionUmaUtil, which is the same as desktop.

BUG=606138
==========

[commit-bot: I haz the power, 2016-11-30 09:44:52]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2016-11-30 09:46:54]

Description was changed from

==========
Make dismiss works for PermissionPromptAndroid

Dismiss permission prompt will close the current bubble and display any
pending requests. This CL also makes it to collect UMA metrics through
PermissionUmaUtil, which is the same as desktop.

BUG=606138
==========

to

==========
Make dismiss works for PermissionPromptAndroid

Dismiss permission prompt will close the current bubble and display any
pending requests. This CL also makes it to collect UMA metrics through
PermissionUmaUtil, which is the same as desktop.

BUG=606138

Committed: https://crrev.com/d8c80d88fe3890f202ab7b0b08a823004b5a97de
Cr-Commit-Position: refs/heads/master@{#435200}
==========

[commit-bot: I haz the power, 2016-11-30 09:46:55]

Patchset 3 (id:??) landed as
https://crrev.com/d8c80d88fe3890f202ab7b0b08a823004b5a97de
Cr-Commit-Position: refs/heads/master@{#435200}