[Autofill] Split server card data and metadata updates.

This caused a bug where the use_date would be reset when adding a new server card.
The metadata should not be updated in this case.

BUG=696365
Review-Url: https://codereview.chromium.org/2719723002
Cr-Commit-Position: refs/heads/master@{#453210}
Committed: https://chromium.googlesource.com/chromium/src/+/464db18fe97e6e7f73667db90a761239447d8e97

[sebsg, 2017-02-26 21:40:43]

Description was changed from

==========
WIP [Autofill] Split server card data and metadata updates.

BUG=
==========

to

==========
This caused a bug where the use_date would be reset when adding a new server
card.
The metadata should not be updated in this case.

BUG=696365
==========

[sebsg, 2017-02-26 21:40:43]

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

[sebsg, 2017-02-26 21:41:01]

Hi Math, PTAL?

[sebsg, 2017-02-26 21:41:15]

Patchset #1 (id:1) has been deleted

[sebsg, 2017-02-26 21:41:46]

The CQ bit was checked by sebsg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-26 21:41:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Mathieu, 2017-02-26 22:16:44]

Thanks for fixing this, a little confused about the test...

https://codereview.chromium.org/2719723002/diff/20001/components/autofill/cor...
File components/autofill/core/browser/webdata/autofill_table_unittest.cc
(right):

https://codereview.chromium.org/2719723002/diff/20001/components/autofill/cor...
components/autofill/core/browser/webdata/autofill_table_unittest.cc:1767:
EXPECT_NE(base::Time(), outputs[0]->use_date());
should we have better values than base::Time() in this test?

https://codereview.chromium.org/2719723002/diff/20001/components/autofill/cor...
components/autofill/core/browser/webdata/autofill_table_unittest.cc:1793:
EXPECT_EQ(base::Time(), outputs[0]->use_date());
this is the bug, right?

[commit-bot: I haz the power, 2017-02-26 22:27:02]

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

[commit-bot: I haz the power, 2017-02-26 22:27:03]

Dry run: This issue passed the CQ dry run.

[sebsg, 2017-02-26 22:55:58]

Thanks, another look?

https://codereview.chromium.org/2719723002/diff/20001/components/autofill/cor...
File components/autofill/core/browser/webdata/autofill_table_unittest.cc
(right):

https://codereview.chromium.org/2719723002/diff/20001/components/autofill/cor...
components/autofill/core/browser/webdata/autofill_table_unittest.cc:1767:
EXPECT_NE(base::Time(), outputs[0]->use_date());
On 2017/02/26 22:16:44, Mathieu Perreault wrote:
> should we have better values than base::Time() in this test? 

AutofillClock to the rescue!

https://codereview.chromium.org/2719723002/diff/20001/components/autofill/cor...
components/autofill/core/browser/webdata/autofill_table_unittest.cc:1793:
EXPECT_EQ(base::Time(), outputs[0]->use_date());
On 2017/02/26 22:16:44, Mathieu Perreault wrote:
> this is the bug, right?

Good eye, I changed the test to make it more obvious.

[Mathieu, 2017-02-26 23:43:12]

lgtm with nit

https://codereview.chromium.org/2719723002/diff/40001/components/autofill/cor...
File components/autofill/core/browser/webdata/autofill_table_unittest.cc
(right):

https://codereview.chromium.org/2719723002/diff/40001/components/autofill/cor...
components/autofill/core/browser/webdata/autofill_table_unittest.cc:1812:
table_->SetServerCreditCards(inputs);
is this useful?

https://codereview.chromium.org/2719723002/diff/40001/components/autofill/cor...
components/autofill/core/browser/webdata/autofill_table_unittest.cc:1817:
inputs.back().set_use_count(1000U);
this seems to be tested on 1783?

[sebsg, 2017-02-27 13:54:53]

Thanks sending to CQ

https://codereview.chromium.org/2719723002/diff/40001/components/autofill/cor...
File components/autofill/core/browser/webdata/autofill_table_unittest.cc
(right):

https://codereview.chromium.org/2719723002/diff/40001/components/autofill/cor...
components/autofill/core/browser/webdata/autofill_table_unittest.cc:1812:
table_->SetServerCreditCards(inputs);
On 2017/02/26 23:43:12, Mathieu Perreault wrote:
> is this useful?

It clears all the data. It is there to show that if the data is cleared, it can
be put back again after.

https://codereview.chromium.org/2719723002/diff/40001/components/autofill/cor...
components/autofill/core/browser/webdata/autofill_table_unittest.cc:1817:
inputs.back().set_use_count(1000U);
On 2017/02/26 23:43:12, Mathieu Perreault wrote:
> this seems to be tested on 1783?

Done.

[sebsg, 2017-02-27 13:54:57]

The CQ bit was checked by sebsg@chromium.org

[sebsg, 2017-02-27 13:54:57]

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

[commit-bot: I haz the power, 2017-02-27 13:55:09]

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-02-27 14:43:16]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1488203697188670,
"parent_rev": "da47dac5f72b5289fb1155d1f73497ed26afac2a", "commit_rev":
"464db18fe97e6e7f73667db90a761239447d8e97"}

[commit-bot: I haz the power, 2017-02-27 14:43:52]

Description was changed from

==========
This caused a bug where the use_date would be reset when adding a new server
card.
The metadata should not be updated in this case.

BUG=696365
==========

to

==========
This caused a bug where the use_date would be reset when adding a new server
card.
The metadata should not be updated in this case.

BUG=696365

Review-Url: https://codereview.chromium.org/2719723002
Cr-Commit-Position: refs/heads/master@{#453210}
Committed:
https://chromium.googlesource.com/chromium/src/+/464db18fe97e6e7f73667db90a76...
==========

[commit-bot: I haz the power, 2017-02-27 14:43:53]

Committed patchset #3 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/464db18fe97e6e7f73667db90a76...

[sebsg, 2017-03-01 20:10:06]

A revert of this CL (patchset #3 id:60001) has been created in
https://codereview.chromium.org/2728733002/ by sebsg@chromium.org.

The reason for reverting is: Breaks the Wallet address conversion feature.