Fix exiting WebVR via Android UI not fully exiting fullscreen.

Fix exiting WebVR via Android UI not fully exiting fullscreen.

BUG=389343
Committed: https://crrev.com/94ff3d8045484adcb0daba7a05465edf813679a0
Cr-Commit-Position: refs/heads/master@{#432081}

[bajones, 2016-11-11 20:38:21]

bajones@chromium.org changed reviewers:
+ klausw@chromium.org, mthiesse@chromium.org

[bajones, 2016-11-11 20:38:22]

Just fixing a minor regression after the Mojo refactor

[mthiesse, 2016-11-14 16:24:12]

https://codereview.chromium.org/2493063004/diff/1/device/vr/android/gvr/gvr_d...
File device/vr/android/gvr/gvr_device.cc (right):

https://codereview.chromium.org/2493063004/diff/1/device/vr/android/gvr/gvr_d...
device/vr/android/gvr/gvr_device.cc:211: presenting_service_ = service;
Can we make presenting_service_ private to vr_device.h, and add a setter method
for it? This code's hard to reason about as-is.

https://codereview.chromium.org/2493063004/diff/1/device/vr/android/gvr/gvr_d...
File device/vr/android/gvr/gvr_device_provider.cc (right):

https://codereview.chromium.org/2493063004/diff/1/device/vr/android/gvr/gvr_d...
device/vr/android/gvr/gvr_device_provider.cc:68: // VR presentation exit
requested by the API
super nit: periods at the end of comments, here and below.

https://codereview.chromium.org/2493063004/diff/1/device/vr/android/gvr/gvr_d...
device/vr/android/gvr/gvr_device_provider.cc:72:
SwitchToNonPresentingDelegate();
Why do we not call vr_device_->OnExitPresent() here?

[bajones, 2016-11-14 19:35:43]

mthiesse@: Addressed your feedback, and took the opportunity to refactor how
some of the validation is done so that we only have to code it on one place and
not for every new VRDevice type.

https://codereview.chromium.org/2493063004/diff/1/device/vr/android/gvr/gvr_d...
File device/vr/android/gvr/gvr_device_provider.cc (right):

https://codereview.chromium.org/2493063004/diff/1/device/vr/android/gvr/gvr_d...
device/vr/android/gvr/gvr_device_provider.cc:72:
SwitchToNonPresentingDelegate();
On 2016/11/14 16:24:12, mthiesse wrote:
> Why do we not call vr_device_->OnExitPresent() here?

This is confusing (need to look at how to make this clearer) but it's because
this is called the vr_device_->ExitPresent, which itself calls OnExitPresent

[mthiesse, 2016-11-14 20:06:49]

lgtm

https://codereview.chromium.org/2493063004/diff/1/device/vr/android/gvr/gvr_d...
File device/vr/android/gvr/gvr_device_provider.cc (right):

https://codereview.chromium.org/2493063004/diff/1/device/vr/android/gvr/gvr_d...
device/vr/android/gvr/gvr_device_provider.cc:72:
SwitchToNonPresentingDelegate();
On 2016/11/14 19:35:43, bajones wrote:
> On 2016/11/14 16:24:12, mthiesse wrote:
> > Why do we not call vr_device_->OnExitPresent() here?
> 
> This is confusing (need to look at how to make this clearer) but it's because
> this is called the vr_device_->ExitPresent, which itself calls OnExitPresent

Fine to leave it for now I guess, but that definitely needs a refactoring.
Exiting presentation differently based on whether or not it was started by this
class is really not great.

[bajones, 2016-11-14 20:32:42]

The CQ bit was checked by bajones@chromium.org

[commit-bot: I haz the power, 2016-11-14 20:34:57]

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-14 23:18:06]

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

[commit-bot: I haz the power, 2016-11-14 23:18:07]

Failed to apply patch for device/vr/android/gvr/gvr_device.h:
While running git apply --index -p1;
  error: patch failed: device/vr/android/gvr/gvr_device.h:24
  error: device/vr/android/gvr/gvr_device.h: patch does not apply

Patch:       device/vr/android/gvr/gvr_device.h
Index: device/vr/android/gvr/gvr_device.h
diff --git a/device/vr/android/gvr/gvr_device.h
b/device/vr/android/gvr/gvr_device.h
index
339b30df77a0e96995038846b1a13bc207cd2f13..70ad353af817ebbce5e7479fc7c8a42777a0b7bf
100644
--- a/device/vr/android/gvr/gvr_device.h
+++ b/device/vr/android/gvr/gvr_device.h
@@ -24,15 +24,14 @@ class GvrDevice : public VRDevice {
 
   // VRDevice
   mojom::VRDisplayInfoPtr GetVRDevice() override;
-  mojom::VRPosePtr GetPose(VRServiceImpl* service) override;
-  void ResetPose(VRServiceImpl* service) override;
+  mojom::VRPosePtr GetPose() override;
+  void ResetPose() override;
 
-  bool RequestPresent(VRServiceImpl* service, bool secure_origin) override;
-  void ExitPresent(VRServiceImpl* service) override;
+  bool RequestPresent(bool secure_origin) override;
+  void ExitPresent() override;
 
-  void SubmitFrame(VRServiceImpl* service, mojom::VRPosePtr pose) override;
-  void UpdateLayerBounds(VRServiceImpl* service,
-                         mojom::VRLayerBoundsPtr leftBounds,
+  void SubmitFrame(mojom::VRPosePtr pose) override;
+  void UpdateLayerBounds(mojom::VRLayerBoundsPtr leftBounds,
                          mojom::VRLayerBoundsPtr rightBounds) override;
 
   void SetDelegate(GvrDelegate* delegate);

[bajones, 2016-11-14 23:50:38]

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

[commit-bot: I haz the power, 2016-11-14 23:51:22]

Dry run: 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-15 01:11:22]

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

[commit-bot: I haz the power, 2016-11-15 01:11:23]

Dry run: 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...)

[bajones, 2016-11-15 01:53:19]

The CQ bit was checked by bajones@chromium.org

[bajones, 2016-11-15 01:53:19]

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/2493063004/#ps60001
(title: "Rebase pt. 2")

[commit-bot: I haz the power, 2016-11-15 01:54:27]

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-15 03:48:25]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-11-15 03:51:24]

Description was changed from

==========
Fix exiting WebVR via Android UI not fully exiting fullscreen.

BUG=389343
==========

to

==========
Fix exiting WebVR via Android UI not fully exiting fullscreen.

BUG=389343

Committed: https://crrev.com/94ff3d8045484adcb0daba7a05465edf813679a0
Cr-Commit-Position: refs/heads/master@{#432081}
==========

[commit-bot: I haz the power, 2016-11-15 03:51:25]

Patchset 4 (id:??) landed as
https://crrev.com/94ff3d8045484adcb0daba7a05465edf813679a0
Cr-Commit-Position: refs/heads/master@{#432081}