Disable HTTP Auth dialog while in VR

Disable HTTP Auth dialog while in VR

This change disabled display of an auth dialog while in VR.

BUG=725525
Review-Url: https://codereview.chromium.org/2895303005
Cr-Commit-Position: refs/heads/master@{#474439}
Committed: https://chromium.googlesource.com/chromium/src/+/fe6673255f7a912b1433bd2d970de54fc437be9b

[billorr, 2017-05-22 22:26:25]

billorr@chromium.org changed reviewers:
+ asimjour@chromium.org, tedchoc@chromium.org, vollick@chromium.org

[billorr, 2017-05-22 22:26:25]

tedchoc@, please owners-review this change.

vollick@, asimjour@, please review to ensure this follows the expected patterns
for disabling popups.

[billorr, 2017-05-23 16:08:10]

Description was changed from

==========
Disable HTTP Auth dialog while in VR

This change disabled display of an auth dialog while in VR.

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

to

==========
Disable HTTP Auth dialog while in VR

This change disabled display of an auth dialog while in VR.

BUG=725525
==========

[Ian Vollick, 2017-05-24 17:35:21]

On 2017/05/22 22:26:25, billorr wrote:
> tedchoc@, please owners-review this change.
> 
> vollick@, asimjour@, please review to ensure this follows the expected
patterns
> for disabling popups.

Yep, this does indeed follow the established pattern.

[Ted C, 2017-05-24 17:40:44]

https://codereview.chromium.org/2895303005/diff/1/chrome/browser/ui/android/l...
File chrome/browser/ui/android/login_handler_android.cc (right):

https://codereview.chromium.org/2895303005/diff/1/chrome/browser/ui/android/l...
chrome/browser/ui/android/login_handler_android.cc:62: #if BUILDFLAG(ENABLE_VR)
just for readability, I'd be tempted to pull this out and above this condition

#if BUILDFLAG(ENABLE_VR)
  if (!vr_shell::VrTabHelper::IsInVr(web_contents)) {
    CancelAuth();
    return;
  }
#endif

Then you don't have the ifdef in the else below that scares me (where it looks
too easy to add something and with the lack of braces see it not get called)

[billorr, 2017-05-24 18:00:42]

https://codereview.chromium.org/2895303005/diff/1/chrome/browser/ui/android/l...
File chrome/browser/ui/android/login_handler_android.cc (right):

https://codereview.chromium.org/2895303005/diff/1/chrome/browser/ui/android/l...
chrome/browser/ui/android/login_handler_android.cc:62: #if BUILDFLAG(ENABLE_VR)
On 2017/05/24 17:40:44, Ted C wrote:
> just for readability, I'd be tempted to pull this out and above this condition
> 
> #if BUILDFLAG(ENABLE_VR)
>   if (!vr_shell::VrTabHelper::IsInVr(web_contents)) {
>     CancelAuth();
>     return;
>   }
> #endif
> 
> Then you don't have the ifdef in the else below that scares me (where it looks
> too easy to add something and with the lack of braces see it not get called)

Done.

https://codereview.chromium.org/2895303005/diff/20001/chrome/browser/ui/andro...
File chrome/browser/ui/android/login_handler_android.cc (right):

https://codereview.chromium.org/2895303005/diff/20001/chrome/browser/ui/andro...
chrome/browser/ui/android/login_handler_android.cc:86: "missing";
oops - I'll revert this whitespace change.

[Ted C, 2017-05-24 19:04:33]

lgtm

[billorr, 2017-05-24 20:37:30]

The CQ bit was checked by billorr@chromium.org

[commit-bot: I haz the power, 2017-05-24 20:38: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-24 22:11:42]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1495658250959900,
"parent_rev": "c053b933a242aaff6949fd5425154619130e103e", "commit_rev":
"f334aa0df5c87fe22c06adde6d23407751a7bb3f"}

[commit-bot: I haz the power, 2017-05-24 22:12:22]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1495658250959900,
"parent_rev": "7afef278595357100d354548df92c0fc9a52e2ba", "commit_rev":
"fe6673255f7a912b1433bd2d970de54fc437be9b"}

[commit-bot: I haz the power, 2017-05-24 22:12:37]

Description was changed from

==========
Disable HTTP Auth dialog while in VR

This change disabled display of an auth dialog while in VR.

BUG=725525
==========

to

==========
Disable HTTP Auth dialog while in VR

This change disabled display of an auth dialog while in VR.

BUG=725525

Review-Url: https://codereview.chromium.org/2895303005
Cr-Commit-Position: refs/heads/master@{#474439}
Committed:
https://chromium.googlesource.com/chromium/src/+/fe6673255f7a912b1433bd2d970d...
==========

[commit-bot: I haz the power, 2017-05-24 22:12:38]

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