Fixes WebVR presentation from within VRShell.

Fixes WebVR presentation from within VRShell.

- VrShellDelegate delays the call of a successfull request present callback until the GL thread is ready.
- By doing so, VrShellDelegate is able to pass the VRSubmitFrameClient to the GL thread no matter if the Gl thread was ready at the time of the request.
- Previously, the VRSubmitFrameClient was only passed correctly if the GL thread was not ready at the time of the request. Thus, WebVR presentation crashed from within VRShell.

BUG=703324
Review-Url: https://codereview.chromium.org/2774933003
Cr-Commit-Position: refs/heads/master@{#462511}
Committed: https://chromium.googlesource.com/chromium/src/+/18732d771d2b10a39707d4bdda57e3a367dac55d

[tiborg, 2017-03-24 21:03:27]

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

[tiborg, 2017-03-24 21:03:27]

Please take a look.

[tiborg, 2017-03-24 21:58:51]

https://codereview.chromium.org/2774933003/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/vr_shell.cc (right):

https://codereview.chromium.org/2774933003/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/vr_shell.cc:415:
delegate_provider_->SetPresentingDelegate(this, gvr_api_);
It would be helpful if this call was made in the constructor of VrShell. By
doing so, the presenting delegate in VrShellDelegate would be already set by the
time SetPresentResult is called. However, moving this to the constructor breaks
entering VRShell from 2D pages. The rendering does not start. Is there a way to
make this work?

[tiborg, 2017-03-24 22:12:14]

tiborg@chromium.org changed reviewers:
+ bajones@chromium.org

[tiborg, 2017-03-24 22:12:15]

bajones@chromium.org: Please take a look.

[mthiesse, 2017-03-30 15:29:46]

lgtm

https://codereview.chromium.org/2774933003/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/vr_shell.cc (right):

https://codereview.chromium.org/2774933003/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/vr_shell.cc:415:
delegate_provider_->SetPresentingDelegate(this, gvr_api_);
On 2017/03/24 21:58:51, tiborg wrote:
> It would be helpful if this call was made in the constructor of VrShell. By
> doing so, the presenting delegate in VrShellDelegate would be already set by
the
> time SetPresentResult is called. However, moving this to the constructor
breaks
> entering VRShell from 2D pages. The rendering does not start. Is there a way
to
> make this work?

What do you mean by the rendering doesn't start? How are you even moving this to
the constructor, when you don't have a submit client when not presenting?

https://codereview.chromium.org/2774933003/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/vr_shell_delegate.cc (right):

https://codereview.chromium.org/2774933003/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/vr_shell_delegate.cc:25:
VrShellDelegate::~VrShellDelegate() {
While you're cleaning this up, can you make sure that if the present_callback_
isn't null that we make sure it's run here?

https://codereview.chromium.org/2774933003/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/vr_shell_delegate.cc:68:
present_callback_.Run(true);
nit: base::ResetAndReturn(presenting_delegate_).Run(true)

Also below.

[tiborg, 2017-03-30 22:19:57]

https://codereview.chromium.org/2774933003/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/vr_shell.cc (right):

https://codereview.chromium.org/2774933003/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/vr_shell.cc:415:
delegate_provider_->SetPresentingDelegate(this, gvr_api_);
On 2017/03/30 15:29:46, mthiesse wrote:
> On 2017/03/24 21:58:51, tiborg wrote:
> > It would be helpful if this call was made in the constructor of VrShell. By
> > doing so, the presenting delegate in VrShellDelegate would be already set by
> the
> > time SetPresentResult is called. However, moving this to the constructor
> breaks
> > entering VRShell from 2D pages. The rendering does not start. Is there a way
> to
> > make this work?
> 
> What do you mean by the rendering doesn't start? How are you even moving this
to
> the constructor, when you don't have a submit client when not presenting?

It goes into the binocular view but it does not render any elements. This call
doesn't need a submit client. The submit client is passed once a present request
is successful. Is was just wondering if there is an easy way to do it. If not I
would just leave it for now.

https://codereview.chromium.org/2774933003/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/vr_shell_delegate.cc (right):

https://codereview.chromium.org/2774933003/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/vr_shell_delegate.cc:25:
VrShellDelegate::~VrShellDelegate() {
On 2017/03/30 15:29:46, mthiesse wrote:
> While you're cleaning this up, can you make sure that if the present_callback_
> isn't null that we make sure it's run here?

Done.

https://codereview.chromium.org/2774933003/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/vr_shell_delegate.cc:68:
present_callback_.Run(true);
On 2017/03/30 15:29:46, mthiesse wrote:
> nit: base::ResetAndReturn(presenting_delegate_).Run(true)
> 
> Also below.

Done.

[bajones, 2017-04-06 15:14:44]

lgtm

[tiborg, 2017-04-06 15:34:25]

The CQ bit was checked by tiborg@chromium.org

[tiborg, 2017-04-06 15:34:26]

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/2774933003/#ps20001
(title: "Rebased on ToT, nits")

[commit-bot: I haz the power, 2017-04-06 15:34:53]

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:52:06]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1491492865835100,
"parent_rev": "0d83127c40b39d0e9a54c1c1a474632017b9fc34", "commit_rev":
"18732d771d2b10a39707d4bdda57e3a367dac55d"}

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

Description was changed from

==========
Fixes WebVR presentation from within VRShell.

- VrShellDelegate delays the call of a successfull request present callback
until the GL thread is ready.
- By doing so, VrShellDelegate is able to pass the VRSubmitFrameClient to the GL
thread no matter if the Gl thread was ready at the time of the request.
- Previously, the VRSubmitFrameClient was only passed correctly if the GL thread
was not ready at the time of the request. Thus, WebVR presentation crashed from
within VRShell.

BUG=703324
==========

to

==========
Fixes WebVR presentation from within VRShell.

- VrShellDelegate delays the call of a successfull request present callback
until the GL thread is ready.
- By doing so, VrShellDelegate is able to pass the VRSubmitFrameClient to the GL
thread no matter if the Gl thread was ready at the time of the request.
- Previously, the VRSubmitFrameClient was only passed correctly if the GL thread
was not ready at the time of the request. Thus, WebVR presentation crashed from
within VRShell.

BUG=703324

Review-Url: https://codereview.chromium.org/2774933003
Cr-Commit-Position: refs/heads/master@{#462511}
Committed:
https://chromium.googlesource.com/chromium/src/+/18732d771d2b10a39707d4bdda57...
==========

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

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