Activate prepopulated fieldtrials

Activate prepopulated fieldtrials

BUG=504075
Committed: https://crrev.com/907bad2715b5ee5bbdabf5b6743ab04ae8f2a187
Cr-Commit-Position: refs/heads/master@{#339092}

[danduong, 2015-06-24 23:32:05]

danduong@chromium.org changed reviewers:
+ asvitkine@chromium.org

[Alexei Svitkine (slow), 2015-07-08 17:10:17]

https://codereview.chromium.org/1200383005/diff/40001/chrome/common/variation...
File chrome/common/variations/variations_util.cc (right):

https://codereview.chromium.org/1200383005/diff/40001/chrome/common/variation...
chrome/common/variations/variations_util.cc:68: bool
AssociateParamsFromFieldTrialConfig() {
Can you add a test?

Maybe make a helper function that takes the struct as a param, so that it can be
tested with mock data. The test can then verify whether params and field trials
have been registered based on the contents of the struct.

https://codereview.chromium.org/1200383005/diff/40001/chrome/common/variation...
chrome/common/variations/variations_util.cc:71: if (group.params_size) {
Nit: Since this is a number, explicitly != 0 for clarity.

[danduong, 2015-07-09 21:35:54]

https://codereview.chromium.org/1200383005/diff/40001/chrome/common/variation...
File chrome/common/variations/variations_util.cc (right):

https://codereview.chromium.org/1200383005/diff/40001/chrome/common/variation...
chrome/common/variations/variations_util.cc:68: bool
AssociateParamsFromFieldTrialConfig() {
On 2015/07/08 at 17:10:17, Alexei Svitkine wrote:
> Can you add a test?
> 
> Maybe make a helper function that takes the struct as a param, so that it can
be tested with mock data. The test can then verify whether params and field
trials have been registered based on the contents of the struct.

Done.

https://codereview.chromium.org/1200383005/diff/40001/chrome/common/variation...
chrome/common/variations/variations_util.cc:71: if (group.params_size) {
On 2015/07/08 at 17:10:17, Alexei Svitkine wrote:
> Nit: Since this is a number, explicitly != 0 for clarity.

Done.

[danduong, 2015-07-09 21:59:44]

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

[danduong, 2015-07-09 21:59:44]

brettw for chrome_browser_main.

[Alexei Svitkine (slow), 2015-07-10 15:24:32]

https://codereview.chromium.org/1200383005/diff/80001/chrome/browser/chrome_b...
File chrome/browser/chrome_browser_main.cc (right):

https://codereview.chromium.org/1200383005/diff/80001/chrome/browser/chrome_b...
chrome/browser/chrome_browser_main.cc:637: if
(command_line->HasSwitch(switches::kEnableFieldTrialTestingConfig)) {
Didn't we want this to be on by default for Chromium builds?

Should we just not have a command-line flag and instead do this unconditionally
inside:

#if !defined(GOOGLE_CHROME_BUILD)

Or maybe I don't have the full context. How were you thinking that this would
work?

https://codereview.chromium.org/1200383005/diff/80001/chrome/common/variation...
File chrome/common/variations/variations_util.h (right):

https://codereview.chromium.org/1200383005/diff/80001/chrome/common/variation...
chrome/common/variations/variations_util.h:24: bool
AssociateParamsFromFieldTrialConfig(const FieldTrialTestingConfig& config);
Put this in an chrome_variations::internal namespace. Add a comment.

https://codereview.chromium.org/1200383005/diff/80001/chrome/common/variation...
chrome/common/variations/variations_util.h:25: bool
AssociateParamsFromFieldTrialConfig();
Add a comment. Name this differently than the other functions - overloads are
generally discouraged. Maybe something like:
AssociateParamsFromDefaultFieldTrialConfig().

[danduong, 2015-07-10 16:24:20]

https://codereview.chromium.org/1200383005/diff/80001/chrome/browser/chrome_b...
File chrome/browser/chrome_browser_main.cc (right):

https://codereview.chromium.org/1200383005/diff/80001/chrome/browser/chrome_b...
chrome/browser/chrome_browser_main.cc:637: if
(command_line->HasSwitch(switches::kEnableFieldTrialTestingConfig)) {
On 2015/07/10 at 15:24:32, Alexei Svitkine wrote:
> Didn't we want this to be on by default for Chromium builds?
> 
> Should we just not have a command-line flag and instead do this
unconditionally inside:
> 
> #if !defined(GOOGLE_CHROME_BUILD)
> 
> Or maybe I don't have the full context. How were you thinking that this would
work?

This is a good topic of discussion. My first thought is to just have it default
enabled on a non-official build, but disable-able from command-line. We'll also
have to be careful on how this effected by the other forcing flags for field
trials. I'm thinking if --force-field-trial is used, always disable this flag.

std::string enable_field_trial_testing =
    command_line->GetSwitchValueASCII(switches::kEnableFieldTrialTestingConfig);
if (enable_field_trial_testing.empty()) {
#if defined(GOOGLE_CHROME_BUILD) && defined(OFFICIAL_BUILD)
  enable_field_trial_testing = "0";
#else
  enable_field_trial_testing = "1";
#endif
}

if (enable_field_trial_testing == '1' &&
!command_line->HasSwitch(switches::kForceFieldTrials) {
 ...
}

[Alexei Svitkine (slow), 2015-07-10 19:05:12]

https://codereview.chromium.org/1200383005/diff/80001/chrome/browser/chrome_b...
File chrome/browser/chrome_browser_main.cc (right):

https://codereview.chromium.org/1200383005/diff/80001/chrome/browser/chrome_b...
chrome/browser/chrome_browser_main.cc:637: if
(command_line->HasSwitch(switches::kEnableFieldTrialTestingConfig)) {
On 2015/07/10 16:24:20, danduong wrote:
> On 2015/07/10 at 15:24:32, Alexei Svitkine wrote:
> > Didn't we want this to be on by default for Chromium builds?
> > 
> > Should we just not have a command-line flag and instead do this
> unconditionally inside:
> > 
> > #if !defined(GOOGLE_CHROME_BUILD)
> > 
> > Or maybe I don't have the full context. How were you thinking that this
would
> work?
> 
> This is a good topic of discussion. My first thought is to just have it
default
> enabled on a non-official build, but disable-able from command-line. We'll
also
> have to be careful on how this effected by the other forcing flags for field
> trials. I'm thinking if --force-field-trial is used, always disable this flag.
> 
> std::string enable_field_trial_testing =
>    
command_line->GetSwitchValueASCII(switches::kEnableFieldTrialTestingConfig);
> if (enable_field_trial_testing.empty()) {
> #if defined(GOOGLE_CHROME_BUILD) && defined(OFFICIAL_BUILD)
>   enable_field_trial_testing = "0";
> #else
>   enable_field_trial_testing = "1";
> #endif
> }
> 
> if (enable_field_trial_testing == '1' &&
> !command_line->HasSwitch(switches::kForceFieldTrials) {
>  ...
> }

Default for unofficial builds but disable-able from the command-line SGTM. Have
you checked whether try bots pass this? (I'm hoping the default configs we have
don't cause any test failures, but it might indeed be the case. :\)

I think we shouldn't make it enable-able for official builds. We should just not
compile it in (i.e. so the whole block should be inside GOOGLE_CHROME_BUILD).
This will let the linker dead-code-eliminate the associated function / struct
and thus we wouldn't be paying any binary size penalty for this in official
builds.

[danduong, 2015-07-10 19:14:42]

On 2015/07/10 at 19:05:12, asvitkine wrote:
>
https://codereview.chromium.org/1200383005/diff/80001/chrome/browser/chrome_b...
> File chrome/browser/chrome_browser_main.cc (right):
> 
>
https://codereview.chromium.org/1200383005/diff/80001/chrome/browser/chrome_b...
> chrome/browser/chrome_browser_main.cc:637: if
(command_line->HasSwitch(switches::kEnableFieldTrialTestingConfig)) {
> On 2015/07/10 16:24:20, danduong wrote:
> > On 2015/07/10 at 15:24:32, Alexei Svitkine wrote:
> > > Didn't we want this to be on by default for Chromium builds?
> > > 
> > > Should we just not have a command-line flag and instead do this
> > unconditionally inside:
> > > 
> > > #if !defined(GOOGLE_CHROME_BUILD)
> > > 
> > > Or maybe I don't have the full context. How were you thinking that this
would
> > work?
> > 
> > This is a good topic of discussion. My first thought is to just have it
default
> > enabled on a non-official build, but disable-able from command-line. We'll
also
> > have to be careful on how this effected by the other forcing flags for field
> > trials. I'm thinking if --force-field-trial is used, always disable this
flag.
> > 
> > std::string enable_field_trial_testing =
> >    
command_line->GetSwitchValueASCII(switches::kEnableFieldTrialTestingConfig);
> > if (enable_field_trial_testing.empty()) {
> > #if defined(GOOGLE_CHROME_BUILD) && defined(OFFICIAL_BUILD)
> >   enable_field_trial_testing = "0";
> > #else
> >   enable_field_trial_testing = "1";
> > #endif
> > }
> > 
> > if (enable_field_trial_testing == '1' &&
> > !command_line->HasSwitch(switches::kForceFieldTrials) {
> >  ...
> > }
> 
> Default for unofficial builds but disable-able from the command-line SGTM.
Have you checked whether try bots pass this? (I'm hoping the default configs we
have don't cause any test failures, but it might indeed be the case. :\)
> 
> I think we shouldn't make it enable-able for official builds. We should just
not compile it in (i.e. so the whole block should be inside
GOOGLE_CHROME_BUILD). This will let the linker dead-code-eliminate the
associated function / struct and thus we wouldn't be paying any binary size
penalty for this in official builds.

Done.

[Alexei Svitkine (slow), 2015-07-10 19:22:30]

https://codereview.chromium.org/1200383005/diff/120001/chrome/browser/chrome_...
File chrome/browser/chrome_browser_main.cc (right):

https://codereview.chromium.org/1200383005/diff/120001/chrome/browser/chrome_...
chrome/browser/chrome_browser_main.cc:637: #if defined(GOOGLE_CHROME_BUILD) &&
defined(OFFICIAL_BUILD)
I think OFFICIAL_BUILD is not needed here. I think that macro is not meant to
change behavior - only control some optimization settings for the build. (Though
there may be existing code that does it wrong).

https://codereview.chromium.org/1200383005/diff/120001/chrome/browser/chrome_...
chrome/browser/chrome_browser_main.cc:639:
switches::kEnableFieldTrialTestingConfig);
Convention for switches that start with "enable_*" is just to test for their
presence.

Since you actually want a disable switch, let's follow the convention for that.

Call the switch kDisableFieldTrialTestingConfig and then simplify the code below
to just be

if (!command_line->HasSwitch(kDisable..) && !command_line->HasSwitch(kForce...))
{
  ...
}

I think the other thing this should be checking for is if the user specified a
custom variations-server-url - since that actually turns on variations service
for unofficial builds - and in that case, we shouldn't use the default config
either.

[danduong, 2015-07-10 19:36:56]

https://codereview.chromium.org/1200383005/diff/120001/chrome/browser/chrome_...
File chrome/browser/chrome_browser_main.cc (right):

https://codereview.chromium.org/1200383005/diff/120001/chrome/browser/chrome_...
chrome/browser/chrome_browser_main.cc:637: #if defined(GOOGLE_CHROME_BUILD) &&
defined(OFFICIAL_BUILD)
On 2015/07/10 at 19:22:30, Alexei Svitkine wrote:
> I think OFFICIAL_BUILD is not needed here. I think that macro is not meant to
change behavior - only control some optimization settings for the build. (Though
there may be existing code that does it wrong).

Hmm... If we were to do this, would I still be able to get field trial config
enabled by default for developers testing Google branded features?

https://codereview.chromium.org/1200383005/diff/120001/chrome/browser/chrome_...
chrome/browser/chrome_browser_main.cc:639:
switches::kEnableFieldTrialTestingConfig);
On 2015/07/10 at 19:22:29, Alexei Svitkine wrote:
> Convention for switches that start with "enable_*" is just to test for their
presence.
> 
> Since you actually want a disable switch, let's follow the convention for
that.
> 
> Call the switch kDisableFieldTrialTestingConfig and then simplify the code
below to just be
> 
> if (!command_line->HasSwitch(kDisable..) &&
!command_line->HasSwitch(kForce...)) {
>   ...
> }
> 
> I think the other thing this should be checking for is if the user specified a
custom variations-server-url - since that actually turns on variations service
for unofficial builds - and in that case, we shouldn't use the default config
either.

I can add a check for the variations server stuff.

I'm a little concerned by the loss of flexibility if I switch it to purely a
disable flag. I know laforge was interested in an easy way for the test team to
enable this configuration on certain builds and it'd be nice to simplify the
configuration of the perf bot builds (add 1 flag versus several flags that we
currently have).

[brettw, 2015-07-10 19:41:21]

Are you sure this is correct? It looks like you're doing all this stuff with
default testing configs for official builds. I thought we wanted to do the
reverse: no change in released builds, developers get default configuration.

https://codereview.chromium.org/1200383005/diff/120001/chrome/browser/chrome_...
File chrome/browser/chrome_browser_main.cc (right):

https://codereview.chromium.org/1200383005/diff/120001/chrome/browser/chrome_...
chrome/browser/chrome_browser_main.cc:637: #if defined(GOOGLE_CHROME_BUILD) &&
defined(OFFICIAL_BUILD)
Yeah, I always get these confused but "chrome_build" is what I think we want.

https://codereview.chromium.org/1200383005/diff/120001/chrome/browser/chrome_...
chrome/browser/chrome_browser_main.cc:640: if
(enable_field_trial_testing.empty()) {
No {} one one-line conditionals for consistency.

https://codereview.chromium.org/1200383005/diff/120001/chrome/common/variatio...
File chrome/common/variations/variations_util.h (right):

https://codereview.chromium.org/1200383005/diff/120001/chrome/common/variatio...
chrome/common/variations/variations_util.h:24: bool
AssociateParamsFromFieldTrialConfig(const FieldTrialTestingConfig& config);
Comments?

[Alexei Svitkine (slow), 2015-07-10 19:44:41]

https://codereview.chromium.org/1200383005/diff/120001/chrome/browser/chrome_...
File chrome/browser/chrome_browser_main.cc (right):

https://codereview.chromium.org/1200383005/diff/120001/chrome/browser/chrome_...
chrome/browser/chrome_browser_main.cc:637: #if defined(GOOGLE_CHROME_BUILD) &&
defined(OFFICIAL_BUILD)
On 2015/07/10 19:36:56, danduong wrote:
> On 2015/07/10 at 19:22:30, Alexei Svitkine wrote:
> > I think OFFICIAL_BUILD is not needed here. I think that macro is not meant
to
> change behavior - only control some optimization settings for the build.
(Though
> there may be existing code that does it wrong).
> 
> Hmm... If we were to do this, would I still be able to get field trial config
> enabled by default for developers testing Google branded features?

Interesting. I had assumed that's not really a use case - but interested in
understanding the exact use case you have in mind.

If we do want to support this, we should disable VariationsService created
trials, if the config is used instead. (In fact, perhaps this code should live
inside VariationsService instead of here?)

https://codereview.chromium.org/1200383005/diff/120001/chrome/browser/chrome_...
chrome/browser/chrome_browser_main.cc:639:
switches::kEnableFieldTrialTestingConfig);
On 2015/07/10 19:36:56, danduong wrote:
> On 2015/07/10 at 19:22:29, Alexei Svitkine wrote:
> > Convention for switches that start with "enable_*" is just to test for their
> presence.
> > 
> > Since you actually want a disable switch, let's follow the convention for
> that.
> > 
> > Call the switch kDisableFieldTrialTestingConfig and then simplify the code
> below to just be
> > 
> > if (!command_line->HasSwitch(kDisable..) &&
> !command_line->HasSwitch(kForce...)) {
> >   ...
> > }
> > 
> > I think the other thing this should be checking for is if the user specified
a
> custom variations-server-url - since that actually turns on variations service
> for unofficial builds - and in that case, we shouldn't use the default config
> either.
> 
> I can add a check for the variations server stuff.
> 
> I'm a little concerned by the loss of flexibility if I switch it to purely a
> disable flag. I know laforge was interested in an easy way for the test team
to
> enable this configuration on certain builds and it'd be nice to simplify the
> configuration of the perf bot builds (add 1 flag versus several flags that we
> currently have).

Right... I remember now - because the perf bots actually do run Chrome builds
rather than Chromium? In that case, it makes sense to have available for those
builds. But we should figure out what we want to do about UMA reporting.
--force-fieldtrials currently disables UMA. Should we do likewise if the config
is used? Otherwise, if people end up using this in the wild, they may affect our
percentages.

In fact, we probably should have a histogram reporting that this was used, so we
can monitor if this starts getting used in the wild. (e.g. malware may set this
to force a "known" configuration?)

Sigh... the more I think about this, the less I like having official builds
support this. Perhaps we can just keep the existing perfbot set up for official
builds?

[danduong, 2015-07-10 23:02:36]

https://codereview.chromium.org/1200383005/diff/120001/chrome/browser/chrome_...
File chrome/browser/chrome_browser_main.cc (right):

https://codereview.chromium.org/1200383005/diff/120001/chrome/browser/chrome_...
chrome/browser/chrome_browser_main.cc:637: #if defined(GOOGLE_CHROME_BUILD) &&
defined(OFFICIAL_BUILD)
On 2015/07/10 at 19:44:41, Alexei Svitkine wrote:
> On 2015/07/10 19:36:56, danduong wrote:
> > On 2015/07/10 at 19:22:30, Alexei Svitkine wrote:
> > > I think OFFICIAL_BUILD is not needed here. I think that macro is not meant
to
> > change behavior - only control some optimization settings for the build.
(Though
> > there may be existing code that does it wrong).
> > 
> > Hmm... If we were to do this, would I still be able to get field trial
config
> > enabled by default for developers testing Google branded features?
> 
> Interesting. I had assumed that's not really a use case - but interested in
understanding the exact use case you have in mind.
> 
> If we do want to support this, we should disable VariationsService created
trials, if the config is used instead. (In fact, perhaps this code should live
inside VariationsService instead of here?)

I guess that would be a lesser use-case. For simplicity, I switched to if
!defined(GOOGLE_CHROME_BUILD)

https://codereview.chromium.org/1200383005/diff/120001/chrome/browser/chrome_...
chrome/browser/chrome_browser_main.cc:639:
switches::kEnableFieldTrialTestingConfig);
On 2015/07/10 at 19:44:41, Alexei Svitkine wrote:
> On 2015/07/10 19:36:56, danduong wrote:
> > On 2015/07/10 at 19:22:29, Alexei Svitkine wrote:
> > > Convention for switches that start with "enable_*" is just to test for
their
> > presence.
> > > 
> > > Since you actually want a disable switch, let's follow the convention for
> > that.
> > > 
> > > Call the switch kDisableFieldTrialTestingConfig and then simplify the code
> > below to just be
> > > 
> > > if (!command_line->HasSwitch(kDisable..) &&
> > !command_line->HasSwitch(kForce...)) {
> > >   ...
> > > }
> > > 
> > > I think the other thing this should be checking for is if the user
specified a
> > custom variations-server-url - since that actually turns on variations
service
> > for unofficial builds - and in that case, we shouldn't use the default
config
> > either.
> > 
> > I can add a check for the variations server stuff.
> > 
> > I'm a little concerned by the loss of flexibility if I switch it to purely a
> > disable flag. I know laforge was interested in an easy way for the test team
to
> > enable this configuration on certain builds and it'd be nice to simplify the
> > configuration of the perf bot builds (add 1 flag versus several flags that
we
> > currently have).
> 
> Right... I remember now - because the perf bots actually do run Chrome builds
rather than Chromium? In that case, it makes sense to have available for those
builds. But we should figure out what we want to do about UMA reporting.
--force-fieldtrials currently disables UMA. Should we do likewise if the config
is used? Otherwise, if people end up using this in the wild, they may affect our
percentages.
> 
> In fact, we probably should have a histogram reporting that this was used, so
we can monitor if this starts getting used in the wild. (e.g. malware may set
this to force a "known" configuration?)
> 
> Sigh... the more I think about this, the less I like having official builds
support this. Perhaps we can just keep the existing perfbot set up for official
builds?

Yup. Let's simplify with just using a disable flag.

https://codereview.chromium.org/1200383005/diff/120001/chrome/browser/chrome_...
chrome/browser/chrome_browser_main.cc:640: if
(enable_field_trial_testing.empty()) {
On 2015/07/10 at 19:41:21, brettw wrote:
> No {} one one-line conditionals for consistency.

Removed this piece of code altogether.

[Alexei Svitkine (slow), 2015-07-13 15:19:49]

LGTM

https://codereview.chromium.org/1200383005/diff/160001/chrome/common/variatio...
File chrome/common/variations/variations_util.h (right):

https://codereview.chromium.org/1200383005/diff/160001/chrome/common/variatio...
chrome/common/variations/variations_util.h:25: bool
AssociateDefaultFieldTrialConfig();
Nit: Add a comment above each of these, please.

[danduong, 2015-07-13 17:19:34]

https://codereview.chromium.org/1200383005/diff/160001/chrome/common/variatio...
File chrome/common/variations/variations_util.h (right):

https://codereview.chromium.org/1200383005/diff/160001/chrome/common/variatio...
chrome/common/variations/variations_util.h:25: bool
AssociateDefaultFieldTrialConfig();
On 2015/07/13 at 15:19:49, Alexei Svitkine wrote:
> Nit: Add a comment above each of these, please.

Done.

[danduong, 2015-07-13 19:39:39]

brettw: PTAL

[brettw, 2015-07-15 17:56:51]

lgtm

https://codereview.chromium.org/1200383005/diff/180001/chrome/common/variatio...
File chrome/common/variations/variations_util.cc (right):

https://codereview.chromium.org/1200383005/diff/180001/chrome/common/variatio...
chrome/common/variations/variations_util.cc:83: return true;
This function never fails, but the code carefully propagates the values and
CHECKS later on. I'd just remove all of that and have it return void.

[danduong, 2015-07-15 22:29:08]

The CQ bit was checked by danduong@chromium.org

[danduong, 2015-07-15 22:29:08]

The patchset sent to the CQ was uploaded after l-g-t-m from
asvitkine@chromium.org, brettw@chromium.org
Link to the patchset: https://codereview.chromium.org/1200383005/#ps220001
(title: " ")

[commit-bot: I haz the power, 2015-07-15 22:29:45]

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

[commit-bot: I haz the power, 2015-07-15 23:06:40]

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

[commit-bot: I haz the power, 2015-07-15 23:06:41]

Try jobs failed on following builders:
  linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

(exceeded global retry quota)

[danduong, 2015-07-16 01:12:39]

On 2015/07/15 at 23:06:41, commit-bot wrote:
> Try jobs failed on following builders:
>   linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
>   linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
> 
> (exceeded global retry quota)

Interesting... SyncBackingDatabase32K study broke a sync test.

Alexei and Brett, should we run browser tests with the flag on or off by
default?

[Alexei Svitkine (slow), 2015-07-16 14:41:51]

I think they should run the configs. It's one of the benefits of this
system - that we can find if there are unexpected functional regressions
for field trials that we otherwise wouldn't detect until much later.

Sounds like for this specific case, you should file a bug against that
trial (and maybe cc the test's owners) and remove it from the testing
config until they fix it.

On Wed, Jul 15, 2015 at 9:12 PM, <danduong@chromium.org> wrote:

> On 2015/07/15 at 23:06:41, commit-bot wrote:
>
>> Try jobs failed on following builders:
>>    linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_FAILED,
>>
>
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...
> )
>
>>    linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
>>
>
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...
> )
>
>  (exceeded global retry quota)
>>
>
> Interesting... SyncBackingDatabase32K study broke a sync test.
>
> Alexei and Brett, should we run browser tests with the flag on or off by
> default?
>
> https://codereview.chromium.org/1200383005/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[danduong, 2015-07-16 16:55:02]

On 2015/07/16 at 14:41:51, asvitkine wrote:
> I think they should run the configs. It's one of the benefits of this
> system - that we can find if there are unexpected functional regressions
> for field trials that we otherwise wouldn't detect until much later.
> 
> Sounds like for this specific case, you should file a bug against that
> trial (and maybe cc the test's owners) and remove it from the testing
> config until they fix it.
> 
> On Wed, Jul 15, 2015 at 9:12 PM, <danduong@chromium.org> wrote:
> 
> > On 2015/07/15 at 23:06:41, commit-bot wrote:
> >
> >> Try jobs failed on following builders:
> >>    linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_FAILED,
> >>
> >
> >
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...
> > )
> >
> >>    linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
> >>
> >
> >
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...
> > )
> >
> >  (exceeded global retry quota)
> >>
> >
> > Interesting... SyncBackingDatabase32K study broke a sync test.
> >
> > Alexei and Brett, should we run browser tests with the flag on or off by
> > default?
> >
> > https://codereview.chromium.org/1200383005/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email to chromium-reviews+unsubscribe@chromium.org.
> 

SyncBackingDatabase32K testing config removed and bug filed:
https://code.google.com/p/chromium/issues/detail?id=510660

[danduong, 2015-07-16 16:55:24]

The CQ bit was checked by danduong@chromium.org

[danduong, 2015-07-16 16:55:24]

The patchset sent to the CQ was uploaded after l-g-t-m from brettw@chromium.org,
asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/1200383005/#ps240001
(title: "remove SyncBackingDatabase32K from testing config until fixed")

[commit-bot: I haz the power, 2015-07-16 16:57:13]

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

[danduong, 2015-07-16 16:58:24]

The CQ bit was unchecked by danduong@chromium.org

[danduong, 2015-07-16 17:01:49]

The CQ bit was checked by danduong@chromium.org

[danduong, 2015-07-16 17:01:49]

The patchset sent to the CQ was uploaded after l-g-t-m from brettw@chromium.org,
asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/1200383005/#ps260001
(title: " ")

[commit-bot: I haz the power, 2015-07-16 17:03:14]

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

[commit-bot: I haz the power, 2015-07-16 17:08:25]

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

[commit-bot: I haz the power, 2015-07-16 17:08:26]

Try jobs failed on following builders:
  linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

(exceeded global retry quota)

[danduong, 2015-07-16 17:28:08]

The CQ bit was checked by danduong@chromium.org

[danduong, 2015-07-16 17:28:09]

The patchset sent to the CQ was uploaded after l-g-t-m from brettw@chromium.org,
asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/1200383005/#ps280001
(title: "rebase")

[commit-bot: I haz the power, 2015-07-16 17:28:43]

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

[commit-bot: I haz the power, 2015-07-16 18:31:19]

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

[commit-bot: I haz the power, 2015-07-16 18:31:20]

Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[danduong, 2015-07-16 18:38:02]

The CQ bit was checked by danduong@chromium.org

[commit-bot: I haz the power, 2015-07-16 18:40:30]

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

[commit-bot: I haz the power, 2015-07-16 19:18:21]

Committed patchset #15 (id:280001)

[commit-bot: I haz the power, 2015-07-16 19:19:10]

Patchset 15 (id:??) landed as
https://crrev.com/907bad2715b5ee5bbdabf5b6743ab04ae8f2a187
Cr-Commit-Position: refs/heads/master@{#339092}