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

[bajones, 2016-09-22 17:35:37]

Description was changed from

==========
Show warning in WebVR for insecure origins

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

to

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

Will enable display of warnings in WebVR for insecure origins

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

[bajones, 2016-09-22 17:38:35]

bajones@chromium.org changed reviewers:
+ dcheng@chromium.org, kenrb@chromium.org

[bajones, 2016-09-22 17:38:36]

dcheng@ or kenrb@: Could you approve the .mojom change?

[dcheng, 2016-09-22 18:02:24]

So my understanding is that clamy@ is currently working on making it possible to
determine isSecureContext() in the browser process. If possible, I'd like to see
if we can just use that instead of plumbing it back from the renderer like this:
plumbing it through the mojom feels a little out of place.

clamy@, any idea on what the timeline would be?

[bajones, 2016-09-22 18:51:59]

On 2016/09/22 18:02:24, dcheng wrote:
> So my understanding is that clamy@ is currently working on making it possible
to
> determine isSecureContext() in the browser process. If possible, I'd like to
see
> if we can just use that instead of plumbing it back from the renderer like
this:
> plumbing it through the mojom feels a little out of place.
> 
> clamy@, any idea on what the timeline would be?

That would definitely be preferable on my end! We are trying to hit a fairly
short deadline with this particular bit of code, though (55 branchpoint). Unless
clamy@'s answer is "Landing today/tommorrow!" I would ask that maybe we be able
to land this path and then strip it out and replace it with the preferred route
as soon as it's available?

[clamy, 2016-09-23 09:58:08]

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

[clamy, 2016-09-23 09:58:09]

Unfortunately no it won't be today or tomorrow. In particular, changes I make
there would need to be reviewed by mkwst@, who is ooo until next week.

[dcheng, 2016-09-23 17:39:00]

On 2016/09/23 09:58:09, clamy wrote:
> Unfortunately no it won't be today or tomorrow. In particular, changes I make
> there would need to be reviewed by mkwst@, who is ooo until next week.

OK, is there a bug to we can track, so we won't lose track of this cleanup?

[clamy, 2016-09-26 09:40:38]

On 2016/09/23 17:39:00, dcheng wrote:
> On 2016/09/23 09:58:09, clamy wrote:
> > Unfortunately no it won't be today or tomorrow. In particular, changes I
make
> > there would need to be reviewed by mkwst@, who is ooo until next week.
> 
> OK, is there a bug to we can track, so we won't lose track of this cleanup?

I have filed crbug.com/650203.

[klausw, 2016-09-27 20:24:15]

On 2016/09/26 09:40:38, clamy wrote:
> On 2016/09/23 17:39:00, dcheng wrote:
> > On 2016/09/23 09:58:09, clamy wrote:
> > > Unfortunately no it won't be today or tomorrow. In particular, changes I
> make
> > > there would need to be reviewed by mkwst@, who is ooo until next week.
> > 
> > OK, is there a bug to we can track, so we won't lose track of this cleanup?
> 
> I have filed crbug.com/650203.

Since the suggested new approach isn't ready yet, is it ok to proceed with this
CL as-is and simplify it later once crbug.com/650203 is complete?

My CL https://codereview.chromium.org/2363553003/ depends on this CL, it
implements displaying the security warnings.

[klausw, 2016-09-28 18:43:59]

On 2016/09/27 20:24:15, klausw wrote:
> On 2016/09/26 09:40:38, clamy wrote:
> > On 2016/09/23 17:39:00, dcheng wrote:
> > > On 2016/09/23 09:58:09, clamy wrote:
> > > > Unfortunately no it won't be today or tomorrow. In particular, changes I
> > make
> > > > there would need to be reviewed by mkwst@, who is ooo until next week.
> > > 
> > > OK, is there a bug to we can track, so we won't lose track of this
cleanup?
> > 
> > I have filed crbug.com/650203.
> 
> Since the suggested new approach isn't ready yet, is it ok to proceed with
this
> CL as-is and simplify it later once crbug.com/650203 is complete?
> 
> My CL https://codereview.chromium.org/2363553003/ depends on this CL, it
> implements displaying the security warnings.

dcheng@ or kenrb@, friendly ping for approval of the .mojom changes?

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

On 2016/09/28 18:43:59, klausw wrote:
> On 2016/09/27 20:24:15, klausw wrote:
> > On 2016/09/26 09:40:38, clamy wrote:
> > > On 2016/09/23 17:39:00, dcheng wrote:
> > > > On 2016/09/23 09:58:09, clamy wrote:
> > > > > Unfortunately no it won't be today or tomorrow. In particular, changes
I
> > > make
> > > > > there would need to be reviewed by mkwst@, who is ooo until next week.
> > > > 
> > > > OK, is there a bug to we can track, so we won't lose track of this
> cleanup?
> > > 
> > > I have filed crbug.com/650203.
> > 
> > Since the suggested new approach isn't ready yet, is it ok to proceed with
> this
> > CL as-is and simplify it later once crbug.com/650203 is complete?
> > 
> > My CL https://codereview.chromium.org/2363553003/ depends on this CL, it
> > implements displaying the security warnings.
> 
> dcheng@ or kenrb@, friendly ping for approval of the .mojom changes?

FYI, since bajones@ is currently out on leave, I've cloned this issue in
https://codereview.chromium.org/2374153002/ and rebased it to current
origin/master. The rebase was trivial (automatic 3-way merge for one file only),
but I'm not sure if it's possible to submit this one as-is. I'll add you as
reviewers there too, if possible can you please approve both?

[klausw, 2016-09-28 19:03:12]

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

[commit-bot: I haz the power, 2016-09-28 19:03:37]

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

[dcheng, 2016-09-28 19:04:04]

On 2016/09/28 18:43:59, klausw wrote:
> On 2016/09/27 20:24:15, klausw wrote:
> > On 2016/09/26 09:40:38, clamy wrote:
> > > On 2016/09/23 17:39:00, dcheng wrote:
> > > > On 2016/09/23 09:58:09, clamy wrote:
> > > > > Unfortunately no it won't be today or tomorrow. In particular, changes
I
> > > make
> > > > > there would need to be reviewed by mkwst@, who is ooo until next week.
> > > > 
> > > > OK, is there a bug to we can track, so we won't lose track of this
> cleanup?
> > > 
> > > I have filed crbug.com/650203.
> > 
> > Since the suggested new approach isn't ready yet, is it ok to proceed with
> this
> > CL as-is and simplify it later once crbug.com/650203 is complete?
> > 
> > My CL https://codereview.chromium.org/2363553003/ depends on this CL, it
> > implements displaying the security warnings.
> 
> dcheng@ or kenrb@, friendly ping for approval of the .mojom changes?

I'll write a more detailed reply, but if we have all the bits we need in the
browser already, we should just add the glue to expose it.

[commit-bot: I haz the power, 2016-09-28 19:28:12]

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

[commit-bot: I haz the power, 2016-09-28 19:28:13]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

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

On 2016/09/28 19:04:04, dcheng wrote:
> On 2016/09/28 18:43:59, klausw wrote:
> > On 2016/09/27 20:24:15, klausw wrote:
> > > On 2016/09/26 09:40:38, clamy wrote:
> > > > On 2016/09/23 17:39:00, dcheng wrote:
> > > > > On 2016/09/23 09:58:09, clamy wrote:
> > > > > > Unfortunately no it won't be today or tomorrow. In particular,
changes
> I
> > > > make
> > > > > > there would need to be reviewed by mkwst@, who is ooo until next
week.
> > > > > 
> > > > > OK, is there a bug to we can track, so we won't lose track of this
> > cleanup?
> > > > 
> > > > I have filed crbug.com/650203.
> > > 
> > > Since the suggested new approach isn't ready yet, is it ok to proceed with
> > this
> > > CL as-is and simplify it later once crbug.com/650203 is complete?
> > > 
> > > My CL https://codereview.chromium.org/2363553003/ depends on this CL, it
> > > implements displaying the security warnings.
> > 
> > dcheng@ or kenrb@, friendly ping for approval of the .mojom changes?
> 
> I'll write a more detailed reply, but if we have all the bits we need in the
> browser already, we should just add the glue to expose it.

Everyone, please continue the review in the cloned change
https://codereview.chromium.org/2374153002/ . The CQ dry run had an error due to
an inconsistency in vr_service_impl_unittest, I uploaded a fix for it there. As
noted, bajones@ is out on leave.