Exit VR Shell on pause (And fix a bunch of minor bugs).

Exit VR Shell on pause (And fix a bunch of minor bugs).

Also fixed a possible bug where we'd incorrectly reset rotation.
Fixed a bug where the control container was hidden on resume, and pages without scrolling couldn't bring it back.
Fixed a crash exiting VR shortly after entering.

BUG=653182, 659707
Committed: https://crrev.com/db320fb20abdc36642a1148fa24224a7fabaa620
Cr-Commit-Position: refs/heads/master@{#428132}

[mthiesse, 2016-10-26 22:10:21]

mthiesse@chromium.org changed reviewers:
+ bshe@chromium.org

[mthiesse, 2016-10-26 22:10:22]

PTAL

[cjgrant, 2016-10-27 14:33:16]

cjgrant@chromium.org changed reviewers:
+ cjgrant@chromium.org

[cjgrant, 2016-10-27 14:33:17]

https://codereview.chromium.org/2453943002/diff/1/chrome/browser/ui/webui/vr_...
File chrome/browser/ui/webui/vr_shell/vr_shell_ui_message_handler.cc (right):

https://codereview.chromium.org/2453943002/diff/1/chrome/browser/ui/webui/vr_...
chrome/browser/ui/webui/vr_shell/vr_shell_ui_message_handler.cc:45: if
(!vr_shell_)
Why proceed here with no UI?

[mthiesse, 2016-10-27 14:37:03]

https://codereview.chromium.org/2453943002/diff/1/chrome/browser/ui/webui/vr_...
File chrome/browser/ui/webui/vr_shell/vr_shell_ui_message_handler.cc (right):

https://codereview.chromium.org/2453943002/diff/1/chrome/browser/ui/webui/vr_...
chrome/browser/ui/webui/vr_shell/vr_shell_ui_message_handler.cc:45: if
(!vr_shell_)
On 2016/10/27 14:33:17, cjgrant wrote:
> Why proceed here with no UI?

This can only happen if vr shell either died, or doesn't exist. If vr shell
died, this webui page will very soon be dead too (there's a destruction race).
If vr shell doesn't exist (the user loaded chrome://vr-shell-ui manually), we
should just silently do nothing.

I ran into this when exiting VR shortly after entering it; we definitely should
crash in that case.

[bshe, 2016-10-27 14:37:11]

https://codereview.chromium.org/2453943002/diff/1/chrome/browser/ui/webui/vr_...
File chrome/browser/ui/webui/vr_shell/vr_shell_ui_message_handler.cc (right):

https://codereview.chromium.org/2453943002/diff/1/chrome/browser/ui/webui/vr_...
chrome/browser/ui/webui/vr_shell/vr_shell_ui_message_handler.cc:45: if
(!vr_shell_)
On 2016/10/27 14:33:17, cjgrant wrote:
> Why proceed here with no UI?

I have logged a bug about this here:
https://bugs.chromium.org/p/chromium/issues/detail?id=659707

The root issue is that we didn't try to destroy ui_content before vr_shell is
destroyed. We should fix that I think. And then I think we dont need vr_shell a
weak ptr here.

[bshe, 2016-10-27 14:39:25]

On 2016/10/27 14:37:11, bshe wrote:
>
https://codereview.chromium.org/2453943002/diff/1/chrome/browser/ui/webui/vr_...
> File chrome/browser/ui/webui/vr_shell/vr_shell_ui_message_handler.cc (right):
> 
>
https://codereview.chromium.org/2453943002/diff/1/chrome/browser/ui/webui/vr_...
> chrome/browser/ui/webui/vr_shell/vr_shell_ui_message_handler.cc:45: if
> (!vr_shell_)
> On 2016/10/27 14:33:17, cjgrant wrote:
> > Why proceed here with no UI?
> 
> I have logged a bug about this here:
> https://bugs.chromium.org/p/chromium/issues/detail?id=659707
> 
> The root issue is that we didn't try to destroy ui_content before vr_shell is
> destroyed. We should fix that I think. And then I think we dont need vr_shell
a
> weak ptr here.

lgtm. Make sense, user might manually load chrome://vr-shell-ui.
Would be nice to add comment so that we dont forget.

[mthiesse, 2016-10-27 14:41:30]

Description was changed from

==========
Exit VR Shell on pause (And fix a bunch of minor bugs).

Also fixed a possible bug where we'd incorrectly reset rotation.
Fixed a bug where the control container was hidden on resume, and pages without
scrolling couldn't bring it back.
Fixed a crash exiting VR shortly after entering.

BUG=653182
==========

to

==========
Exit VR Shell on pause (And fix a bunch of minor bugs).

Also fixed a possible bug where we'd incorrectly reset rotation.
Fixed a bug where the control container was hidden on resume, and pages without
scrolling couldn't bring it back.
Fixed a crash exiting VR shortly after entering.

BUG=653182,659707
==========

[mthiesse, 2016-10-27 14:45:55]

https://codereview.chromium.org/2453943002/diff/1/chrome/browser/ui/webui/vr_...
File chrome/browser/ui/webui/vr_shell/vr_shell_ui_message_handler.cc (right):

https://codereview.chromium.org/2453943002/diff/1/chrome/browser/ui/webui/vr_...
chrome/browser/ui/webui/vr_shell/vr_shell_ui_message_handler.cc:45: if
(!vr_shell_)
On 2016/10/27 14:37:11, bshe wrote:
> On 2016/10/27 14:33:17, cjgrant wrote:
> > Why proceed here with no UI?
> 
> I have logged a bug about this here:
> https://bugs.chromium.org/p/chromium/issues/detail?id=659707
> 
> The root issue is that we didn't try to destroy ui_content before vr_shell is
> destroyed. We should fix that I think. And then I think we dont need vr_shell
a
> weak ptr here.

Added comment and added bug# to CL.

[mthiesse, 2016-10-27 14:47:12]

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

[mthiesse, 2016-10-27 14:47:13]

+dfalcantara@chromium.org for OWNERS stamp on chrome/android

[gone, 2016-10-27 18:02:57]

lgtm

[mthiesse, 2016-10-27 18:30:22]

The CQ bit was checked by mthiesse@chromium.org

[mthiesse, 2016-10-27 18:30:23]

The patchset sent to the CQ was uploaded after l-g-t-m from bshe@chromium.org
Link to the patchset: https://codereview.chromium.org/2453943002/#ps20001
(title: "Add comment")

[commit-bot: I haz the power, 2016-10-27 18:31:17]

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-10-27 18:51:25]

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

[commit-bot: I haz the power, 2016-10-27 18:51:26]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[mthiesse, 2016-10-27 20:03:14]

The CQ bit was checked by mthiesse@chromium.org

[mthiesse, 2016-10-27 20:03:15]

The patchset sent to the CQ was uploaded after l-g-t-m from
dfalcantara@chromium.org, bshe@chromium.org
Link to the patchset: https://codereview.chromium.org/2453943002/#ps40001
(title: "Rebase")

[commit-bot: I haz the power, 2016-10-27 20:04:03]

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-10-27 21:00:31]

Description was changed from

==========
Exit VR Shell on pause (And fix a bunch of minor bugs).

Also fixed a possible bug where we'd incorrectly reset rotation.
Fixed a bug where the control container was hidden on resume, and pages without
scrolling couldn't bring it back.
Fixed a crash exiting VR shortly after entering.

BUG=653182,659707
==========

to

==========
Exit VR Shell on pause (And fix a bunch of minor bugs).

Also fixed a possible bug where we'd incorrectly reset rotation.
Fixed a bug where the control container was hidden on resume, and pages without
scrolling couldn't bring it back.
Fixed a crash exiting VR shortly after entering.

BUG=653182,659707
==========

[commit-bot: I haz the power, 2016-10-27 21:00:32]

Committed patchset #3 (id:40001)

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

Description was changed from

==========
Exit VR Shell on pause (And fix a bunch of minor bugs).

Also fixed a possible bug where we'd incorrectly reset rotation.
Fixed a bug where the control container was hidden on resume, and pages without
scrolling couldn't bring it back.
Fixed a crash exiting VR shortly after entering.

BUG=653182,659707
==========

to

==========
Exit VR Shell on pause (And fix a bunch of minor bugs).

Also fixed a possible bug where we'd incorrectly reset rotation.
Fixed a bug where the control container was hidden on resume, and pages without
scrolling couldn't bring it back.
Fixed a crash exiting VR shortly after entering.

BUG=653182,659707

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

[commit-bot: I haz the power, 2016-10-27 21:02:24]

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