Fix WebVR does not properly stop presenting if requestPresent rejects.

Fix WebVR does not properly stop presenting if requestPresent rejects.

Directly call forceExitPresent won't stop
presenting correctly.

BUG=680209
Review-Url: https://codereview.chromium.org/2689563008
Cr-Commit-Position: refs/heads/master@{#451544}
Committed: https://chromium.googlesource.com/chromium/src/+/a8725caafe812944aa590f27e8711eab810bf1a1

[shaobo.yan, 2017-02-13 02:28:49]

shaobo.yan@intel.com changed reviewers:
+ bajones@chromium.org, bsheedy@chromium.org, ddorwin@chromium.org

[shaobo.yan, 2017-02-13 02:28:49]

PTAL.

[shaobo.yan, 2017-02-13 03:01:43]

Description was changed from

==========
Fix WebVR does not properly stop presenting if requestPresent rejects.

Directly call forceExitPresent won't stop
presenting correctly.

BUG=680209
==========

to

==========
Fix WebVR does not properly stop presenting if requestPresent rejects.

Directly call forceExitPresent won't stop
presenting correctly.

BUG=680209
==========

[mthiesse, 2017-02-13 15:33:34]

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

[mthiesse, 2017-02-13 15:33:34]

https://codereview.chromium.org/2689563008/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/vr/VRDisplay.cpp (right):

https://codereview.chromium.org/2689563008/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/vr/VRDisplay.cpp:254: if (m_isPresenting) {
You should fix VRDisplay::forceExitPresent(), rather than fixing all of the
callsites.

[shaobo.yan, 2017-02-14 00:57:31]

On 2017/02/13 15:33:34, mthiesse wrote:
>
https://codereview.chromium.org/2689563008/diff/1/third_party/WebKit/Source/m...
> File third_party/WebKit/Source/modules/vr/VRDisplay.cpp (right):
> 
>
https://codereview.chromium.org/2689563008/diff/1/third_party/WebKit/Source/m...
> third_party/WebKit/Source/modules/vr/VRDisplay.cpp:254: if (m_isPresenting) {
> You should fix VRDisplay::forceExitPresent(), rather than fixing all of the
> callsites.

I've considerd fix forceExitPresent(), But it seems this function is used to
close the "fullscreen mode" VR.(especially with the comments here:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/vr/VRD...).
If you're sure the function of this method could be extended, I'd be very happy
to fix forceExitPresent. But I'd like to double check with you.

[shaobo.yan, 2017-02-14 01:08:12]

On 2017/02/14 00:57:31, shaobo.yan wrote:
> On 2017/02/13 15:33:34, mthiesse wrote:
> >
>
https://codereview.chromium.org/2689563008/diff/1/third_party/WebKit/Source/m...
> > File third_party/WebKit/Source/modules/vr/VRDisplay.cpp (right):
> > 
> >
>
https://codereview.chromium.org/2689563008/diff/1/third_party/WebKit/Source/m...
> > third_party/WebKit/Source/modules/vr/VRDisplay.cpp:254: if (m_isPresenting)
{
> > You should fix VRDisplay::forceExitPresent(), rather than fixing all of the
> > callsites.
> 
> I've considerd fix forceExitPresent(), But it seems this function is used to
> close the "fullscreen mode" VR.(especially with the comments here:
>
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/vr/VRD...).
> If you're sure the function of this method could be extended, I'd be very
happy
> to fix forceExitPresent. But I'd like to double check with you.

One more thing, the implementation of exitPresent
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/vr/VRD...
is also one of the reason I concern about expanding forceExitPresent function
yesterday. Because I understand it as a hint. If I understand wrong, pls correct
me.

[shaobo.yan, 2017-02-14 04:19:34]

On 2017/02/13 15:33:34, mthiesse wrote:
>
https://codereview.chromium.org/2689563008/diff/1/third_party/WebKit/Source/m...
> File third_party/WebKit/Source/modules/vr/VRDisplay.cpp (right):
> 
>
https://codereview.chromium.org/2689563008/diff/1/third_party/WebKit/Source/m...
> third_party/WebKit/Source/modules/vr/VRDisplay.cpp:254: if (m_isPresenting) {
> You should fix VRDisplay::forceExitPresent(), rather than fixing all of the
> callsites.

Sorry. Just go through the code during launch and find I'm totally wrong. I'll
update the patch. Thx for comments.

[shaobo.yan, 2017-02-14 06:12:35]

https://codereview.chromium.org/2689563008/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/vr/VRDisplay.cpp (right):

https://codereview.chromium.org/2689563008/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/vr/VRDisplay.cpp:254: if (m_isPresenting) {
On 2017/02/13 15:33:34, mthiesse wrote:
> You should fix VRDisplay::forceExitPresent(), rather than fixing all of the
> callsites.

Done.

[mthiesse, 2017-02-14 16:06:36]

lgtm

[shaobo.yan, 2017-02-15 00:46:08]

The CQ bit was checked by shaobo.yan@intel.com

[commit-bot: I haz the power, 2017-02-15 00:47:27]

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

[mthiesse, 2017-02-15 00:49:23]

The CQ bit was unchecked by mthiesse@chromium.org

[mthiesse, 2017-02-15 00:50:57]

I don't have OWNERS here, you'll need to wait for bajones to review.

[shaobo.yan, 2017-02-15 00:52:51]

mthiesse@:thx
bajones@ PTAL

[bajones, 2017-02-17 00:07:33]

LGTM

[shaobo.yan, 2017-02-20 00:38:27]

The CQ bit was checked by shaobo.yan@intel.com

[commit-bot: I haz the power, 2017-02-20 00:38:36]

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-02-20 02:22:24]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1487551107670080,
"parent_rev": "f5c1ca2bcc7f2cecd48866670dd7f0ddf3c0f24c", "commit_rev":
"a8725caafe812944aa590f27e8711eab810bf1a1"}

[commit-bot: I haz the power, 2017-02-20 02:23:00]

Description was changed from

==========
Fix WebVR does not properly stop presenting if requestPresent rejects.

Directly call forceExitPresent won't stop
presenting correctly.

BUG=680209
==========

to

==========
Fix WebVR does not properly stop presenting if requestPresent rejects.

Directly call forceExitPresent won't stop
presenting correctly.

BUG=680209

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

[commit-bot: I haz the power, 2017-02-20 02:23:01]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/a8725caafe812944aa590f27e871...