[Payments] Dedupe contact info, and phone and email editor suggestions.

[Payments] Dedupe contact info, and phone and email editor suggestions.

BUG=624869
Committed: https://crrev.com/24fa769ff3ee4d66bc7867ae6f508636b372ef30
Cr-Commit-Position: refs/heads/master@{#403651}

[sebsg, 2016-07-03 19:52:40]

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

[sebsg, 2016-07-03 19:52:40]

PTAL

[please use gerrit instead, 2016-07-03 20:02:28]

https://codereview.chromium.org/2122493003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/AddressEditor.java
(right):

https://codereview.chromium.org/2122493003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/payments/AddressEditor.java:38:
private final HashSet<CharSequence> mPhoneNumbers;
Set

https://codereview.chromium.org/2122493003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
(right):

https://codereview.chromium.org/2122493003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:249:
HashSet<String> contact_infos = new HashSet<String>();
Set<String> uniqueContactInfos = new HashSet<>();

https://codereview.chromium.org/2122493003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:264:
String contact_info = phone + email;
// Different profiles can have identical contact info.
// Do not add the same contact info to the list twice.
uniqueContactInfo

https://codereview.chromium.org/2122493003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/EditorFieldModel.java
(right):

https://codereview.chromium.org/2122493003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/EditorFieldModel.java:116:
@Nullable HashSet<CharSequence> suggestions, @Nullable EditorFieldValidator
validator,
Set

[sebsg, 2016-07-03 20:16:57]

Could you please take another look? thanks!

https://codereview.chromium.org/2122493003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/AddressEditor.java
(right):

https://codereview.chromium.org/2122493003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/payments/AddressEditor.java:38:
private final HashSet<CharSequence> mPhoneNumbers;
On 2016/07/03 20:02:28, Rouslan (ツ) wrote:
> Set

Done.

https://codereview.chromium.org/2122493003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
(right):

https://codereview.chromium.org/2122493003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:249:
HashSet<String> contact_infos = new HashSet<String>();
On 2016/07/03 20:02:28, Rouslan (ツ) wrote:
> Set<String> uniqueContactInfos = new HashSet<>();

Done.

https://codereview.chromium.org/2122493003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:264:
String contact_info = phone + email;
On 2016/07/03 20:02:28, Rouslan (ツ) wrote:
> // Different profiles can have identical contact info.
> // Do not add the same contact info to the list twice.
> uniqueContactInfo

Done.

https://codereview.chromium.org/2122493003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/EditorFieldModel.java
(right):

https://codereview.chromium.org/2122493003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/EditorFieldModel.java:116:
@Nullable HashSet<CharSequence> suggestions, @Nullable EditorFieldValidator
validator,
On 2016/07/03 20:02:28, Rouslan (ツ) wrote:
> Set

Done.

[please use gerrit instead, 2016-07-03 20:20:39]

lgtm % nits i mentioned offline

[sebsg, 2016-07-03 20:23:54]

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

[sebsg, 2016-07-03 20:23:54]

Hi bauerb@, could you please take a look at this CL? Thanks!

[Bernhard Bauer, 2016-07-04 07:57:16]

lgtm

https://codereview.chromium.org/2122493003/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/AddressEditor.java
(right):

https://codereview.chromium.org/2122493003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/AddressEditor.java:51:
mPhoneNumbers = new HashSet<>();
Nit: You could initialize these where they're declared -- less trivial code in
the constructor that way.

https://codereview.chromium.org/2122493003/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/EditorFieldModel.java
(right):

https://codereview.chromium.org/2122493003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/EditorFieldModel.java:122:
mSuggestions = new ArrayList<CharSequence>(suggestions);
|suggestion| is declared as nullable, but this constructor would throw an
exception if null is passed to it. Given that the current callers pass in final
and non-null members, you could just remove the annotation?

[sebsg, 2016-07-04 08:20:55]

Patchset #4 (id:60001) has been deleted

[sebsg, 2016-07-04 08:21:22]

Thanks for the quick review, sending to CQ.

https://codereview.chromium.org/2122493003/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/AddressEditor.java
(right):

https://codereview.chromium.org/2122493003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/AddressEditor.java:51:
mPhoneNumbers = new HashSet<>();
On 2016/07/04 07:57:16, Bernhard Bauer wrote:
> Nit: You could initialize these where they're declared -- less trivial code in
> the constructor that way.

Done.

https://codereview.chromium.org/2122493003/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/EditorFieldModel.java
(right):

https://codereview.chromium.org/2122493003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/EditorFieldModel.java:122:
mSuggestions = new ArrayList<CharSequence>(suggestions);
On 2016/07/04 07:57:16, Bernhard Bauer wrote:
> |suggestion| is declared as nullable, but this constructor would throw an
> exception if null is passed to it. Given that the current callers pass in
final
> and non-null members, you could just remove the annotation?

Done.

[sebsg, 2016-07-04 08:21:29]

The CQ bit was checked by sebsg@chromium.org

[sebsg, 2016-07-04 08:21:29]

The patchset sent to the CQ was uploaded after l-g-t-m from
rouslan@chromium.org, bauerb@chromium.org
Link to the patchset: https://codereview.chromium.org/2122493003/#ps80001
(title: "Addressed bauerb's comments")

[commit-bot: I haz the power, 2016-07-04 08:21:38]

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, 2016-07-04 08:54:48]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-07-04 08:56:12]

Description was changed from

==========
[Payments] Dedupe contact info, and phone and email editor suggestions.

BUG=624869
==========

to

==========
[Payments] Dedupe contact info, and phone and email editor suggestions.

BUG=624869

Committed: https://crrev.com/24fa769ff3ee4d66bc7867ae6f508636b372ef30
Cr-Commit-Position: refs/heads/master@{#403651}
==========

[commit-bot: I haz the power, 2016-07-04 08:56:13]

Patchset 4 (id:??) landed as
https://crrev.com/24fa769ff3ee4d66bc7867ae6f508636b372ef30
Cr-Commit-Position: refs/heads/master@{#403651}