[Payments] Update metadata instead of billing address for server cards.

[Payments] Update metadata instead of billing address for server cards.

Changes the function signature used in android code to update the billing_address_id of server cards. Now sends the whole card to the C++ instead of just the id and billing_address_id.

BUG=680180
Review-Url: https://codereview.chromium.org/2625413002
Cr-Commit-Position: refs/heads/master@{#443907}
Committed: https://chromium.googlesource.com/chromium/src/+/0ed6d578394cf6648a30dbb8714bc22b99ab91e4

[sebsg, 2017-01-12 20:33:44]

Patchset #1 (id:1) has been deleted

[sebsg, 2017-01-12 20:39:12]

Description was changed from

==========
[Payments] Change method name for saving server cards metadata.

BUG=680180
==========

to

==========
[Payments] Change method name for saving server cards metadata.

This CL should land after https://codereview.chromium.org/2626843004

BUG=680180
==========

[sebsg, 2017-01-12 20:41:36]

Description was changed from

==========
[Payments] Change method name for saving server cards metadata.

This CL should land after https://codereview.chromium.org/2626843004

BUG=680180
==========

to

==========
[Payments] Update metadata instead of billing address for server cards.

This CL should land after https://codereview.chromium.org/2626843004

BUG=680180
==========

[sebsg, 2017-01-12 20:41:53]

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

[sebsg, 2017-01-12 20:41:54]

Hi Rouslan, PTAL

[please use gerrit instead, 2017-01-12 20:57:04]

https://codereview.chromium.org/2625413002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java
(right):

https://codereview.chromium.org/2625413002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java:768:
pdm.updateServerCardMetadata(card);
I would prefer if this method was still called updateServerCardBillingAddress(),
because that conveys its meaning and purpose better. It's OK to remove the
billing address ID parameter. It's also OK if updateServerCardBillingAddress()
in personal_data_manager_android.cc calls updateServerCardMetadata() under the
hood.

https://codereview.chromium.org/2625413002/diff/20001/chrome/browser/autofill...
File chrome/browser/autofill/android/personal_data_manager_android.cc (right):

https://codereview.chromium.org/2625413002/diff/20001/chrome/browser/autofill...
chrome/browser/autofill/android/personal_data_manager_android.cc:663: LOG(ERROR)
<< "Got the card";
remove logs

[sebsg, 2017-01-12 21:12:42]

Thanks! Another look?

https://codereview.chromium.org/2625413002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java
(right):

https://codereview.chromium.org/2625413002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java:768:
pdm.updateServerCardMetadata(card);
On 2017/01/12 20:57:04, rouslan wrote:
> I would prefer if this method was still called
updateServerCardBillingAddress(),
> because that conveys its meaning and purpose better. It's OK to remove the
> billing address ID parameter. It's also OK if updateServerCardBillingAddress()
> in personal_data_manager_android.cc calls updateServerCardMetadata() under the
> hood.

Done.

https://codereview.chromium.org/2625413002/diff/20001/chrome/browser/autofill...
File chrome/browser/autofill/android/personal_data_manager_android.cc (right):

https://codereview.chromium.org/2625413002/diff/20001/chrome/browser/autofill...
chrome/browser/autofill/android/personal_data_manager_android.cc:663: LOG(ERROR)
<< "Got the card";
On 2017/01/12 20:57:04, rouslan wrote:
> remove logs

Done.

[please use gerrit instead, 2017-01-13 14:31:09]

lgtm % comment

https://codereview.chromium.org/2625413002/diff/40001/components/autofill/cor...
File components/autofill/core/browser/personal_data_manager.h (right):

https://codereview.chromium.org/2625413002/diff/40001/components/autofill/cor...
components/autofill/core/browser/personal_data_manager.h:155: // Updates the
metadata for the server |credit_card|. Looks up the card by
specify exactly the data that's going to be updated.

[sebsg, 2017-01-13 20:23:58]

Thanks Rouslan. Looking at the CL, I don't think I have to way for the CL that
adds the fields to the DB to land this. WDYT?

https://codereview.chromium.org/2625413002/diff/40001/components/autofill/cor...
File components/autofill/core/browser/personal_data_manager.h (right):

https://codereview.chromium.org/2625413002/diff/40001/components/autofill/cor...
components/autofill/core/browser/personal_data_manager.h:155: // Updates the
metadata for the server |credit_card|. Looks up the card by
On 2017/01/13 14:31:09, rouslan wrote:
> specify exactly the data that's going to be updated.

Done.

[please use gerrit instead, 2017-01-13 20:30:23]

You're right, no need to wait. Update the CL description to remove mention of
the other CL and make it clear that you're changing function signature, not data
on disk.

[sebsg, 2017-01-13 20:35:49]

Description was changed from

==========
[Payments] Update metadata instead of billing address for server cards.

This CL should land after https://codereview.chromium.org/2626843004

BUG=680180
==========

to

==========
[Payments] Update metadata instead of billing address for server cards.

Changes the function signature used in android code to update the
billing_address_id of server cards. Now sends the whole card to the C++ instead
of just the id and billing_address_id.

BUG=680180
==========

[sebsg, 2017-01-13 20:36:13]

The CQ bit was checked by sebsg@chromium.org

[sebsg, 2017-01-13 20:36:13]

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

[commit-bot: I haz the power, 2017-01-13 20:37:17]

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-01-13 20:48:44]

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

[commit-bot: I haz the power, 2017-01-13 20:48:45]

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

[sebsg, 2017-01-13 20:52:10]

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

[sebsg, 2017-01-13 20:52:10]

Hi Ted, could you please review AutofillServerCardEditor.java ?

Thanks!

[Ted C, 2017-01-13 23:42:10]

On 2017/01/13 20:52:10, sebsg wrote:
> Hi Ted, could you please review AutofillServerCardEditor.java ?
> 
> Thanks!

AutofillServerCardEditor.java - lgtm

[sebsg, 2017-01-16 16:02:04]

The CQ bit was checked by sebsg@chromium.org

[commit-bot: I haz the power, 2017-01-16 16:02:24]

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-01-16 16:58:36]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1484582524910530,
"parent_rev": "6b8b86255aca589f4eb99b80a79edf117377fe5f", "commit_rev":
"0ed6d578394cf6648a30dbb8714bc22b99ab91e4"}

[commit-bot: I haz the power, 2017-01-16 16:59:07]

Description was changed from

==========
[Payments] Update metadata instead of billing address for server cards.

Changes the function signature used in android code to update the
billing_address_id of server cards. Now sends the whole card to the C++ instead
of just the id and billing_address_id.

BUG=680180
==========

to

==========
[Payments] Update metadata instead of billing address for server cards.

Changes the function signature used in android code to update the
billing_address_id of server cards. Now sends the whole card to the C++ instead
of just the id and billing_address_id.

BUG=680180

Review-Url: https://codereview.chromium.org/2625413002
Cr-Commit-Position: refs/heads/master@{#443907}
Committed:
https://chromium.googlesource.com/chromium/src/+/0ed6d578394cf6648a30dbb8714b...
==========

[commit-bot: I haz the power, 2017-01-16 16:59:09]

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