Adds button to VRShell menu mode, which lets the user manually exit WebVR presentation.

Adds button to VRShell menu mode, which lets the user manually exit WebVR presentation.
- Adds exit presentation button to the UI, which is shown in place of the back button when in WebVR presentation mode.
- Clicking the button will send a command to native and native triggers presentation exit.

BUG=644784
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2784303002
Cr-Commit-Position: refs/heads/master@{#462542}
Committed: https://chromium.googlesource.com/chromium/src/+/5ca8b58d19df1aeb5d3f9e746a5d4b3f862e1087

[tiborg, 2017-03-30 15:15:02]

Description was changed from

==========
Adds button to VRShell menu mode, which lets the user manually exit WebVR
presentation.
- Adds exit presentation button to the UI, which is shown in place of the back
button when in WebVR presentation mode.
- Clicking the button will send a command to native and native triggers
presentation exit.

BUG=644784
==========

to

==========
Adds button to VRShell menu mode, which lets the user manually exit WebVR
presentation.
- Adds exit presentation button to the UI, which is shown in place of the back
button when in WebVR presentation mode.
- Clicking the button will send a command to native and native triggers
presentation exit.

BUG=644784
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[tiborg, 2017-03-30 15:16:09]

tiborg@chromium.org changed reviewers:
+ cjgrant@chromium.org, mthiesse@chromium.org

[tiborg, 2017-03-30 15:16:10]

cjgrant@chromium.org: Please take a look.

mthiesse@chromium.org: Please take a look.

[cjgrant, 2017-03-30 15:35:28]

https://codereview.chromium.org/2784303002/diff/1/chrome/browser/resources/vr...
File chrome/browser/resources/vr_shell/vr_shell_ui.js (right):

https://codereview.chromium.org/2784303002/diff/1/chrome/browser/resources/vr...
chrome/browser/resources/vr_shell/vr_shell_ui.js:420: 0, 'exitPresentButton',
'#exit-present-button',
I'm mulling why we don't just use one button, and change the image+caption on
the fly.  Does translation make that not work easily?

https://codereview.chromium.org/2784303002/diff/1/chrome/browser/resources/vr...
chrome/browser/resources/vr_shell/vr_shell_ui.js:478: /** If true shows the exit
present button instead of the back button. */
- "true" isn't the parameter name.
- set and show are saying almost the same thing.  This could be
setExitPresentButtonVisible(visible)

https://codereview.chromium.org/2784303002/diff/1/chrome/browser/resources/vr...
chrome/browser/resources/vr_shell/vr_shell_ui.js:1237:
this.controls.setShowExitPresentButton(mode == api.Mode.WEB_VR);
Another thing we need to resolve with the UXEs is the fact that we generate tons
of calls to native that aren't actually changing anything.  We should maybe put
in some generic method of caching state (maybe at the scene layer).  Lets
discuss offline.

[mthiesse, 2017-03-30 15:36:57]

https://codereview.chromium.org/2784303002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
(left):

https://codereview.chromium.org/2784303002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java:535:
shutdownVR(
We still need to call shutdownVR if VR Shell isn't enabled so that we don't
return to VR Shell.

[tiborg, 2017-03-30 20:53:29]

https://codereview.chromium.org/2784303002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
(left):

https://codereview.chromium.org/2784303002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java:535:
shutdownVR(
On 2017/03/30 15:36:57, mthiesse wrote:
> We still need to call shutdownVR if VR Shell isn't enabled so that we don't
> return to VR Shell.

Done.

https://codereview.chromium.org/2784303002/diff/1/chrome/browser/resources/vr...
File chrome/browser/resources/vr_shell/vr_shell_ui.js (right):

https://codereview.chromium.org/2784303002/diff/1/chrome/browser/resources/vr...
chrome/browser/resources/vr_shell/vr_shell_ui.js:420: 0, 'exitPresentButton',
'#exit-present-button',
On 2017/03/30 15:35:28, cjgrant wrote:
> I'm mulling why we don't just use one button, and change the image+caption on
> the fly.  Does translation make that not work easily?

As discussed offline, let's keep separate buttons for now since this plays more
nicely with the sprite sheet.

https://codereview.chromium.org/2784303002/diff/1/chrome/browser/resources/vr...
chrome/browser/resources/vr_shell/vr_shell_ui.js:478: /** If true shows the exit
present button instead of the back button. */
On 2017/03/30 15:35:28, cjgrant wrote:
> - "true" isn't the parameter name.
> - set and show are saying almost the same thing.  This could be
> setExitPresentButtonVisible(visible)

Done.

https://codereview.chromium.org/2784303002/diff/1/chrome/browser/resources/vr...
chrome/browser/resources/vr_shell/vr_shell_ui.js:1237:
this.controls.setShowExitPresentButton(mode == api.Mode.WEB_VR);
On 2017/03/30 15:35:28, cjgrant wrote:
> Another thing we need to resolve with the UXEs is the fact that we generate
tons
> of calls to native that aren't actually changing anything.  We should maybe
put
> in some generic method of caching state (maybe at the scene layer).  Lets
> discuss offline.

Acknowledged. Let's discuss offline.

[mthiesse, 2017-04-06 15:45:13]

lgtm

[tiborg, 2017-04-06 15:48:18]

The CQ bit was checked by tiborg@chromium.org

[commit-bot: I haz the power, 2017-04-06 15:48:55]

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-04-06 16:04:26]

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

[commit-bot: I haz the power, 2017-04-06 16:04:27]

Try jobs failed on following builders:
  closure_compilation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/closure_compila...)

[tiborg, 2017-04-06 16:20:07]

The CQ bit was checked by tiborg@chromium.org

[tiborg, 2017-04-06 16:20:07]

The patchset sent to the CQ was uploaded after l-g-t-m from
mthiesse@chromium.org
Link to the patchset: https://codereview.chromium.org/2784303002/#ps40001
(title: "Fixed closure compiler error")

[commit-bot: I haz the power, 2017-04-06 16:32:48]

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-04-06 18:01:11]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1491495607127050,
"parent_rev": "742bb98afe29eff3d693328bc39a62f680f83a1d", "commit_rev":
"5ca8b58d19df1aeb5d3f9e746a5d4b3f862e1087"}

[commit-bot: I haz the power, 2017-04-06 18:01:56]

Description was changed from

==========
Adds button to VRShell menu mode, which lets the user manually exit WebVR
presentation.
- Adds exit presentation button to the UI, which is shown in place of the back
button when in WebVR presentation mode.
- Clicking the button will send a command to native and native triggers
presentation exit.

BUG=644784
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Adds button to VRShell menu mode, which lets the user manually exit WebVR
presentation.
- Adds exit presentation button to the UI, which is shown in place of the back
button when in WebVR presentation mode.
- Clicking the button will send a command to native and native triggers
presentation exit.

BUG=644784
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2784303002
Cr-Commit-Position: refs/heads/master@{#462542}
Committed:
https://chromium.googlesource.com/chromium/src/+/5ca8b58d19df1aeb5d3f9e746a5d...
==========

[commit-bot: I haz the power, 2017-04-06 18:01:57]

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