Offer to save card locally if user manually enters same card number as

Offer to save card locally if user manually enters same card number as
masked server card.

BUG=717665
Review-Url: https://codereview.chromium.org/2855943002
Cr-Commit-Position: refs/heads/master@{#469093}
Committed: https://chromium.googlesource.com/chromium/src/+/42c3e10afe8f751a662458b535fe001bdcd26f28

[csashi, 2017-05-02 19:16:37]

csashi@google.com changed reviewers:
+ mathp@chromium.org

[csashi, 2017-05-02 19:16:38]

Hi Mathieu,
Please take a look or reassign if overloaded. Thanks!
-sashi.

[csashi, 2017-05-02 19:16:50]

The CQ bit was checked by csashi@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-02 19:17:26]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Mathieu, 2017-05-02 19:18:07]

Description was changed from

==========
Offer to save card locally if user manually enters same card number as
masked server card.

BUG=717665
==========

to

==========
Offer to save card locally if user manually enters same card number as
masked server card.

BUG=717665
==========

[Mathieu, 2017-05-02 19:18:07]

mathp@chromium.org changed reviewers:
+ sebsg@chromium.org
- mathp@chromium.org

[Mathieu, 2017-05-02 19:18:14]

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

[Mathieu, 2017-05-02 19:18:15]

Over to Seb :)

[commit-bot: I haz the power, 2017-05-02 19:21:06]

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

[commit-bot: I haz the power, 2017-05-02 19:21:07]

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

[csashi, 2017-05-02 19:30:56]

The CQ bit was checked by csashi@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-02 19:31:16]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-02 19:41:04]

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

[commit-bot: I haz the power, 2017-05-02 19:41:05]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[csashi, 2017-05-02 20:41:56]

The CQ bit was checked by csashi@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-02 20:42:25]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-02 21:38:26]

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

[commit-bot: I haz the power, 2017-05-02 21:38:27]

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

[csashi, 2017-05-02 21:56:02]

The CQ bit was checked by csashi@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-02 21:56:24]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-02 23:32:45]

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

[commit-bot: I haz the power, 2017-05-02 23:32:46]

Dry run: This issue passed the CQ dry run.

[sebsg, 2017-05-03 17:23:30]

One small comment :)

https://codereview.chromium.org/2855943002/diff/60001/components/autofill/cor...
File components/autofill/core/browser/personal_data_manager_unittest.cc (right):

https://codereview.chromium.org/2855943002/diff/60001/components/autofill/cor...
components/autofill/core/browser/personal_data_manager_unittest.cc:4251:
DuplicateMaskedServerCard_DontDuplicateFullServerCard) {
Can you split this one in two? Like DontDuplicateFullSercerCard and
SaveCopyOfMaskedCardLocally (or something similar).

[csashi, 2017-05-03 17:43:09]

Hi Sebastien,
Please take a look. Thanks! -sashi.

https://codereview.chromium.org/2855943002/diff/60001/components/autofill/cor...
File components/autofill/core/browser/personal_data_manager_unittest.cc (right):

https://codereview.chromium.org/2855943002/diff/60001/components/autofill/cor...
components/autofill/core/browser/personal_data_manager_unittest.cc:4251:
DuplicateMaskedServerCard_DontDuplicateFullServerCard) {
On 2017/05/03 17:23:30, sebsg wrote:
> Can you split this one in two? Like DontDuplicateFullSercerCard and
> SaveCopyOfMaskedCardLocally (or something similar).

Done.

[csashi, 2017-05-03 17:43:16]

The CQ bit was checked by csashi@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-03 17:43:50]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sebsg, 2017-05-03 17:58:23]

LGTM % a small comment.

Thank you!

https://codereview.chromium.org/2855943002/diff/80001/components/autofill/cor...
File components/autofill/core/browser/personal_data_manager_unittest.cc (right):

https://codereview.chromium.org/2855943002/diff/80001/components/autofill/cor...
components/autofill/core/browser/personal_data_manager_unittest.cc:4353:
EXPECT_FALSE(imported_credit_card);
Can you also
EXPECT_FALSE(imported_credit_card_matches_masked_server_credit_card)? Thanks
you!

[csashi, 2017-05-03 18:05:49]

https://codereview.chromium.org/2855943002/diff/80001/components/autofill/cor...
File components/autofill/core/browser/personal_data_manager_unittest.cc (right):

https://codereview.chromium.org/2855943002/diff/80001/components/autofill/cor...
components/autofill/core/browser/personal_data_manager_unittest.cc:4353:
EXPECT_FALSE(imported_credit_card);
On 2017/05/03 17:58:22, sebsg wrote:
> Can you also
> EXPECT_FALSE(imported_credit_card_matches_masked_server_credit_card)? Thanks
> you!

Done.

[csashi, 2017-05-03 18:06:46]

On 2017/05/03 18:05:49, csashi wrote:
>
https://codereview.chromium.org/2855943002/diff/80001/components/autofill/cor...
> File components/autofill/core/browser/personal_data_manager_unittest.cc
(right):
> 
>
https://codereview.chromium.org/2855943002/diff/80001/components/autofill/cor...
> components/autofill/core/browser/personal_data_manager_unittest.cc:4353:
> EXPECT_FALSE(imported_credit_card);
> On 2017/05/03 17:58:22, sebsg wrote:
> > Can you also
> > EXPECT_FALSE(imported_credit_card_matches_masked_server_credit_card)? Thanks
> > you!
> 
> Done.

Hi Mathieu,
Please let me know if you have bandwidth to review or I can commit because I
have all approvals. Thanks!
-sashi.

[csashi, 2017-05-03 18:06:53]

The CQ bit was checked by csashi@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-03 18:07:51]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Mathieu, 2017-05-03 18:21:18]

I trust Seb! Make sure you have complete test coverage would be my only
suggestion from a quick glance.

[commit-bot: I haz the power, 2017-05-03 18:59:30]

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

[commit-bot: I haz the power, 2017-05-03 18:59:31]

Dry run: This issue passed the CQ dry run.

[csashi, 2017-05-03 19:56:14]

The CQ bit was checked by csashi@google.com

[csashi, 2017-05-03 19:56:15]

The patchset sent to the CQ was uploaded after l-g-t-m from sebsg@chromium.org
Link to the patchset: https://codereview.chromium.org/2855943002/#ps30007
(title: "Creates separate tests for duplicating masked server card and not
duplicating full server card.")

[commit-bot: I haz the power, 2017-05-03 19:57:04]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-03 20:04:12]

CQ is committing da patch.

Bot data: {"patchset_id": 30007, "attempt_start_ts": 1493841374768420,
"parent_rev": "eba2b9c32c0df7536d5884ab0d645b2701815206", "commit_rev":
"42c3e10afe8f751a662458b535fe001bdcd26f28"}

[commit-bot: I haz the power, 2017-05-03 20:04:20]

Description was changed from

==========
Offer to save card locally if user manually enters same card number as
masked server card.

BUG=717665
==========

to

==========
Offer to save card locally if user manually enters same card number as
masked server card.

BUG=717665

Review-Url: https://codereview.chromium.org/2855943002
Cr-Commit-Position: refs/heads/master@{#469093}
Committed:
https://chromium.googlesource.com/chromium/src/+/42c3e10afe8f751a662458b535fe...
==========

[commit-bot: I haz the power, 2017-05-03 20:04:21]

Committed patchset #6 (id:30007) as
https://chromium.googlesource.com/chromium/src/+/42c3e10afe8f751a662458b535fe...