1238954 - Assertion failure: HasScope(aRegistration->mPrincipal, aRegistration->mScope) when unregistering a ServiceWorkerRegistration right after registering it in e10s

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, defect)

Description

[(no longer active)]

See M-e10s-5 failures in <https://treeherder.mozilla.org/#/jobs?repo=try&revision=1a5c58558ea8>.  STR:

Apply this patch:

diff --git a/dom/security/test/csp/file_child-src_service_worker.html b/dom/security/test/csp/file_child-src_service_worker.html
index c970dc4..882216c 100644
--- a/dom/security/test/csp/file_child-src_service_worker.html
+++ b/dom/security/test/csp/file_child-src_service_worker.html
@@ -15,7 +15,9 @@
             ).then(function(reg)
               {
                 // registration worked
-                window.parent.postMessage({id:page_id, message:"allowed"}, 'http://mochi.test:8888');
+                reg.unregister().then(function() {
+                  window.parent.postMessage({id:page_id, message:"allowed"}, 'http://mochi.test:8888');
+                });
               }).catch(function(error) {
               // registration failed
               window.parent.postMessage({id:page_id, message:"blocked"}, 'http://mochi.test:8888');

Run:

./mach mochitest --e10s -f plain dom/security/test/csp/

Assertion failure: HasScope(aRegistration->mPrincipal, aRegistration->mScope), at /Users/ehsan/moz/src/dom/workers/ServiceWorkerManager.cpp:4288
#01: mozilla::dom::workers::ServiceWorkerManager::RemoveRegistration(mozilla::dom::workers::ServiceWorkerRegistrationInfo*) (ServiceWorkerManager.cpp:4288, in XUL)
#02: mozilla::dom::workers::ServiceWorkerManager::StopControllingADocument(mozilla::dom::workers::ServiceWorkerRegistrationInfo*) (ServiceWorkerManager.cpp:3344, in XUL)
#03: mozilla::dom::workers::ServiceWorkerManager::MaybeStopControlling(nsIDocument*) (ServiceWorkerManager.cpp:3296, in XUL)
#04: nsDocument::RemovedFromDocShell() (nsDocument.cpp:8840, in XUL)
#05: nsHTMLDocument::RemovedFromDocShell() (nsHTMLDocument.cpp:3685, in XUL)
#06: nsDocumentViewer::Close(nsISHEntry*) (nsDocumentViewer.cpp:1467, in XUL)
#07: nsDocShell::Destroy() (nsDocShell.cpp:5721, in XUL)
#08: non-virtual thunk to nsDocShell::Destroy() (nsDocShell.cpp:5648, in XUL)
#09: nsFrameLoader::DestroyDocShell() (nsFrameLoader.cpp:1509, in XUL)
#10: nsFrameLoaderDestroyRunnable::Run() (nsFrameLoader.cpp:1454, in XUL)
#11: nsDocument::MaybeInitializeFinalizeFrameLoaders() (nsDocument.cpp:7318, in XUL)
#12: void nsRunnableMethodArguments<>::apply<nsDocument, void (nsDocument::*)()>(nsDocument*, void (nsDocument::*)()) (nsThreadUtils.h:664, in XUL)
#13: nsRunnableMethodImpl<void (nsDocument::*)(), true>::Run() (nsThreadUtils.h:870, in XUL)
#14: nsContentUtils::AddScriptRunner(nsIRunnable*) (nsContentUtils.cpp:5225, in XUL)
#15: nsDocument::FinalizeFrameLoader(nsFrameLoader*, nsIRunnable*) (nsDocument.cpp:7274, in XUL)
#16: nsFrameLoader::StartDestroy() (nsFrameLoader.cpp:1437, in XUL)
#17: nsFrameLoader::Destroy() (nsFrameLoader.cpp:1337, in XUL)
#18: nsGenericHTMLFrameElement::DestroyContent() (nsGenericHTMLFrameElement.cpp:404, in XUL)
#19: mozilla::dom::FragmentOrElement::DestroyContent() (FragmentOrElement.cpp:1186, in XUL)
#20: mozilla::dom::FragmentOrElement::DestroyContent() (FragmentOrElement.cpp:1186, in XUL)
#21: mozilla::dom::FragmentOrElement::DestroyContent() (FragmentOrElement.cpp:1186, in XUL)
#22: nsDocument::Destroy() (nsDocument.cpp:8805, in XUL)
#23: nsDocumentViewer::Destroy() (nsDocumentViewer.cpp:1641, in XUL)
#24: nsDocumentViewer::Show() (nsDocumentViewer.cpp:1959, in XUL)
#25: nsPresContext::EnsureVisible() (nsPresContext.cpp:2221, in XUL)
#26: PresShell::UnsuppressAndInvalidate() (nsPresShell.cpp:3692, in XUL)
#27: PresShell::UnsuppressPainting() (nsPresShell.cpp:3737, in XUL)
#28: PresShell::sPaintSuppressionCallback(nsITimer*, void*) (nsPresShell.cpp:1736, in XUL)
#29: nsTimerImpl::Fire() (nsTimerImpl.cpp:527, in XUL)
#30: nsTimerEvent::Run() (TimerThread.cpp:290, in XUL)
#31: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:990, in XUL)
#32: NS_ProcessPendingEvents(nsIThread*, unsigned int) (nsThreadUtils.cpp:239, in XUL)
#33: nsBaseAppShell::NativeEventCallback() (nsBaseAppShell.cpp:98, in XUL)
#34: nsAppShell::ProcessGeckoEvents(void*) (nsAppShell.mm:387, in XUL)
#35: __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ (in CoreFoundation) + 17
#36: __CFRunLoopDoSources0 (in CoreFoundation) + 556
#37: __CFRunLoopRun (in CoreFoundation) + 927
#38: CFRunLoopRunSpecific (in CoreFoundation) + 296
#39: RunCurrentEventLoopInMode (in HIToolbox) + 235
#40: ReceiveNextEventCommon (in HIToolbox) + 432
#41: _BlockUntilNextEventMatchingListInModeWithFilter (in HIToolbox) + 71
#42: _DPSNextEvent (in AppKit) + 1067
#43: -[NSApplication _nextEventMatchingEventMask:untilDate:inMode:dequeue:] (in AppKit) + 454
#44: -[GeckoNSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] (nsAppShell.mm:121, in XUL)
#45: -[NSApplication run] (in AppKit) + 682
#46: nsAppShell::Run() (nsAppShell.mm:661, in XUL)
#47: XRE_RunAppShell (nsEmbedFunctions.cpp:789, in XUL)
#48: mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) (MessagePump.cpp:259, in XUL)
#49: MessageLoop::RunInternal() (message_loop.cc:235, in XUL)
#50: MessageLoop::RunHandler() (message_loop.cc:228, in XUL)
#51: MessageLoop::Run() (message_loop.cc:201, in XUL)
#52: XRE_InitChildProcess (nsEmbedFunctions.cpp:625, in XUL)
#53: content_process_main(int, char**) (plugin-container.cpp:237, in plugin-container)
#54: main (MozillaRuntimeMain.cpp:11, in plugin-container)

Comment 1

[Brad Lassey [:blassey] (use needinfo?)]

Ehsan, is the ServiceWorkers team taking care of this?

Comment 2

[(no longer active)]

(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #1)
> Ehsan, is the ServiceWorkers team taking care of this?

I need to retest this on top of bug 1238990.  That bug may fix this.  But to answer your question, yes!

Comment 3

[(no longer active)]

This still happens with bug 1238990 landed.

Ben, do you mind taking a look?  Irrespective of what to do with bug 1237363, this shouldn't assert in e10s...

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

I'll take a look, but I will be on PTO starting Friday through middle of next week.  Probably won't get to it before I leave.

Comment 5

[(no longer active)]

No rush, thanks a lot!

Comment 6

[Ben Kelly [:bkelly, not reviewing]]

Part of the issue here is the test is open many iframes that each try to register the same service worker in parallel.  Your patch in comment 0 makes them all try to unregister at the same time.

The test tries to avoid this by using register(url + '#' + page_id), but url fragments do not count for service worker scripts.  Also, they would all have the same scope anyways.

A better approach is to have each frame put the page_id in their register()'s scope.  I'll provide a patch to do that.

Besides that, though, I'll still try to understand whats going on with the assert.

Comment 7

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P1 Purge potentially controlled document entries when evicting service worker registrations. r=ehsan

Comment 8

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P2 Fix csp service worker tests to register and unregister separate scopes. r=ehsan

Comment 9

[Ben Kelly [:bkelly, not reviewing]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1a44dc1d1e35

Comment 10

[Ben Kelly [:bkelly, not reviewing]]

Apparently I have a syntax error somewhere that try caught.  Will fix tomorrow morning.

Comment 11

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P2 Fix csp service worker tests to register and unregister separate scopes. r=ehsan

Forget to add a semicolon back when I put my fix back in.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=767a3859e3ab

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d1bc57a69090
https://hg.mozilla.org/integration/mozilla-inbound/rev/0b53d7dbaaf2

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d1bc57a69090
https://hg.mozilla.org/mozilla-central/rev/0b53d7dbaaf2