[Password Manager] Switch password manager code to use "base/feature_list.h".

[Password Manager] Switch password manager code to use "base/feature_list.h".

Changes so far:
  A. 'enable-automatic-password-saving' feature.
  B. 'enable-password-force-saving' feature.
  C. 'enable-password-change-support' feature.

BUG=561322
Committed: https://crrev.com/906f9aee610d3f45e1cb98ed3f5aef9fb424b873
Cr-Commit-Position: refs/heads/master@{#363525}

[Pritam Nikam, 2015-12-01 13:36:41]

Description was changed from

==========
[Password Manager] Switch password manager code to use "base/feature_list.h".

BUG=561322
==========

to

==========
[Password Manager] Switch password manager code to use "base/feature_list.h".

Changes so far:
  A. 'enable-automatic-password-saving' feature.
  B. 'enable-password-force-saving' feature.
  C. 'enable-password-change-support' feature.

BUG=561322
==========

[Pritam Nikam, 2015-12-03 06:01:48]

Description was changed from

==========
[Password Manager] Switch password manager code to use "base/feature_list.h".

Changes so far:
  A. 'enable-automatic-password-saving' feature.
  B. 'enable-password-force-saving' feature.
  C. 'enable-password-change-support' feature.

BUG=561322
==========

to

==========
[Password Manager] Switch password manager code to use "base/feature_list.h".

Changes so far:
  A. 'enable-automatic-password-saving' feature.
  B. 'enable-password-force-saving' feature.
  C. 'enable-password-change-support' feature.

BUG=561322
R=SamsungPeerReview
==========

[Pritam Nikam, 2015-12-03 06:01:48]

pritam.nikam@samsung.com changed reviewers:
+ vivek.vg@samsung.com, vivekg@chromium.org

[Pritam Nikam, 2015-12-03 06:08:06]

On 2015/12/03 06:01:48, Pritam Nikam wrote:
> mailto:pritam.nikam@samsung.com changed reviewers:
> + mailto:vivek.vg@samsung.com, mailto:vivekg@chromium.org

Hi Vivek,

Motivation behind this patch is introduction of new way of enabling/disabling
the command line switches/feature.
Related chromium-dev thread:
https://groups.google.com/a/chromium.org/forum/?fromgroups#!topic/chromium-de...

I've planned to split this feature into 2 parts - patch-1 targets password
manger *switches without field-trials* (& its quite simple to port), & patch-2
(its WIP)  porting *switches with field-trials* handled. Please have a look at
the patch and help with your review inputs.

Thanks!

[Pritam Nikam, 2015-12-04 12:33:07]

Description was changed from

==========
[Password Manager] Switch password manager code to use "base/feature_list.h".

Changes so far:
  A. 'enable-automatic-password-saving' feature.
  B. 'enable-password-force-saving' feature.
  C. 'enable-password-change-support' feature.

BUG=561322
R=SamsungPeerReview
==========

to

==========
[Password Manager] Switch password manager code to use "base/feature_list.h".

Changes so far:
  A. 'enable-automatic-password-saving' feature.
  B. 'enable-password-force-saving' feature.
  C. 'enable-password-change-support' feature.

BUG=561322
==========

[Pritam Nikam, 2015-12-04 12:33:07]

pritam.nikam@samsung.com changed reviewers:
+ vabr@chromium.org

[Pritam Nikam, 2015-12-04 12:36:20]

Hi Vaclav,

I've planned to split this feature into 2 parts:
A. Patch-1 targets porting of password manager *switches without field-trials*
(& its quite simple to port), 
B. Patch-2 (it's WIP) targets porting of *switches with field-trials* handled.
Please have a look at the patch and help with your review inputs.

Thanks!

[vabr (Chromium), 2015-12-07 13:34:33]

LGTM, very nice!

I checked that the default-disabled settings are correct. In particular, while
kEnablePasswordChangeSupport is not used at the moment at all, you did the
correct thing to keep it in and changing it to "disabled by default" -- it will
soon be used for adding the change password form support on Mac and Android, and
there the default will be off.

I also looked for the constants being used on iOS downstream, and there seems to
be no usage, so this is safe to land upstream.

Thank you,
Vaclav

https://codereview.chromium.org/1488763003/diff/20001/components/password_man...
File components/password_manager/core/common/password_manager_features.h
(right):

https://codereview.chromium.org/1488763003/diff/20001/components/password_man...
components/password_manager/core/common/password_manager_features.h:5: // This
file defines all the base::FeatureList features for the Password Manager
nit: Move this comment below the #include guard on lines 8-9. The only text
above that guard should be the copyright notice.

[Pritam Nikam, 2015-12-07 15:33:41]

Thanks Vaclav for review. I've addressed nit in new patch set. Since its LGTM
form your side I'll go ahead with commit.
Thanks!

https://codereview.chromium.org/1488763003/diff/20001/components/password_man...
File components/password_manager/core/common/password_manager_features.h
(right):

https://codereview.chromium.org/1488763003/diff/20001/components/password_man...
components/password_manager/core/common/password_manager_features.h:5: // This
file defines all the base::FeatureList features for the Password Manager
On 2015/12/07 13:34:33, vabr (Chromium) wrote:
> nit: Move this comment below the #include guard on lines 8-9. The only text
> above that guard should be the copyright notice.

Done.

[Pritam Nikam, 2015-12-07 15:33:52]

The CQ bit was checked by pritam.nikam@samsung.com

[Pritam Nikam, 2015-12-07 15:33:52]

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/1488763003/#ps40001
(title: "Addresses nit.")

[commit-bot: I haz the power, 2015-12-07 15:34:47]

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

[vabr (Chromium), 2015-12-07 15:36:24]

LGTM.

(And I acknowledge that you did the right thing starting to land this, because
you did have my approval and addressed the comment.)

Cheers,
Vaclav

[commit-bot: I haz the power, 2015-12-07 16:04:23]

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

[commit-bot: I haz the power, 2015-12-07 16:04:23]

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_...)
  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_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Pritam Nikam, 2015-12-07 17:22:51]

The CQ bit was checked by pritam.nikam@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2015-12-07 17:23:43]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1488763003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1488763003/40001

[commit-bot: I haz the power, 2015-12-07 18:18:33]

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

[commit-bot: I haz the power, 2015-12-07 18:18:34]

Dry run: This issue passed the CQ dry run.

[Pritam Nikam, 2015-12-07 18:19:13]

The CQ bit was checked by pritam.nikam@samsung.com

[commit-bot: I haz the power, 2015-12-07 18:22:27]

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

[commit-bot: I haz the power, 2015-12-07 18:36:46]

Description was changed from

==========
[Password Manager] Switch password manager code to use "base/feature_list.h".

Changes so far:
  A. 'enable-automatic-password-saving' feature.
  B. 'enable-password-force-saving' feature.
  C. 'enable-password-change-support' feature.

BUG=561322
==========

to

==========
[Password Manager] Switch password manager code to use "base/feature_list.h".

Changes so far:
  A. 'enable-automatic-password-saving' feature.
  B. 'enable-password-force-saving' feature.
  C. 'enable-password-change-support' feature.

BUG=561322
==========

[commit-bot: I haz the power, 2015-12-07 18:36:46]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-12-07 18:38:07]

Description was changed from

==========
[Password Manager] Switch password manager code to use "base/feature_list.h".

Changes so far:
  A. 'enable-automatic-password-saving' feature.
  B. 'enable-password-force-saving' feature.
  C. 'enable-password-change-support' feature.

BUG=561322
==========

to

==========
[Password Manager] Switch password manager code to use "base/feature_list.h".

Changes so far:
  A. 'enable-automatic-password-saving' feature.
  B. 'enable-password-force-saving' feature.
  C. 'enable-password-change-support' feature.

BUG=561322

Committed: https://crrev.com/906f9aee610d3f45e1cb98ed3f5aef9fb424b873
Cr-Commit-Position: refs/heads/master@{#363525}
==========

[commit-bot: I haz the power, 2015-12-07 18:38:08]

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