Move ChromiumMultiDex to BuildConfig.

Also adds debug build flag.

BUG=583690
Committed: https://crrev.com/b0d6314305ddd99c0808a0a41e90b7feb89a79ea
Cr-Commit-Position: refs/heads/master@{#375666}

[Peter Wen, 2016-01-18 16:46:09]

wnwen@chromium.org changed reviewers:
+ yfriedman@chromium.org

[Peter Wen, 2016-01-18 16:46:09]

Hi Yaron,

Is this the type of crash-guard that you were thinking of?

cc'ing Benoit as he recently worked on RecordHistogram.java and initialization.

Peter

[Peter Wen, 2016-01-18 16:50:22]

On 2016/01/18 16:46:09, Peter Wen wrote:
> Hi Yaron,
> 
> Is this the type of crash-guard that you were thinking of?
> 
> cc'ing Benoit as he recently worked on RecordHistogram.java and
initialization.
> 
> Peter

Forgot that RecordHistogram is in base, so can't depend on browser's
ChromeVersionInfo stuff. :-/

[Yaron, 2016-01-18 17:51:10]

https://codereview.chromium.org/1597273005/diff/1/base/android/java/src/org/c...
File base/android/java/src/org/chromium/base/metrics/RecordHistogram.java
(right):

https://codereview.chromium.org/1597273005/diff/1/base/android/java/src/org/c...
base/android/java/src/org/chromium/base/metrics/RecordHistogram.java:10: import
org.chromium.chrome.browser.ChromeVersionInfo;
Unfortunately this dep won't work because you're in base.

https://codereview.chromium.org/1597273005/diff/1/base/android/java/src/org/c...
base/android/java/src/org/chromium/base/metrics/RecordHistogram.java:32:
Log.w(TAG, "Calling RecordHistogram before native is loaded.");
Can you make the histogram name a parameter to this function and include it
here? For the exception case, you'd know what went wrong but if we see this for
a non-local build, you can't tell what metric is dropped.

[Peter Wen, 2016-01-18 17:55:35]

On 2016/01/18 17:51:10, Yaron wrote:
>
https://codereview.chromium.org/1597273005/diff/1/base/android/java/src/org/c...
> File base/android/java/src/org/chromium/base/metrics/RecordHistogram.java
> (right):
> 
>
https://codereview.chromium.org/1597273005/diff/1/base/android/java/src/org/c...
> base/android/java/src/org/chromium/base/metrics/RecordHistogram.java:10:
import
> org.chromium.chrome.browser.ChromeVersionInfo;
> Unfortunately this dep won't work because you're in base.
> 
>
https://codereview.chromium.org/1597273005/diff/1/base/android/java/src/org/c...
> base/android/java/src/org/chromium/base/metrics/RecordHistogram.java:32:
> Log.w(TAG, "Calling RecordHistogram before native is loaded.");
> Can you make the histogram name a parameter to this function and include it
> here? For the exception case, you'd know what went wrong but if we see this
for
> a non-local build, you can't tell what metric is dropped.

Hmm... how would we distinguish builds without access to ChromeVersionInfo? Is
there something more fundamental in base/?

[Yaron, 2016-01-18 17:56:27]

On 2016/01/18 17:55:35, Peter Wen wrote:
> On 2016/01/18 17:51:10, Yaron wrote:
> >
>
https://codereview.chromium.org/1597273005/diff/1/base/android/java/src/org/c...
> > File base/android/java/src/org/chromium/base/metrics/RecordHistogram.java
> > (right):
> > 
> >
>
https://codereview.chromium.org/1597273005/diff/1/base/android/java/src/org/c...
> > base/android/java/src/org/chromium/base/metrics/RecordHistogram.java:10:
> import
> > org.chromium.chrome.browser.ChromeVersionInfo;
> > Unfortunately this dep won't work because you're in base.
> > 
> >
>
https://codereview.chromium.org/1597273005/diff/1/base/android/java/src/org/c...
> > base/android/java/src/org/chromium/base/metrics/RecordHistogram.java:32:
> > Log.w(TAG, "Calling RecordHistogram before native is loaded.");
> > Can you make the histogram name a parameter to this function and include it
> > here? For the exception case, you'd know what went wrong but if we see this
> for
> > a non-local build, you can't tell what metric is dropped.
> 
> Hmm... how would we distinguish builds without access to ChromeVersionInfo? Is
> there something more fundamental in base/?

You could do debug vs release

[Peter Wen, 2016-01-18 22:18:08]

Switched to using @RemovableInRelease. Tested locally with debug/release builds
and works great. Exception in debug and log warning in release.

Now testing on trybots (dbg and rel). Will remove trigger line in
TabPersistentStore when done.

Thanks, PTAL.

https://codereview.chromium.org/1597273005/diff/1/base/android/java/src/org/c...
File base/android/java/src/org/chromium/base/metrics/RecordHistogram.java
(right):

https://codereview.chromium.org/1597273005/diff/1/base/android/java/src/org/c...
base/android/java/src/org/chromium/base/metrics/RecordHistogram.java:10: import
org.chromium.chrome.browser.ChromeVersionInfo;
On 2016/01/18 17:51:10, Yaron wrote:
> Unfortunately this dep won't work because you're in base.

Acknowledged.

https://codereview.chromium.org/1597273005/diff/1/base/android/java/src/org/c...
base/android/java/src/org/chromium/base/metrics/RecordHistogram.java:32:
Log.w(TAG, "Calling RecordHistogram before native is loaded.");
On 2016/01/18 17:51:10, Yaron wrote:
> Can you make the histogram name a parameter to this function and include it
> here? For the exception case, you'd know what went wrong but if we see this
for
> a non-local build, you can't tell what metric is dropped.

Done.

[Yaron, 2016-01-19 15:57:27]

lgtm when that's gone (assuming trybots go green with it)

https://codereview.chromium.org/1597273005/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabPersistentStore.java
(right):

https://codereview.chromium.org/1597273005/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabPersistentStore.java:166:
logExecutionTime("DO NOT SUBMIT", 10);
remove

[Peter Wen, 2016-01-19 19:58:04]

On 2016/01/19 15:57:27, Yaron wrote:
> lgtm when that's gone (assuming trybots go green with it)
> 
>
https://codereview.chromium.org/1597273005/diff/20001/chrome/android/java/src...
> File
>
chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabPersistentStore.java
> (right):
> 
>
https://codereview.chromium.org/1597273005/diff/20001/chrome/android/java/src...
>
chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabPersistentStore.java:166:
> logExecutionTime("DO NOT SUBMIT", 10);
> remove

Currently webview and content shell do not optimize out @RemovableInRelease
annotated methods. As a result of adding this check, found a stat that's
recorded before initialization, http://crbug.com/579188.

Looking for a way to either distinguish between chrome and non-chrome builds or
better distinguish debug/release builds for all.

[Peter Wen, 2016-01-20 20:14:03]

Hi Yaron,

I've found a more comprehensive way of using .template to match java's debug
definition with native's. We can build java-based DCHECKs with this.

Please take another look.

Peter

https://codereview.chromium.org/1597273005/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabPersistentStore.java
(right):

https://codereview.chromium.org/1597273005/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabPersistentStore.java:166:
logExecutionTime("DO NOT SUBMIT", 10);
On 2016/01/19 15:57:26, Yaron wrote:
> remove

Done.

[Peter Wen, 2016-01-20 22:15:40]

wnwen@chromium.org changed reviewers:
+ jbudorick@chromium.org

[Peter Wen, 2016-01-20 22:15:40]

Fixed as discussed offline.

Adding jbudorick@ for multidex and build change.

[jbudorick, 2016-01-21 01:51:15]

https://codereview.chromium.org/1597273005/diff/120001/base/android/java/src/...
File base/android/java/src/org/chromium/base/metrics/RecordHistogram.java
(right):

https://codereview.chromium.org/1597273005/diff/120001/base/android/java/src/...
base/android/java/src/org/chromium/base/metrics/RecordHistogram.java:29: private
static void raiseExceptionInDebug(String name) {
This no longer appears to be called.

https://codereview.chromium.org/1597273005/diff/120001/build/config/android/r...
File build/config/android/rules.gni (right):

https://codereview.chromium.org/1597273005/diff/120001/build/config/android/r...
build/config/android/rules.gni:1488:
java_cpp_template("${_template_name}__multidex_config_java") {
This name should change.

https://codereview.chromium.org/1597273005/diff/120001/build/java_apk.gypi
File build/java_apk.gypi (right):

https://codereview.chromium.org/1597273005/diff/120001/build/java_apk.gypi#ne...
build/java_apk.gypi:126: 'multidex_config_template':
'<(DEPTH)/base/android/java/templates/ChromiumBuildConfig.template',
As should these five -- "multidex_config" should be replaced by "build_config"
or something along those lines.

[Peter Wen, 2016-01-21 15:18:24]

Thanks for the quick review, PTAL.

https://codereview.chromium.org/1597273005/diff/120001/build/config/android/r...
File build/config/android/rules.gni (right):

https://codereview.chromium.org/1597273005/diff/120001/build/config/android/r...
build/config/android/rules.gni:1488:
java_cpp_template("${_template_name}__multidex_config_java") {
On 2016/01/21 01:51:15, jbudorick wrote:
> This name should change.

Done. Also switched this to always build, even under test.

https://codereview.chromium.org/1597273005/diff/120001/build/java_apk.gypi
File build/java_apk.gypi (right):

https://codereview.chromium.org/1597273005/diff/120001/build/java_apk.gypi#ne...
build/java_apk.gypi:126: 'multidex_config_template':
'<(DEPTH)/base/android/java/templates/ChromiumBuildConfig.template',
On 2016/01/21 01:51:15, jbudorick wrote:
> As should these five -- "multidex_config" should be replaced by "build_config"
> or something along those lines.

Done, replaced with chromium_build_config for easier grepping than build_config
alone.

[Peter Wen, 2016-01-21 20:27:18]

I am no longer planning to change RecordHistogram since
RecordHistogram.initialize() is not actually required, since native calls the
underlying initialize function by itself, the java function is just a shortcut
for preloading.

Is it still valuable to hijack this CL to rename ChromiumMultiDex to
ChromiumBuildConfig and add in a debug flag so we can do DCHECKs in java?

If not I can just close this CL. Thanks for all your help!

Peter

[Yaron, 2016-02-02 22:44:00]

On 2016/01/21 20:27:18, Peter Wen wrote:
> I am no longer planning to change RecordHistogram since
> RecordHistogram.initialize() is not actually required, since native calls the
> underlying initialize function by itself, the java function is just a shortcut
> for preloading.
> 
> Is it still valuable to hijack this CL to rename ChromiumMultiDex to
> ChromiumBuildConfig and add in a debug flag so we can do DCHECKs in java?
> 
> If not I can just close this CL. Thanks for all your help!
> 
> Peter

Seems it could be since it just came up elsewhere: crbug/581927

[Peter Wen, 2016-02-03 17:46:52]

Description was changed from

==========
Add native initialization guard to histogram calls.

UMA histogram calls should fail with logcat only on official builds.

BUG=578845
==========

to

==========
Adds debug build flag.

BUG=583690
==========

[Peter Wen, 2016-02-08 16:24:48]

@jbudorick - PTAL, this CL now just renames ChromiumMultidex and adds a debug
flag for java. :)

[Peter Wen, 2016-02-10 21:41:01]

Friendly ping. :)

[jbudorick, 2016-02-10 21:45:46]

https://codereview.chromium.org/1597273005/diff/220001/build/config/android/r...
File build/config/android/rules.gni (left):

https://codereview.chromium.org/1597273005/diff/220001/build/config/android/r...
build/config/android/rules.gni:1487: if (!defined(invoker.apk_under_test)) {
Removing this condition makes me worry that we're going to hit duplicate
definition of ChromiumBuildConfig between APK under test + test APK. I saw this
manifest as a crash w/ "class resolved to unexpected dex" or something along
those lines appearing in the logcat.

[jbudorick, 2016-02-10 21:46:18]

On 2016/02/10 21:41:01, Peter Wen wrote:
> Friendly ping. :)

sorry about the delay, lost this in the inbox

[Peter Wen, 2016-02-10 21:56:48]

Thanks! :)

https://codereview.chromium.org/1597273005/diff/220001/build/config/android/r...
File build/config/android/rules.gni (left):

https://codereview.chromium.org/1597273005/diff/220001/build/config/android/r...
build/config/android/rules.gni:1487: if (!defined(invoker.apk_under_test)) {
On 2016/02/10 21:45:46, jbudorick wrote:
> Removing this condition makes me worry that we're going to hit duplicate
> definition of ChromiumBuildConfig between APK under test + test APK. I saw
this
> manifest as a crash w/ "class resolved to unexpected dex" or something along
> those lines appearing in the logcat.

Makes sense. I'll check the logcat again after adding back this check.

Do we have a bot building multidex? Or is there a way for me to make sure I
don't break multidex locally?

[jbudorick, 2016-02-10 22:00:44]

On 2016/02/10 21:56:48, Peter Wen wrote:
> Thanks! :)
> 
>
https://codereview.chromium.org/1597273005/diff/220001/build/config/android/r...
> File build/config/android/rules.gni (left):
> 
>
https://codereview.chromium.org/1597273005/diff/220001/build/config/android/r...
> build/config/android/rules.gni:1487: if (!defined(invoker.apk_under_test)) {
> On 2016/02/10 21:45:46, jbudorick wrote:
> > Removing this condition makes me worry that we're going to hit duplicate
> > definition of ChromiumBuildConfig between APK under test + test APK. I saw
> this
> > manifest as a crash w/ "class resolved to unexpected dex" or something along
> > those lines appearing in the logcat.
> 
> Makes sense. I'll check the logcat again after adding back this check.
> 
> Do we have a bot building multidex? Or is there a way for me to make sure I
> don't break multidex locally?

Debug builds of chrome_public_apk and chrome_apk are multidex. There are several
bots that do this.

You'd want to build debug and run either the ChromePublic tests or the Chrome
tests to see if it breaks.

[jbudorick, 2016-02-10 22:38:34]

On 2016/02/10 22:00:44, jbudorick wrote:
> On 2016/02/10 21:56:48, Peter Wen wrote:
> > Thanks! :)
> > 
> >
>
https://codereview.chromium.org/1597273005/diff/220001/build/config/android/r...
> > File build/config/android/rules.gni (left):
> > 
> >
>
https://codereview.chromium.org/1597273005/diff/220001/build/config/android/r...
> > build/config/android/rules.gni:1487: if (!defined(invoker.apk_under_test)) {
> > On 2016/02/10 21:45:46, jbudorick wrote:
> > > Removing this condition makes me worry that we're going to hit duplicate
> > > definition of ChromiumBuildConfig between APK under test + test APK. I saw
> > this
> > > manifest as a crash w/ "class resolved to unexpected dex" or something
along
> > > those lines appearing in the logcat.
> > 
> > Makes sense. I'll check the logcat again after adding back this check.
> > 
> > Do we have a bot building multidex? Or is there a way for me to make sure I
> > don't break multidex locally?
> 
> Debug builds of chrome_public_apk and chrome_apk are multidex. There are
several
> bots that do this.
> 
> You'd want to build debug and run either the ChromePublic tests or the Chrome
> tests to see if it breaks.

Also, linux_android_dbg_ng will cover this case, though it's not in the default
set of try bots and has _very_ limited capacity. I've kicked off a run.

[Peter Wen, 2016-02-10 23:21:03]

On 2016/02/10 22:38:34, jbudorick wrote:
> On 2016/02/10 22:00:44, jbudorick wrote:
> > Debug builds of chrome_public_apk and chrome_apk are multidex. There are
> several
> > bots that do this.
> > 
> > You'd want to build debug and run either the ChromePublic tests or the
Chrome
> > tests to see if it breaks.
> 
> Also, linux_android_dbg_ng will cover this case, though it's not in the
default
> set of try bots and has _very_ limited capacity. I've kicked off a run.

Thanks! My local instrumentation build doesn't seem to want to run (likely due
to the android version). I'll flash M and test locally tomorrow if issues come
up when the buildbot finishes.

I already kicked off a linux_android_dbg_ng build so I've stopped the newer one.
Thanks for the reminder! :)

Seems it's fine compiling on debug and ChromePublic instrumentation tests
already passes:
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...

[jbudorick, 2016-02-11 01:54:59]

On 2016/02/10 23:21:03, Peter Wen wrote:
> On 2016/02/10 22:38:34, jbudorick wrote:
> > On 2016/02/10 22:00:44, jbudorick wrote:
> > > Debug builds of chrome_public_apk and chrome_apk are multidex. There are
> > several
> > > bots that do this.
> > > 
> > > You'd want to build debug and run either the ChromePublic tests or the
> Chrome
> > > tests to see if it breaks.
> > 
> > Also, linux_android_dbg_ng will cover this case, though it's not in the
> default
> > set of try bots and has _very_ limited capacity. I've kicked off a run.
> 
> Thanks! My local instrumentation build doesn't seem to want to run (likely due
> to the android version). I'll flash M and test locally tomorrow if issues come
> up when the buildbot finishes.

Let me know how this goes. I might give this a spin locally as well.

> 
> I already kicked off a linux_android_dbg_ng build so I've stopped the newer
one.
> Thanks for the reminder! :)
> 

Oh, I missed that. Perfect.

> Seems it's fine compiling on debug and ChromePublic instrumentation tests
> already passes:
>
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...

[Peter Wen, 2016-02-11 20:39:27]

On 2016/02/11 01:54:59, jbudorick wrote:
> On 2016/02/10 23:21:03, Peter Wen wrote:
> > Thanks! My local instrumentation build doesn't seem to want to run (likely
due
> > to the android version). I'll flash M and test locally tomorrow if issues
come
> > up when the buildbot finishes.
> 
> Let me know how this goes. I might give this a spin locally as well.

The debug buildbot (and all others) are green. My local tests are progressing
fine, it'll take a long while before they finish.

Let me know if there's anything else I should do.

[jbudorick, 2016-02-11 20:41:53]

On 2016/02/11 20:39:27, Peter Wen wrote:
> On 2016/02/11 01:54:59, jbudorick wrote:
> > On 2016/02/10 23:21:03, Peter Wen wrote:
> > > Thanks! My local instrumentation build doesn't seem to want to run (likely
> due
> > > to the android version). I'll flash M and test locally tomorrow if issues
> come
> > > up when the buildbot finishes.
> > 
> > Let me know how this goes. I might give this a spin locally as well.
> 
> The debug buildbot (and all others) are green. My local tests are progressing
> fine, it'll take a long while before they finish.
> 
> Let me know if there's anything else I should do.

Not that I can think of. lgtm

[Yaron, 2016-02-11 20:44:39]

On 2016/02/11 20:41:53, jbudorick wrote:
> On 2016/02/11 20:39:27, Peter Wen wrote:
> > On 2016/02/11 01:54:59, jbudorick wrote:
> > > On 2016/02/10 23:21:03, Peter Wen wrote:
> > > > Thanks! My local instrumentation build doesn't seem to want to run
(likely
> > due
> > > > to the android version). I'll flash M and test locally tomorrow if
issues
> > come
> > > > up when the buildbot finishes.
> > > 
> > > Let me know how this goes. I might give this a spin locally as well.
> > 
> > The debug buildbot (and all others) are green. My local tests are
progressing
> > fine, it'll take a long while before they finish.
> > 
> > Let me know if there's anything else I should do.
> 
> Not that I can think of. lgtm

Could we rename it to BuildConfig? Is there a reason to have "Chromium" in the
name?

[jbudorick, 2016-02-11 20:47:04]

On 2016/02/11 20:44:39, Yaron wrote:
> On 2016/02/11 20:41:53, jbudorick wrote:
> > On 2016/02/11 20:39:27, Peter Wen wrote:
> > > On 2016/02/11 01:54:59, jbudorick wrote:
> > > > On 2016/02/10 23:21:03, Peter Wen wrote:
> > > > > Thanks! My local instrumentation build doesn't seem to want to run
> (likely
> > > due
> > > > > to the android version). I'll flash M and test locally tomorrow if
> issues
> > > come
> > > > > up when the buildbot finishes.
> > > > 
> > > > Let me know how this goes. I might give this a spin locally as well.
> > > 
> > > The debug buildbot (and all others) are green. My local tests are
> progressing
> > > fine, it'll take a long while before they finish.
> > > 
> > > Let me know if there's anything else I should do.
> > 
> > Not that I can think of. lgtm
> 
> Could we rename it to BuildConfig? Is there a reason to have "Chromium" in the
> name?

The only one I can think of would be to avoid name conflicts with the
(generated) android class of the same name, though I'm not sure if that's part
of the public API. (That was why it was "ChromiumMultiDex" instead of
"MultiDex")

[Yaron, 2016-02-11 20:55:25]

On 2016/02/11 20:47:04, jbudorick wrote:
> On 2016/02/11 20:44:39, Yaron wrote:
> > On 2016/02/11 20:41:53, jbudorick wrote:
> > > On 2016/02/11 20:39:27, Peter Wen wrote:
> > > > On 2016/02/11 01:54:59, jbudorick wrote:
> > > > > On 2016/02/10 23:21:03, Peter Wen wrote:
> > > > > > Thanks! My local instrumentation build doesn't seem to want to run
> > (likely
> > > > due
> > > > > > to the android version). I'll flash M and test locally tomorrow if
> > issues
> > > > come
> > > > > > up when the buildbot finishes.
> > > > > 
> > > > > Let me know how this goes. I might give this a spin locally as well.
> > > > 
> > > > The debug buildbot (and all others) are green. My local tests are
> > progressing
> > > > fine, it'll take a long while before they finish.
> > > > 
> > > > Let me know if there's anything else I should do.
> > > 
> > > Not that I can think of. lgtm
> > 
> > Could we rename it to BuildConfig? Is there a reason to have "Chromium" in
the
> > name?
> 
> The only one I can think of would be to avoid name conflicts with the
> (generated) android class of the same name, though I'm not sure if that's part
> of the public API. (That was why it was "ChromiumMultiDex" instead of
> "MultiDex")

Not aware of it in Android - I was thinking this could be like Android Studio
which has a BuildConfig class (based on your discussion with Andrew in the other
thread). That is unless we someday support Android Studio ? :)

[jbudorick, 2016-02-11 20:58:30]

On 2016/02/11 20:55:25, Yaron wrote:
> On 2016/02/11 20:47:04, jbudorick wrote:
> > On 2016/02/11 20:44:39, Yaron wrote:
> > > On 2016/02/11 20:41:53, jbudorick wrote:
> > > > On 2016/02/11 20:39:27, Peter Wen wrote:
> > > > > On 2016/02/11 01:54:59, jbudorick wrote:
> > > > > > On 2016/02/10 23:21:03, Peter Wen wrote:
> > > > > > > Thanks! My local instrumentation build doesn't seem to want to run
> > > (likely
> > > > > due
> > > > > > > to the android version). I'll flash M and test locally tomorrow if
> > > issues
> > > > > come
> > > > > > > up when the buildbot finishes.
> > > > > > 
> > > > > > Let me know how this goes. I might give this a spin locally as well.
> > > > > 
> > > > > The debug buildbot (and all others) are green. My local tests are
> > > progressing
> > > > > fine, it'll take a long while before they finish.
> > > > > 
> > > > > Let me know if there's anything else I should do.
> > > > 
> > > > Not that I can think of. lgtm
> > > 
> > > Could we rename it to BuildConfig? Is there a reason to have "Chromium" in
> the
> > > name?
> > 
> > The only one I can think of would be to avoid name conflicts with the
> > (generated) android class of the same name, though I'm not sure if that's
part
> > of the public API. (That was why it was "ChromiumMultiDex" instead of
> > "MultiDex")
> 
> Not aware of it in Android - I was thinking this could be like Android Studio
> which has a BuildConfig class (based on your discussion with Andrew in the
other
> thread). That is unless we someday support Android Studio ? :)

Oh, yeah, maybe I'm mixing the two up.

I think Studio may be a bit far off :)

[Peter Wen, 2016-02-11 21:01:07]

Description was changed from

==========
Adds debug build flag.

BUG=583690
==========

to

==========
Also adds debug build flag.

BUG=583690
==========

[Peter Wen, 2016-02-11 21:17:57]

wnwen@chromium.org changed reviewers:
+ thestig@chromium.org

[Peter Wen, 2016-02-11 21:17:58]

+thestig@ for OWNERS base/BUILD.gn and base/base.gyp.

[Lei Zhang, 2016-02-11 21:47:51]

On 2016/02/11 21:17:58, Peter Wen wrote:
> +thestig@ for OWNERS base/BUILD.gn and base/base.gyp.

lgtm

[Peter Wen, 2016-02-11 22:48:20]

The CQ bit was checked by wnwen@chromium.org

[Peter Wen, 2016-02-11 22:48:21]

The patchset sent to the CQ was uploaded after l-g-t-m from
yfriedman@chromium.org, jbudorick@chromium.org, thestig@chromium.org
Link to the patchset: https://codereview.chromium.org/1597273005/#ps280001
(title: "Add back missing template.")

[commit-bot: I haz the power, 2016-02-11 22:50:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1597273005/280001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1597273005/280001

[Peter Wen, 2016-02-12 00:01:06]

The CQ bit was unchecked by wnwen@chromium.org

[Peter Wen, 2016-02-16 20:33:27]

The CQ bit was checked by wnwen@chromium.org

[Peter Wen, 2016-02-16 20:33:27]

The patchset sent to the CQ was uploaded after l-g-t-m from
thestig@chromium.org, yfriedman@chromium.org, jbudorick@chromium.org
Link to the patchset: https://codereview.chromium.org/1597273005/#ps300001
(title: "Rebase.")

[commit-bot: I haz the power, 2016-02-16 20:35:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1597273005/300001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1597273005/300001

[commit-bot: I haz the power, 2016-02-16 21:29:59]

Description was changed from

==========
Also adds debug build flag.

BUG=583690
==========

to

==========
Also adds debug build flag.

BUG=583690
==========

[commit-bot: I haz the power, 2016-02-16 21:30:00]

Committed patchset #16 (id:300001)

[commit-bot: I haz the power, 2016-02-16 22:55:01]

Description was changed from

==========
Also adds debug build flag.

BUG=583690
==========

to

==========
Also adds debug build flag.

BUG=583690

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

[commit-bot: I haz the power, 2016-02-16 22:55:02]

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