Add a flag for the password manager force-saving experiment.

Add a flag for the password manager force-saving experiment.

There is no functionality behind this flag yet.

I kept the name "force-saving", since this name of the feature is quite established now, although I wonder whether saying "manually save" wouldn't be more understandable in the description.

BUG=484891
Committed: https://crrev.com/e2ed8aaf2f40fb30ab0d4fb383ab4f945ac9fb55
Cr-Commit-Position: refs/heads/master@{#330438}

[msramek, 2015-05-15 11:30:46]

Patchset #1 (id:1) has been deleted

[msramek, 2015-05-15 11:36:06]

msramek@chromium.org changed reviewers:
+ vabr@chromium.org

[msramek, 2015-05-15 11:36:07]

Hi Václav,

could you PTAL?

Thanks,
Martin

[vabr (Chromium), 2015-05-15 11:53:11]

LGTM.

Could you perhaps also add a Boolean function into
components/password_manager/core/common/experiments.* to easily check if
force-saving is enabled?

Also, please explicitly mention in the CL description, that no functionality is
introduced by this flag yet.

I favour force-saving against manually saving. While the latter sounds less
aggressive and more natural, it can be misunderstood, IMO: we already have
--enable-automatic-password-saving to skip the save-password confirmation, so
just "manually saving" could be understood as not having the automatic saving
on.

Cheers,
Vaclav

https://codereview.chromium.org/1140043003/diff/20001/chrome/browser/about_fl...
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/1140043003/diff/20001/chrome/browser/about_fl...
chrome/browser/about_flags.cc:1151: kOsDesktop,
At some point we will need to add this to mobile as well, so maybe use kOsAll.

[msramek, 2015-05-15 12:19:07]

https://codereview.chromium.org/1140043003/diff/20001/chrome/browser/about_fl...
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/1140043003/diff/20001/chrome/browser/about_fl...
chrome/browser/about_flags.cc:1151: kOsDesktop,
On 2015/05/15 11:53:11, vabr (Chromium) wrote:
> At some point we will need to add this to mobile as well, so maybe use kOsAll.

Done.

[vabr (Chromium), 2015-05-15 13:06:30]

lgtm

[msramek, 2015-05-18 09:32:05]

It should be after branch point (latest build shows 45), so I'm landing this.

[msramek, 2015-05-18 09:32:10]

The CQ bit was checked by msramek@chromium.org

[commit-bot: I haz the power, 2015-05-18 09:32:28]

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

[commit-bot: I haz the power, 2015-05-18 10:29:53]

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

[commit-bot: I haz the power, 2015-05-18 10:29:54]

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

[msramek, 2015-05-18 14:13:34]

I just found out that every flag needs an entry in histograms.xml.

+Ilya, PTAL for ownership of histograms.

[msramek, 2015-05-18 14:14:22]

msramek@chromium.org changed reviewers:
+ isherman@chromium.org

[Ilya Sherman, 2015-05-18 21:11:24]

histograms.xml lgtm

[Ilya Sherman, 2015-05-18 21:11:36]

The CQ bit was checked by isherman@chromium.org

[Ilya Sherman, 2015-05-18 21:11:36]

The patchset sent to the CQ was uploaded after l-g-t-m from vabr@chromium.org
Link to the patchset: https://codereview.chromium.org/1140043003/#ps60001
(title: "Added the required experiment metric.")

[commit-bot: I haz the power, 2015-05-18 21:12:37]

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

[commit-bot: I haz the power, 2015-05-18 22:11:30]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2015-05-18 22:44:36]

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