Reland gvr upgrade

Reland gvr upgrade

The original CL is here: https://codereview.chromium.org/2367673002/
It was reverted because of compile failure with gn flags:
is_debug=false
is_component_build=true
target_os="android"
The reason is gvr uses API that only available in Android N SDK. For
release build, proguard detected the dependancy and results a compile
error. N SDK is in the process of rolling to chromium
https://codereview.chromium.org/2350013004/
In this CL, I restricted WebVr to debug build only. This could unblock
us. And I will remove the is_debug restriction once N is in Chromium

BUG=389343
TBR=dtrainor@

Committed: https://crrev.com/511e7f727be80e20d41fc5fd07cf8b5244cafbf0
Cr-Commit-Position: refs/heads/master@{#423005}

[bshe, 2016-10-04 22:28:01]

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

[commit-bot: I haz the power, 2016-10-04 22:28:37]

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

[bshe, 2016-10-04 23:05:11]

Description was changed from

==========
Reland gvr upgrade

The original CL is here: https://codereview.chromium.org/2367673002/
It was reverted because of compile failure with gn flags:
is_debug=false
is_component_build=true
target_os="android"
The reason is gvr uses API that only available in Android N SDK. For
release build, proguard detected the dependancy and results a compile
error. N SDK is in the process of rolling to chromium
https://codereview.chromium.org/2350013004/
In this CL, I restricted WebVr to debug build only. This could unblock
us. And I will remove the is_debug restriction once N is in Chromium


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

to

==========
Reland gvr upgrade

The original CL is here: https://codereview.chromium.org/2367673002/
It was reverted because of compile failure with gn flags:
is_debug=false
is_component_build=true
target_os="android"
The reason is gvr uses API that only available in Android N SDK. For
release build, proguard detected the dependancy and results a compile
error. N SDK is in the process of rolling to chromium
https://codereview.chromium.org/2350013004/
In this CL, I restricted WebVr to debug build only. This could unblock
us. And I will remove the is_debug restriction once N is in Chromium


BUG=389343
tbr=dtrainor
==========

[commit-bot: I haz the power, 2016-10-04 23:32:51]

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

[commit-bot: I haz the power, 2016-10-04 23:32:52]

Dry run: This issue passed the CQ dry run.

[bshe, 2016-10-04 23:36:42]

The CQ bit was checked by bshe@chromium.org

[commit-bot: I haz the power, 2016-10-04 23:37:27]

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-10-04 23:37:28]

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

[commit-bot: I haz the power, 2016-10-04 23:37:29]

No L-G-T-M from a valid reviewer yet. 
CQ run can only be started by full committers or once the patch has
received an L-G-T-M from a full committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[bshe, 2016-10-04 23:39:30]

Description was changed from

==========
Reland gvr upgrade

The original CL is here: https://codereview.chromium.org/2367673002/
It was reverted because of compile failure with gn flags:
is_debug=false
is_component_build=true
target_os="android"
The reason is gvr uses API that only available in Android N SDK. For
release build, proguard detected the dependancy and results a compile
error. N SDK is in the process of rolling to chromium
https://codereview.chromium.org/2350013004/
In this CL, I restricted WebVr to debug build only. This could unblock
us. And I will remove the is_debug restriction once N is in Chromium


BUG=389343
tbr=dtrainor
==========

to

==========
Reland gvr upgrade

The original CL is here: https://codereview.chromium.org/2367673002/
It was reverted because of compile failure with gn flags:
is_debug=false
is_component_build=true
target_os="android"
The reason is gvr uses API that only available in Android N SDK. For
release build, proguard detected the dependancy and results a compile
error. N SDK is in the process of rolling to chromium
https://codereview.chromium.org/2350013004/
In this CL, I restricted WebVr to debug build only. This could unblock
us. And I will remove the is_debug restriction once N is in Chromium


BUG=389343
TBR=dtrainor@
==========

[bshe, 2016-10-04 23:39:45]

The CQ bit was checked by bshe@chromium.org

[commit-bot: I haz the power, 2016-10-04 23:40:05]

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-10-04 23:46:51]

Description was changed from

==========
Reland gvr upgrade

The original CL is here: https://codereview.chromium.org/2367673002/
It was reverted because of compile failure with gn flags:
is_debug=false
is_component_build=true
target_os="android"
The reason is gvr uses API that only available in Android N SDK. For
release build, proguard detected the dependancy and results a compile
error. N SDK is in the process of rolling to chromium
https://codereview.chromium.org/2350013004/
In this CL, I restricted WebVr to debug build only. This could unblock
us. And I will remove the is_debug restriction once N is in Chromium


BUG=389343
TBR=dtrainor@
==========

to

==========
Reland gvr upgrade

The original CL is here: https://codereview.chromium.org/2367673002/
It was reverted because of compile failure with gn flags:
is_debug=false
is_component_build=true
target_os="android"
The reason is gvr uses API that only available in Android N SDK. For
release build, proguard detected the dependancy and results a compile
error. N SDK is in the process of rolling to chromium
https://codereview.chromium.org/2350013004/
In this CL, I restricted WebVr to debug build only. This could unblock
us. And I will remove the is_debug restriction once N is in Chromium


BUG=389343
TBR=dtrainor@
==========

[commit-bot: I haz the power, 2016-10-04 23:46:53]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-10-04 23:50:35]

Description was changed from

==========
Reland gvr upgrade

The original CL is here: https://codereview.chromium.org/2367673002/
It was reverted because of compile failure with gn flags:
is_debug=false
is_component_build=true
target_os="android"
The reason is gvr uses API that only available in Android N SDK. For
release build, proguard detected the dependancy and results a compile
error. N SDK is in the process of rolling to chromium
https://codereview.chromium.org/2350013004/
In this CL, I restricted WebVr to debug build only. This could unblock
us. And I will remove the is_debug restriction once N is in Chromium


BUG=389343
TBR=dtrainor@
==========

to

==========
Reland gvr upgrade

The original CL is here: https://codereview.chromium.org/2367673002/
It was reverted because of compile failure with gn flags:
is_debug=false
is_component_build=true
target_os="android"
The reason is gvr uses API that only available in Android N SDK. For
release build, proguard detected the dependancy and results a compile
error. N SDK is in the process of rolling to chromium
https://codereview.chromium.org/2350013004/
In this CL, I restricted WebVr to debug build only. This could unblock
us. And I will remove the is_debug restriction once N is in Chromium

BUG=389343
TBR=dtrainor@

Committed: https://crrev.com/511e7f727be80e20d41fc5fd07cf8b5244cafbf0
Cr-Commit-Position: refs/heads/master@{#423005}
==========

[commit-bot: I haz the power, 2016-10-04 23:50:36]

Patchset 1 (id:??) landed as
https://crrev.com/511e7f727be80e20d41fc5fd07cf8b5244cafbf0
Cr-Commit-Position: refs/heads/master@{#423005}

[brettw, 2016-10-14 19:58:18]

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

[brettw, 2016-10-14 19:58:18]

You landed this change to the root build configuration files TBR without
actually listing a reviewer or sending mail. This kind of change needs proper
review.

[brettw, 2016-10-14 20:01:20]

https://codereview.chromium.org/2396663002/diff/1/build/config/features.gni
File build/config/features.gni (right):

https://codereview.chromium.org/2396663002/diff/1/build/config/features.gni#n...
build/config/features.gni:120: enable_webvr = is_android && is_component_build
&& is_debug &&
In addition to the lack of review, please update the comment to reference why
this is debug only, which makes no sense without reading the CL description.

[bshe, 2016-10-14 21:12:39]

I tbred the author who originally reverted the CL and confirmed with him
offline. Do you mean I should also add an owner for features.gni file? Since the
change only touches webvr, I thought this is a relatively trivial and simple
change that is safe to tbr

https://codereview.chromium.org/2396663002/diff/1/build/config/features.gni
File build/config/features.gni (right):

https://codereview.chromium.org/2396663002/diff/1/build/config/features.gni#n...
build/config/features.gni:120: enable_webvr = is_android && is_component_build
&& is_debug &&
On 2016/10/14 20:01:20, brettw (ping on IM after 24h) wrote:
> In addition to the lack of review, please update the comment to reference why
> this is debug only, which makes no sense without reading the CL description.

will do in a new CL.

[brettw, 2016-10-14 22:26:09]

On 2016/10/14 21:12:39, bshe wrote:
> I tbred the author who originally reverted the CL and confirmed with him
> offline. Do you mean I should also add an owner for features.gni file? Since
the
> change only touches webvr, I thought this is a relatively trivial and simple
> change that is safe to tbr

The original change did not have a change in this file so the build/config
change was not a "reland."

"TBR" means "To Be Reviewed" not "Skip review". You still need to add a reviewer
in Reitveld, send mail, and make sure you have the right reviewers. Even if you
had done a traditional review with the original author, that author is not an
owner of the build/config directory and would have been insufficient.

[bshe, 2016-10-14 22:39:10]

On 2016/10/14 22:26:09, brettw (ping on IM after 24h) wrote:
> On 2016/10/14 21:12:39, bshe wrote:
> > I tbred the author who originally reverted the CL and confirmed with him
> > offline. Do you mean I should also add an owner for features.gni file? Since
> the
> > change only touches webvr, I thought this is a relatively trivial and simple
> > change that is safe to tbr
> 
> The original change did not have a change in this file so the build/config
> change was not a "reland."
> 
> "TBR" means "To Be Reviewed" not "Skip review". You still need to add a
reviewer
> in Reitveld, send mail, and make sure you have the right reviewers. Even if
you
> had done a traditional review with the original author, that author is not an
> owner of the build/config directory and would have been insufficient.

Ack. Thanks for clarification!
It looks like N sdk is now in Chromium a few hours ago. I will test it locally
and remove is_debug
check.

[brettw, 2016-10-14 22:46:55]

Great, thanks!