[Payments] Don't show Organization in billing address selection label.

[Payments] Don't show Organization in billing address selection label.

BUG=662161
Committed: https://crrev.com/c6af37456ab84d08503af9f99077ef7e12f27092
Cr-Commit-Position: refs/heads/master@{#430655}

[sebsg, 2016-11-04 13:46:49]

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

[sebsg, 2016-11-04 13:46:50]

Hi Rouslan, PTAL?

[please use gerrit instead, 2016-11-04 21:27:33]

LGTM % comments

https://codereview.chromium.org/2472043003/diff/1/chrome/browser/autofill/and...
File chrome/browser/autofill/android/personal_data_manager_android.cc (right):

https://codereview.chromium.org/2472043003/diff/1/chrome/browser/autofill/and...
chrome/browser/autofill/android/personal_data_manager_android.cc:391: return
GetProfileLabels(env, false, false, true,
Please add a comment here about the meaning of each of these three boolean
parameters. Unfortunately, a list of boolean parameters is a common anti-pattern
that makes reading code harder. Comments is one way to fight the anti-pattern,
although not the most ideal, because the compiler cannot check your comments for
correctness. In the interest of time for M56 release, let's just use comments.

https://codereview.chromium.org/2472043003/diff/1/chrome/browser/autofill/and...
chrome/browser/autofill/android/personal_data_manager_android.cc:401: return
GetProfileLabels(env, true, include_name_in_label,
Please add a comment about the meaning of this "true" boolean parameter.

[sebsg, 2016-11-07 15:53:02]

Thanks for the comments, sending to CQ.

https://codereview.chromium.org/2472043003/diff/1/chrome/browser/autofill/and...
File chrome/browser/autofill/android/personal_data_manager_android.cc (right):

https://codereview.chromium.org/2472043003/diff/1/chrome/browser/autofill/and...
chrome/browser/autofill/android/personal_data_manager_android.cc:391: return
GetProfileLabels(env, false, false, true,
On 2016/11/04 21:27:32, rouslan wrote:
> Please add a comment here about the meaning of each of these three boolean
> parameters. Unfortunately, a list of boolean parameters is a common
anti-pattern
> that makes reading code harder. Comments is one way to fight the anti-pattern,
> although not the most ideal, because the compiler cannot check your comments
for
> correctness. In the interest of time for M56 release, let's just use comments.

Done.

https://codereview.chromium.org/2472043003/diff/1/chrome/browser/autofill/and...
chrome/browser/autofill/android/personal_data_manager_android.cc:401: return
GetProfileLabels(env, true, include_name_in_label,
On 2016/11/04 21:27:32, rouslan wrote:
> Please add a comment about the meaning of this "true" boolean parameter.

Done.

[sebsg, 2016-11-07 15:53:05]

The CQ bit was checked by sebsg@chromium.org

[sebsg, 2016-11-07 15:53:05]

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/2472043003/#ps20001
(title: "Added comments")

[commit-bot: I haz the power, 2016-11-07 15:53:31]

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-07 16:02:24]

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

[commit-bot: I haz the power, 2016-11-07 16:02:25]

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, 2016-11-07 16:49:57]

Patchset #3 (id:40001) has been deleted

[sebsg, 2016-11-07 17:28:11]

Patchset #4 (id:80001) has been deleted

[sebsg, 2016-11-07 17:29:41]

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

[sebsg, 2016-11-07 17:29:41]

Hi Dan, could you please review PersonalDataManager.java?

Hi Math could you please review the 2 C++ files?

Thanks!

[sebsg, 2016-11-07 18:02:01]

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

[commit-bot: I haz the power, 2016-11-07 18:02:24]

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

[gone, 2016-11-07 18:50:26]

That file lgtm

https://codereview.chromium.org/2472043003/diff/100001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/autofill/PersonalDataManager.java
(right):

https://codereview.chromium.org/2472043003/diff/100001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/autofill/PersonalDataManager.java:563:
// TODO(crbug.com/616102): Reduce the number of Java to Native calls when
getting profiles.
Plop this in as part of the function javadoc?

[commit-bot: I haz the power, 2016-11-07 19:28:54]

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

[commit-bot: I haz the power, 2016-11-07 19:28:55]

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

[sebsg, 2016-11-07 19:58:28]

I addressed Dan's comment and updated the test.

https://codereview.chromium.org/2472043003/diff/100001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/autofill/PersonalDataManager.java
(right):

https://codereview.chromium.org/2472043003/diff/100001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/autofill/PersonalDataManager.java:563:
// TODO(crbug.com/616102): Reduce the number of Java to Native calls when
getting profiles.
On 2016/11/07 18:50:26, dfalcantara (check my queue) wrote:
> Plop this in as part of the function javadoc?

Done.

[sebsg, 2016-11-07 20:17:09]

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

[commit-bot: I haz the power, 2016-11-07 20:17:54]

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-07 20:49:42]

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

[commit-bot: I haz the power, 2016-11-07 20:49:43]

Dry run: This issue passed the CQ dry run.

[Mathieu, 2016-11-08 17:21:26]

c++ lgtm

[sebsg, 2016-11-08 18:11:57]

The CQ bit was checked by sebsg@chromium.org

[sebsg, 2016-11-08 18:11:57]

The patchset sent to the CQ was uploaded after l-g-t-m from
rouslan@chromium.org, dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/2472043003/#ps120001
(title: "Updated test + Addressed Dan's comment")

[commit-bot: I haz the power, 2016-11-08 18:12:35]

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-08 18:19:02]

Committed patchset #5 (id:120001)

[commit-bot: I haz the power, 2016-11-08 18:38:36]

Description was changed from

==========
[Payments] Don't show Organization in billing address selection label.

BUG=662161
==========

to

==========
[Payments] Don't show Organization in billing address selection label.

BUG=662161

Committed: https://crrev.com/c6af37456ab84d08503af9f99077ef7e12f27092
Cr-Commit-Position: refs/heads/master@{#430655}
==========

[commit-bot: I haz the power, 2016-11-08 18:38:37]

Patchset 5 (id:??) landed as
https://crrev.com/c6af37456ab84d08503af9f99077ef7e12f27092
Cr-Commit-Position: refs/heads/master@{#430655}