[WebPayments] Crash when normalizing addresses in Incognito Mode |
|||||||||
Issue description1. Open an incognito window 2. Go to https://rsolomakhin.github.io/pr/us/ 3. Add an address and hit "Done" 4. Crash: [16468:16468:0814/121259.089554:FATAL:url_fetcher_core.cc(248)] Check failed: request_context_getter. #0 0x7fd436a8107d base::debug::StackTrace::StackTrace() #1 0x7fd436a7f44c base::debug::StackTrace::StackTrace() #2 0x7fd436a80a35 base::debug::(anonymous namespace)::StackDumpSignalHandler() #3 0x7fd4370e2330 <unknown> #4 0x7fd41de7dc37 gsignal #5 0x7fd41de81028 abort #6 0x7fd436a7c446 base::debug::(anonymous namespace)::DebugBreak() #7 0x7fd436a7c428 base::debug::BreakDebugger() #8 0x7fd436b105a6 logging::LogMessage::~LogMessage() #9 0x7fd434b0ff79 net::URLFetcherCore::SetRequestContext() #10 0x7fd434b1d83c net::URLFetcherImpl::SetRequestContext() #11 0x55d4fda37d9a autofill::ChromeMetadataSource::Download() #12 0x55d4fda37be5 autofill::ChromeMetadataSource::Get() #13 0x55d4fda5433f i18n::addressinput::(anonymous namespace)::Helper::OnValidatedDataReady() #14 0x55d4fda54553 i18n::addressinput::(anonymous namespace)::CallbackImpl<>::operator()() #15 0x55d4fda54ca9 i18n::addressinput::(anonymous namespace)::Helper::OnWrappedDataReady() #16 0x55d4fda54ea3 i18n::addressinput::(anonymous namespace)::CallbackImpl<>::operator()() #17 0x55d4fda3b942 autofill::ChromeStorageImpl::DoGet() #18 0x55d4fda3b135 autofill::ChromeStorageImpl::Get() #19 0x55d4fda54ad8 i18n::addressinput::(anonymous namespace)::Helper::Helper() #20 0x55d4fda54a23 i18n::addressinput::ValidatingStorage::Get() #21 0x55d4fda53d71 i18n::addressinput::(anonymous namespace)::Helper::Helper() #22 0x55d4fda53b74 i18n::addressinput::Retriever::Retrieve() #23 0x55d4fda3e8d7 i18n::addressinput::(anonymous namespace)::Helper::Helper() #24 0x55d4fda3e1f0 i18n::addressinput::PreloadSupplier::LoadRules() #25 0x55d500be2370 autofill::AddressValidator::LoadRules() #26 0x55d4fdb03ffc payments::AddressNormalizerImpl::LoadRulesForRegion() #27 0x55d4fdb04e80 payments::AddressNormalizerImpl::StartAddressNormalization() #28 0x55d4ff6d1b23 payments::PaymentRequestState::SetSelectedShippingProfile() #29 0x55d4ff6d1a70 payments::PaymentRequestState::AddAutofillShippingProfile() #30 0x55d4fc38fec8 _ZN4base8internal13FunctorTraitsIMN11cloud_print29PrivetLocalPrintOperationImplEFvbRKNS_8FilePathEEvE6InvokeIPS3_JbS6_EEEvS8_OT_DpOT0_ #31 0x55d4fc38fe1a _ZN4base8internal12InvokeHelperILb0EvE8MakeItSoIRKMN11cloud_print29PrivetLocalPrintOperationImplEFvbRKNS_8FilePathEEJPS5_bS8_EEEvOT_DpOT0_ #32 0x55d4fef8ac5d _ZN4base8internal7InvokerINS0_9BindStateIMN8payments19PaymentRequestStateEFvbRKN8autofill15AutofillProfileEEJNS0_17UnretainedWrapperIS4_EEbEEEFvS8_EE7RunImplISA_NSt3__15tupleIJSC_bEEEJLm0ELm1EEEEvOT_OT0_NS_13IndexSequenceIJXspT1_EEEES8_ #33 0x55d4fef8ab69 _ZN4base8internal7InvokerINS0_9BindStateIMN8payments19PaymentRequestStateEFvbRKN8autofill15AutofillProfileEEJNS0_17UnretainedWrapperIS4_EEbEEEFvS8_EE7RunOnceEPNS0_13BindStateBaseES8_ #34 0x55d4fa640261 _ZNO4base8CallbackIFvRKNSt3__16vectorIhNS1_9allocatorIhEEEEELNS_8internal8CopyModeE0ELNS9_10RepeatModeE0EE3RunES7_ #35 0x55d4fef932d8 payments::ShippingAddressEditorViewController::ValidateModelAndSave() #36 0x55d4ff2617e6 payments::EditorViewController::ButtonPressed() #37 0x55d4fef74d79 payments::PaymentRequestSheetController::PerformPrimaryButtonAction() #38 0x55d4fb5f7e2d _ZN4base8internal13FunctorTraitsIMN10extensions12EventEmitterEFbvEvE6InvokeIPS3_JEEEbS5_OT_DpOT0_ #39 0x55d4fb5f7da4 _ZN4base8internal12InvokeHelperILb0EbE8MakeItSoIRKMN10extensions12EventEmitterEFbvEJPS5_EEEbOT_DpOT0_ #40 0x55d4fef77635 _ZN4base8internal7InvokerINS0_9BindStateIMN8payments29PaymentRequestSheetControllerEFbvEJNS0_17UnretainedWrapperIS4_EEEEEFbvEE7RunImplIRKS6_RKNSt3__15tupleIJS8_EEEJLm0EEEEbOT_OT0_NS_13IndexSequenceIJXspT1_EEEE #41 0x55d4fef7757c _ZN4base8internal7InvokerINS0_9BindStateIMN8payments29PaymentRequestSheetControllerEFbvEJNS0_17UnretainedWrapperIS4_EEEEEFbvEE3RunEPNS0_13BindStateBaseE #42 0x55d4fc23338d _ZNKR4base8CallbackIFbvELNS_8internal8CopyModeE1ELNS2_10RepeatModeE1EE3RunEv #43 0x55d4fef761db payments::(anonymous namespace)::SheetView::AcceleratorPressed()
,
Aug 14 2017
Looks like request_context_getter is null in incognito. That's surprising to me. We need to find the correct request_context_getter to use in this situation. See https://cs.chromium.org/chromium/src/chrome/browser/payments/chrome_payment_request_delegate.cc?rcl=41b9e4cbe05cd7c9f16496e7a2a058da2d7205af&l=49 --- that's where the url_context_getter is being created.
,
Aug 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ef6e100e869af31681d62bfdb1b886e5cc18a963 commit ef6e100e869af31681d62bfdb1b886e5cc18a963 Author: Anthony Vallee-Dubois <anthonyvd@chromium.org> Date: Tue Aug 15 17:37:43 2017 [Web Payments] Use the correct PersonalDataManager in incognito Before this change, adding an address in incognito mode from the Payment Request editor would cause a Browser crash because Payment Request attempted to use an ill-initiated PersonalDataManager's URLRequestContext. This is because autofill uses the original profile's PersonalDataManager rather than instantiate a new one. This change brings Payment Request in line with autofill and uses the original profile's PersonalDataManager, preventing the crash. Bug: 755161 Change-Id: Iba1ca7cc86d2c0ef50030c4d9fe84c85c551b71f Reviewed-on: https://chromium-review.googlesource.com/615185 Commit-Queue: Anthony Vallee-Dubois <anthonyvd@chromium.org> Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org> Cr-Commit-Position: refs/heads/master@{#494432} [modify] https://crrev.com/ef6e100e869af31681d62bfdb1b886e5cc18a963/chrome/browser/payments/chrome_payment_request_delegate.cc
,
Aug 15 2017
This should be merged to 61 since it fixes a crasher.
,
Aug 15 2017
Before we approve merge to M61, please answer followings: * Is this M61 regression? Is it critical? * Is the change well baked/verified in Canary, having enough automation tests coverage and safe to merge to M61? * Any other important details to justify the merge. Please note M61 is already in Beta, so merge bar is VERY high. Thank you.
,
Aug 15 2017
* This fixes a crashing bug for a launch happening in m61, so I would say it's critical. * It just landed, so no bake time. We'll return back after it's baked a day or two on canary and let you know. thanks!
,
Aug 15 2017
OK, pls update the bug with Canary result. Also wanted to check on automation coverage for this change.
,
Aug 15 2017
Re: automation. This feature has extended test coverage so we're confident this change isn't introducing any regression.
,
Aug 16 2017
Tested this issue on Windows 7,Mac 10.12.6 & Ubuntu 14.04 using latest canary-62.0.3187.0 as per steps mentioned in comment#0. Observed that user able to add shipping address successfully without any crash on the above URL.As it is working as intended, adding TE Verified labels. Please find the attached screencast for reference. Thanks..!!
,
Aug 16 2017
This bug requires manual review: M61 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 17 2017
The NextAction date has arrived: 2017-08-17
,
Aug 17 2017
zkoch@ & anthonyvd@, how is the change looking on Canary so far?
,
Aug 17 2017
Looking good! Crash is resolved and everything is stable.
,
Aug 17 2017
Approving merge to M61 branch 3163 based on comments #8, #9 and #13. Please merge ASAP. Thank you.
,
Aug 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/118bdc3b680949ef5d6cb41317522b6f7b362767 commit 118bdc3b680949ef5d6cb41317522b6f7b362767 Author: Anthony Vallee-Dubois <anthonyvd@chromium.org> Date: Thu Aug 17 18:32:11 2017 Merge - [Web Payments] Use the correct PersonalDataManager in incognito Before this change, adding an address in incognito mode from the Payment Request editor would cause a Browser crash because Payment Request attempted to use an ill-initiated PersonalDataManager's URLRequestContext. This is because autofill uses the original profile's PersonalDataManager rather than instantiate a new one. This change brings Payment Request in line with autofill and uses the original profile's PersonalDataManager, preventing the crash. TBR=anthonyvd@chromium.org (cherry picked from commit ef6e100e869af31681d62bfdb1b886e5cc18a963) Bug: 755161 Change-Id: Iba1ca7cc86d2c0ef50030c4d9fe84c85c551b71f Reviewed-on: https://chromium-review.googlesource.com/615185 Commit-Queue: Anthony Vallee-Dubois <anthonyvd@chromium.org> Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#494432} Reviewed-on: https://chromium-review.googlesource.com/618803 Reviewed-by: Anthony Vallee-Dubois <anthonyvd@chromium.org> Cr-Commit-Position: refs/branch-heads/3163@{#632} Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528} [modify] https://crrev.com/118bdc3b680949ef5d6cb41317522b6f7b362767/chrome/browser/payments/chrome_payment_request_delegate.cc
,
Aug 17 2017
,
Aug 23 2017
Tested this issue using #61.0.3163.59 on Windows 7, Mac 10.12.6 & Ubuntu 14.04 as per steps mentioned in original comment. No Crash is seen on adding address. Please find the screen cast for the same. Hence adding verified labels. Thanks!! |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by anthonyvd@chromium.org
, Aug 14 2017