PaymentRequest: Sync up PaymentAddress with the payment request spec.

PaymentRequest: Sync up PaymentAddress with the payment request spec.

Some attributes in PaymentAddress was renamed and added. This CL syncs up
the PaymentAddress with the spec[1].

[1] https://w3c.github.io/browser-payment-api/specs/paymentrequest.html#paymentaddress-interface

BUG=617190
Committed: https://crrev.com/49b94a9305230f24b934d79dc6b56df517b71a05
Cr-Commit-Position: refs/heads/master@{#398455}

[zino, 2016-06-06 14:15:44]

Description was changed from

==========
hello

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

to

==========
PaymentRequest: Sync up PaymentAddress with the payment request spec.

Some attributes in PaymentAddress was renamed and added. This CL syncs up
the PaymentAddress with the spec[1].

[1]
https://w3c.github.io/browser-payment-api/specs/paymentrequest.html#paymentad...
==========

[zino, 2016-06-06 14:15:44]

jinho.bang@samsung.com changed reviewers:
+ rouslan@chromium.org

[zino, 2016-06-06 14:16:22]

Description was changed from

==========
PaymentRequest: Sync up PaymentAddress with the payment request spec.

Some attributes in PaymentAddress was renamed and added. This CL syncs up
the PaymentAddress with the spec[1].

[1]
https://w3c.github.io/browser-payment-api/specs/paymentrequest.html#paymentad...
==========

to

==========
PaymentRequest: Sync up PaymentAddress with the payment request spec.

Some attributes in PaymentAddress was renamed and added. This CL syncs up
the PaymentAddress with the spec[1].

[1]
https://w3c.github.io/browser-payment-api/specs/paymentrequest.html#paymentad...

BUG=617190
==========

[zino, 2016-06-06 14:16:44]

PTAL

[please use gerrit instead, 2016-06-06 16:01:15]

Excellent! Let's add phone and careOf everywhere as well.

https://codereview.chromium.org/2039863002/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillAddress.java
(right):

https://codereview.chromium.org/2039863002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillAddress.java:68:
result.recipient = mProfile.getFullName();
Please also add "careOf" and "phone".

result.careOf = "";
result.phone = mProfile.getPhoneNumber();

https://codereview.chromium.org/2039863002/diff/60001/chrome/android/junit/sr...
File
chrome/android/junit/src/org/chromium/chrome/browser/payments/AutofillAddressTest.java
(right):

https://codereview.chromium.org/2039863002/diff/60001/chrome/android/junit/sr...
chrome/android/junit/src/org/chromium/chrome/browser/payments/AutofillAddressTest.java:53:
Assert.assertEquals("Latn", output.scriptCode);
Test that output.careOf is empty string and output.phone is "phone number".

https://codereview.chromium.org/2039863002/diff/60001/chrome/test/data/androi...
File chrome/test/data/android/payments/util.js (right):

https://codereview.chromium.org/2039863002/diff/60001/chrome/test/data/androi...
chrome/test/data/android/payments/util.js:32: dict.recipient = addr.recipient;
careOf and phone.

https://codereview.chromium.org/2039863002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
(right):

https://codereview.chromium.org/2039863002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt:4192:
method constructor
+ phone

https://codereview.chromium.org/2039863002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/payments/PaymentAddress.cpp (right):

https://codereview.chromium.org/2039863002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentAddress.cpp:19: ,
m_recipient(address->recipient)
+careOf
+phone

https://codereview.chromium.org/2039863002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/payments/PaymentAddress.h (right):

https://codereview.chromium.org/2039863002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentAddress.h:36: const String&
careOf() const { return m_careOf; }
+phone

https://codereview.chromium.org/2039863002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/payments/PaymentAddress.idl (right):

https://codereview.chromium.org/2039863002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentAddress.idl:20: readonly
attribute DOMString careOf;
+phone

https://codereview.chromium.org/2039863002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentAddress.idl:24: // readonly
attribute DOMString phone;
Yes, let's add it here. http://crbug.com/617195 is for PaymentResponse. So
changes to PaymentAddress best fit into your patch.

https://codereview.chromium.org/2039863002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/payments/PaymentAddressTest.cpp (right):

https://codereview.chromium.org/2039863002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentAddressTest.cpp:29:
input->recipient = "Jon Doe";
+careOf
+phone

https://codereview.chromium.org/2039863002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentAddressTest.cpp:45:
EXPECT_EQ("Jon Doe", output.recipient());
+careOf
+phone

https://codereview.chromium.org/2039863002/diff/60001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/modules/payments/payment_request.mojom
(right):

https://codereview.chromium.org/2039863002/diff/60001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/payments/payment_request.mojom:30:
string recipient;
+careOf
+phone

[zino, 2016-06-06 17:23:16]

Thank you for your review!

I addressed all of your comments.

Also, the bot error will disappear after
https://codereview.chromium.org/2038333002/ landed.

https://codereview.chromium.org/2039863002/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillAddress.java
(right):

https://codereview.chromium.org/2039863002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillAddress.java:68:
result.recipient = mProfile.getFullName();
On 2016/06/06 16:01:14, Rouslan (ツ) wrote:
> Please also add "careOf" and "phone".
> 
> result.careOf = "";
> result.phone = mProfile.getPhoneNumber();

Done.

https://codereview.chromium.org/2039863002/diff/60001/chrome/android/junit/sr...
File
chrome/android/junit/src/org/chromium/chrome/browser/payments/AutofillAddressTest.java
(right):

https://codereview.chromium.org/2039863002/diff/60001/chrome/android/junit/sr...
chrome/android/junit/src/org/chromium/chrome/browser/payments/AutofillAddressTest.java:53:
Assert.assertEquals("Latn", output.scriptCode);
On 2016/06/06 16:01:14, Rouslan (ツ) wrote:
> Test that output.careOf is empty string and output.phone is "phone number".

Done.

https://codereview.chromium.org/2039863002/diff/60001/chrome/test/data/androi...
File chrome/test/data/android/payments/util.js (right):

https://codereview.chromium.org/2039863002/diff/60001/chrome/test/data/androi...
chrome/test/data/android/payments/util.js:32: dict.recipient = addr.recipient;
On 2016/06/06 16:01:14, Rouslan (ツ) wrote:
> careOf and phone.

Done.

https://codereview.chromium.org/2039863002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
(right):

https://codereview.chromium.org/2039863002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt:4192:
method constructor
On 2016/06/06 16:01:15, Rouslan (ツ) wrote:
> + phone

Done.

https://codereview.chromium.org/2039863002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/payments/PaymentAddress.cpp (right):

https://codereview.chromium.org/2039863002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentAddress.cpp:19: ,
m_recipient(address->recipient)
On 2016/06/06 16:01:15, Rouslan (ツ) wrote:
> +careOf
> +phone

Done.

https://codereview.chromium.org/2039863002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/payments/PaymentAddress.h (right):

https://codereview.chromium.org/2039863002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentAddress.h:36: const String&
careOf() const { return m_careOf; }
On 2016/06/06 16:01:15, Rouslan (ツ) wrote:
> +phone

Done.

https://codereview.chromium.org/2039863002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/payments/PaymentAddress.idl (right):

https://codereview.chromium.org/2039863002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentAddress.idl:20: readonly
attribute DOMString careOf;
On 2016/06/06 16:01:15, Rouslan (ツ) wrote:
> +phone

Done.

https://codereview.chromium.org/2039863002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/payments/PaymentAddressTest.cpp (right):

https://codereview.chromium.org/2039863002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentAddressTest.cpp:29:
input->recipient = "Jon Doe";
On 2016/06/06 16:01:15, Rouslan (ツ) wrote:
> +careOf
> +phone

Done.

https://codereview.chromium.org/2039863002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentAddressTest.cpp:45:
EXPECT_EQ("Jon Doe", output.recipient());
On 2016/06/06 16:01:15, Rouslan (ツ) wrote:
> +careOf
> +phone

Done.

https://codereview.chromium.org/2039863002/diff/60001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/modules/payments/payment_request.mojom
(right):

https://codereview.chromium.org/2039863002/diff/60001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/payments/payment_request.mojom:30:
string recipient;
On 2016/06/06 16:01:15, Rouslan (ツ) wrote:
> +careOf
> +phone

Done.

[please use gerrit instead, 2016-06-06 17:35:48]

Only a couple of minor comments remaining.

https://codereview.chromium.org/2039863002/diff/80001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillAddress.java
(right):

https://codereview.chromium.org/2039863002/diff/80001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillAddress.java:50:
assert profile.getPhoneNumber() != null : "Phone number should not be null";
To avoid hitting this assert, you should check for phone number here as well:

https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chr...

To make sure that user is aware of their phone number being sent to the
merchant, you should add "suggested_fields->push_back(PHONE_HOME_WHOLE_NUMBER);"
here:

https://cs.chromium.org/chromium/src/chrome/browser/autofill/android/personal...

[zino, 2016-06-06 18:02:17]

Thank you for review

https://codereview.chromium.org/2039863002/diff/80001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillAddress.java
(right):

https://codereview.chromium.org/2039863002/diff/80001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillAddress.java:50:
assert profile.getPhoneNumber() != null : "Phone number should not be null";
On 2016/06/06 17:35:48, Rouslan (ツ) wrote:
> To avoid hitting this assert, you should check for phone number here as well:
> 
>
https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chr...
> 
> To make sure that user is aware of their phone number being sent to the
> merchant, you should add
"suggested_fields->push_back(PHONE_HOME_WHOLE_NUMBER);"
> here:
> 
>
https://cs.chromium.org/chromium/src/chrome/browser/autofill/android/personal...

Done.

[please use gerrit instead, 2016-06-06 18:12:17]

lgtm

[zino, 2016-06-06 18:12:22]

Patchset #6 (id:100001) has been deleted

[please use gerrit instead, 2016-06-06 18:12:26]

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

[commit-bot: I haz the power, 2016-06-06 18:13:03]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2039863002/120001

[commit-bot: I haz the power, 2016-06-06 18:27:40]

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

[commit-bot: I haz the power, 2016-06-06 18:27:42]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_compile_dbg on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  chromeos_daisy_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

[zino, 2016-06-06 19:14:59]

The CQ bit was checked by jinho.bang@samsung.com

[commit-bot: I haz the power, 2016-06-06 19:15:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2039863002/120001

[commit-bot: I haz the power, 2016-06-06 19:24:46]

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

[commit-bot: I haz the power, 2016-06-06 19:24:47]

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

[zino, 2016-06-06 23:55:09]

The CQ bit was checked by jinho.bang@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-06 23:55:40]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2039863002/120001

[commit-bot: I haz the power, 2016-06-07 00:07:21]

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

[commit-bot: I haz the power, 2016-06-07 00:07:23]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  chromeos_daisy_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

[zino, 2016-06-07 12:13:28]

Patchset #7 (id:140001) has been deleted

[zino, 2016-06-07 12:27:53]

https://codereview.chromium.org/2039863002/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/payments/PaymentAddress.idl (left):

https://codereview.chromium.org/2039863002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/payments/PaymentAddress.idl:11: readonly
attribute sequence<DOMString> addressLine;
When using FrozenArray type, link error occurs in debug build. it causes bot
failure.
I filed a bug (http://crbug.com/617919) and uploaded a patch to fix it but
I'm not sure if it's a right way. So, to avoid blocking landing this, I left a
comment and used sequence<> instead of FrozenArray<>.
What do you think rouslan@?

[zino, 2016-06-07 12:29:33]

jinho.bang@samsung.com changed reviewers:
+ dfalcantara@chromium.or, tsepez@chromium.org

[zino, 2016-06-07 12:30:21]

jinho.bang@samsung.com changed reviewers:
+ dfalcantara@chromium.org, estade@chromium.org
- dfalcantara@chromium.or

[zino, 2016-06-07 12:32:53]

Dear all,

Please confirm this.

+dfalcantara@ for chrome/android/*

+estade@ for chrome/browser/autofill/*

+tsepez@ for *.mojom

Thank you.

[zino, 2016-06-07 12:47:15]

https://codereview.chromium.org/2039863002/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/payments/PaymentAddress.idl (left):

https://codereview.chromium.org/2039863002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/payments/PaymentAddress.idl:11: readonly
attribute sequence<DOMString> addressLine;
On 2016/06/07 12:27:53, zino wrote:
> When using FrozenArray type, link error occurs in debug build. it causes bot
> failure.
> I filed a bug (http://crbug.com/617919) and uploaded a patch to fix it but
> I'm not sure if it's a right way. So, to avoid blocking landing this, I left a
> comment and used sequence<> instead of FrozenArray<>.
> What do you think rouslan@?

Sorry, I've just gotten lgtm from bash1@.
(https://codereview.chromium.org/2044083002/)
So, we can use FrozenArray<> from now.
Never mind my previous comment.

Thank you.

[Tom Sepez, 2016-06-07 16:10:38]

mojom lgtm

[please use gerrit instead, 2016-06-07 17:06:10]

rouslan@chromium.org changed reviewers:
+ mathp@chromium.org
- estade@chromium.org

[please use gerrit instead, 2016-06-07 17:06:11]

+mathp@ for personal_data_manager_android.cc

-estade@, as he is ooo.

[Mathieu, 2016-06-07 17:28:32]

On 2016/06/07 at 17:06:11, rouslan wrote:
> +mathp@ for personal_data_manager_android.cc
> 
> -estade@, as he is ooo.

Hi Rouslan, did you confirm whether adding phone number to GetProfileLabels will
affect the non-PR flow? If so, lgtm.

[gone, 2016-06-07 18:18:35]

android/ lgtm

[please use gerrit instead, 2016-06-07 18:41:24]

On 2016/06/07 17:28:32, Mathieu Perreault wrote:
> Hi Rouslan, did you confirm whether adding phone number to GetProfileLabels
will
> affect the non-PR flow? If so, lgtm.

Only PR flow is affected. We're good.

[zino, 2016-06-07 23:30:39]

The CQ bit was checked by jinho.bang@samsung.com

[zino, 2016-06-07 23:30:40]

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/2039863002/#ps180001
(title: " ")

[commit-bot: I haz the power, 2016-06-07 23:31:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2039863002/180001

[commit-bot: I haz the power, 2016-06-08 01:04:56]

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

[commit-bot: I haz the power, 2016-06-08 01:04:58]

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

[zino, 2016-06-08 01:09:11]

The CQ bit was checked by jinho.bang@samsung.com

[commit-bot: I haz the power, 2016-06-08 01:09:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2039863002/180001

[commit-bot: I haz the power, 2016-06-08 01:53:10]

Description was changed from

==========
PaymentRequest: Sync up PaymentAddress with the payment request spec.

Some attributes in PaymentAddress was renamed and added. This CL syncs up
the PaymentAddress with the spec[1].

[1]
https://w3c.github.io/browser-payment-api/specs/paymentrequest.html#paymentad...

BUG=617190
==========

to

==========
PaymentRequest: Sync up PaymentAddress with the payment request spec.

Some attributes in PaymentAddress was renamed and added. This CL syncs up
the PaymentAddress with the spec[1].

[1]
https://w3c.github.io/browser-payment-api/specs/paymentrequest.html#paymentad...

BUG=617190
==========

[commit-bot: I haz the power, 2016-06-08 01:53:12]

Committed patchset #8 (id:180001)

[commit-bot: I haz the power, 2016-06-08 01:55:13]

Description was changed from

==========
PaymentRequest: Sync up PaymentAddress with the payment request spec.

Some attributes in PaymentAddress was renamed and added. This CL syncs up
the PaymentAddress with the spec[1].

[1]
https://w3c.github.io/browser-payment-api/specs/paymentrequest.html#paymentad...

BUG=617190
==========

to

==========
PaymentRequest: Sync up PaymentAddress with the payment request spec.

Some attributes in PaymentAddress was renamed and added. This CL syncs up
the PaymentAddress with the spec[1].

[1]
https://w3c.github.io/browser-payment-api/specs/paymentrequest.html#paymentad...

BUG=617190

Committed: https://crrev.com/49b94a9305230f24b934d79dc6b56df517b71a05
Cr-Commit-Position: refs/heads/master@{#398455}
==========

[commit-bot: I haz the power, 2016-06-08 01:55:14]

Patchset 8 (id:??) landed as
https://crrev.com/49b94a9305230f24b934d79dc6b56df517b71a05
Cr-Commit-Position: refs/heads/master@{#398455}