Fix crash when variation parameters are set in chrome://flags.

Fix crash when variation parameters are set in chrome://flags.

In chrome://flags, one can now choose among predefined variation
parameters for a feature. Such parameters are registered on startup
with a predefined trial and group name. This causes a crash when the
same trial is specified in Finch config.

This CL skips dealing with a Finch trial where a group is already
selected and this group is not mentioned in the Finch config.

BUG=624807
Committed: https://crrev.com/7ce02460a73a4e47708d5bf96943fe2640dd47e1
Cr-Commit-Position: refs/heads/master@{#403431}

[jkrcal, 2016-06-30 21:45:37]

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

[jkrcal, 2016-06-30 21:45:38]

PTAL.

I hope this will do.

[Alexei Svitkine (slow), 2016-06-30 21:50:59]

lgtm % comments

Please also add a unit test for this.

https://codereview.chromium.org/2119713002/diff/1/components/variations/varia...
File components/variations/variations_seed_processor.cc (right):

https://codereview.chromium.org/2119713002/diff/1/components/variations/varia...
components/variations/variations_seed_processor.cc:282: const std::string&
group_name = trial->group_name();
Nit: Just inline this into the call below.

https://codereview.chromium.org/2119713002/diff/1/components/variations/varia...
components/variations/variations_seed_processor.cc:284: if (experiment_index ==
-1) return;
Nit: Put return on next line.

[jkrcal, 2016-07-01 08:44:10]

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

[commit-bot: I haz the power, 2016-07-01 08:44:19]

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

[jkrcal, 2016-07-01 08:50:34]

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

[commit-bot: I haz the power, 2016-07-01 08:50:40]

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, 2016-07-01 08:57:46]

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

[commit-bot: I haz the power, 2016-07-01 08:57:47]

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

[jkrcal, 2016-07-01 09:00:16]

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

[commit-bot: I haz the power, 2016-07-01 09:00:22]

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, 2016-07-01 09:43:55]

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

[commit-bot: I haz the power, 2016-07-01 09:43:57]

Dry run: This issue passed the CQ dry run.

[jkrcal, 2016-07-01 09:54:11]

The CQ bit was checked by jkrcal@chromium.org

[jkrcal, 2016-07-01 09:54:11]

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

[commit-bot: I haz the power, 2016-07-01 09:54:19]

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-07-01 09:58:12]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-07-01 09:58:15]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-01 09:59:53]

Description was changed from

==========
Fix crash when variation parameters are set in chrome://flags.

In chrome://flags, one can now choose among predefined variation
parameters for a feature. Such parameters are registered on startup
with a predefined trial and group name. This causes a crash when the
same trial is specified in Finch config.

This CL skips dealing with a Finch trial where a group is already
selected and this group is not mentioned in the Finch config.

BUG=624807
==========

to

==========
Fix crash when variation parameters are set in chrome://flags.

In chrome://flags, one can now choose among predefined variation
parameters for a feature. Such parameters are registered on startup
with a predefined trial and group name. This causes a crash when the
same trial is specified in Finch config.

This CL skips dealing with a Finch trial where a group is already
selected and this group is not mentioned in the Finch config.

BUG=624807

Committed: https://crrev.com/7ce02460a73a4e47708d5bf96943fe2640dd47e1
Cr-Commit-Position: refs/heads/master@{#403431}
==========

[commit-bot: I haz the power, 2016-07-01 09:59:54]

Patchset 5 (id:??) landed as
https://crrev.com/7ce02460a73a4e47708d5bf96943fe2640dd47e1
Cr-Commit-Position: refs/heads/master@{#403431}

[jkrcal, 2016-07-01 10:40:54]

Alexei, I moved it up to make it even more clear that there is nothing to do for
such trials.

(Also I had to replace group_name() by GetGroupNameWithoutActivation(), the
former did not work being called so early.)

If you dislike it this way, let me know, I'll move it back down in another CL.

https://codereview.chromium.org/2119713002/diff/1/components/variations/varia...
File components/variations/variations_seed_processor.cc (right):

https://codereview.chromium.org/2119713002/diff/1/components/variations/varia...
components/variations/variations_seed_processor.cc:282: const std::string&
group_name = trial->group_name();
On 2016/06/30 21:50:59, Alexei Svitkine (very slow) wrote:
> Nit: Just inline this into the call below.

Done.

https://codereview.chromium.org/2119713002/diff/1/components/variations/varia...
components/variations/variations_seed_processor.cc:284: if (experiment_index ==
-1) return;
On 2016/06/30 21:50:59, Alexei Svitkine (very slow) wrote:
> Nit: Put return on next line.

Done.

[Alexei Svitkine (slow), 2016-07-04 17:15:32]

On 2016/07/01 10:40:54, jkrcal wrote:
> Alexei, I moved it up to make it even more clear that there is nothing to do
for
> such trials.
> 
> (Also I had to replace group_name() by GetGroupNameWithoutActivation(), the
> former did not work being called so early.)
> 
> If you dislike it this way, let me know, I'll move it back down in another CL.
> 
>
https://codereview.chromium.org/2119713002/diff/1/components/variations/varia...
> File components/variations/variations_seed_processor.cc (right):
> 
>
https://codereview.chromium.org/2119713002/diff/1/components/variations/varia...
> components/variations/variations_seed_processor.cc:282: const std::string&
> group_name = trial->group_name();
> On 2016/06/30 21:50:59, Alexei Svitkine (very slow) wrote:
> > Nit: Just inline this into the call below.
> 
> Done.
> 
>
https://codereview.chromium.org/2119713002/diff/1/components/variations/varia...
> components/variations/variations_seed_processor.cc:284: if (experiment_index
==
> -1) return;
> On 2016/06/30 21:50:59, Alexei Svitkine (very slow) wrote:
> > Nit: Put return on next line.
> 
> Done.

Seems fine. I don't think it makes sense to do any of the work when a group we
don't know about is active.