Expose secure origin state to WebVR renderer

Expose secure origin state to WebVR renderer

Will enable display of warnings in WebVR for insecure origins

BUG=389343
patch from issue 2361533003 at patchset 20001 (http://crrev.com/2361533003#ps20001)

Committed: https://crrev.com/7bdd119442049affe8ad5e1c3f96070e0531a543
Cr-Commit-Position: refs/heads/master@{#421883}

[klausw, 2016-09-28 19:02:46]

Description was changed from

==========
Expose secure origin state to WebVR renderer

Will enable display of warnings in WebVR for insecure origins

BUG=389343

patch from issue 2361533003 at patchset 20001
(http://crrev.com/2361533003#ps20001)
==========

to

==========
Expose secure origin state to WebVR renderer

Will enable display of warnings in WebVR for insecure origins

BUG=389343

patch from issue 2361533003 at patchset 20001
(http://crrev.com/2361533003#ps20001)
==========

[klausw, 2016-09-28 19:02:46]

klausw@chromium.org changed reviewers:
+ clamy@chromium.org, dcheng@chromium.org, kenrb@chromium.org

[klausw, 2016-09-28 20:38:12]

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

[klausw, 2016-09-28 20:42:31]

On 2016/09/28 20:38:12, klausw wrote:
> mailto:klausw@chromium.org changed reviewers:
> + mailto:bajones@chromium.org, mailto:bshe@chromium.org

Everyone, this is a cloned copy of bajones@'s
https://codereview.chromium.org/2361533003/ since he is out on leave. It rebases
to current origin/master and fixes a unit test.

[dcheng, 2016-09-28 22:08:53]

Just to understand how this is going to work once we have a bit that VR can
query. Is the VR service associated with one RenderFrame so that it will be an
easy migration? Or is it more complicated?

[klausw, 2016-09-28 22:31:59]

On 2016/09/28 22:08:53, dcheng wrote:
> Just to understand how this is going to work once we have a bit that VR can
> query. Is the VR service associated with one RenderFrame so that it will be an
> easy migration? Or is it more complicated?

Sorry, I'm new to Chrome and having a hard time following the various
abstractions. I haven't come across RenderFrame specifically.

Currently, the WebVR presentation works on top of VrShell which has a pointer to
a ContentViewCore, that has a GetWebContents accessor, and we also have a
compositor which AFAIK is based on render_widget_host_view_android which in turn
is related to a delegated_frame_host_android. Do any of these sound as if they'd
be suitable for getting access to a RenderFrame? This is Android specific, but
that's currently the only WebVR plaform available at top-of-tree. The desktop
WebVR versions (SteamVR/Oculus) are in a separate experimental branch and aren't
an immediate concern at this point.

Conceptually, while in WebVR mode, the current WebVR display should be
associated with exactly one web page that's being rendered there, similar to
being in fullscreen mode.

[dcheng, 2016-09-29 00:00:09]

mojo lgtm, as discussed offline, we'll remove the bit from the mojom once we add
the secure context plumbing.

[bajones, 2016-09-29 04:22:24]

On 2016/09/28 22:08:53, dcheng wrote:
> Just to understand how this is going to work once we have a bit that VR can
> query. Is the VR service associated with one RenderFrame so that it will be an
> easy migration? Or is it more complicated?

Just to confirm: yes, it's one service per frame.

[klausw, 2016-09-29 05:03:54]

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

[commit-bot: I haz the power, 2016-09-29 05:04:08]

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

[klausw, 2016-09-29 05:08:38]

klausw@chromium.org changed reviewers:
+ kbr@chromium.org

[klausw, 2016-09-29 05:09:59]

On 2016/09/29 04:22:24, bajones wrote:
> On 2016/09/28 22:08:53, dcheng wrote:
> > Just to understand how this is going to work once we have a bit that VR can
> > query. Is the VR service associated with one RenderFrame so that it will be
an
> > easy migration? Or is it more complicated?
> 
> Just to confirm: yes, it's one service per frame.

+kbr@ for OWNERS review - as noted earlier in the thread this is basically a
clone of bajones's https://codereview.chromium.org/2361533003/ so I'm guessing
his OWNERS approval is implicit, but want to make sure someone else can
greenlight it.

[commit-bot: I haz the power, 2016-09-29 06:15:37]

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

[commit-bot: I haz the power, 2016-09-29 06:15:37]

Dry run: This issue passed the CQ dry run.

[bshe, 2016-09-29 12:46:47]

vr_shell lgtm with nits

https://codereview.chromium.org/2374153002/diff/20001/chrome/browser/android/...
File chrome/browser/android/vr_shell/vr_shell.h (right):

https://codereview.chromium.org/2374153002/diff/20001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_shell.h:144: bool webvr_secure_origin_;
set default to false?

https://codereview.chromium.org/2374153002/diff/20001/device/vr/android/gvr/g...
File device/vr/android/gvr/gvr_device.cc (right):

https://codereview.chromium.org/2374153002/diff/20001/device/vr/android/gvr/g...
device/vr/android/gvr/gvr_device.cc:189: LOG(ERROR) <<
"GvrDevice::RequestPresent: " << secure_origin;
Do you still need this log message?

[bajones, 2016-09-29 15:26:35]

On 2016/09/29 12:46:47, bshe wrote:
> vr_shell lgtm with nits
> 
>
https://codereview.chromium.org/2374153002/diff/20001/chrome/browser/android/...
> File chrome/browser/android/vr_shell/vr_shell.h (right):
> 
>
https://codereview.chromium.org/2374153002/diff/20001/chrome/browser/android/...
> chrome/browser/android/vr_shell/vr_shell.h:144: bool webvr_secure_origin_;
> set default to false?
> 
>
https://codereview.chromium.org/2374153002/diff/20001/device/vr/android/gvr/g...
> File device/vr/android/gvr/gvr_device.cc (right):
> 
>
https://codereview.chromium.org/2374153002/diff/20001/device/vr/android/gvr/g...
> device/vr/android/gvr/gvr_device.cc:189: LOG(ERROR) <<
> "GvrDevice::RequestPresent: " << secure_origin;
> Do you still need this log message?

/device/vr and /modules/vr LGTM with the log message removed.

[klausw, 2016-09-29 17:17:08]

On 2016/09/29 12:46:47, bshe wrote:
> vr_shell lgtm with nits
> 
>
https://codereview.chromium.org/2374153002/diff/20001/chrome/browser/android/...
> File chrome/browser/android/vr_shell/vr_shell.h (right):
> 
>
https://codereview.chromium.org/2374153002/diff/20001/chrome/browser/android/...
> chrome/browser/android/vr_shell/vr_shell.h:144: bool webvr_secure_origin_;
> set default to false?

Done.

>
https://codereview.chromium.org/2374153002/diff/20001/device/vr/android/gvr/g...
> File device/vr/android/gvr/gvr_device.cc (right):
> 
>
https://codereview.chromium.org/2374153002/diff/20001/device/vr/android/gvr/g...
> device/vr/android/gvr/gvr_device.cc:189: LOG(ERROR) <<
> "GvrDevice::RequestPresent: " << secure_origin;
> Do you still need this log message?

That wasn't supposed to be there, removed. Note to self, use INFO for temp
logging since IIRC a presubmit will complain about those.

I've rebased to current origin/master, running a few final tests before hitting
"submit" on it. Thanks everyone!

[klausw, 2016-09-29 17:24:58]

The CQ bit was checked by klausw@chromium.org

[klausw, 2016-09-29 17:24:59]

The patchset sent to the CQ was uploaded after l-g-t-m from
bajones@chromium.org, dcheng@chromium.org, bshe@chromium.org
Link to the patchset: https://codereview.chromium.org/2374153002/#ps40001
(title: "Rebase, remove stray logging, set explicit default")

[commit-bot: I haz the power, 2016-09-29 17:25:37]

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-09-29 18:44:43]

Description was changed from

==========
Expose secure origin state to WebVR renderer

Will enable display of warnings in WebVR for insecure origins

BUG=389343

patch from issue 2361533003 at patchset 20001
(http://crrev.com/2361533003#ps20001)
==========

to

==========
Expose secure origin state to WebVR renderer

Will enable display of warnings in WebVR for insecure origins

BUG=389343

patch from issue 2361533003 at patchset 20001
(http://crrev.com/2361533003#ps20001)
==========

[commit-bot: I haz the power, 2016-09-29 18:44:44]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-09-29 18:48:08]

Description was changed from

==========
Expose secure origin state to WebVR renderer

Will enable display of warnings in WebVR for insecure origins

BUG=389343

patch from issue 2361533003 at patchset 20001
(http://crrev.com/2361533003#ps20001)
==========

to

==========
Expose secure origin state to WebVR renderer

Will enable display of warnings in WebVR for insecure origins

BUG=389343

patch from issue 2361533003 at patchset 20001
(http://crrev.com/2361533003#ps20001)

Committed: https://crrev.com/7bdd119442049affe8ad5e1c3f96070e0531a543
Cr-Commit-Position: refs/heads/master@{#421883}
==========

[commit-bot: I haz the power, 2016-09-29 18:48:09]

Patchset 3 (id:??) landed as
https://crrev.com/7bdd119442049affe8ad5e1c3f96070e0531a543
Cr-Commit-Position: refs/heads/master@{#421883}