Add a field trial to enable plugin power saver for all users.

Add a field trial to enable plugin power saver for all users.

R=tommycli@chromium.org,bauerb@chromium.org
BUG=478918
Committed: https://crrev.com/f377cefbea76754a5a0eb43049640bb28bcff8dd
Cr-Commit-Position: refs/heads/master@{#327546}

[tommycli, 2015-04-28 21:57:56]

https://codereview.chromium.org/1112723002/diff/1/components/content_settings...
File components/content_settings/core/browser/plugins_field_trial.cc (right):

https://codereview.chromium.org/1112723002/diff/1/components/content_settings...
components/content_settings/core/browser/plugins_field_trial.cc:14: const char
PluginsFieldTrial::kEnableFieldTrial[] = "PluginPowerSaver";
Add a TODO to remove the redundant one when we're ready.

https://codereview.chromium.org/1112723002/diff/1/components/content_settings...
File components/content_settings/core/browser/plugins_field_trial_unittest.cc
(right):

https://codereview.chromium.org/1112723002/diff/1/components/content_settings...
components/content_settings/core/browser/plugins_field_trial_unittest.cc:25:
void SetForceFieldTrialGroup(const std::string& group) {
It's a shame that the test has been weakened by adding another Field trial name.

But testing all 4 combinations of these two field trials is also lame. Is there
a way out of this?

[Dan Beam, 2015-04-28 22:17:55]

https://codereview.chromium.org/1112723002/diff/1/components/content_settings...
File components/content_settings/core/browser/plugins_field_trial.cc (right):

https://codereview.chromium.org/1112723002/diff/1/components/content_settings...
components/content_settings/core/browser/plugins_field_trial.cc:14: const char
PluginsFieldTrial::kEnableFieldTrial[] = "PluginPowerSaver";
On 2015/04/28 21:57:56, tommycli wrote:
> Add a TODO to remove the redundant one when we're ready.

Done. (in the header)

https://codereview.chromium.org/1112723002/diff/1/components/content_settings...
File components/content_settings/core/browser/plugins_field_trial_unittest.cc
(right):

https://codereview.chromium.org/1112723002/diff/1/components/content_settings...
components/content_settings/core/browser/plugins_field_trial_unittest.cc:25:
void SetForceFieldTrialGroup(const std::string& group) {
On 2015/04/28 21:57:56, tommycli wrote:
> It's a shame that the test has been weakened by adding another Field trial
name.
> 
> But testing all 4 combinations of these two field trials is also lame. Is
there
> a way out of this?

Done. (minimal lameness achieved?)

[tommycli, 2015-04-28 22:28:23]

lgtm. I guess the lameness will (hopefully) only last until branch cut.

[Dan Beam, 2015-04-28 22:29:21]

dbeam@chromium.org changed reviewers:
+ bauerb@chromium.org

[Dan Beam, 2015-04-28 22:29:35]

+bauerb@ for components/content_settings/OWNERS

[Bernhard Bauer, 2015-04-29 16:23:24]

https://codereview.chromium.org/1112723002/diff/20001/components/content_sett...
File components/content_settings/core/browser/plugins_field_trial.cc (right):

https://codereview.chromium.org/1112723002/diff/20001/components/content_sett...
components/content_settings/core/browser/plugins_field_trial.cc:41: std::string
group = base::FieldTrialList::FindFullName(kEnableFieldTrial);
According to go/finch-and-flags, you should get the trial group first, to ensure
that the experiment state is reported properly.

https://codereview.chromium.org/1112723002/diff/20001/components/content_sett...
File components/content_settings/core/browser/plugins_field_trial_unittest.cc
(right):

https://codereview.chromium.org/1112723002/diff/20001/components/content_sett...
components/content_settings/core/browser/plugins_field_trial_unittest.cc:22:
const auto& kEnableFieldTrial = PluginsFieldTrial::kEnableFieldTrial;
This is const char*, right? Could you just use that, as it's not longer than
const auto&?

[Dan Beam, 2015-04-29 17:59:28]

https://codereview.chromium.org/1112723002/diff/20001/components/content_sett...
File components/content_settings/core/browser/plugins_field_trial.cc (right):

https://codereview.chromium.org/1112723002/diff/20001/components/content_sett...
components/content_settings/core/browser/plugins_field_trial.cc:41: std::string
group = base::FieldTrialList::FindFullName(kEnableFieldTrial);
On 2015/04/29 16:23:24, Bernhard Bauer wrote:
> According to go/finch-and-flags, you should get the trial group first, to
ensure
> that the experiment state is reported properly.

Done.

https://codereview.chromium.org/1112723002/diff/20001/components/content_sett...
File components/content_settings/core/browser/plugins_field_trial_unittest.cc
(right):

https://codereview.chromium.org/1112723002/diff/20001/components/content_sett...
components/content_settings/core/browser/plugins_field_trial_unittest.cc:22:
const auto& kEnableFieldTrial = PluginsFieldTrial::kEnableFieldTrial;
On 2015/04/29 16:23:24, Bernhard Bauer wrote:
> This is const char*, right? Could you just use that, as it's not longer than
> const auto&?

Done. (this is just want clang told me to do, btw)

[Bernhard Bauer, 2015-04-29 18:07:55]

lgtm

[Dan Beam, 2015-04-29 18:13:15]

The CQ bit was checked by dbeam@chromium.org

[Dan Beam, 2015-04-29 18:13:15]

The patchset sent to the CQ was uploaded after l-g-t-m from
tommycli@chromium.org
Link to the patchset: https://codereview.chromium.org/1112723002/#ps60001
(title: "humans are bad compilers")

[commit-bot: I haz the power, 2015-04-29 18:13:41]

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

[commit-bot: I haz the power, 2015-04-29 19:31:05]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-04-29 19:31:54]

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