CREDENTIAL: Disable auto sign-in by default for existing credentials.

CREDENTIAL: Disable auto sign-in by default for existing credentials.

The changes to libsecret and gnome backends are fairly hideous, but I am assured
that this is How Things are Done. *sigh*

BUG=587440
R=vasilii@chromium.org

Committed: https://crrev.com/77be16dcee58c4067891103fc6a8f11ec01825e1
Cr-Commit-Position: refs/heads/master@{#377884}

[Mike West, 2016-02-26 10:17:26]

Description was changed from

==========
CREDENTIAL: Disable auto sign-in by default for existing credentials.

BUG=587440
R=vasilii@chromium.org
==========

to

==========
CREDENTIAL: Disable auto sign-in by default for existing credentials.

The changes to libsecret and gnome backends are fairly hideous, but I am assured
that this is How Things are Done. *sigh*

BUG=587440
R=vasilii@chromium.org
==========

[vasilii, 2016-02-26 11:46:31]

https://codereview.chromium.org/1734193003/diff/1/chrome/browser/password_man...
File chrome/browser/password_manager/native_backend_gnome_x.cc (right):

https://codereview.chromium.org/1734193003/diff/1/chrome/browser/password_man...
chrome/browser/password_manager/native_backend_gnome_x.cc:163:
form->skip_zero_click = uint_attr_map["should_skip_zero_click"];
I'd prefer an explicit "true" here if it's not found.

https://codereview.chromium.org/1734193003/diff/1/chrome/browser/password_man...
File chrome/browser/password_manager/native_backend_kwallet_x.cc (right):

https://codereview.chromium.org/1734193003/diff/1/chrome/browser/password_man...
chrome/browser/password_manager/native_backend_kwallet_x.cc:248:
form->skip_zero_click = true;
It's a bug. You read "true" always even if you save "false" later.
You should add something to NativeBackendKWalletPickleTest

https://codereview.chromium.org/1734193003/diff/1/chrome/browser/password_man...
File chrome/browser/password_manager/native_backend_libsecret.cc (right):

https://codereview.chromium.org/1734193003/diff/1/chrome/browser/password_man...
chrome/browser/password_manager/native_backend_libsecret.cc:199: // value from
the backend. Otherwise, leave it alone.
Any reason not to assign the member in all the cases?

[Mike West, 2016-02-26 12:29:34]

Thanks. I think this addresses your concerns.

https://codereview.chromium.org/1734193003/diff/1/chrome/browser/password_man...
File chrome/browser/password_manager/native_backend_kwallet_x.cc (right):

https://codereview.chromium.org/1734193003/diff/1/chrome/browser/password_man...
chrome/browser/password_manager/native_backend_kwallet_x.cc:248:
form->skip_zero_click = true;
On 2016/02/26 at 11:46:30, vasilii wrote:
> It's a bug. You read "true" always even if you save "false" later.
> You should add something to NativeBackendKWalletPickleTest

Yeah, I misunderstood what was going on here. Added a test.

https://codereview.chromium.org/1734193003/diff/1/chrome/browser/password_man...
File chrome/browser/password_manager/native_backend_libsecret.cc (right):

https://codereview.chromium.org/1734193003/diff/1/chrome/browser/password_man...
chrome/browser/password_manager/native_backend_libsecret.cc:199: // value from
the backend. Otherwise, leave it alone.
On 2016/02/26 at 11:46:31, vasilii wrote:
> Any reason not to assign the member in all the cases?

*shrug* I can add an assignment.

[vasilii, 2016-02-26 13:02:07]

lgtm

https://codereview.chromium.org/1734193003/diff/20001/chrome/browser/password...
File chrome/browser/password_manager/native_backend_kwallet_x.cc (right):

https://codereview.chromium.org/1734193003/diff/20001/chrome/browser/password...
chrome/browser/password_manager/native_backend_kwallet_x.cc:247: if (version ==
7)
<= 7, I'd move it upper to the place where we actually read this member.

[Mike West, 2016-02-26 13:05:12]

The CQ bit was checked by mkwst@chromium.org

[Mike West, 2016-02-26 13:05:12]

The patchset sent to the CQ was uploaded after l-g-t-m from vasilii@chromium.org
Link to the patchset: https://codereview.chromium.org/1734193003/#ps40001
(title: "Yay")

[commit-bot: I haz the power, 2016-02-26 13:05:46]

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

[commit-bot: I haz the power, 2016-02-26 13:13:46]

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

[commit-bot: I haz the power, 2016-02-26 13:13:47]

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

[vasilii, 2016-02-26 13:27:28]

lgtm

https://codereview.chromium.org/1734193003/diff/40001/chrome/browser/password...
File chrome/browser/password_manager/native_backend_kwallet_x.cc (right):

https://codereview.chromium.org/1734193003/diff/40001/chrome/browser/password...
chrome/browser/password_manager/native_backend_kwallet_x.cc:224: if (version ==
7)
<= 7

[vabr (Chromium), 2016-02-26 13:33:52]

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

[vabr (Chromium), 2016-02-26 13:33:53]

components/autofill/core/common/password_form.cc LGTM

[Mike West, 2016-02-26 13:36:09]

The CQ bit was checked by mkwst@chromium.org

[Mike West, 2016-02-26 13:36:09]

The patchset sent to the CQ was uploaded after l-g-t-m from vasilii@chromium.org
Link to the patchset: https://codereview.chromium.org/1734193003/#ps60001
(title: "Ugh.")

[commit-bot: I haz the power, 2016-02-26 13:36:51]

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

[commit-bot: I haz the power, 2016-02-26 14:56:45]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-02-26 14:57:37]

Description was changed from

==========
CREDENTIAL: Disable auto sign-in by default for existing credentials.

The changes to libsecret and gnome backends are fairly hideous, but I am assured
that this is How Things are Done. *sigh*

BUG=587440
R=vasilii@chromium.org
==========

to

==========
CREDENTIAL: Disable auto sign-in by default for existing credentials.

The changes to libsecret and gnome backends are fairly hideous, but I am assured
that this is How Things are Done. *sigh*

BUG=587440
R=vasilii@chromium.org

Committed: https://crrev.com/77be16dcee58c4067891103fc6a8f11ec01825e1
Cr-Commit-Position: refs/heads/master@{#377884}
==========

[commit-bot: I haz the power, 2016-02-26 14:57:39]

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