Remove ENABLE_VR_SHELL compile-time define.

Remove ENABLE_VR_SHELL compile-time define.

This feature is now controlled at runtime. Also, remove VR-shell
specific FYI bot builder. Fix lint warnings along the way.

BUG=None

Review-Url: https://codereview.chromium.org/2680723003
Cr-Commit-Position: refs/heads/master@{#450017}
Committed: https://chromium.googlesource.com/chromium/src/+/ffd9fb25d65c80160879eb47bfc5d1b0fdfff57f

[cjgrant, 2017-02-07 19:19:59]

cjgrant@chromium.org changed reviewers:
+ bshe@chromium.org, mthiesse@chromium.org

[cjgrant, 2017-02-07 19:20:00]

Guys, if/when this looks okay to you, I want to loop in amp@ and bsheedy@ as
well.

[amp, 2017-02-07 19:25:29]

On 2017/02/07 19:20:00, cjgrant wrote:
> Guys, if/when this looks okay to you, I want to loop in amp@ and bsheedy@ as
> well.

Too late, I already saw it. :)  I'll only add a few comments.

[amp, 2017-02-07 19:26:02]

https://codereview.chromium.org/2680723003/diff/1/chrome/browser/about_flags.cc
File chrome/browser/about_flags.cc (left):

https://codereview.chromium.org/2680723003/diff/1/chrome/browser/about_flags....
chrome/browser/about_flags.cc:1601: #if defined(ENABLE_VR_SHELL)
We only want this to show up on platforms that can support vr.  If we are
getting rid of the vr shell build flag, we should wrap this in the web vr build
flag.

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

https://codereview.chromium.org/2680723003/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/vr_shell.cc:76:
feature_enabled_(base::FeatureList::IsEnabled(features::kVrShell)),
I think the preference is to check the feature directly where it is used, rather
than save the value like this, but I'll defer to someone more familiar with
feature flags to say for sure.

[bshe, 2017-02-07 19:26:27]

On 2017/02/07 19:20:00, cjgrant wrote:
> Guys, if/when this looks okay to you, I want to loop in amp@ and bsheedy@ as
> well.

I am planning to use this flag to measure binary size increase before we ship
ChromeVR.
I dont expect that we have a large binary increase as most of the code and
resources already
compiled into chrome as part of WebVR. But I think we will have more ChromeVR
specific resources
in the future. So it probably still make sense to keep this flag before we ship
ChromeVR?

[amp, 2017-02-07 19:28:07]

On 2017/02/07 19:26:27, bshe wrote:
> On 2017/02/07 19:20:00, cjgrant wrote:
> > Guys, if/when this looks okay to you, I want to loop in amp@ and bsheedy@ as
> > well.
> 
> I am planning to use this flag to measure binary size increase before we ship
> ChromeVR.
> I dont expect that we have a large binary increase as most of the code and
> resources already
> compiled into chrome as part of WebVR. But I think we will have more ChromeVR
> specific resources
> in the future. So it probably still make sense to keep this flag before we
ship
> ChromeVR?

Also, see go/vrflags for some more thoughts about the existing flags and where
we want to go with them (and feel free to update the doc as needed).

[mthiesse, 2017-02-07 19:33:01]

On 2017/02/07 19:28:07, amp wrote:
> On 2017/02/07 19:26:27, bshe wrote:
> > On 2017/02/07 19:20:00, cjgrant wrote:
> > > Guys, if/when this looks okay to you, I want to loop in amp@ and bsheedy@
as
> > > well.
> > 
> > I am planning to use this flag to measure binary size increase before we
ship
> > ChromeVR.
> > I dont expect that we have a large binary increase as most of the code and
> > resources already
> > compiled into chrome as part of WebVR. But I think we will have more
ChromeVR
> > specific resources
> > in the future. So it probably still make sense to keep this flag before we
> ship
> > ChromeVR?
> 
> Also, see go/vrflags for some more thoughts about the existing flags and where
> we want to go with them (and feel free to update the doc as needed).

Keeping the flag is fine, but we have almost no actual uses of it right now.
Replacing all of the webvr || vr_shell with just webvr seems like the right
thing to do.

[mthiesse, 2017-02-07 19:33:26]

https://codereview.chromium.org/2680723003/diff/1/chrome/browser/about_flags.cc
File chrome/browser/about_flags.cc (left):

https://codereview.chromium.org/2680723003/diff/1/chrome/browser/about_flags....
chrome/browser/about_flags.cc:1601: #if defined(ENABLE_VR_SHELL)
On 2017/02/07 19:26:01, amp wrote:
> We only want this to show up on platforms that can support vr.  If we are
> getting rid of the vr shell build flag, we should wrap this in the web vr
build
> flag.

+1, I think this is the only useful use of ENABLE_VR_SHELL currently. The rest
of the changes seem like the right thing to me.

[cjgrant, 2017-02-07 20:10:17]

https://codereview.chromium.org/2680723003/diff/1/chrome/browser/about_flags.cc
File chrome/browser/about_flags.cc (left):

https://codereview.chromium.org/2680723003/diff/1/chrome/browser/about_flags....
chrome/browser/about_flags.cc:1601: #if defined(ENABLE_VR_SHELL)
On 2017/02/07 19:26:01, amp wrote:
> We only want this to show up on platforms that can support vr.  If we are
> getting rid of the vr shell build flag, we should wrap this in the web vr
build
> flag.

It's interesting though that the "enable-webvr" flag above isn't wrapped in a
WebVR build flag.  By this logic, should it not be?

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

https://codereview.chromium.org/2680723003/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/vr_shell.cc:76:
feature_enabled_(base::FeatureList::IsEnabled(features::kVrShell)),
On 2017/02/07 19:26:01, amp wrote:
> I think the preference is to check the feature directly where it is used,
rather
> than save the value like this, but I'll defer to someone more familiar with
> feature flags to say for sure.

Why would we want to incur this extra overhead on every button-press?  But more
importantly, we initialize VR components based on the feature... I think
allowing behavior to change after the fact would be dangerous.

https://codereview.chromium.org/2680723003/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/vr_shell.cc:411: 
I'll remove this line.

[amp, 2017-02-07 20:23:11]

https://codereview.chromium.org/2680723003/diff/1/chrome/browser/about_flags.cc
File chrome/browser/about_flags.cc (left):

https://codereview.chromium.org/2680723003/diff/1/chrome/browser/about_flags....
chrome/browser/about_flags.cc:1601: #if defined(ENABLE_VR_SHELL)
On 2017/02/07 20:10:17, cjgrant wrote:
> On 2017/02/07 19:26:01, amp wrote:
> > We only want this to show up on platforms that can support vr.  If we are
> > getting rid of the vr shell build flag, we should wrap this in the web vr
> build
> > flag.
> 
> It's interesting though that the "enable-webvr" flag above isn't wrapped in a
> WebVR build flag.  By this logic, should it not be?

Maybe it should be.  I think I had that same question when I added this build
check, but platform features are odd though (because they just add an API, not
new experiences like VrShell would).

I'll defer to the about flags OWNER review to make that call.  It is possible to
turn the flag on for any platform, even if it doesn't show up in about://flags,
so maybe it doesn't make a big difference either way.

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

https://codereview.chromium.org/2680723003/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/vr_shell.cc:76:
feature_enabled_(base::FeatureList::IsEnabled(features::kVrShell)),
On 2017/02/07 20:10:17, cjgrant wrote:
> On 2017/02/07 19:26:01, amp wrote:
> > I think the preference is to check the feature directly where it is used,
> rather
> > than save the value like this, but I'll defer to someone more familiar with
> > feature flags to say for sure.
> 
> Why would we want to incur this extra overhead on every button-press?  But
more
> importantly, we initialize VR components based on the feature... I think
> allowing behavior to change after the fact would be dangerous.

I'm not familiar with the overhead differences in c++ of having it saved as a
local property versus pulling it statically.  I kind of thought they would be
pretty similar (but I don't really have anything to base that assumption on).

https://codereview.chromium.org/2680723003/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/vr_shell.cc:339: if (feature_enabled_)
Isn't this going to be supported in WebVR for menu mode soon anyways?  (or am I
confusing what the app button does).

[cjgrant, 2017-02-07 20:28:25]

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

https://codereview.chromium.org/2680723003/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/vr_shell.cc:339: if (feature_enabled_)
On 2017/02/07 20:23:10, amp wrote:
> Isn't this going to be supported in WebVR for menu mode soon anyways?  (or am
I
> confusing what the app button does).

You're right.  Once the app button works properly in WebVR, and menu mode is
tested in WebVR, this check will disappear.

[bsheedy, 2017-02-07 20:53:08]

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

[bsheedy, 2017-02-07 20:53:09]

Shouldn't really affect anything on the test side AFAIK. If/when the
enable_vr_shell GN arg is removed, you'll have to update the bots to stop using
it.

https://codereview.chromium.org/2680723003/diff/1/chrome/android/BUILD.gn
File chrome/android/BUILD.gn (right):

https://codereview.chromium.org/2680723003/diff/1/chrome/android/BUILD.gn#new...
chrome/android/BUILD.gn:464: if (enable_webvr) {
If we're throwing all the VR related things under enable_webvr now, you can get
rid of this conditional and include the java_files append in the line above.

https://codereview.chromium.org/2680723003/diff/1/chrome/android/BUILD.gn#new...
chrome/android/BUILD.gn:471: # WebVR instrumentation files added conditionally
here in future
Likewise, can remove this comment if we're throwing everything under
enable_webvr.

[bsheedy, 2017-02-07 20:53:13]

Shouldn't really affect anything on the test side AFAIK. If/when the
enable_vr_shell GN arg is removed, you'll have to update the bots to stop using
it.

[bshe, 2017-02-07 22:07:10]

https://codereview.chromium.org/2680723003/diff/1/chrome/android/BUILD.gn
File chrome/android/BUILD.gn (right):

https://codereview.chromium.org/2680723003/diff/1/chrome/android/BUILD.gn#new...
chrome/android/BUILD.gn:258: if (enable_webvr) {
should we rename enable_webvr to enable_vr then? we shouldn't let enable_webvr
represent both WebVR and ChromeVR.

[cjgrant, 2017-02-07 22:12:37]

On 2017/02/07 19:33:01, mthiesse wrote:
> On 2017/02/07 19:28:07, amp wrote:
> > On 2017/02/07 19:26:27, bshe wrote:
> > > On 2017/02/07 19:20:00, cjgrant wrote:
> > > > Guys, if/when this looks okay to you, I want to loop in amp@ and
bsheedy@
> as
> > > > well.
> > > 
> > > I am planning to use this flag to measure binary size increase before we
> ship
> > > ChromeVR.
> > > I dont expect that we have a large binary increase as most of the code and
> > > resources already
> > > compiled into chrome as part of WebVR. But I think we will have more
> ChromeVR
> > > specific resources
> > > in the future. So it probably still make sense to keep this flag before we
> > ship
> > > ChromeVR?
> > 
> > Also, see go/vrflags for some more thoughts about the existing flags and
where
> > we want to go with them (and feel free to update the doc as needed).
> 
> Keeping the flag is fine, but we have almost no actual uses of it right now.
> Replacing all of the webvr || vr_shell with just webvr seems like the right
> thing to do.

Biao indicated offline that he's okay to lose the vr_shell-specific flag.

[cjgrant, 2017-02-07 22:16:54]

https://codereview.chromium.org/2680723003/diff/1/chrome/android/BUILD.gn
File chrome/android/BUILD.gn (right):

https://codereview.chromium.org/2680723003/diff/1/chrome/android/BUILD.gn#new...
chrome/android/BUILD.gn:258: if (enable_webvr) {
On 2017/02/07 22:07:09, bshe wrote:
> should we rename enable_webvr to enable_vr then? we shouldn't let enable_webvr
> represent both WebVR and ChromeVR.

That seems reasonable to me, but some buy-in from others would be good.  This
also implies changing the #define ENABLE_WEBVR, so doing this in a separate
change may also be good.

https://codereview.chromium.org/2680723003/diff/1/chrome/android/BUILD.gn#new...
chrome/android/BUILD.gn:464: if (enable_webvr) {
On 2017/02/07 20:53:09, bsheedy wrote:
> If we're throwing all the VR related things under enable_webvr now, you can
get
> rid of this conditional and include the java_files append in the line above.

Done.

https://codereview.chromium.org/2680723003/diff/1/chrome/android/BUILD.gn#new...
chrome/android/BUILD.gn:471: # WebVR instrumentation files added conditionally
here in future
On 2017/02/07 20:53:09, bsheedy wrote:
> Likewise, can remove this comment if we're throwing everything under
> enable_webvr.

Done.

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

https://codereview.chromium.org/2680723003/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/vr_shell.cc:411: 
On 2017/02/07 20:10:17, cjgrant wrote:
> I'll remove this line.

Done.

[mthiesse, 2017-02-07 22:33:11]

lgtm

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

https://codereview.chromium.org/2680723003/diff/20001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_shell.h:186: bool feature_enabled_;
nit: vr_shell_enabled_?

[cjgrant, 2017-02-07 22:45:06]

cjgrant@chromium.org changed reviewers:
+ agrieve@chromium.org, dbeam@chromium.org

[cjgrant, 2017-02-07 22:45:06]

agrieve@chromium.org: Please review changes in 
chrome/android/BUILD.gn

dbeam@chromium.org: Please review changes in 
chrome/browser/android/...

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

https://codereview.chromium.org/2680723003/diff/20001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_shell.h:186: bool feature_enabled_;
On 2017/02/07 22:33:11, mthiesse wrote:
> nit: vr_shell_enabled_?

Done.

[cjgrant, 2017-02-07 22:49:48]

cjgrant@chromium.org changed reviewers:
+ dtrainor@chromium.org

[cjgrant, 2017-02-07 22:49:49]

dtrainor@chromium.org: Please review changes in 
chrome/browser/android/chrome_jni_registrar.cc

dbeam@chromium.org: Please review changes in
chrome/browser/ui/...

[amp, 2017-02-07 22:50:07]

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

[amp, 2017-02-07 22:50:08]

lgtm

https://codereview.chromium.org/2680723003/diff/1/chrome/android/BUILD.gn
File chrome/android/BUILD.gn (right):

https://codereview.chromium.org/2680723003/diff/1/chrome/android/BUILD.gn#new...
chrome/android/BUILD.gn:258: if (enable_webvr) {
On 2017/02/07 22:16:54, cjgrant wrote:
> On 2017/02/07 22:07:09, bshe wrote:
> > should we rename enable_webvr to enable_vr then? we shouldn't let
enable_webvr
> > represent both WebVR and ChromeVR.
> 
> That seems reasonable to me, but some buy-in from others would be good.  This
> also implies changing the #define ENABLE_WEBVR, so doing this in a separate
> change may also be good.

+1 to renaming to a more general vr single flag (and also doing it in a separate
change).

[agrieve, 2017-02-08 01:31:27]

https://codereview.chromium.org/2680723003/diff/40001/chrome/android/BUILD.gn
File chrome/android/BUILD.gn (left):

https://codereview.chromium.org/2680723003/diff/40001/chrome/android/BUILD.gn...
chrome/android/BUILD.gn:258: if (enable_vr_shell || enable_webvr) {
Why not remove the definition in src/chrome/common/features.gni?

Is it still useful to keep the compile-time define for non-arm architectures?

[David Trainor- moved to gerrit, 2017-02-08 07:43:43]

chrome_jni_registrar.cc lgtm

[cjgrant, 2017-02-08 16:38:10]

On 2017/02/08 01:31:27, agrieve wrote:
> https://codereview.chromium.org/2680723003/diff/40001/chrome/android/BUILD.gn
> File chrome/android/BUILD.gn (left):
> 
>
https://codereview.chromium.org/2680723003/diff/40001/chrome/android/BUILD.gn...
> chrome/android/BUILD.gn:258: if (enable_vr_shell || enable_webvr) {
> Why not remove the definition in src/chrome/common/features.gni?
> 
> Is it still useful to keep the compile-time define for non-arm architectures?

Nope, I'll pull it from here too.  Thanks for flagging this.  We'll add a
replacement flag if/when we need it.

[cjgrant, 2017-02-08 16:50:50]

bsheedy@, can you please check the change in tools/mb/mb_config.pyl?  Do we
still need that config for any reason?

[cjgrant, 2017-02-08 16:52:33]

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

[commit-bot: I haz the power, 2017-02-08 16:52:57]

Dry run: 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-02-08 16:55:05]

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

[commit-bot: I haz the power, 2017-02-08 16:55:06]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[bsheedy, 2017-02-08 17:25:43]

The entry in mb_config.pyl is still very much necessary, as otherwise several
FYI bots won't know what GN args to use.

https://codereview.chromium.org/2680723003/diff/60001/tools/mb/mb_config.pyl
File tools/mb/mb_config.pyl (left):

https://codereview.chromium.org/2680723003/diff/60001/tools/mb/mb_config.pyl#...
tools/mb/mb_config.pyl:100: 'Android Builder (dbg)':
'android_debug_static_bot_vr_shell',
Keep this line, but change the value to 'android_debug_static_bot'

https://codereview.chromium.org/2680723003/diff/60001/tools/mb/mb_config.pyl#...
tools/mb/mb_config.pyl:834: 'android_debug_static_bot_vr_shell': [
Change this to 'android_debug_static_bot' and remove the 'vr_shell' element in
the list.

[agrieve, 2017-02-08 18:13:57]

On 2017/02/08 17:25:43, bsheedy wrote:
> The entry in mb_config.pyl is still very much necessary, as otherwise several
> FYI bots won't know what GN args to use.
> 
> https://codereview.chromium.org/2680723003/diff/60001/tools/mb/mb_config.pyl
> File tools/mb/mb_config.pyl (left):
> 
>
https://codereview.chromium.org/2680723003/diff/60001/tools/mb/mb_config.pyl#...
> tools/mb/mb_config.pyl:100: 'Android Builder (dbg)':
> 'android_debug_static_bot_vr_shell',
> Keep this line, but change the value to 'android_debug_static_bot'
> 
>
https://codereview.chromium.org/2680723003/diff/60001/tools/mb/mb_config.pyl#...
> tools/mb/mb_config.pyl:834: 'android_debug_static_bot_vr_shell': [
> Change this to 'android_debug_static_bot' and remove the 'vr_shell' element in
> the list.

BUILD.gn lgtm

[cjgrant, 2017-02-08 18:55:55]

https://codereview.chromium.org/2680723003/diff/60001/tools/mb/mb_config.pyl
File tools/mb/mb_config.pyl (left):

https://codereview.chromium.org/2680723003/diff/60001/tools/mb/mb_config.pyl#...
tools/mb/mb_config.pyl:100: 'Android Builder (dbg)':
'android_debug_static_bot_vr_shell',
On 2017/02/08 17:25:43, bsheedy wrote:
> Keep this line, but change the value to 'android_debug_static_bot'

Done.  Thanks, I understand this better now.

https://codereview.chromium.org/2680723003/diff/60001/tools/mb/mb_config.pyl#...
tools/mb/mb_config.pyl:834: 'android_debug_static_bot_vr_shell': [
On 2017/02/08 17:25:43, bsheedy wrote:
> Change this to 'android_debug_static_bot' and remove the 'vr_shell' element in
> the list.

'android_debug_static_bot' is already defined in the list above, so we can
remove this entry.

[cjgrant, 2017-02-08 18:56:07]

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

[commit-bot: I haz the power, 2017-02-08 18:56:49]

Dry run: 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-02-08 20:30:47]

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

[commit-bot: I haz the power, 2017-02-08 20:30:54]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[cjgrant, 2017-02-09 15:12:34]

cjgrant@chromium.org changed reviewers:
+ brettw@chromium.org

[cjgrant, 2017-02-09 15:12:35]

brettw@chromium.org: Please review changes in 

/tools/mb/

[amp, 2017-02-09 18:45:31]

Does the Android manifest need to be updated as well?

Or are these references not to the build flag?
https://cs.chromium.org/chromium/src/chrome/android/java/AndroidManifest.xml?...

[cjgrant, 2017-02-10 23:08:59]

Description was changed from

==========
Remove ENABLE_VR_SHELL compile-time define.

This feature is now controlled at runtime.

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

to

==========
Remove ENABLE_VR_SHELL compile-time define.

This feature is now controlled at runtime.

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

[cjgrant, 2017-02-10 23:08:59]

cjgrant@chromium.org changed reviewers:
- brettw@chromium.org

[cjgrant, 2017-02-10 23:09:28]

cjgrant@chromium.org changed reviewers:
+ dpranke@chromium.org

[cjgrant, 2017-02-10 23:09:28]

dpranke@chromium.org: Please review changes in 

tools/mb

[cjgrant, 2017-02-10 23:12:37]

cjgrant@chromium.org changed reviewers:
+ thakis@chromium.org

[cjgrant, 2017-02-10 23:12:37]

thakis@chromium.org: Please review changes in 

chrome/common/features.gni

[Nico, 2017-02-10 23:13:32]

features.gni lgt

[Nico, 2017-02-10 23:13:38]

lgtm

[cjgrant, 2017-02-10 23:14:54]

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

[commit-bot: I haz the power, 2017-02-10 23:15:26]

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

[Dirk Pranke, 2017-02-10 23:17:49]

lgtm

[commit-bot: I haz the power, 2017-02-10 23:24:09]

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

[commit-bot: I haz the power, 2017-02-10 23:24:10]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Dan Beam, 2017-02-11 22:59:18]

lgtm

[cjgrant, 2017-02-13 17:05:36]

Description was changed from

==========
Remove ENABLE_VR_SHELL compile-time define.

This feature is now controlled at runtime.

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

to

==========
Remove ENABLE_VR_SHELL compile-time define.

This feature is now controlled at runtime. Also, remove VR-shell
specific FYI bot builder. Fix lint warnings along the way.

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

[bajones, 2017-02-13 17:08:56]

device/vr/ LGTM

[girard, 2017-02-13 17:09:08]

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

[girard, 2017-02-13 17:09:09]

lgtm

[cjgrant, 2017-02-13 17:10:05]

The CQ bit was checked by cjgrant@chromium.org

[cjgrant, 2017-02-13 17:10:06]

The patchset sent to the CQ was uploaded after l-g-t-m from
mthiesse@chromium.org, dtrainor@chromium.org, amp@chromium.org,
agrieve@chromium.org, dpranke@chromium.org, dbeam@chromium.org,
thakis@chromium.org
Link to the patchset: https://codereview.chromium.org/2680723003/#ps160001
(title: "Fix lint warnings generated by vr_shell's new pickier checks.")

[commit-bot: I haz the power, 2017-02-13 17:10:47]

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-02-13 18:24:22]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1487005805615270,
"parent_rev": "ed35a0fe3dbb990c4782489c3982de49fa6f9059", "commit_rev":
"ffd9fb25d65c80160879eb47bfc5d1b0fdfff57f"}

[commit-bot: I haz the power, 2017-02-13 18:25:00]

Description was changed from

==========
Remove ENABLE_VR_SHELL compile-time define.

This feature is now controlled at runtime. Also, remove VR-shell
specific FYI bot builder. Fix lint warnings along the way.

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

to

==========
Remove ENABLE_VR_SHELL compile-time define.

This feature is now controlled at runtime. Also, remove VR-shell
specific FYI bot builder. Fix lint warnings along the way.

BUG=None

Review-Url: https://codereview.chromium.org/2680723003
Cr-Commit-Position: refs/heads/master@{#450017}
Committed:
https://chromium.googlesource.com/chromium/src/+/ffd9fb25d65c80160879eb47bfc5...
==========

[commit-bot: I haz the power, 2017-02-13 18:25:02]

Committed patchset #9 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/ffd9fb25d65c80160879eb47bfc5...