WebVR: avoid race conditions for partially-initialized display

WebVR: avoid race conditions for partially-initialized display

The GvrNonPresentingDelegate was acting as a pseudo VRDisplay device and
reporting successful requestPresent even though vital information such
as render resolution is not available yet.

Instead of the "fallback" 2048x1024 resolution, report a 0x0 resolution
so that other code can recognize this as an invalid device and defer
actions such as resizing.

One specific issue was that a vrpresentchange was getting fired prematurely,
leading to wrong canvas sizing by the JS application.

BUG=667327
Committed: https://crrev.com/772bdaaac52114f3ebbc7626a3c630a3918b03d0
Cr-Commit-Position: refs/heads/master@{#435824}

[klausw, 2016-12-01 21:12:58]

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

[mthiesse, 2016-12-01 21:46:57]

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

https://codereview.chromium.org/2544713002/diff/1/device/vr/android/gvr/gvr_d...
device/vr/android/gvr/gvr_device.cc:94: delegate_->SetWebVRRenderSurfaceSize(0,
0);
nit: use constant

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

https://codereview.chromium.org/2544713002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/vr/VRDisplay.cpp:511: m_isPresenting = false;
why this change?

[klausw, 2016-12-01 23:08:56]

PTAL

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

https://codereview.chromium.org/2544713002/diff/1/device/vr/android/gvr/gvr_d...
device/vr/android/gvr/gvr_device.cc:94: delegate_->SetWebVRRenderSurfaceSize(0,
0);
On 2016/12/01 21:46:57, mthiesse wrote:
> nit: use constant

Done.

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

https://codereview.chromium.org/2544713002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/vr/VRDisplay.cpp:511: m_isPresenting = false;
On 2016/12/01 21:46:57, mthiesse wrote:
> why this change?

OnPresentChange() looks at m_isPresenting to tell if presentation is
starting or stopping. The rest of the code was consistent in changing 
the flag before calling OnPresentChange, but this method did it the 
other way around. I just moved the existing assignment, the overall
effect is the same. (There's no need to set m_isPresenting to false
in the "else" branch where it's false already.)

[mthiesse, 2016-12-01 23:16:56]

lgtm

[bajones, 2016-12-01 23:42:28]

On 2016/12/01 23:16:56, mthiesse wrote:
> lgtm

LGTM as a short term fix. We definitely need to tighten up the lifetime of the
GvrApi objects and the presentation flow so that we don't hit these situations.
:P

[bajones, 2016-12-02 00:04:19]

The CQ bit was checked by bajones@chromium.org

[commit-bot: I haz the power, 2016-12-02 00:04:51]

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-12-02 02:10:50]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1480637059777100,
"parent_rev": "0ba16f3c8ca5c926f931c4255810ae78c7530cdc", "commit_rev":
"65c752733ff940244fac32bb1a182d8e5d39093f"}

[commit-bot: I haz the power, 2016-12-02 02:11:57]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-12-02 02:16:01]

Description was changed from

==========
WebVR: avoid race conditions for partially-initialized display

The GvrNonPresentingDelegate was acting as a pseudo VRDisplay device and
reporting successful requestPresent even though vital information such
as render resolution is not available yet.

Instead of the "fallback" 2048x1024 resolution, report a 0x0 resolution
so that other code can recognize this as an invalid device and defer
actions such as resizing.

One specific issue was that a vrpresentchange was getting fired prematurely,
leading to wrong canvas sizing by the JS application.

BUG=667327
==========

to

==========
WebVR: avoid race conditions for partially-initialized display

The GvrNonPresentingDelegate was acting as a pseudo VRDisplay device and
reporting successful requestPresent even though vital information such
as render resolution is not available yet.

Instead of the "fallback" 2048x1024 resolution, report a 0x0 resolution
so that other code can recognize this as an invalid device and defer
actions such as resizing.

One specific issue was that a vrpresentchange was getting fired prematurely,
leading to wrong canvas sizing by the JS application.

BUG=667327

Committed: https://crrev.com/772bdaaac52114f3ebbc7626a3c630a3918b03d0
Cr-Commit-Position: refs/heads/master@{#435824}
==========

[commit-bot: I haz the power, 2016-12-02 02:16:02]

Patchset 2 (id:??) landed as
https://crrev.com/772bdaaac52114f3ebbc7626a3c630a3918b03d0
Cr-Commit-Position: refs/heads/master@{#435824}