Update the |skip_zero_click| flag of a credential when selected in the account chooser.

Update the |skip_zero_click| flag of a credential when selected in the account chooser.

When we create a PasswordForm from a CredentialInfo, we're stripping out
data that PasswordStore requires in order to find the form in the
database. This breaks the auto-sign-in bit toggle for the credential if
it was saved via our heuristics, and not via the API.

This patch stops truncating PasswordForm to CredentialInfo when passing to CredentialManagerDispatcher.

BUG=594110
R=vasilii@chromium.org

patch from issue 1785273002 at patchset 20001 (http://crrev.com/1785273002#ps20001)

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

[vasilii, 2016-03-24 17:58:19]

vasilii@chromium.org changed reviewers:
+ mkwst@chromium.org, vabr@chromium.org
- vasilii@chromium.org

[vasilii, 2016-03-24 17:58:20]

Hi Vaclav,

please review. Mike is on vacation.

[vabr (Chromium), 2016-03-29 08:45:55]

lgtm

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

https://codereview.chromium.org/1832933002/diff/1/chrome/browser/password_man...
chrome/browser/password_manager/account_chooser_dialog_android.cc:212:
DCHECK(type == CredentialType::CREDENTIAL_TYPE_PASSWORD);
optional nit: One could also either define operator<< for CredentialType to make
DCHECK_EQ(CredentialType::CREDENTIAL_TYPE_PASSWORD, type) work, or at least use
DCHECK(...) << "type is " << static_cast<int>(type) to make the error log a bit
more helpful.

Similarly on line 298 in
chrome/browser/ui/passwords/manage_passwords_ui_controller.cc.

https://codereview.chromium.org/1832933002/diff/1/chrome/browser/password_man...
File chrome/browser/password_manager/chrome_password_manager_client.h (right):

https://codereview.chromium.org/1832933002/diff/1/chrome/browser/password_man...
chrome/browser/password_manager/chrome_password_manager_client.h:157: // Called
as a response to PromptUserToChooseCredentials.
nit: Could you comment on what does form being nullptr represent?

https://codereview.chromium.org/1832933002/diff/1/components/password_manager...
File
components/password_manager/content/browser/credential_manager_dispatcher_unittest.cc
(right):

https://codereview.chromium.org/1832933002/diff/1/components/password_manager...
components/password_manager/content/browser/credential_manager_dispatcher_unittest.cc:436:
const uint32_t kMsgID = CredentialManagerMsg_SendCredential::ID;
heads-up: If you get rebasing conflicts or post-rebaseing errors, check out
https://codereview.chromium.org/1762603002/ which, once landed, will switch from
IPC to Mojo.

[vasilii, 2016-03-29 09:57:44]

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

https://codereview.chromium.org/1832933002/diff/1/chrome/browser/password_man...
chrome/browser/password_manager/account_chooser_dialog_android.cc:212:
DCHECK(type == CredentialType::CREDENTIAL_TYPE_PASSWORD);
On 2016/03/29 08:45:55, vabr (Chromium) wrote:
> optional nit: One could also either define operator<< for CredentialType to
make
> DCHECK_EQ(CredentialType::CREDENTIAL_TYPE_PASSWORD, type) work, or at least
use
> DCHECK(...) << "type is " << static_cast<int>(type) to make the error log a
bit
> more helpful.
> 
> Similarly on line 298 in
> chrome/browser/ui/passwords/manage_passwords_ui_controller.cc.

Done.

https://codereview.chromium.org/1832933002/diff/1/chrome/browser/password_man...
File chrome/browser/password_manager/chrome_password_manager_client.h (right):

https://codereview.chromium.org/1832933002/diff/1/chrome/browser/password_man...
chrome/browser/password_manager/chrome_password_manager_client.h:157: // Called
as a response to PromptUserToChooseCredentials.
On 2016/03/29 08:45:55, vabr (Chromium) wrote:
> nit: Could you comment on what does form being nullptr represent?

Done.

https://codereview.chromium.org/1832933002/diff/1/components/password_manager...
File
components/password_manager/content/browser/credential_manager_dispatcher_unittest.cc
(right):

https://codereview.chromium.org/1832933002/diff/1/components/password_manager...
components/password_manager/content/browser/credential_manager_dispatcher_unittest.cc:436:
const uint32_t kMsgID = CredentialManagerMsg_SendCredential::ID;
On 2016/03/29 08:45:55, vabr (Chromium) wrote:
> heads-up: If you get rebasing conflicts or post-rebaseing errors, check out
> https://codereview.chromium.org/1762603002/ which, once landed, will switch
from
> IPC to Mojo.

No conflicts.

[vasilii, 2016-03-29 09:57:55]

The CQ bit was checked by vasilii@chromium.org

[vasilii, 2016-03-29 09:57:55]

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/1832933002/#ps40001
(title: "comments")

[commit-bot: I haz the power, 2016-03-29 09:58:06]

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

[commit-bot: I haz the power, 2016-03-29 10:43:55]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-03-29 10:44:53]

Description was changed from

==========
Update the |skip_zero_click| flag of a credential when selected in the account
chooser.

When we create a PasswordForm from a CredentialInfo, we're stripping out
data that PasswordStore requires in order to find the form in the
database. This breaks the auto-sign-in bit toggle for the credential if
it was saved via our heuristics, and not via the API.

This patch stops truncating PasswordForm to CredentialInfo when passing to
CredentialManagerDispatcher.

BUG=594110
R=vasilii@chromium.org

patch from issue 1785273002 at patchset 20001
(http://crrev.com/1785273002#ps20001)
==========

to

==========
Update the |skip_zero_click| flag of a credential when selected in the account
chooser.

When we create a PasswordForm from a CredentialInfo, we're stripping out
data that PasswordStore requires in order to find the form in the
database. This breaks the auto-sign-in bit toggle for the credential if
it was saved via our heuristics, and not via the API.

This patch stops truncating PasswordForm to CredentialInfo when passing to
CredentialManagerDispatcher.

BUG=594110
R=vasilii@chromium.org

patch from issue 1785273002 at patchset 20001
(http://crrev.com/1785273002#ps20001)

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

[commit-bot: I haz the power, 2016-03-29 10:44:54]

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