CREDENTIAL: Only toggle 'skip_zero_click' state if API is used.

CREDENTIAL: Only toggle 'skip_zero_click' state if API is used.

Currently, we're toggling 'skip_zero_click' to 'false' if the user logs
in successfully after autofill. We should restrict that a bit to ensure
that the site understands the credential management API (and can therefore
be assumed to explain to us when the user signs out).

This patch adjusts the API's behavior to send the relevant PasswordForm
to the client whenever it could have been handed over but wasn't. Upon
successful autofill-based login with that same credential, the client is
responsible for toggling its 'skip_zero_click' value, and for popping up
the first-run experience if necessary.

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

Committed: https://crrev.com/ce3eee6988a1d901f47f95cf192b77874b85bdb2
Cr-Commit-Position: refs/heads/master@{#378190}

[Mike West, 2016-02-26 14:23:00]

Mind taking a look? Based on my conversations with Sabine yesterday, I think
this gets us a bit closer to her ideal state.

-mike

[vabr (Chromium), 2016-02-26 14:50:27]

LGTM.

Cheers,
Vaclav

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

https://codereview.chromium.org/1735013004/diff/1/chrome/browser/password_man...
chrome/browser/password_manager/password_manager_browsertest.cc:2968:
signin_form.skip_zero_click = true;
nit: Any reason why signin_form.origin is not needed here but is needed on line
2921?

[Mike West, 2016-02-26 14:54:45]

The CQ bit was checked by mkwst@chromium.org

[Mike West, 2016-02-26 14:54:46]

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/1735013004/#ps20001
(title: "vabr@")

[commit-bot: I haz the power, 2016-02-26 14:55:12]

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

[Mike West, 2016-02-26 14:55:50]

On 2016/02/26 at 14:50:27, vabr wrote:
> LGTM.
> 
> Cheers,
> Vaclav
> 
>
https://codereview.chromium.org/1735013004/diff/1/chrome/browser/password_man...
> File chrome/browser/password_manager/password_manager_browsertest.cc (right):
> 
>
https://codereview.chromium.org/1735013004/diff/1/chrome/browser/password_man...
> chrome/browser/password_manager/password_manager_browsertest.cc:2968:
signin_form.skip_zero_click = true;
> nit: Any reason why signin_form.origin is not needed here but is needed on
line 2921?

Just oversight. It wasn't necessary before, because we weren't checking the form
we submitted against API-based forms. Now we need a match between the form we
provided in the API call and the form we're submitting, so I added the field in.
I should have added it here as well. Done now.

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

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

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

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_...)

[vasilii, 2016-02-26 15:42:36]

lgtm

https://codereview.chromium.org/1735013004/diff/20001/components/password_man...
File
components/password_manager/core/browser/credential_manager_pending_request_task.cc
(right):

https://codereview.chromium.org/1735013004/diff/20001/components/password_man...
components/password_manager/core/browser/credential_manager_pending_request_task.cc:103:
scoped_ptr<autofill::PasswordForm> potential_autosignin_form(
move the definition down to where it's used (before local_results is moved).

[Mike West, 2016-02-29 09:28:22]

The CQ bit was checked by mkwst@chromium.org

[Mike West, 2016-02-29 09:28:23]

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

[commit-bot: I haz the power, 2016-02-29 09:28:31]

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

[commit-bot: I haz the power, 2016-02-29 10:34:44]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-02-29 10:35:55]

Description was changed from

==========
CREDENTIAL: Only toggle 'skip_zero_click' state if API is used.

Currently, we're toggling 'skip_zero_click' to 'false' if the user logs
in successfully after autofill. We should restrict that a bit to ensure
that the site understands the credential management API (and can therefore
be assumed to explain to us when the user signs out).

This patch adjusts the API's behavior to send the relevant PasswordForm
to the client whenever it could have been handed over but wasn't. Upon
successful autofill-based login with that same credential, the client is
responsible for toggling its 'skip_zero_click' value, and for popping up
the first-run experience if necessary.

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

to

==========
CREDENTIAL: Only toggle 'skip_zero_click' state if API is used.

Currently, we're toggling 'skip_zero_click' to 'false' if the user logs
in successfully after autofill. We should restrict that a bit to ensure
that the site understands the credential management API (and can therefore
be assumed to explain to us when the user signs out).

This patch adjusts the API's behavior to send the relevant PasswordForm
to the client whenever it could have been handed over but wasn't. Upon
successful autofill-based login with that same credential, the client is
responsible for toggling its 'skip_zero_click' value, and for popping up
the first-run experience if necessary.

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

Committed: https://crrev.com/ce3eee6988a1d901f47f95cf192b77874b85bdb2
Cr-Commit-Position: refs/heads/master@{#378190}
==========

[commit-bot: I haz the power, 2016-02-29 10:35:56]

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