[WebPayments] Show labels on incomplete profiles

[WebPayments] Show labels on incomplete profiles

Adds labels describing missing information (e.g., "Add Email Address") to shipping + contact info views. Adds validation for shipping addresses. Adds cacheing to PaymentsProfileComparator, and adds an instance to PaymentRequestState so that cached values can be used throughout the request.

BUG=711358, 709454
,
TBR=mahmadi@chromium.org

Review-Url: https://codereview.chromium.org/2847503002
Cr-Commit-Position: refs/heads/master@{#468242}
Committed: https://chromium.googlesource.com/chromium/src/+/01c3eb8fd187b96cf17164ebcf15c69da1fa660e

[tmartino, 2017-04-26 20:04:32]

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

[commit-bot: I haz the power, 2017-04-26 20:05:41]

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, 2017-04-26 20:15:16]

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

[commit-bot: I haz the power, 2017-04-26 20:15:17]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[tmartino, 2017-04-26 22:29:02]

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

[commit-bot: I haz the power, 2017-04-26 22:29:45]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tmartino, 2017-04-26 22:33:13]

Description was changed from

==========
[WebPayments] Validation and labels

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

to

==========
[WebPayments] Show labels on incomplete profiles

Adds labels describing missing information (e.g., "Add Email Address") to
shipping + contact info views. Adds validation for shipping addresses. Adds
cacheing to PaymentsProfileComparator, and adds an instance to
PaymentRequestState so that cached values can be used throughout the request.

BUG=711358,709454,
==========

[commit-bot: I haz the power, 2017-04-26 22:41:06]

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

[commit-bot: I haz the power, 2017-04-26 22:41:06]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[tmartino, 2017-04-27 14:49:39]

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

[commit-bot: I haz the power, 2017-04-27 14:50:02]

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, 2017-04-27 14:59:18]

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

[commit-bot: I haz the power, 2017-04-27 14:59:19]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[tmartino, 2017-04-27 15:06:57]

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

[commit-bot: I haz the power, 2017-04-27 15:07:30]

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, 2017-04-27 15:17:22]

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

[commit-bot: I haz the power, 2017-04-27 15:17:25]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[tmartino, 2017-04-27 15:28:20]

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

[commit-bot: I haz the power, 2017-04-27 15:28:50]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tmartino, 2017-04-27 15:36:02]

tmartino@chromium.org changed reviewers:
+ mathp@chromium.org

[tmartino, 2017-04-27 15:36:02]

Hey, PTAL.

Note that I'm planning to rename profile_util.h to
payments_profile_comparator.h, but I wanted to let you review it first so that
the diffs wouldn't break.

[tmartino, 2017-04-27 15:36:44]

tmartino@chromium.org changed reviewers:
- mathp@chromium.org

[commit-bot: I haz the power, 2017-04-27 16:56:03]

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

[commit-bot: I haz the power, 2017-04-27 16:56:04]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Mathieu, 2017-04-27 17:32:57]

mathp@chromium.org changed reviewers:
+ mathp@chromium.org

[tmartino, 2017-04-27 17:55:44]

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

[commit-bot: I haz the power, 2017-04-27 17:56:11]

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, 2017-04-27 18:18:21]

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

[commit-bot: I haz the power, 2017-04-27 18:18:22]

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

[MAD, 2017-04-27 18:30:39]

Shouldn't we make PaymentRequestState a PersonalDataManagerObserver so that
it can update its cache when changes are made to the list of profiles
(e.g., via sync, or Chrome settings)?

Le jeu. 27 avr. 2017 à 13:32, <mathp@chromium.org> a écrit :

> https://codereview.chromium.org/2847503002/
>
-- 
BYE
MAD...

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Mathieu, 2017-04-27 18:34:01]

I like it a LOT!

I have a sense that some methods in profile_util are lacking test coverage?
Would be good to test some of that boolean counting logic, or the warnings that
are returned "more information required", etc.

https://codereview.chromium.org/2847503002/diff/80001/chrome/browser/ui/views...
File chrome/browser/ui/views/payments/payment_request_dialog_view_ids.h (right):

https://codereview.chromium.org/2847503002/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/payment_request_dialog_view_ids.h:48: // Used
in "three line labels" to annotate each Label of the grouping.
fix comment

https://codereview.chromium.org/2847503002/diff/80001/chrome/browser/ui/views...
File chrome/browser/ui/views/payments/profile_list_view_controller.h (right):

https://codereview.chromium.org/2847503002/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/profile_list_view_controller.h:68: virtual bool
ValidateProfile(autofill::AutofillProfile* profile) = 0;
If it doesn't have side-effects, perhaps IsValidProfile is a better wording?
ValidateProfile feels like it's going to do all sorts of things to the |profile|

https://codereview.chromium.org/2847503002/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/profile_list_view_controller.h:68: virtual bool
ValidateProfile(autofill::AutofillProfile* profile) = 0;
can this be const ref?

https://codereview.chromium.org/2847503002/diff/80001/components/autofill/cor...
File components/autofill/core/browser/autofill_profile_comparator.h (right):

https://codereview.chromium.org/2847503002/diff/80001/components/autofill/cor...
components/autofill/core/browser/autofill_profile_comparator.h:218: const
std::string app_locale_;
hmm, I think the style is to have a getter() that's visible and app_locale_
stays private

https://codereview.chromium.org/2847503002/diff/80001/components/payments/con...
File components/payments/content/payment_request_state.cc (right):

https://codereview.chromium.org/2847503002/diff/80001/components/payments/con...
components/payments/content/payment_request_state.cc:245: // TODO(mathp): Have a
measure of shipping address completeness.
fix or add proper TODO?

https://codereview.chromium.org/2847503002/diff/80001/components/payments/cor...
File components/payments/core/profile_util.cc (right):

https://codereview.chromium.org/2847503002/diff/80001/components/payments/cor...
components/payments/core/profile_util.cc:38: ProfileFields missing = 0;
Could we put lines 38-68 in a private function, and have

if (!cache_.count(profile->guid())) {
  cache_[profile->guid()] = GetMissingProfileFields(profile);
} else {
  // Profile was in cache. In Debug mode, check that invalidation
  // was properly handled.
  DCHECK_EQ(cache_[profile->guid()], GetMissingProfileFields(profile)) <<
"profile was not invalidated properly!";
}

This is still as efficient because the DCHECK won't run for non-devs, but at
least we will catch dev errors who forget to invalidate?

https://codereview.chromium.org/2847503002/diff/80001/components/payments/cor...
components/payments/core/profile_util.cc:50: autofill::HTML_TYPE_COUNTRY_CODE,
autofill::HTML_MODE_NONE))
Would something like this work?
https://cs.chromium.org/chromium/src/components/payments/content/payment_requ...

If not, we should also fix the other entry

https://codereview.chromium.org/2847503002/diff/80001/components/payments/cor...
components/payments/core/profile_util.cc:160: // int desired_score =
options_.request_payer_name() +
fix?

https://codereview.chromium.org/2847503002/diff/80001/components/payments/cor...
components/payments/core/profile_util.cc:218: base::string16
PaymentsProfileComparator::ProfileFieldsToUIString(
would recommend a slightly more specific name to indicate you are getting the
string to indicate what is missing

https://codereview.chromium.org/2847503002/diff/80001/ios/chrome/browser/paym...
File ios/chrome/browser/payments/payment_request.mm (right):

https://codereview.chromium.org/2847503002/diff/80001/ios/chrome/browser/paym...
ios/chrome/browser/payments/payment_request.mm:104: // TODO(crbug.com/602666):
Implement deduplication and prioritization rules
is this resolved then?

[Mathieu, 2017-04-27 18:34:31]

https://codereview.chromium.org/2847503002/diff/80001/components/autofill/cor...
File components/autofill/core/browser/autofill_profile_comparator.h (right):

https://codereview.chromium.org/2847503002/diff/80001/components/autofill/cor...
components/autofill/core/browser/autofill_profile_comparator.h:218: const
std::string app_locale_;
On 2017/04/27 18:34:01, Mathieu wrote:
> hmm, I think the style is to have a getter() that's visible and app_locale_
> stays private

Also do you need it, can you get it another way?

[Mathieu, 2017-04-27 18:35:54]

On 2017/04/27 18:34:31, Mathieu wrote:
>
https://codereview.chromium.org/2847503002/diff/80001/components/autofill/cor...
> File components/autofill/core/browser/autofill_profile_comparator.h (right):
> 
>
https://codereview.chromium.org/2847503002/diff/80001/components/autofill/cor...
> components/autofill/core/browser/autofill_profile_comparator.h:218: const
> std::string app_locale_;
> On 2017/04/27 18:34:01, Mathieu wrote:
> > hmm, I think the style is to have a getter() that's visible and app_locale_
> > stays private
> 
> Also do you need it, can you get it another way?

MAD: It's intentional that our UI is _not_ a PersonalDataManagerObserver, to
ensure a stable UI experience. Hence why we build a profile_cache_ and work with
that.

[tmartino, 2017-04-28 17:09:06]

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

[commit-bot: I haz the power, 2017-04-28 17:09:41]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tmartino, 2017-04-28 17:10:06]

Thanks! PTAL

https://codereview.chromium.org/2847503002/diff/80001/chrome/browser/ui/views...
File chrome/browser/ui/views/payments/payment_request_dialog_view_ids.h (right):

https://codereview.chromium.org/2847503002/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/payment_request_dialog_view_ids.h:48: // Used
in "three line labels" to annotate each Label of the grouping.
On 2017/04/27 at 18:34:01, Mathieu wrote:
> fix comment

Done

https://codereview.chromium.org/2847503002/diff/80001/chrome/browser/ui/views...
File chrome/browser/ui/views/payments/profile_list_view_controller.h (right):

https://codereview.chromium.org/2847503002/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/profile_list_view_controller.h:68: virtual bool
ValidateProfile(autofill::AutofillProfile* profile) = 0;
On 2017/04/27 at 18:34:01, Mathieu wrote:
> If it doesn't have side-effects, perhaps IsValidProfile is a better wording?
ValidateProfile feels like it's going to do all sorts of things to the |profile|

Done to both

https://codereview.chromium.org/2847503002/diff/80001/components/autofill/cor...
File components/autofill/core/browser/autofill_profile_comparator.h (right):

https://codereview.chromium.org/2847503002/diff/80001/components/autofill/cor...
components/autofill/core/browser/autofill_profile_comparator.h:218: const
std::string app_locale_;
On 2017/04/27 at 18:34:30, Mathieu wrote:
> On 2017/04/27 18:34:01, Mathieu wrote:
> > hmm, I think the style is to have a getter() that's visible and app_locale_
> > stays private
> 
> Also do you need it, can you get it another way?

We take it in at construct-time and pass it to the AutofillProfileComparator. I
guess the alternative would be to store a copy at the subclass level, but that
seems silly. Added a getter.

https://codereview.chromium.org/2847503002/diff/80001/components/payments/con...
File components/payments/content/payment_request_state.cc (right):

https://codereview.chromium.org/2847503002/diff/80001/components/payments/con...
components/payments/content/payment_request_state.cc:245: // TODO(mathp): Have a
measure of shipping address completeness.
On 2017/04/27 at 18:34:01, Mathieu wrote:
> fix or add proper TODO?

Fixed.

https://codereview.chromium.org/2847503002/diff/80001/components/payments/cor...
File components/payments/core/profile_util.cc (right):

https://codereview.chromium.org/2847503002/diff/80001/components/payments/cor...
components/payments/core/profile_util.cc:38: ProfileFields missing = 0;
On 2017/04/27 at 18:34:01, Mathieu wrote:
> Could we put lines 38-68 in a private function, and have
> 
> if (!cache_.count(profile->guid())) {
>   cache_[profile->guid()] = GetMissingProfileFields(profile);
> } else {
>   // Profile was in cache. In Debug mode, check that invalidation
>   // was properly handled.
>   DCHECK_EQ(cache_[profile->guid()], GetMissingProfileFields(profile)) <<
"profile was not invalidated properly!";
> }
> 
> This is still as efficient because the DCHECK won't run for non-devs, but at
least we will catch dev errors who forget to invalidate?

Great idea! Done.

https://codereview.chromium.org/2847503002/diff/80001/components/payments/cor...
components/payments/core/profile_util.cc:50: autofill::HTML_TYPE_COUNTRY_CODE,
autofill::HTML_MODE_NONE))
On 2017/04/27 at 18:34:01, Mathieu wrote:
> Would something like this work?
https://cs.chromium.org/chromium/src/components/payments/content/payment_requ...
> 
> If not, we should also fix the other entry

Done

https://codereview.chromium.org/2847503002/diff/80001/components/payments/cor...
components/payments/core/profile_util.cc:160: // int desired_score =
options_.request_payer_name() +
On 2017/04/27 at 18:34:01, Mathieu wrote:
> fix?

Done

https://codereview.chromium.org/2847503002/diff/80001/components/payments/cor...
components/payments/core/profile_util.cc:218: base::string16
PaymentsProfileComparator::ProfileFieldsToUIString(
On 2017/04/27 at 18:34:01, Mathieu wrote:
> would recommend a slightly more specific name to indicate you are getting the
string to indicate what is missing

OK, done. Just named it GetStringForMissingFields to be consistent with the
methods it's helping.

https://codereview.chromium.org/2847503002/diff/80001/ios/chrome/browser/paym...
File ios/chrome/browser/payments/payment_request.mm (right):

https://codereview.chromium.org/2847503002/diff/80001/ios/chrome/browser/paym...
ios/chrome/browser/payments/payment_request.mm:104: // TODO(crbug.com/602666):
Implement deduplication and prioritization rules
On 2017/04/27 at 18:34:01, Mathieu wrote:
> is this resolved then?

Nope. We'll want to write a shipping method that's analogous to
FilterProfilesForContact. It would move complete profiles to the top (but
otherwise preserve frecency order) and would do any filtering/merging we need
(though I don't think there's anything we have planned like that for shipping).

[commit-bot: I haz the power, 2017-04-28 17:14:32]

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

[commit-bot: I haz the power, 2017-04-28 17:14:33]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[tmartino, 2017-04-28 17:35:26]

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

[commit-bot: I haz the power, 2017-04-28 17:36:14]

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, 2017-04-28 17:41:49]

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

[commit-bot: I haz the power, 2017-04-28 17:41:50]

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

[Mathieu, 2017-04-28 18:36:01]

This lgtm with the file rename and the missing tests for strings, and as much
const ref as possible :)

Thanks a lot, very thorough code!

https://codereview.chromium.org/2847503002/diff/140001/components/payments/co...
File components/payments/core/profile_util.h (right):

https://codereview.chromium.org/2847503002/diff/140001/components/payments/co...
components/payments/core/profile_util.h:5: #ifndef
COMPONENTS_PAYMENTS_CONTENT_PROFILE_UTIL_H_
should rename this file before landing it

https://codereview.chromium.org/2847503002/diff/140001/components/payments/co...
components/payments/core/profile_util.h:48: const autofill::AutofillProfile*
profile) const;
can this be const ref

https://codereview.chromium.org/2847503002/diff/140001/components/payments/co...
components/payments/core/profile_util.h:100: const autofill::AutofillProfile*
profile) const;
can this be const ref

[tmartino, 2017-04-28 21:33:30]

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

[commit-bot: I haz the power, 2017-04-28 21:34:24]

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, 2017-04-28 21:44:05]

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

[commit-bot: I haz the power, 2017-04-28 21:44:07]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)

[tmartino, 2017-04-28 22:47:42]

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

[commit-bot: I haz the power, 2017-04-28 22:48:10]

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, 2017-04-28 23:41:01]

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

[commit-bot: I haz the power, 2017-04-28 23:41:02]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tmartino, 2017-04-29 12:30:00]

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

[commit-bot: I haz the power, 2017-04-29 12:30:07]

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, 2017-04-29 13:33:23]

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

[commit-bot: I haz the power, 2017-04-29 13:33:24]

Dry run: This issue passed the CQ dry run.

[tmartino, 2017-04-29 13:36:06]

Description was changed from

==========
[WebPayments] Show labels on incomplete profiles

Adds labels describing missing information (e.g., "Add Email Address") to
shipping + contact info views. Adds validation for shipping addresses. Adds
cacheing to PaymentsProfileComparator, and adds an instance to
PaymentRequestState so that cached values can be used throughout the request.

BUG=711358,709454,
==========

to

==========
[WebPayments] Show labels on incomplete profiles

Adds labels describing missing information (e.g., "Add Email Address") to
shipping + contact info views. Adds validation for shipping addresses. Adds
cacheing to PaymentsProfileComparator, and adds an instance to
PaymentRequestState so that cached values can be used throughout the request.

BUG=711358,709454,
TBR=mahmadi@chromium.org
==========

[tmartino, 2017-04-29 13:36:06]

tmartino@chromium.org changed reviewers:
+ mahmadi@chromium.org

[tmartino, 2017-04-29 13:37:44]

https://codereview.chromium.org/2847503002/diff/140001/components/payments/co...
File components/payments/core/profile_util.h (right):

https://codereview.chromium.org/2847503002/diff/140001/components/payments/co...
components/payments/core/profile_util.h:48: const autofill::AutofillProfile*
profile) const;
On 2017/04/28 at 18:36:00, Mathieu wrote:
> can this be const ref

We handle the nullptr case, returning that all fields are missing. (This is used
when, e.g., there is no selected profile.)

https://codereview.chromium.org/2847503002/diff/140001/components/payments/co...
components/payments/core/profile_util.h:100: const autofill::AutofillProfile*
profile) const;
On 2017/04/28 at 18:36:00, Mathieu wrote:
> can this be const ref

Yes, this one can be.

[tmartino, 2017-04-29 13:37:52]

The CQ bit was checked by tmartino@chromium.org

[tmartino, 2017-04-29 13:37:53]

The patchset sent to the CQ was uploaded after l-g-t-m from mathp@chromium.org
Link to the patchset: https://codereview.chromium.org/2847503002/#ps200001
(title: "test fix")

[commit-bot: I haz the power, 2017-04-29 13:38:04]

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-04-29 15:18:42]

CQ is committing da patch.

Bot data: {"patchset_id": 200001, "attempt_start_ts": 1493473072711140,
"parent_rev": "b75aa7267969141e12f5ece554be20d81d39b044", "commit_rev":
"01c3eb8fd187b96cf17164ebcf15c69da1fa660e"}

[commit-bot: I haz the power, 2017-04-29 15:18:50]

Description was changed from

==========
[WebPayments] Show labels on incomplete profiles

Adds labels describing missing information (e.g., "Add Email Address") to
shipping + contact info views. Adds validation for shipping addresses. Adds
cacheing to PaymentsProfileComparator, and adds an instance to
PaymentRequestState so that cached values can be used throughout the request.

BUG=711358,709454,
TBR=mahmadi@chromium.org
==========

to

==========
[WebPayments] Show labels on incomplete profiles

Adds labels describing missing information (e.g., "Add Email Address") to
shipping + contact info views. Adds validation for shipping addresses. Adds
cacheing to PaymentsProfileComparator, and adds an instance to
PaymentRequestState so that cached values can be used throughout the request.

BUG=711358,709454,
TBR=mahmadi@chromium.org

Review-Url: https://codereview.chromium.org/2847503002
Cr-Commit-Position: refs/heads/master@{#468242}
Committed:
https://chromium.googlesource.com/chromium/src/+/01c3eb8fd187b96cf17164ebcf15...
==========

[commit-bot: I haz the power, 2017-04-29 15:18:51]

Committed patchset #11 (id:200001) as
https://chromium.googlesource.com/chromium/src/+/01c3eb8fd187b96cf17164ebcf15...