[Payments] Show required information for contact details and set appropriate editor title

Show required information for contact details and set appropriate editor title

BUG=673855
Committed: https://crrev.com/6c71598c9a27ee0e48a76851eae954ad94b61f39
Cr-Commit-Position: refs/heads/master@{#438626}

[gogerald1, 2016-11-25 18:42:48]

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

[commit-bot: I haz the power, 2016-11-25 18:43:00]

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

[gogerald1, 2016-11-25 18:44:29]

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

[commit-bot: I haz the power, 2016-11-25 18:44:40]

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, 2016-11-25 19:07:33]

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

[commit-bot: I haz the power, 2016-11-25 19:07:34]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)

[gogerald1, 2016-11-25 19:20:13]

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

[commit-bot: I haz the power, 2016-11-25 19:20:31]

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

[gogerald1, 2016-11-25 19:20:54]

Patchset #2 (id:20001) has been deleted

[gogerald1, 2016-11-25 19:21:06]

Patchset #2 (id:40001) has been deleted

[gogerald1, 2016-11-25 19:21:20]

Description was changed from

==========
format


format


show missing information for contact info

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

to

==========
show missing information for contact info

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

[gogerald1, 2016-11-25 19:22:24]

Description was changed from

==========
show missing information for contact info

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

to

==========
Show required information for contact and set appropriate editor title

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

[gogerald1, 2016-11-25 20:24:32]

Patchset #2 (id:60001) has been deleted

[gogerald1, 2016-11-25 20:24:37]

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

[commit-bot: I haz the power, 2016-11-25 20:24:51]

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

[gogerald1, 2016-11-25 21:33:59]

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

[commit-bot: I haz the power, 2016-11-25 21:34:09]

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, 2016-11-25 21:58:41]

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

[commit-bot: I haz the power, 2016-11-25 21:58:42]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[gogerald1, 2016-11-25 22:02:19]

Patchset #3 (id:100001) has been deleted

[gogerald1, 2016-11-25 22:02:25]

Patchset #2 (id:80001) has been deleted

[gogerald1, 2016-12-13 20:43:04]

Patchset #1 (id:1) has been deleted

[gogerald1, 2016-12-13 20:43:24]

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

[gogerald1, 2016-12-13 20:43:41]

Description was changed from

==========
Show required information for contact and set appropriate editor title

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

to

==========
Show required information for contact and set appropriate editor title

BUG=673855
==========

[commit-bot: I haz the power, 2016-12-13 20:43:47]

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

[gogerald1, 2016-12-13 20:44:05]

Description was changed from

==========
Show required information for contact and set appropriate editor title

BUG=673855
==========

to

==========
Show required information for contact details and set appropriate editor title

BUG=673855
==========

[gogerald1, 2016-12-13 20:48:11]

Patchset #1 (id:120001) has been deleted

[commit-bot: I haz the power, 2016-12-13 21:20:25]

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

[commit-bot: I haz the power, 2016-12-13 21:20:26]

Dry run: This issue passed the CQ dry run.

[gogerald1, 2016-12-13 21:42:44]

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

[commit-bot: I haz the power, 2016-12-13 21:43:19]

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

[gogerald1, 2016-12-13 21:45:55]

gogerald@chromium.org changed reviewers:
+ rouslan@chromium.org, tedchoc@chromium.org

[gogerald1, 2016-12-13 21:45:56]

Hi rouslan@ and tedchoc@, PTAL,

[commit-bot: I haz the power, 2016-12-13 22:30:46]

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

[commit-bot: I haz the power, 2016-12-13 22:30:47]

Dry run: This issue passed the CQ dry run.

[please use gerrit instead, 2016-12-14 15:32:35]

lgtm % comments

https://codereview.chromium.org/2529043003/diff/160001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillContact.java
(right):

https://codereview.chromium.org/2529043003/diff/160001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillContact.java:38:
@Nullable String phone, @Nullable String email, int completionStatus) {
Can you put @CompletionStatus here?

https://codereview.chromium.org/2529043003/diff/160001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillContact.java:102:

Would a switch statement look cleaner? It will save a few lines of code, for
sure.

https://codereview.chromium.org/2529043003/diff/160001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillContact.java:130:
}
return;
}

assert false : "Unknown completion status code";

https://codereview.chromium.org/2529043003/diff/160001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/ContactEditor.java
(right):

https://codereview.chromium.org/2529043003/diff/160001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/ContactEditor.java:142:
checkContactCompletionStatus(null, null, null))
You can use INVALID_MULTIPLE_FIELDS here directly to save yourself one function
call.

https://codereview.chromium.org/2529043003/diff/160001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/ContactEditor.java:171:
EditorModel editor = new EditorModel(toEdit == null
You can replace everything inside of () with contact.getEditTitle().

https://codereview.chromium.org/2529043003/diff/160001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
(right):

https://codereview.chromium.org/2529043003/diff/160001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:429:
int completionStatus =
Can you put @CompletionStatus here?

[gogerald1, 2016-12-14 18:35:43]

Hi Ted, PTAL of the patch 3 after addressing rouslan's comments.

https://codereview.chromium.org/2529043003/diff/160001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillContact.java
(right):

https://codereview.chromium.org/2529043003/diff/160001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillContact.java:38:
@Nullable String phone, @Nullable String email, int completionStatus) {
On 2016/12/14 15:32:35, rouslan wrote:
> Can you put @CompletionStatus here?

Done.

https://codereview.chromium.org/2529043003/diff/160001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillContact.java:102:

On 2016/12/14 15:32:35, rouslan wrote:
> Would a switch statement look cleaner? It will save a few lines of code, for
> sure.

Done.

https://codereview.chromium.org/2529043003/diff/160001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillContact.java:130:
}
On 2016/12/14 15:32:35, rouslan wrote:
> return;
> }
> 
> assert false : "Unknown completion status code";

Done.

https://codereview.chromium.org/2529043003/diff/160001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/ContactEditor.java
(right):

https://codereview.chromium.org/2529043003/diff/160001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/ContactEditor.java:142:
checkContactCompletionStatus(null, null, null))
On 2016/12/14 15:32:35, rouslan wrote:
> You can use INVALID_MULTIPLE_FIELDS here directly to save yourself one
function
> call.

Done.

https://codereview.chromium.org/2529043003/diff/160001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/ContactEditor.java:171:
EditorModel editor = new EditorModel(toEdit == null
On 2016/12/14 15:32:35, rouslan wrote:
> You can replace everything inside of () with contact.getEditTitle().

Can not do that here since we display "Add contact info" instead of "Add more
information" for adding new contact, right? The 'completionStatus' for
constructing contact above is only used to set correct completion status in this
case.

https://codereview.chromium.org/2529043003/diff/160001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
(right):

https://codereview.chromium.org/2529043003/diff/160001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:429:
int completionStatus =
On 2016/12/14 15:32:35, rouslan wrote:
> Can you put @CompletionStatus here?

Done.

[gogerald1, 2016-12-14 18:35:48]

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

[commit-bot: I haz the power, 2016-12-14 18:36:59]

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, 2016-12-14 19:10:05]

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

[commit-bot: I haz the power, 2016-12-14 19:10:06]

Dry run: This issue passed the CQ dry run.

[Ted C, 2016-12-14 19:15:16]

lgtm

https://codereview.chromium.org/2529043003/diff/180001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/ContactEditor.java
(right):

https://codereview.chromium.org/2529043003/diff/180001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/ContactEditor.java:172:
? mContext.getString(R.string.payments_add_contact_details_label)
seems indented too much

[gogerald1, 2016-12-14 19:25:43]

https://codereview.chromium.org/2529043003/diff/180001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/ContactEditor.java
(right):

https://codereview.chromium.org/2529043003/diff/180001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/ContactEditor.java:172:
? mContext.getString(R.string.payments_add_contact_details_label)
On 2016/12/14 19:15:16, Ted C wrote:
> seems indented too much

Done.

[gogerald1, 2016-12-14 19:25:46]

The CQ bit was checked by gogerald@chromium.org

[gogerald1, 2016-12-14 19:25:47]

The patchset sent to the CQ was uploaded after l-g-t-m from
rouslan@chromium.org, tedchoc@chromium.org
Link to the patchset: https://codereview.chromium.org/2529043003/#ps200001
(title: "correct indentation")

[commit-bot: I haz the power, 2016-12-14 19:26:36]

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-12-14 21:20:19]

CQ is committing da patch.

Bot data: {"patchset_id": 200001, "attempt_start_ts": 1481743546688200,
"parent_rev": "78c102c8cd7f856290acc8b0dafdda73c5ef412b", "commit_rev":
"61112eaebde5f943ec73fb27ef1aa0288c8e0a8e"}

[commit-bot: I haz the power, 2016-12-14 21:20:50]

Description was changed from

==========
Show required information for contact details and set appropriate editor title

BUG=673855
==========

to

==========
Show required information for contact details and set appropriate editor title

BUG=673855

Review-Url: https://codereview.chromium.org/2529043003
==========

[commit-bot: I haz the power, 2016-12-14 21:20:51]

Committed patchset #4 (id:200001)

[commit-bot: I haz the power, 2016-12-14 21:23:01]

Description was changed from

==========
Show required information for contact details and set appropriate editor title

BUG=673855

Review-Url: https://codereview.chromium.org/2529043003
==========

to

==========
Show required information for contact details and set appropriate editor title

BUG=673855

Committed: https://crrev.com/6c71598c9a27ee0e48a76851eae954ad94b61f39
Cr-Commit-Position: refs/heads/master@{#438626}
==========

[commit-bot: I haz the power, 2016-12-14 21:23:02]

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