1255621 - Opening a site with an existing service worker in PBM crashes the content process

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, defect)

Description

[Lina Butler [:lina]]

STR:

1. In non-private mode, open a page that registers a service worker (for example, https://www.pinterest.com).
2. Open a private window.
3. Navigate to the page in the private window.

That navigation will trip this assertion: https://dxr.mozilla.org/mozilla-central/rev/dd1abe874252e507b825a0a4e1063b0e13578288/dom/workers/ServiceWorkerManager.cpp#3751

Comment 1

[Lina Butler [:lina]]

Stack trace:

Assertion failure: !nsContentUtils::IsInPrivateBrowsing(aDoc), at /Users/k/Projects/hg/central/dom/workers/ServiceWorkerManager.cpp:3751
#01: mozilla::image::ImageCacheKey::ImageCacheKey(nsIURI*, nsIDOMDocument*)[/Users/k/Projects/hg/central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x1028acd]
#02: imgLoader::FindEntryProperties(nsIURI*, nsIDOMDocument*, nsIProperties**)[/Users/k/Projects/hg/central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x10041b5]
#03: non-virtual thunk to imgLoader::FindEntryProperties(nsIURI*, nsIDOMDocument*, nsIProperties**)[/Users/k/Projects/hg/central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x100435d]
#04: nsContentUtils::IsImageInCache(nsIURI*, nsIDocument*)[/Users/k/Projects/hg/central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x109f5f2]
#05: nsDocument::MaybePreLoadImage(nsIURI*, nsAString_internal const&, mozilla::net::ReferrerPolicy)[/Users/k/Projects/hg/central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x12735b8]
#06: nsHtml5TreeOpExecutor::PreloadImage(nsAString_internal const&, nsAString_internal const&, nsAString_internal const&, nsAString_internal const&, nsAString_internal const&)[/Users/k/Projects/hg/central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0xd2d582]
#07: nsHtml5SpeculativeLoad::Perform(nsHtml5TreeOpExecutor*)[/Users/k/Projects/hg/central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0xd2cb24]
#08: nsHtml5TreeOpExecutor::RunFlushLoop()[/Users/k/Projects/hg/central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0xd4a7ab]
#09: nsHtml5ExecutorFlusher::Run()[/Users/k/Projects/hg/central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0xd5cd81]
#10: nsThread::ProcessNextEvent(bool, bool*)[/Users/k/Projects/hg/central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0xeb6a4]
#11: NS_ProcessPendingEvents(nsIThread*, unsigned int)[/Users/k/Projects/hg/central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x12aa7f]
#12: nsBaseAppShell::NativeEventCallback()[/Users/k/Projects/hg/central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x2cc507a]
#13: nsAppShell::ProcessGeckoEvents(void*)[/Users/k/Projects/hg/central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x2d2cc45]
#14: __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__[/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation +0x7f5b1]
#15: __CFRunLoopDoSources0[/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation +0x70c62]
#16: __CFRunLoopRun[/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation +0x703ef]
#17: CFRunLoopRunSpecific[/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation +0x6fe75]
#18: RunCurrentEventLoopInMode[/System/Library/Frameworks/Carbon.framework/Versions/A/Frameworks/HIToolbox.framework/Versions/A/HIToolbox +0x2ea0d]
#19: ReceiveNextEventCommon[/System/Library/Frameworks/Carbon.framework/Versions/A/Frameworks/HIToolbox.framework/Versions/A/HIToolbox +0x2e7b7]
#20: _BlockUntilNextEventMatchingListInModeWithFilter[/System/Library/Frameworks/Carbon.framework/Versions/A/Frameworks/HIToolbox.framework/Versions/A/HIToolbox +0x2e5bc]
#21: _DPSNextEvent[/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit +0x2424e]
#22: -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:][/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit +0x2389b]
#23: -[GeckoNSApplication nextEventMatchingMask:untilDate:inMode:dequeue:][/Users/k/Projects/hg/central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x2d2be46]
#24: -[NSApplication run][/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit +0x1799c]
#25: nsAppShell::Run()[/Users/k/Projects/hg/central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x2d2d380]
#26: XRE_RunAppShell[/Users/k/Projects/hg/central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x39302a4]
#27: mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*)[/Users/k/Projects/hg/central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x53a852]
#28: MessageLoop::Run()[/Users/k/Projects/hg/central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x4fca00]
#29: XRE_InitChildProcess[/Users/k/Projects/hg/central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x392fd44]
#30: content_process_main(int, char**)[/Users/k/Projects/hg/central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/plugin-container.app/Contents/MacOS/plugin-container +0x3ef8]

Comment 2

[Andrew Overholt [:overholt]]

Ben, can you take a look at this?

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

I think we just need to make IsControlled() returned false if we are in a private browsing session.  Kit, any chance you would be willing to make that change?

Comment 4

[Lina Butler [:lina]]

Oh, nice. :-) Sure, I can do it now. I'll just add to the test we already have for PBM.

Comment 5

[Lina Butler [:lina]]

Attachment: pbm.patch

Ugh, sorry; got sidetracked. WDYT?

Comment 6

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8737420 [details] [diff] [review]
pbm.patch

Review of attachment 8737420 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good.  Thanks!  r=me

::: dom/workers/ServiceWorkerManager.cpp
@@ +3757,5 @@
>  ServiceWorkerManager::IsControlled(nsIDocument* aDoc, ErrorResult& aRv)
>  {
>    MOZ_ASSERT(aDoc);
>  
> +  if (nsContentUtils::IsInPrivateBrowsing(aDoc)) {

Can you add a comment that this is necessary to handle the case where a service worker was registered in a non-PB window?

Comment 7

[Lina Butler [:lina]]

Attachment: pbm.patch

Added comment, carrying forward r+.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8f81375c88fb

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4191a98791bb

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4191a98791bb