Issue metadata
Sign in to add a comment
|
Hit DCHECK failure on ServiceWorkerStorage |
||||||||||||||||||||||
Issue description<Environment> Chromium 57.0.2946.0 (Developer Build) (64-bit) Revision d8244df22299cee8a52451031abf3254902cbd44-refs/heads/master@{#437403} OS Linux <Log> $ ./out/Default/chrome [5322:5398:1209/153039.534712:ERROR:entry.cc(167)] Entry::Deserialize: dictionary has no interface_provider_specs key [5322:5398:1209/153040.487169:ERROR:gcm_store_impl.cc(929)] Failed to restore security token. [5322:5392:1209/153040.596636:FATAL:service_worker_storage.cc(691)] Check failed: state_ == INITIALIZED || state_ == DISABLED. 1 #0 0x7f783f07b1fe base::debug::StackTrace::StackTrace() #1 0x7f783f0e869f logging::LogMessage::~LogMessage() #2 0x7f78391447f9 content::ServiceWorkerStorage::ClearUserData() #3 0x7f783906f939 content::ServiceWorkerContextWrapper::ClearRegistrationUserData() #4 0x7f7837b77a44 content::(anonymous namespace)::ClearPushSubscriptionIdOnIO() #5 0x7f7837b7bbbf _ZN4base8internal13FunctorTraitsIPFv13scoped_refptrIN7content27ServiceWorkerContextWrapperEElRKNS_8CallbackIFvvELNS0_8CopyModeE1ELNS0_10RepeatModeE1EEEEvE6InvokeIJRKS5_RKlSC_EEEvSE_DpOT_ #6 0x7f7837b7baf2 _ZN4base8internal12InvokeHelperILb0EvE8MakeItSoIRKPFv13scoped_refptrIN7content27ServiceWorkerContextWrapperEElRKNS_8CallbackIFvvELNS0_8CopyModeE1ELNS0_10RepeatModeE1EEEEJRKS7_RKlSE_EEEvOT_DpOT0_ #7 0x7f7837b7ba84 _ZN4base8internal7InvokerINS0_9BindStateIPFv13scoped_refptrIN7content27ServiceWorkerContextWrapperEElRKNS_8CallbackIFvvELNS0_8CopyModeE1ELNS0_10RepeatModeE1EEEEJS6_lSB_EEES8_E7RunImplIRKSF_RKSt5tupleIJS6_lSB_EEJLm0ELm1ELm2EEEEvOT_OT0_NS_13IndexSequenceIJXspT1_EEEE #8 0x7f7837b7b95c _ZN4base8internal7InvokerINS0_9BindStateIPFv13scoped_refptrIN7content27ServiceWorkerContextWrapperEElRKNS_8CallbackIFvvELNS0_8CopyModeE1ELNS0_10RepeatModeE1EEEEJS6_lSB_EEES8_E3RunEPNS0_13BindStateBaseE #9 0x7f783f081021 _ZNO4base8internal8RunMixinINS_8CallbackIFvvELNS0_8CopyModeE0ELNS0_10RepeatModeE0EEEE3RunEv #10 0x7f783f0809f2 base::debug::TaskAnnotator::RunTask() #11 0x7f783f11195a base::MessageLoop::RunTask() #12 0x7f783f111be4 base::MessageLoop::DeferOrRunPendingTask() #13 0x7f783f111ece base::MessageLoop::DoWork() #14 0x7f783f12c74e base::MessagePumpLibevent::Run() #15 0x7f783f1114da base::MessageLoop::RunHandler() #16 0x7f783f1be1f3 base::RunLoop::Run() #17 0x7f783f266028 base::Thread::Run() #18 0x7f783866dad6 content::BrowserThreadImpl::IOThreadRun() #19 0x7f783866dddb content::BrowserThreadImpl::Run() #20 0x7f783f26696a base::Thread::ThreadMain() #21 0x7f783f24dcba base::(anonymous namespace)::ThreadFunc() #22 0x7f783f4c6184 start_thread #23 0x7f782baf737d clone Aborted (core dumped)
,
Dec 9 2016
s/bros/browser/
,
Dec 14 2016
nhiroki@ did this happen on browser startup?
ClearPushSubscriptionIdOnIO is a post task by PushMessagingService::ClearPushSubscriptionId. This can be called by:
- PushMessagingServiceImpl::OnStoreReset
- this is an GCMAppHandler override called when "GCM store is reset"
- PushMessagingServiceImpl::UnsubscribeInternal
called by
- PushMessagingServiceImpl::DeliverMessageCallback
- when there's a fatal error delivering a push message
- PushMessagingServiceImpl::Unsubscribe
- called from PushSubscription.unsubscribe() API
- PushMessagingServiceImpl::OnContentSettingsChanged
- probably called when content settings changed
- PushMessagingServiceImpl::UnsubscribeBecausePermissionRevoked
- also called when content settings changed
Assuming nhiroki@ wasn't changing content settings or permissions, either the GCM store was reset or a push message delivery failed fatally. Since push message delivery happens on startup, that seems plausible.
Push message delivery involves looking up a registration which would initalize SW storage. But it early exits if context_core_ isn't set yet.
Could it be possible context_core_ is not set at the time we try to deliver the push message, but is set by the time the push delivery happens?
Also, why does (Get|Store|Delete)UserData assume that LazyInitialize has already been called? Should they just call LazyInitialize in the same way as Find* functions do?
,
Dec 14 2016
On the other hand: [5322:5398:1209/153040.487169:ERROR:gcm_store_impl.cc(929)] Failed to restore security token. seems to lead to a OnStoreReset call, so that might be the cause.
,
Dec 14 2016
I could repro this but had to force the failure by saying if (true) to cause the security token error and then avoid triggering SW storage initialization by avoiding the NewTabPage and also disabled BackgroundSyncManager which would call LazyInitialize. I think the (Get|Store|Delete)UserData functions just need to call LazyInitialize. GetUserDataForAllRegistrations does this. I think maybe the assumption was you needed to have initialized SWStorage in order to have a registration id, but Push seems to store reg id in Prefs.
,
Dec 15 2016
Thank you for investigating this! > I think maybe the assumption was you needed to have initialized SWStorage in order to have a registration id, but Push seems to store reg id in Prefs. Yes, registration_id was supposed to be retrieved from the storage because registration_id can be reused after the storage is wiped out by data corruption etc. If we allow to store it somewhere else like Prefs, we need to make sure all storage clients verify a registration retrieved with a stored registration_id before they use it.
,
Dec 15 2016
Or, instead storage clients should delete data in Prefs when the service worker storage is wiped out. You can receive a notification via ServiceWorkerContextObserver::OnStorageWiped().
,
Dec 15 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/88bb637513212580567ef844f6b6e3edc16554f7 commit 88bb637513212580567ef844f6b6e3edc16554f7 Author: falken <falken@chromium.org> Date: Thu Dec 15 04:53:06 2016 Make ServiceWorkerStorage's UserData functions call LazyInitialize The callers can have a registration id even before storage is initialized, for example if registration ids are stored in preferences. BUG= 672716 Review-Url: https://codereview.chromium.org/2569353003 Cr-Commit-Position: refs/heads/master@{#438748} [modify] https://crrev.com/88bb637513212580567ef844f6b6e3edc16554f7/content/browser/service_worker/service_worker_storage.cc [modify] https://crrev.com/88bb637513212580567ef844f6b6e3edc16554f7/content/browser/service_worker/service_worker_storage_unittest.cc
,
Dec 15 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/88bb637513212580567ef844f6b6e3edc16554f7 commit 88bb637513212580567ef844f6b6e3edc16554f7 Author: falken <falken@chromium.org> Date: Thu Dec 15 04:53:06 2016 Make ServiceWorkerStorage's UserData functions call LazyInitialize The callers can have a registration id even before storage is initialized, for example if registration ids are stored in preferences. BUG= 672716 Review-Url: https://codereview.chromium.org/2569353003 Cr-Commit-Position: refs/heads/master@{#438748} [modify] https://crrev.com/88bb637513212580567ef844f6b6e3edc16554f7/content/browser/service_worker/service_worker_storage.cc [modify] https://crrev.com/88bb637513212580567ef844f6b6e3edc16554f7/content/browser/service_worker/service_worker_storage_unittest.cc
,
Dec 15 2016
OK marking fixed as this DCHECK should no longer happen. peter@ should confirm the suggestions in comment 6 and comment 7.
,
Dec 20 2016
Regarding #7 - awdf@ is working on that right now. We don't just want to clear prefs in that case, but also unsubscribe the affected push subscriptions. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by nhiroki@chromium.org
, Dec 9 2016