Introduce --vr-shell flag on Android

Introduce --vr-shell flag on Android

BUG=617946
Committed: https://crrev.com/ccf69a1673986f7af139e2940a71418fe4e3d890
Cr-Commit-Position: refs/heads/master@{#399025}

[bshe, 2016-06-02 21:27:05]

Description was changed from

==========
Introduce --vr-shell flag


BUG=None
==========

to

==========
Introduce --vr-shell flag on Android


BUG=None
==========

[bshe, 2016-06-07 13:47:22]

Description was changed from

==========
Introduce --vr-shell flag on Android


BUG=None
==========

to

==========
Introduce --vr-shell flag on Android


BUG=617946
==========

[bshe, 2016-06-07 13:47:23]

bshe@chromium.org changed reviewers:
+ isherman@chromium.org, sievers@chromium.org

[bshe, 2016-06-07 13:48:41]

Hi guys. Do you mind take a look at this CL? Thanks

+sievers for content/*
+isherman for tools/metrics/*

[Ilya Sherman, 2016-06-07 19:26:53]

histograms.xml lgtm

[no sievers, 2016-06-07 21:42:13]

why is it a content flag though? 'shell' sounds like something embedder specific
which content shouldn't know about.

(also you are adding unused code)

[bshe, 2016-06-07 22:37:42]

On 2016/06/07 21:42:13, sievers wrote:
> why is it a content flag though? 'shell' sounds like something embedder
specific
> which content shouldn't know about.
The shell refers to an ash shell like shell. It is specific for vr environment.
The reason I add it here is because content probably need to know if it is in
vr_shell or not. Some of the UIs need to be customized
in vr_shell. Link disambiguate popup window is an example.

We might need to plumb this flag to lower layer too and I thought content is a
good place to start. Do you have any suggestion?
> 
> (also you are adding unused code)
Right. the code is currently unused but will be used to hide vr code in
following up CLs.

[no sievers, 2016-06-07 22:45:29]

On 2016/06/07 22:37:42, bshe wrote:
> On 2016/06/07 21:42:13, sievers wrote:
> > why is it a content flag though? 'shell' sounds like something embedder
> specific
> > which content shouldn't know about.
> The shell refers to an ash shell like shell. It is specific for vr
environment.
> The reason I add it here is because content probably need to know if it is in
> vr_shell or not. Some of the UIs need to be customized
> in vr_shell. Link disambiguate popup window is an example.
> 
> We might need to plumb this flag to lower layer too and I thought content is a
> good place to start. Do you have any suggestion?


It'd be nice if we can make them more specific wrt what this does *in* the
content layer (rather then the content layer knowing about the embedder and then
configuring itself in a way), for example a flag to configure the disambiguate
popup behavior.

We also started cleaning up the content layer for Android and as part of that
I'd like to make it less hardcoded to the Java upper layer concepts (and
untangle the Java content-public things to wrt the content public API). So let
me know if things get in the way in this regard where you want to take another
codepath for example.


> > 
> > (also you are adding unused code)
> Right. the code is currently unused but will be used to hide vr code in
> following up CLs.

[bshe, 2016-06-09 12:42:26]

Patchset #3 (id:40001) has been deleted

[bshe, 2016-06-09 12:42:36]

Patchset #3 (id:60001) has been deleted

[bshe, 2016-06-09 13:57:22]

On 2016/06/07 22:45:29, sievers wrote:
> On 2016/06/07 22:37:42, bshe wrote:
> > On 2016/06/07 21:42:13, sievers wrote:
> > > why is it a content flag though? 'shell' sounds like something embedder
> > specific
> > > which content shouldn't know about.
> > The shell refers to an ash shell like shell. It is specific for vr
> environment.
> > The reason I add it here is because content probably need to know if it is
in
> > vr_shell or not. Some of the UIs need to be customized
> > in vr_shell. Link disambiguate popup window is an example.
> > 
> > We might need to plumb this flag to lower layer too and I thought content is
a
> > good place to start. Do you have any suggestion?
> 
> 
> It'd be nice if we can make them more specific wrt what this does *in* the
> content layer (rather then the content layer knowing about the embedder and
then
> configuring itself in a way), for example a flag to configure the disambiguate
> popup behavior.
> 
> We also started cleaning up the content layer for Android and as part of that
> I'd like to make it less hardcoded to the Java upper layer concepts (and
> untangle the Java content-public things to wrt the content public API). So let
> me know if things get in the way in this regard where you want to take another
> codepath for example.
> 
> 
> > > 
> > > (also you are adding unused code)
> > Right. the code is currently unused but will be used to hide vr code in
> > following up CLs.

Moved the flag to Chrome. As discussed offline, content customization could be
done through delegate or using existing functions. PTAL

[no sievers, 2016-06-09 17:59:14]

On 2016/06/09 13:57:22, bshe wrote:
> On 2016/06/07 22:45:29, sievers wrote:
> > On 2016/06/07 22:37:42, bshe wrote:
> > > On 2016/06/07 21:42:13, sievers wrote:
> > > > why is it a content flag though? 'shell' sounds like something embedder
> > > specific
> > > > which content shouldn't know about.
> > > The shell refers to an ash shell like shell. It is specific for vr
> > environment.
> > > The reason I add it here is because content probably need to know if it is
> in
> > > vr_shell or not. Some of the UIs need to be customized
> > > in vr_shell. Link disambiguate popup window is an example.
> > > 
> > > We might need to plumb this flag to lower layer too and I thought content
is
> a
> > > good place to start. Do you have any suggestion?
> > 
> > 
> > It'd be nice if we can make them more specific wrt what this does *in* the
> > content layer (rather then the content layer knowing about the embedder and
> then
> > configuring itself in a way), for example a flag to configure the
disambiguate
> > popup behavior.
> > 
> > We also started cleaning up the content layer for Android and as part of
that
> > I'd like to make it less hardcoded to the Java upper layer concepts (and
> > untangle the Java content-public things to wrt the content public API). So
let
> > me know if things get in the way in this regard where you want to take
another
> > codepath for example.
> > 
> > 
> > > > 
> > > > (also you are adding unused code)
> > > Right. the code is currently unused but will be used to hide vr code in
> > > following up CLs.
> 
> Moved the flag to Chrome. As discussed offline, content customization could be
> done through delegate or using existing functions. PTAL

now you need a new reviewer :)

[bshe, 2016-06-09 18:11:57]

On 2016/06/09 17:59:14, sievers wrote:
> On 2016/06/09 13:57:22, bshe wrote:
> > On 2016/06/07 22:45:29, sievers wrote:
> > > On 2016/06/07 22:37:42, bshe wrote:
> > > > On 2016/06/07 21:42:13, sievers wrote:
> > > > > why is it a content flag though? 'shell' sounds like something
embedder
> > > > specific
> > > > > which content shouldn't know about.
> > > > The shell refers to an ash shell like shell. It is specific for vr
> > > environment.
> > > > The reason I add it here is because content probably need to know if it
is
> > in
> > > > vr_shell or not. Some of the UIs need to be customized
> > > > in vr_shell. Link disambiguate popup window is an example.
> > > > 
> > > > We might need to plumb this flag to lower layer too and I thought
content
> is
> > a
> > > > good place to start. Do you have any suggestion?
> > > 
> > > 
> > > It'd be nice if we can make them more specific wrt what this does *in* the
> > > content layer (rather then the content layer knowing about the embedder
and
> > then
> > > configuring itself in a way), for example a flag to configure the
> disambiguate
> > > popup behavior.
> > > 
> > > We also started cleaning up the content layer for Android and as part of
> that
> > > I'd like to make it less hardcoded to the Java upper layer concepts (and
> > > untangle the Java content-public things to wrt the content public API). So
> let
> > > me know if things get in the way in this regard where you want to take
> another
> > > codepath for example.
> > > 
> > > 
> > > > > 
> > > > > (also you are adding unused code)
> > > > Right. the code is currently unused but will be used to hide vr code in
> > > > following up CLs.
> > 
> > Moved the flag to Chrome. As discussed offline, content customization could
be
> > done through delegate or using existing functions. PTAL
> 
> now you need a new reviewer :)

huh. Looks like anyone can be a reviewer for chrome_switches.* Can I use you
still if you dont mind? :)

[no sievers, 2016-06-09 19:07:26]

lgtm

[bshe, 2016-06-09 21:12:37]

The CQ bit was checked by bshe@chromium.org

[bshe, 2016-06-09 21:12:38]

The patchset sent to the CQ was uploaded after l-g-t-m from
isherman@chromium.org
Link to the patchset: https://codereview.chromium.org/2030223002/#ps80001
(title: " ")

[commit-bot: I haz the power, 2016-06-09 21:13:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2030223002/80001

[commit-bot: I haz the power, 2016-06-09 21:36:02]

Description was changed from

==========
Introduce --vr-shell flag on Android


BUG=617946
==========

to

==========
Introduce --vr-shell flag on Android


BUG=617946
==========

[commit-bot: I haz the power, 2016-06-09 21:36:05]

Committed patchset #3 (id:80001)

[commit-bot: I haz the power, 2016-06-09 21:36:25]

CQ bit was unchecked

[commit-bot: I haz the power, 2016-06-09 21:38:30]

Description was changed from

==========
Introduce --vr-shell flag on Android


BUG=617946
==========

to

==========
Introduce --vr-shell flag on Android

BUG=617946

Committed: https://crrev.com/ccf69a1673986f7af139e2940a71418fe4e3d890
Cr-Commit-Position: refs/heads/master@{#399025}
==========

[commit-bot: I haz the power, 2016-06-09 21:38:33]

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