Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2625183002:
[WebPayments] Adding Shipping Address and Contact Info display to order summary (Closed)
Description[WebPayments] Adding Shipping Address and Contact Info display to order summary
BUG=679449
Review-Url: https://codereview.chromium.org/2625183002
Cr-Commit-Position: refs/heads/master@{#444778}
Committed: https://chromium.googlesource.com/chromium/src/+/68c0a27692c2ef52a59e01661bb6c8ff1a1aef65
Patch Set 1 #
Total comments: 12
Patch Set 2 : Rebasing with anthonyvd changes #
Total comments: 28
Patch Set 3 : Rouslan feedback #
Total comments: 2
Patch Set 4 : mathp nit #
Total comments: 11
Patch Set 5 : sky comments #
Messages
Total messages: 33 (18 generated)
tmartino@chromium.org changed reviewers: + anthonyvd@chromium.org, mathp@chromium.org
The CQ bit was checked by tmartino@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromeos_amd64-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...) chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
Looking good, thanks! Few comments. https://codereview.chromium.org/2625183002/diff/1/chrome/browser/ui/views/pay... File chrome/browser/ui/views/payments/payment_request_address_util.cc (right): https://codereview.chromium.org/2625183002/diff/1/chrome/browser/ui/views/pay... chrome/browser/ui/views/payments/payment_request_address_util.cc:38: std::unique_ptr<autofill::AutofillProfile> GetDummyProfile() { Since we know the currently selected profile will be on the PaymentRequest object, I'd replace this with a GetCurrentlySelectedProfile() function on PaymentRequest. It can probably just return the first element in PersonalDataManager::GetProfiles() for now instead of a dummy profile too. WDYT? https://codereview.chromium.org/2625183002/diff/1/chrome/browser/ui/views/pay... File chrome/browser/ui/views/payments/payment_request_address_util.h (right): https://codereview.chromium.org/2625183002/diff/1/chrome/browser/ui/views/pay... chrome/browser/ui/views/payments/payment_request_address_util.h:1: // Copyright 2017 The Chromium Authors. All rights reserved. Can we move the contents of this file in payment_request_views_util? It's meant to contain this sort of functions that can be used in multiple places to create/set up UI pieces. We could also rename it if the name doesn't convey this well :) https://codereview.chromium.org/2625183002/diff/1/chrome/browser/ui/views/pay... chrome/browser/ui/views/payments/payment_request_address_util.h:29: AddressFormatType type, What's this parameter for (and same question for the AddressFormatType enum)? https://codereview.chromium.org/2625183002/diff/1/chrome/browser/ui/views/pay... File chrome/browser/ui/views/payments/payment_sheet_view_controller.cc (right): https://codereview.chromium.org/2625183002/diff/1/chrome/browser/ui/views/pay... chrome/browser/ui/views/payments/payment_sheet_view_controller.cc:46: SHOW_SHIPPING_BUTTON = kFirstTagValue + 1, No need for the explicit value on enum members after the first one per the standard: "For any other enumerator whose definition does not have an initializer, the associated value is the value of the previous enumerator plus one." (TIL!) https://codereview.chromium.org/2625183002/diff/1/chrome/browser/ui/views/pay... chrome/browser/ui/views/payments/payment_sheet_view_controller.cc:141: layout->StartRow(1, 0); Do you have the first parameter here set to 0 on purpose? It's |vertical_resize|, which I imagine would make the row stretch to the bottom of the dialog. https://codereview.chromium.org/2625183002/diff/1/chrome/browser/ui/views/pay... chrome/browser/ui/views/payments/payment_sheet_view_controller.cc:196: AddressFormatType::SHIPPING_SUMMARY, "", GetDummyProfile().get()); GetDummyProfile().get() here would likely delete the underlying pointer before it's used in the function. I think my suggestion in the comment above should address this, otherwise the unique_ptr should be held on until after the payments::GetPaymentRequestAddressLabel call returns.
tmartino@chromium.org changed reviewers: + rouslan@chromium.org - anthonyvd@chromium.org, mathp@chromium.org
+rouslan to R-line mathp, anthonyvd from R-line to CC-line as they're OOO https://codereview.chromium.org/2625183002/diff/1/chrome/browser/ui/views/pay... File chrome/browser/ui/views/payments/payment_request_address_util.cc (right): https://codereview.chromium.org/2625183002/diff/1/chrome/browser/ui/views/pay... chrome/browser/ui/views/payments/payment_request_address_util.cc:38: std::unique_ptr<autofill::AutofillProfile> GetDummyProfile() { On 2017/01/12 at 15:42:11, anthonyvd wrote: > Since we know the currently selected profile will be on the PaymentRequest object, I'd replace this with a GetCurrentlySelectedProfile() function on PaymentRequest. It can probably just return the first element in PersonalDataManager::GetProfiles() for now instead of a dummy profile too. > > WDYT? Done https://codereview.chromium.org/2625183002/diff/1/chrome/browser/ui/views/pay... File chrome/browser/ui/views/payments/payment_request_address_util.h (right): https://codereview.chromium.org/2625183002/diff/1/chrome/browser/ui/views/pay... chrome/browser/ui/views/payments/payment_request_address_util.h:1: // Copyright 2017 The Chromium Authors. All rights reserved. On 2017/01/12 at 15:42:11, anthonyvd wrote: > Can we move the contents of this file in payment_request_views_util? It's meant to contain this sort of functions that can be used in multiple places to create/set up UI pieces. We could also rename it if the name doesn't convey this well :) Done https://codereview.chromium.org/2625183002/diff/1/chrome/browser/ui/views/pay... chrome/browser/ui/views/payments/payment_request_address_util.h:29: AddressFormatType type, On 2017/01/12 at 15:42:11, anthonyvd wrote: > What's this parameter for (and same question for the AddressFormatType enum)? Changed name + documented https://codereview.chromium.org/2625183002/diff/1/chrome/browser/ui/views/pay... File chrome/browser/ui/views/payments/payment_sheet_view_controller.cc (right): https://codereview.chromium.org/2625183002/diff/1/chrome/browser/ui/views/pay... chrome/browser/ui/views/payments/payment_sheet_view_controller.cc:46: SHOW_SHIPPING_BUTTON = kFirstTagValue + 1, On 2017/01/12 at 15:42:11, anthonyvd wrote: > No need for the explicit value on enum members after the first one per the standard: "For any other enumerator whose definition does not have an initializer, the associated value is the value of the previous enumerator plus one." (TIL!) Done https://codereview.chromium.org/2625183002/diff/1/chrome/browser/ui/views/pay... chrome/browser/ui/views/payments/payment_sheet_view_controller.cc:141: layout->StartRow(1, 0); On 2017/01/12 at 15:42:11, anthonyvd wrote: > Do you have the first parameter here set to 0 on purpose? It's |vertical_resize|, which I imagine would make the row stretch to the bottom of the dialog. It doesn't have that effect here; the new row appears correctly including space for the multi-line display. I believe it means "allow the row to grow to fit its content", not "make the row as large as possible." https://codereview.chromium.org/2625183002/diff/1/chrome/browser/ui/views/pay... chrome/browser/ui/views/payments/payment_sheet_view_controller.cc:196: AddressFormatType::SHIPPING_SUMMARY, "", GetDummyProfile().get()); On 2017/01/12 at 15:42:11, anthonyvd wrote: > GetDummyProfile().get() here would likely delete the underlying pointer before it's used in the function. I think my suggestion in the comment above should address this, otherwise the unique_ptr should be held on until after the payments::GetPaymentRequestAddressLabel call returns. Done (per your other suggestion)
Description was changed from ========== [WebPayments] Adding skeleton of address display to order summary BUG=679449 ========== to ========== [WebPayments] Adding Billing Address and Contact Info display to order summary BUG=679449 ==========
https://codereview.chromium.org/2625183002/diff/20001/chrome/browser/ui/views... File chrome/browser/ui/views/payments/payment_request_views_util.cc (right): https://codereview.chromium.org/2625183002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/payments/payment_request_views_util.cc:29: base::string16 GetAddressFromProfile(autofill::AutofillProfile* profile, const-ref parameter https://codereview.chromium.org/2625183002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/payments/payment_request_views_util.cc:30: std::string locale) { const-ref https://codereview.chromium.org/2625183002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/payments/payment_request_views_util.cc:144: return styled_label; nit: Avoid local variables that are used only once. return base::MakeUnique<views::StyledLabel>(base::JoinString(values, base::ASCIIToUTF16("\n")), nullptr); https://codereview.chromium.org/2625183002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/payments/payment_request_views_util.cc:188: return styled_label; Ditto https://codereview.chromium.org/2625183002/diff/20001/chrome/browser/ui/views... File chrome/browser/ui/views/payments/payment_request_views_util.h (right): https://codereview.chromium.org/2625183002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/payments/payment_request_views_util.h:12: #include "components/autofill/core/browser/autofill_profile.h" Forward-declare instead, please. https://codereview.chromium.org/2625183002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/payments/payment_request_views_util.h:67: std::string locale, const-ref https://codereview.chromium.org/2625183002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/payments/payment_request_views_util.h:68: autofill::AutofillProfile* profile); const-ref https://codereview.chromium.org/2625183002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/payments/payment_request_views_util.h:75: std::string locale, const-ref https://codereview.chromium.org/2625183002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/payments/payment_request_views_util.h:76: autofill::AutofillProfile* profile, const-ref https://codereview.chromium.org/2625183002/diff/20001/chrome/browser/ui/views... File chrome/browser/ui/views/payments/payment_sheet_view_controller.cc (right): https://codereview.chromium.org/2625183002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/payments/payment_sheet_view_controller.cc:138: private: Good catch! :-) https://codereview.chromium.org/2625183002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/payments/payment_sheet_view_controller.cc:149: std::vector<int> section_names{ Is this clang-format? https://codereview.chromium.org/2625183002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/payments/payment_sheet_view_controller.cc:271: return payments::GetShippingAddressLabel(AddressStyleType::SUMMARY, "", Add a comment for "" please. For example: "" /* parameterNameHere */ https://codereview.chromium.org/2625183002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/payments/payment_sheet_view_controller.cc:274: return base::MakeUnique<views::Label>(base::string16()); Nit: Prefer single return statement like this: return profile ? payments::GetShippingAddressLabel(AddressStyleType::SUMMARY, "" /* paramNameHere */, profile) : base::MakeUnique<views::Label>(base::string16()); https://codereview.chromium.org/2625183002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/payments/payment_sheet_view_controller.cc:358: return base::MakeUnique<views::Label>(base::string16()); Ditto https://codereview.chromium.org/2625183002/diff/20001/components/autofill_str... File components/autofill_strings.grdp (right): https://codereview.chromium.org/2625183002/diff/20001/components/autofill_str... components/autofill_strings.grdp:357: <message name="IDS_PAYMENT_REQUEST_SHIPPING_SECTION_NAME" desc="The name of the Shipping Address section in the Payment Sheet of the Payment Request dialog."> Can you copy the description from here? https://cs.chromium.org/chromium/src/chrome/android/java/strings/android_chro... Translators will appreciate it. (Also would be nice to use the same strings across platforms eventually.) https://codereview.chromium.org/2625183002/diff/20001/components/autofill_str... components/autofill_strings.grdp:363: <message name="IDS_PAYMENT_REQUEST_CONTACT_INFO_SECTION_NAME" desc="The name of the Contact Info section in the Payment Sheet of the Payment Request dialog."> Ditto https://codereview.chromium.org/2625183002/diff/20001/components/payments/pay... File components/payments/payment_request.cc (right): https://codereview.chromium.org/2625183002/diff/20001/components/payments/pay... components/payments/payment_request.cc:83: auto profiles = data_manager->GetProfiles(); By the way, GetPersonalDataManager() and GetProfiles() are very cheap calls, because PersonalDataManager always exists in memory and has a cache of profiles and credit cards in memory as well. https://codereview.chromium.org/2625183002/diff/20001/components/payments/pay... File components/payments/payment_request.h (right): https://codereview.chromium.org/2625183002/diff/20001/components/payments/pay... components/payments/payment_request.h:13: #include "components/autofill/core/browser/autofill_profile.h" Please forward-declare instead https://codereview.chromium.org/2625183002/diff/20001/components/payments/pay... components/payments/payment_request.h:82: base::Optional<autofill::AutofillProfile*> profile_; Please keep a copy of the selected profile. Profiles can change on disk and in PersonalDataManager due to sync events. If the profile goes away, you may be dealing with an invalid pointer.
PTAL https://codereview.chromium.org/2625183002/diff/20001/chrome/browser/ui/views... File chrome/browser/ui/views/payments/payment_request_views_util.cc (right): https://codereview.chromium.org/2625183002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/payments/payment_request_views_util.cc:144: return styled_label; On 2017/01/18 at 18:50:47, rouslan wrote: > nit: Avoid local variables that are used only once. > > return base::MakeUnique<views::StyledLabel>(base::JoinString(values, base::ASCIIToUTF16("\n")), nullptr); Done to both https://codereview.chromium.org/2625183002/diff/20001/chrome/browser/ui/views... File chrome/browser/ui/views/payments/payment_request_views_util.h (right): https://codereview.chromium.org/2625183002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/payments/payment_request_views_util.h:12: #include "components/autofill/core/browser/autofill_profile.h" On 2017/01/18 at 18:50:47, rouslan wrote: > Forward-declare instead, please. Done https://codereview.chromium.org/2625183002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/payments/payment_request_views_util.h:67: std::string locale, On 2017/01/18 at 18:50:47, rouslan wrote: > const-ref Done to all https://codereview.chromium.org/2625183002/diff/20001/chrome/browser/ui/views... File chrome/browser/ui/views/payments/payment_sheet_view_controller.cc (right): https://codereview.chromium.org/2625183002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/payments/payment_sheet_view_controller.cc:149: std::vector<int> section_names{ On 2017/01/18 at 18:50:48, rouslan wrote: > Is this clang-format? yup https://codereview.chromium.org/2625183002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/payments/payment_sheet_view_controller.cc:271: return payments::GetShippingAddressLabel(AddressStyleType::SUMMARY, "", On 2017/01/18 at 18:50:48, rouslan wrote: > Add a comment for "" please. For example: > > "" /* parameterNameHere */ Added a more descriptive TODO comment instead. https://codereview.chromium.org/2625183002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/payments/payment_sheet_view_controller.cc:274: return base::MakeUnique<views::Label>(base::string16()); On 2017/01/18 at 18:50:48, rouslan wrote: > Nit: Prefer single return statement like this: > > return profile ? payments::GetShippingAddressLabel(AddressStyleType::SUMMARY, "" /* paramNameHere */, profile) : base::MakeUnique<views::Label>(base::string16()); Done to both. https://codereview.chromium.org/2625183002/diff/20001/components/autofill_str... File components/autofill_strings.grdp (right): https://codereview.chromium.org/2625183002/diff/20001/components/autofill_str... components/autofill_strings.grdp:357: <message name="IDS_PAYMENT_REQUEST_SHIPPING_SECTION_NAME" desc="The name of the Shipping Address section in the Payment Sheet of the Payment Request dialog."> On 2017/01/18 at 18:50:48, rouslan wrote: > Can you copy the description from here? > > https://cs.chromium.org/chromium/src/chrome/android/java/strings/android_chro... > > Translators will appreciate it. (Also would be nice to use the same strings across platforms eventually.) Done to both. https://codereview.chromium.org/2625183002/diff/20001/components/payments/pay... File components/payments/payment_request.h (right): https://codereview.chromium.org/2625183002/diff/20001/components/payments/pay... components/payments/payment_request.h:13: #include "components/autofill/core/browser/autofill_profile.h" On 2017/01/18 at 18:50:48, rouslan wrote: > Please forward-declare instead Done https://codereview.chromium.org/2625183002/diff/20001/components/payments/pay... components/payments/payment_request.h:82: base::Optional<autofill::AutofillProfile*> profile_; On 2017/01/18 at 18:50:48, rouslan wrote: > Please keep a copy of the selected profile. Profiles can change on disk and in PersonalDataManager due to sync events. If the profile goes away, you may be dealing with an invalid pointer. Done
Description was changed from ========== [WebPayments] Adding Billing Address and Contact Info display to order summary BUG=679449 ========== to ========== [WebPayments] Adding Shipping Address and Contact Info display to order summary BUG=679449 ==========
LGTM
tmartino@chromium.org changed reviewers: + mathp@chromium.org, pkasting@chromium.org
+mathp for OWNERS on autofill_strings.grdp +pkasting for OWNERS on chrome/browser/ui/*
tmartino@chromium.org changed reviewers: + sky@chromium.org - pkasting@chromium.org
-pkasting, who I'm told is at a conference +sky for OWNERS on c/b/ui/*
autogill lgtm https://codereview.chromium.org/2625183002/diff/40001/components/payments/pay... File components/payments/payment_request.h (right): https://codereview.chromium.org/2625183002/diff/40001/components/payments/pay... components/payments/payment_request.h:18: class AutofillProfile; nit: alphabetical
https://codereview.chromium.org/2625183002/diff/40001/components/payments/pay... File components/payments/payment_request.h (right): https://codereview.chromium.org/2625183002/diff/40001/components/payments/pay... components/payments/payment_request.h:18: class AutofillProfile; On 2017/01/18 at 21:34:16, Mathieu Perreault wrote: > nit: alphabetical Done
https://codereview.chromium.org/2625183002/diff/60001/chrome/browser/ui/views... File chrome/browser/ui/views/payments/payment_request_views_util.cc (right): https://codereview.chromium.org/2625183002/diff/60001/chrome/browser/ui/views... chrome/browser/ui/views/payments/payment_request_views_util.cc:144: std::unique_ptr<views::View> GetContactInfoLabel( I see one call to this that passes in true for all values. Do you plan on having more call sites? https://codereview.chromium.org/2625183002/diff/60001/chrome/browser/ui/views... chrome/browser/ui/views/payments/payment_request_views_util.cc:156: name_value = Why bother with the temp? Why not add directly to values? https://codereview.chromium.org/2625183002/diff/60001/chrome/browser/ui/views... File chrome/browser/ui/views/payments/payment_request_views_util.h (right): https://codereview.chromium.org/2625183002/diff/60001/chrome/browser/ui/views... chrome/browser/ui/views/payments/payment_request_views_util.h:70: const std::string& locale, Do you really need to pass in different locales? https://codereview.chromium.org/2625183002/diff/60001/chrome/browser/ui/views... chrome/browser/ui/views/payments/payment_request_views_util.h:80: bool show_payer_name, optional: Did you consider a bitmask? Generally it's easier to parse than a bunch of true/false values in a call to this function. https://codereview.chromium.org/2625183002/diff/60001/chrome/browser/ui/views... File chrome/browser/ui/views/payments/payment_sheet_view_controller.cc (right): https://codereview.chromium.org/2625183002/diff/60001/chrome/browser/ui/views... chrome/browser/ui/views/payments/payment_sheet_view_controller.cc:274: "", *profile) "" -> std::string() https://codereview.chromium.org/2625183002/diff/60001/chrome/browser/ui/views... chrome/browser/ui/views/payments/payment_sheet_view_controller.cc:356: return profile ? payments::GetContactInfoLabel(AddressStyleType::SUMMARY, "", "" -> std::string()
The CQ bit was checked by tmartino@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2625183002/diff/60001/chrome/browser/ui/views... File chrome/browser/ui/views/payments/payment_request_views_util.cc (right): https://codereview.chromium.org/2625183002/diff/60001/chrome/browser/ui/views... chrome/browser/ui/views/payments/payment_request_views_util.cc:144: std::unique_ptr<views::View> GetContactInfoLabel( On 2017/01/18 at 23:19:25, sky wrote: > I see one call to this that passes in true for all values. Do you plan on having more call sites? The show_payer_* values will be specified by the merchant as part of the request, as will a few other aspects of the UI (e.g., it's possible for the merchant to hide the shipping address entirely). Right now I have everything displaying, but I'll be handling those interactions with the passed request in a follow-up CL. https://codereview.chromium.org/2625183002/diff/60001/chrome/browser/ui/views... chrome/browser/ui/views/payments/payment_request_views_util.cc:156: name_value = On 2017/01/18 at 23:19:25, sky wrote: > Why bother with the temp? Why not add directly to values? For the bold styling described in the TODO below, we will need to use the length of |name_value| to specify the substring to which it will apply. https://codereview.chromium.org/2625183002/diff/60001/chrome/browser/ui/views... File chrome/browser/ui/views/payments/payment_request_views_util.h (right): https://codereview.chromium.org/2625183002/diff/60001/chrome/browser/ui/views... chrome/browser/ui/views/payments/payment_request_views_util.h:70: const std::string& locale, On 2017/01/18 at 23:19:25, sky wrote: > Do you really need to pass in different locales? Yes, I believe we do. This is needed for each call to AutofillProfile::GetInfo, and gets passed down to (e.g.) the phone number formatter. I asked the people on my team who are better versed in Autofill than I am, and they confirmed that it was a necessary part of those calls. https://codereview.chromium.org/2625183002/diff/60001/chrome/browser/ui/views... chrome/browser/ui/views/payments/payment_request_views_util.h:80: bool show_payer_name, On 2017/01/18 at 23:19:25, sky wrote: > optional: Did you consider a bitmask? Generally it's easier to parse than a bunch of true/false values in a call to this function. I did consider it, yeah. If I were planning on leaving these as configuration variables determined statically at each call-site (like they are now) I think that would be preferable. When I switch to reading these values from the request, though, I think it will be cleaner to inline those calls to (e.g.) request()->request_payer_name() than to read those values and use them to twiddle some bits. https://codereview.chromium.org/2625183002/diff/60001/chrome/browser/ui/views... File chrome/browser/ui/views/payments/payment_sheet_view_controller.cc (right): https://codereview.chromium.org/2625183002/diff/60001/chrome/browser/ui/views... chrome/browser/ui/views/payments/payment_sheet_view_controller.cc:274: "", *profile) On 2017/01/18 at 23:19:25, sky wrote: > "" -> std::string() Done to both.
LGTM
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by tmartino@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from mathp@chromium.org, rouslan@chromium.org Link to the patchset: https://codereview.chromium.org/2625183002/#ps80001 (title: "sky comments")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 80001, "attempt_start_ts": 1484847481435860,
"parent_rev": "29acddfbe7b3c92cd85d99e416f920849d011ec1", "commit_rev":
"68c0a27692c2ef52a59e01661bb6c8ff1a1aef65"}
Message was sent while issue was closed.
Description was changed from ========== [WebPayments] Adding Shipping Address and Contact Info display to order summary BUG=679449 ========== to ========== [WebPayments] Adding Shipping Address and Contact Info display to order summary BUG=679449 Review-Url: https://codereview.chromium.org/2625183002 Cr-Commit-Position: refs/heads/master@{#444778} Committed: https://chromium.googlesource.com/chromium/src/+/68c0a27692c2ef52a59e01661bb6... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/src/+/68c0a27692c2ef52a59e01661bb6... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
