Closed Bug 1330512 Opened 3 years ago Closed 3 years ago

DocGroup labeling in nsDocument.cpp and nsGlobalWindow.cpp

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: njn, Assigned: njn)

References

()

Details

Attachments

(3 files)

I have a couple of DocGroup labelling patches.
For all the NS_DispatchToCurrentThread() cases I wasn't sure if we are
guaranteed to be on the main thread, so I added
MOZ_RELEASE_ASSERT(NS_IsMainThread()) to all of them. A try push showed up no
problems there.
Attachment #8826050 - Flags: review?(wmccloskey)
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
For all the NS_DispatchToCurrentThread() cases I wasn't sure if we are
guaranteed to be on the main thread, but nsGlobalWindow::Dispatch() has a
MOZ_RELEASE_ASSERT(NS_IsMainThread()) and a try push showed up no problems
there.
Attachment #8826051 - Flags: review?(wmccloskey)
There's one problem still to be ironed out. In a try push:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=89656aa15fe65144841705e246ec03004e02b501

I'm getting assertion failures in GetDocGroup() in the tc-Fxfn-r-e10s[tier 2] suite. It's possible that I've exposed a latent bug. The stack traces in the logs are unhelpful and I don't know how to run that test suite locally. I'll probably split my patches into lots of little pieces and push them to try individually to determine which change(s) are responsible.
> I'm getting assertion failures in GetDocGroup() in the tc-Fxfn-r-e10s[tier
> 2] suite. It's possible that I've exposed a latent bug. The stack traces in
> the logs are unhelpful and I don't know how to run that test suite locally.
> I'll probably split my patches into lots of little pieces and push them to
> try individually to determine which change(s) are responsible.

I've done this and the problem is the Dispatch("NotifyWindowIDDestroyed", ...) call in part 2. billm, any idea why that one might cause the assertion? I could just revert that change, though it would be good to know what's going wrong.
Flags: needinfo?(wmccloskey)
The window seems to be getting destroyed.  Perhaps the DocGroup is already destroyed?

[task 2017-01-11T07:04:35.749774Z] 07:04:35     INFO -  0  libxul.so!nsIDocument::GetDocGroup [nsDocument.cpp:89656aa15fe6 : 2870 + 0x0]
[task 2017-01-11T07:04:35.750631Z] 07:04:35     INFO -     rax = 0x0000000000627a18   rdx = 0x0000000000000000
[task 2017-01-11T07:04:35.751852Z] 07:04:35     INFO -     rcx = 0x00007fb122e4b664   rbx = 0x00007fb10379e000
[task 2017-01-11T07:04:35.752928Z] 07:04:35     INFO -     rsi = 0x00007fb12f338770   rdi = 0x00007fb12f337540
[task 2017-01-11T07:04:35.753952Z] 07:04:35     INFO -     rbp = 0x00007ffe865b3fb0   rsp = 0x00007ffe865b3f40
[task 2017-01-11T07:04:35.755066Z] 07:04:35     INFO -      r8 = 0x00007fb12f338770    r9 = 0x00007fb130615740
[task 2017-01-11T07:04:35.756154Z] 07:04:35     INFO -     r10 = 0x000000000000004a   r11 = 0x0000000000000000
[task 2017-01-11T07:04:35.757211Z] 07:04:35     INFO -     r12 = 0x00007ffe865b3f40   r13 = 0x0000000000000006
[task 2017-01-11T07:04:35.758162Z] 07:04:35     INFO -     r14 = 0x00007ffe865b4018   r15 = 0x00007fb102d42020
[task 2017-01-11T07:04:35.759151Z] 07:04:35     INFO -     rip = 0x00007fb1201bf35b
[task 2017-01-11T07:04:35.760297Z] 07:04:35     INFO -     Found by: given as instruction pointer in context
[task 2017-01-11T07:04:35.761272Z] 07:04:35     INFO -  1  libxul.so!nsGlobalWindow::Dispatch [nsGlobalWindow.cpp:89656aa15fe6 : 14227 + 0xc]
[task 2017-01-11T07:04:35.762334Z] 07:04:35     INFO -     rbx = 0x00007fb102d42000   rbp = 0x00007ffe865b3ff0
[task 2017-01-11T07:04:35.763432Z] 07:04:35     INFO -     rsp = 0x00007ffe865b3fc0   r12 = 0x00007fb122e26f45
[task 2017-01-11T07:04:35.764405Z] 07:04:35     INFO -     r13 = 0x0000000000000006   r14 = 0x00007ffe865b4018
[task 2017-01-11T07:04:35.765352Z] 07:04:35     INFO -     r15 = 0x00007fb102d42020   rip = 0x00007fb120104282
[task 2017-01-11T07:04:35.766311Z] 07:04:35     INFO -     Found by: call frame info
[task 2017-01-11T07:04:35.767442Z] 07:04:35     INFO -  2  libxul.so!nsGlobalWindow::NotifyWindowIDDestroyed [nsGlobalWindow.cpp:89656aa15fe6 : 9088 + 0x2a]
[task 2017-01-11T07:04:35.768526Z] 07:04:35     INFO -     rbx = 0x00007fb12010421c   rbp = 0x00007ffe865b4040
[task 2017-01-11T07:04:35.769490Z] 07:04:35     INFO -     rsp = 0x00007ffe865b4000   r12 = 0x00007fb102d42000
[task 2017-01-11T07:04:35.770624Z] 07:04:35     INFO -     r13 = 0x00007ffe865b4018   r14 = 0x00007ffe865b4010
[task 2017-01-11T07:04:35.771392Z] 07:04:35     INFO -     r15 = 0x00007fb10477f180   rip = 0x00007fb1200dd155
[task 2017-01-11T07:04:35.772526Z] 07:04:35     INFO -     Found by: call frame info
[task 2017-01-11T07:04:35.773513Z] 07:04:35     INFO -  3  libxul.so!nsGlobalWindow::FreeInnerObjects [nsGlobalWindow.cpp:89656aa15fe6 : 1774 + 0xf]
[task 2017-01-11T07:04:35.774567Z] 07:04:35     INFO -     rbx = 0x00007fb102d42000   rbp = 0x00007ffe865b4070
[task 2017-01-11T07:04:35.775604Z] 07:04:35     INFO -     rsp = 0x00007ffe865b4050   r12 = 0x0000000000000000
[task 2017-01-11T07:04:35.776728Z] 07:04:35     INFO -     r13 = 0x00007fb102d420e0   r14 = 0x00007fb10237a060
[task 2017-01-11T07:04:35.777671Z] 07:04:35     INFO -     r15 = 0x00007fb10477f180   rip = 0x00007fb1200fd697
[task 2017-01-11T07:04:35.778784Z] 07:04:35     INFO -     Found by: call frame info
[task 2017-01-11T07:04:35.779576Z] 07:04:35     INFO -  4  libxul.so!WindowStateHolder::~WindowStateHolder [nsGlobalWindow.cpp:89656aa15fe6 : 2326 + 0x5]
[task 2017-01-11T07:04:35.780629Z] 07:04:35     INFO -     rbx = 0x00007fb102332140   rbp = 0x00007ffe865b4090
[task 2017-01-11T07:04:35.781807Z] 07:04:35     INFO -     rsp = 0x00007ffe865b4080   r12 = 0x0000000000000000
[task 2017-01-11T07:04:35.782742Z] 07:04:35     INFO -     r13 = 0x00007fb1037d6280   r14 = 0x00007fb10237a060
[task 2017-01-11T07:04:35.783714Z] 07:04:35     INFO -     r15 = 0x00007fb10477f180   rip = 0x00007fb1200fd727
[task 2017-01-11T07:04:35.784813Z] 07:04:35     INFO -     Found by: call frame info
[task 2017-01-11T07:04:35.785726Z] 07:04:35     INFO -  5  libxul.so!WindowStateHolder::Release [nsGlobalWindow.cpp:89656aa15fe6 : 2330 + 0x5]
[task 2017-01-11T07:04:35.786517Z] 07:04:35     INFO -     rbx = 0x00007fb102332140   rbp = 0x00007ffe865b40b0
[task 2017-01-11T07:04:35.787727Z] 07:04:35     INFO -     rsp = 0x00007ffe865b40a0   r12 = 0x0000000000000000
[task 2017-01-11T07:04:35.788687Z] 07:04:35     INFO -     r13 = 0x00007fb1037d6280   r14 = 0x00007fb10237a060
[task 2017-01-11T07:04:35.789761Z] 07:04:35     INFO -     r15 = 0x00007fb10477f180   rip = 0x00007fb1200fd808
[task 2017-01-11T07:04:35.790522Z] 07:04:35     INFO -     Found by: call frame info
[task 2017-01-11T07:04:35.791571Z] 07:04:35     INFO -  6  libxul.so!nsSHEntryShared::DropPresentationState [nsCOMPtr.h:89656aa15fe6 : 334 + 0xa]
[task 2017-01-11T07:04:35.792711Z] 07:04:35     INFO -     rbx = 0x00007fb1037d74f0   rbp = 0x00007ffe865b40f0
[task 2017-01-11T07:04:35.793644Z] 07:04:35     INFO -     rsp = 0x00007ffe865b40c0   r12 = 0x00007fb102332140
[task 2017-01-11T07:04:35.794650Z] 07:04:35     INFO -     r13 = 0x00007fb1037d6280   r14 = 0x00007fb10237a060
[task 2017-01-11T07:04:35.795851Z] 07:04:35     INFO -     r15 = 0x00007fb10477f180   rip = 0x00007fb12170b07b
[task 2017-01-11T07:04:35.796788Z] 07:04:35     INFO -     Found by: call frame info
[task 2017-01-11T07:04:35.798006Z] 07:04:35     INFO -  7  libxul.so!nsSHEntryShared::RemoveFromBFCacheSync [nsSHEntryShared.cpp:89656aa15fe6 : 240 + 0x8]
[task 2017-01-11T07:04:35.798988Z] 07:04:35     INFO -     rbx = 0x00007fb1037d74f0   rbp = 0x00007ffe865b4120
[task 2017-01-11T07:04:35.799932Z] 07:04:35     INFO -     rsp = 0x00007ffe865b4100   r12 = 0x00007ffe865b4108
[task 2017-01-11T07:04:35.800934Z] 07:04:35     INFO -     r13 = 0x0000000000000000   r14 = 0x00007fb10237a060
[task 2017-01-11T07:04:35.801991Z] 07:04:35     INFO -     r15 = 0x00007fb10477f180   rip = 0x00007fb12170b1b1
[task 2017-01-11T07:04:35.802939Z] 07:04:35     INFO -     Found by: call frame info
[task 2017-01-11T07:04:35.804108Z] 07:04:35     INFO -  8  libxul.so!nsSHEntryShared::~nsSHEntryShared [nsSHEntryShared.cpp:89656aa15fe6 : 105 + 0x8]
[task 2017-01-11T07:04:35.805043Z] 07:04:35     INFO -     rbx = 0x00007fb1037d74f0   rbp = 0x00007ffe865b4150
[task 2017-01-11T07:04:35.806217Z] 07:04:35     INFO -     rsp = 0x00007ffe865b4130   r12 = 0x0000000000000000
[task 2017-01-11T07:04:35.807144Z] 07:04:35     INFO -     r13 = 0x0000000000000000   r14 = 0x00007fb10237a060
[task 2017-01-11T07:04:35.808150Z] 07:04:35     INFO -     r15 = 0x00007fb10477f180   rip = 0x00007fb12170ada5
[task 2017-01-11T07:04:35.809346Z] 07:04:35     INFO -     Found by: call frame info
[task 2017-01-11T07:04:35.810286Z] 07:04:35     INFO -  9  libxul.so!nsSHEntryShared::Release [nsSHEntryShared.cpp:89656aa15fe6 : 109 + 0x5]
[task 2017-01-11T07:04:35.811279Z] 07:04:35     INFO -     rbx = 0x00007fb1037d74f0   rbp = 0x00007ffe865b4170
[task 2017-01-11T07:04:35.812384Z] 07:04:35     INFO -     rsp = 0x00007ffe865b4160   r12 = 0x0000000000000000
[task 2017-01-11T07:04:35.813305Z] 07:04:35     INFO -     r13 = 0x00000000000001fd   r14 = 0x00007fb103fc0400
[task 2017-01-11T07:04:35.814277Z] 07:04:35     INFO -     r15 = 0x00007fb10477f180   rip = 0x00007fb12170aee2
[task 2017-01-11T07:04:35.815407Z] 07:04:35     INFO -     Found by: call frame info
[task 2017-01-11T07:04:35.816397Z] 07:04:35     INFO - 10  libxul.so!mozilla::SegmentedVector<nsCOMPtr<nsISupports>, 4096ul, mozilla::MallocAllocPolicy>::SegmentImpl<509ul>::~SegmentImpl [nsCOMPtr.h:89656aa15fe6 : 801 + 0x8]
[task 2017-01-11T07:04:35.817479Z] 07:04:35     INFO -     rbx = 0x0000000000000090   rbp = 0x00007ffe865b4190
[task 2017-01-11T07:04:35.818542Z] 07:04:35     INFO -     rsp = 0x00007ffe865b4180   r12 = 0x00007fb104036000
[task 2017-01-11T07:04:35.819749Z] 07:04:35     INFO -     r13 = 0x00000000000001fd   r14 = 0x00007fb103fc0400
[task 2017-01-11T07:04:35.820667Z] 07:04:35     INFO -     r15 = 0x00007fb10477f180   rip = 0x00007fb11f607eab
[task 2017-01-11T07:04:35.821646Z] 07:04:35     INFO -     Found by: call frame info
[task 2017-01-11T07:04:35.822800Z] 07:04:35     INFO - 11  libxul.so!mozilla::SegmentedVector<nsCOMPtr<nsISupports>, 4096ul, mozilla::MallocAllocPolicy>::PopLastN [SegmentedVector.h:89656aa15fe6 : 249 + 0x8]
[task 2017-01-11T07:04:35.823763Z] 07:04:35     INFO -     rbx = 0x00007fb104036000   rbp = 0x00007ffe865b41c0
[task 2017-01-11T07:04:35.824734Z] 07:04:35     INFO -     rsp = 0x00007ffe865b41a0   r12 = 0x0000000000000bee
[task 2017-01-11T07:04:35.825865Z] 07:04:35     INFO -     r13 = 0x00000000000001fd   r14 = 0x00007fb103fc0400
[task 2017-01-11T07:04:35.826776Z] 07:04:35     INFO -     r15 = 0x00007fb10477f180   rip = 0x00007fb11f607f19
[task 2017-01-11T07:04:35.827714Z] 07:04:35     INFO -     Found by: call frame info
[task 2017-01-11T07:04:35.828714Z] 07:04:35     INFO - 12  libxul.so!mozilla::dom::DeferredFinalizerImpl<nsISupports>::DeferredFinalize [BindingUtils.h:89656aa15fe6 : 2788 + 0x8]
[task 2017-01-11T07:04:35.829821Z] 07:04:35     INFO -     rbx = 0x00007fb103fc0400   rbp = 0x00007ffe865b41f0
[task 2017-01-11T07:04:35.830563Z] 07:04:35     INFO -     rsp = 0x00007ffe865b41d0   r12 = 0x0000000000000cd5
[task 2017-01-11T07:04:35.831708Z] 07:04:35     INFO -     r13 = 0x0000000000000000   r14 = 0x0000000000000cd5
[task 2017-01-11T07:04:35.832682Z] 07:04:35     INFO -     r15 = 0x00007fb10477f180   rip = 0x00007fb11f60826b
[task 2017-01-11T07:04:35.833803Z] 07:04:35     INFO -     Found by: call frame info
Here's a clearer stack trace. It's happening during XPCOM shutdown. Definitely looks like a latent problem with shutting down DocGroups.

> Thread 0 (crashed)
>  0  libxul.so!nsIDocument::GetDocGroup [nsDocument.cpp:b4b515b6841a : 2870 + 0x0]
>  1  libxul.so!nsGlobalWindow::Dispatch [nsGlobalWindow.cpp:b4b515b6841a : 14223 + 0xc]
>  2  libxul.so!nsGlobalWindow::NotifyWindowIDDestroyed [nsGlobalWindow.cpp:b4b515b6841a : 9087 + 0x2]
>  3  libxul.so!nsGlobalWindow::FreeInnerObjects [nsGlobalWindow.cpp:b4b515b6841a : 1774 + 0xf]
>  4  libxul.so!WindowStateHolder::~WindowStateHolder [nsGlobalWindow.cpp:b4b515b6841a : 2326 + 0x5]
>  5  libxul.so!WindowStateHolder::Release [nsGlobalWindow.cpp:b4b515b6841a : 2330 + 0x5]
>  6  libxul.so!nsSHEntryShared::DropPresentationState [nsCOMPtr.h:b4b515b6841a : 334 + 0xa]
>  7  libxul.so!nsSHEntryShared::RemoveFromBFCacheSync [nsSHEntryShared.cpp:b4b515b6841a : 240 + 0x8]
>  8  libxul.so!nsSHEntryShared::~nsSHEntryShared [nsSHEntryShared.cpp:b4b515b6841a : 105 + 0x8]
>  9  libxul.so!nsSHEntryShared::Release [nsSHEntryShared.cpp:b4b515b6841a : 109 + 0x5]
> 10  libxul.so!mozilla::SegmentedVector<nsCOMPtr<nsISupports>, 4096ul, mozilla::MallocAllocPolicy>::SegmentImpl<509ul>::~SegmentImpl [nsCOMPtr.h:b4b515b6841a : 801 + 0x8]
> 11  libxul.so!mozilla::SegmentedVector<nsCOMPtr<nsISupports>, 4096ul, mozilla::MallocAllocPolicy>::PopLastN [SegmentedVector.h:b4b515b6841a : 249 + 0x8]
> 12  libxul.so!mozilla::dom::DeferredFinalizerImpl<nsISupports>::DeferredFinalize [BindingUtils.h:b4b515b6841a : 2788 + 0x8]
> 13  libxul.so!mozilla::IncrementalFinalizeRunnable::ReleaseNow [CycleCollectedJSContext.cpp:b4b515b6841a : 1541 + 0xa]
> 14  libxul.so!mozilla::CycleCollectedJSContext::FinalizeDeferredThings [CycleCollectedJSContext.cpp:b4b515b6841a : 1614 + 0xf]
> 15  libxul.so!mozilla::CycleCollectedJSContext::OnGC [CycleCollectedJSContext.cpp:b4b515b6841a : 1660 + 0xe]
> 16  libxul.so!js::gc::GCRuntime::gcCycle [jsgc.cpp:b4b515b6841a : 1391 + 0xd]
> 17  libxul.so!js::gc::GCRuntime::collect [jsgc.cpp:b4b515b6841a : 6337 + 0xe]
> 18  libxul.so!js::gc::GCRuntime::gc [jsgc.cpp:b4b515b6841a : 6398 + 0xc]
> 19  libxul.so!nsCycleCollector::BeginCollection [nsCycleCollector.cpp:b4b515b6841a : 3823 + 0x10]
> 20  libxul.so!nsCycleCollector::Collect [nsCycleCollector.cpp:b4b515b6841a : 3650 + 0xf]
> 21  libxul.so!nsCycleCollector::ShutdownCollect [nsCycleCollector.cpp:b4b515b6841a : 3591 + 0x15]
> 22  libxul.so!nsCycleCollector_shutdown [nsCycleCollector.cpp:b4b515b6841a : 4200 + 0x14]
> 23  libxul.so!mozilla::ShutdownXPCOM [XPCOMInit.cpp:b4b515b6841a : 1016 + 0xa]
> 24  libxul.so!XRE_TermEmbedding [nsEmbedFunctions.cpp:b4b515b6841a : 216 + 0x7]
> 25  libxul.so!mozilla::ipc::ScopedXREEmbed::Stop [ScopedXREEmbed.cpp:b4b515b6841a : 117 + 0x5]
> 26  libxul.so!XRE_InitChildProcess [nsEmbedFunctions.cpp:b4b515b6841a : 760 + 0x11]
> 27  firefox!content_process_main [plugin-container.cpp:b4b515b6841a : 115 + 0x6]
> 28  firefox!main [nsBrowserApp.cpp:b4b515b6841a : 429 + 0xb]
My best guess is that we've shut down the component manager at this point, so when we call DocGroup::GetKey inside nsIDocument::GetDocGroup, we're unable to get the effective TLD service. So we return an incorrect docgroup key and the assertion fails.

I think we just want to avoid asserting if we're unable to get the effective TLD service. We could do that if we folded GetKey and MatchesKey into a single function that would be used for assertion checking. If it couldn't get the service, it would do nothing.

Nick, do you have time to do this? If not, please needinfo me.
Flags: needinfo?(wmccloskey) → needinfo?(n.nethercote)
Comment on attachment 8826050 [details] [diff] [review]
(part 1) - DocGroup labelling in nsDocument.cpp

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

Thanks! I've been thinking about the names we should use for runnables, and I think the best way to go is to use the name of the C++ method or class being dispatched. We can probably elide mozilla:: and mozilla::dom:: in cases where it doesn't seem helpful. Sorry for the churn, but I'm realizing that it's probably important to have a convention early in the process.

::: dom/base/nsDocument.cpp
@@ +4204,4 @@
>      nsCOMPtr<nsIRunnable> notification = NewRunnableMethod(this,
>        &nsDocument::NotifyStyleSheetApplicableStateChanged);
>      mSSApplicableStateNotificationPending =
> +      NS_SUCCEEDED(Dispatch("NotifyStyleSheetApplicableStateChanged",

nsDocument::Notify... for the name.

@@ +6757,5 @@
> +  RefPtr<nsRunnableMethod<nsDocument, void, false>> event =
> +    NewNonOwningRunnableMethod(this, &nsDocument::DoNotifyPossibleTitleChange);
> +  nsCOMPtr<nsIRunnable> event2(event);
> +  nsresult rv =
> +    Dispatch("NotifyPossibleTitleChange", TaskCategory::Other, event2.forget());

Rather than the extra temporary, I think you can pass do_AddRef(event).

Also, nsDocument::DoNotifyPossible... for the name.

@@ +8632,3 @@
>    nsCOMPtr<nsIRunnable> evt = new nsUnblockOnloadEvent(this);
> +  nsresult rv =
> +    Dispatch("PostUnblockOnloadEvent", TaskCategory::Other, evt.forget());

"nsUnblockOnloadEvent"

@@ +9562,5 @@
>  
>    if (aFireEvents) {
> +    MOZ_RELEASE_ASSERT(NS_IsMainThread());
> +    nsCOMPtr<nsIRunnable> ded = new nsDelayedEventDispatcher(args.mDocs);
> +    Dispatch("DelayedEventDispatcher", TaskCategory::Other, ded.forget());

Not sure it's worth it, but you could do:

Dispatch("DelayedEventDispatcher", TaskCategory::Other,
         MakeAndAddRef<nsDelayedEventDispatcher>(args.mDocs));

Also, "nsDelayedEventDispatcher".

@@ +10589,5 @@
>  {
> +  MOZ_RELEASE_ASSERT(NS_IsMainThread());
> +  nsCOMPtr<nsIRunnable> exit = new nsCallExitFullscreen(aDoc);
> +  if (aDoc) {
> +    aDoc->Dispatch("AsyncExitFullscreen", TaskCategory::Other, exit.forget());

"nsCallExitFullscreen"

@@ +10591,5 @@
> +  nsCOMPtr<nsIRunnable> exit = new nsCallExitFullscreen(aDoc);
> +  if (aDoc) {
> +    aDoc->Dispatch("AsyncExitFullscreen", TaskCategory::Other, exit.forget());
> +  } else {
> +    NS_DispatchToCurrentThread(exit);

Please use exit.forget(). I believe we're trying to deprecate the rawptr versions of functions like this. It also avoids unnecessary refcounting.

@@ +10864,5 @@
>  
>    // Request full-screen asynchronously.
> +  MOZ_RELEASE_ASSERT(NS_IsMainThread());
> +  nsCOMPtr<nsIRunnable> event = new nsCallRequestFullScreen(Move(aRequest));
> +  Dispatch("AsyncRequestFullScreen", TaskCategory::Other, event.forget());

Same with the name.
Attachment #8826050 - Flags: review?(wmccloskey) → review+
Comment on attachment 8826051 [details] [diff] [review]
(part 2) - DocGroup labelling in nsGlobalWindow.cpp

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

::: dom/base/nsGlobalWindow.cpp
@@ +533,5 @@
>    RefPtr<IdleRequest> request(mThrottledIdleRequestCallbacks.popFirst());
>    // ownership transferred from mThrottledIdleRequestCallbacks to
>    // mIdleRequestCallbacks
>    mIdleRequestCallbacks.insertBack(request);
> +  Dispatch("PostThrottledIdleCallback", TaskCategory::Other, request.forget());

These aren't equivalent. Please undo this one. We need to create an idle dispatch version eventually and we can fix this then.

@@ +8630,5 @@
>    static nsresult
>    PostCloseEvent(nsGlobalWindow* aWindow, bool aIndirect) {
>      nsCOMPtr<nsIRunnable> ev = new nsCloseEvent(aWindow, aIndirect);
> +    nsresult rv =
> +      aWindow->Dispatch("PostCloseEvent", TaskCategory::Other, ev.forget());

nsCloseEvent

@@ +9110,5 @@
>  nsGlobalWindow::NotifyWindowIDDestroyed(const char* aTopic)
>  {
>    nsCOMPtr<nsIRunnable> runnable = new WindowDestroyedEvent(this, mWindowID, aTopic);
> +  nsresult rv =
> +    Dispatch("NotifyWindowIDDestroyed", TaskCategory::Other, runnable.forget());

WindowDestroyedEvent

@@ +10373,5 @@
>    NS_ConvertUTF8toUTF16 newWideSpec(newSpec);
>  
>    nsCOMPtr<nsIRunnable> callback =
>      new HashchangeCallback(oldWideSpec, newWideSpec, this);
> +  return Dispatch("AsyncHashChange", TaskCategory::Other, callback.forget());

HashchangeCallback

@@ +10977,5 @@
>    nsCOMPtr<nsIRunnable> caller =
>      new NotifyIdleObserverRunnable(aIdleObserverHolder->mIdleObserver,
>                                     aIdleObserverHolder->mTimeInS,
>                                     aCallOnidle, this);
> +  if (NS_FAILED(Dispatch("NotifyIdleObserver", TaskCategory::Other,

...Runnable

@@ +12175,5 @@
>    ~AutoUnblockScriptClosing()
>    {
>      void (nsGlobalWindow::*run)() = &nsGlobalWindow::UnblockScriptedClosing;
> +    nsCOMPtr<nsIRunnable> caller = NewRunnableMethod(mWin, run);
> +    mWin->Dispatch("AutoUnblockScriptClosing", TaskCategory::Other,

nsGlobalWindow::UnblockScriptedClosing
Attachment #8826051 - Flags: review?(wmccloskey) → review+
> I think we just want to avoid asserting if we're unable to get the effective
> TLD service. We could do that if we folded GetKey and MatchesKey into a
> single function that would be used for assertion checking. If it couldn't
> get the service, it would do nothing.
> 
> Nick, do you have time to do this? If not, please needinfo me.

I'm happy to try that.
Flags: needinfo?(n.nethercote)
> we're unable to get the effective TLD service. So we return an incorrect docgroup
> key and the assertion fails.

I did a try push with some diagnostic printfs (because I couldn't reproduce this locally) and you're right: we fail to get TLS service. I'll write the fix tomorrow.
The way the empty string is used in this function is a bit clunky, but it
reflects the pre-existing behaviour.
Attachment #8827365 - Flags: review?(wmccloskey)
Comment on attachment 8827365 [details] [diff] [review]
(part 0) - Add an nsresult return value to DocGroup::GetKey()

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

Thanks!
Attachment #8827365 - Flags: review?(wmccloskey) → review+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.