Closed
Bug 1330512
Opened 8 years ago
Closed 8 years ago
DocGroup labeling in nsDocument.cpp and nsGlobalWindow.cpp
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla53
Tracking | Status | |
---|---|---|
firefox53 | --- | fixed |
People
(Reporter: n.nethercote, Assigned: n.nethercote)
References
()
Details
Attachments
(3 files)
7.82 KB,
patch
|
billm
:
review+
|
Details | Diff | Splinter Review |
4.60 KB,
patch
|
billm
:
review+
|
Details | Diff | Splinter Review |
4.52 KB,
patch
|
billm
:
review+
|
Details | Diff | Splinter Review |
I have a couple of DocGroup labelling patches.
Assignee | ||
Comment 1•8 years ago
|
||
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 | ||
Updated•8 years ago
|
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•8 years ago
|
||
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)
Assignee | ||
Comment 3•8 years ago
|
||
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.
Assignee | ||
Comment 4•8 years ago
|
||
> 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)
Comment 5•8 years ago
|
||
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
Assignee | ||
Comment 6•8 years ago
|
||
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+
Assignee | ||
Comment 10•8 years ago
|
||
> 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)
Assignee | ||
Comment 11•8 years ago
|
||
> 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.
Assignee | ||
Comment 12•8 years ago
|
||
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+
Assignee | ||
Comment 14•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/e9850399c17efdbabadf9b4f4dc08023f88d90fb
Bug 1330512 (part 0) - Add an nsresult return value to DocGroup::GetKey(). r=billm.
https://hg.mozilla.org/integration/mozilla-inbound/rev/63a2802a6c1568901bb26f232be62bc21f56e737
Bug 1330512 (part 1) - DocGroup labelling in nsDocument.cpp. r=billm.
https://hg.mozilla.org/integration/mozilla-inbound/rev/456e087c0df8d83e3de2efc8ac387ddec8810b61
Bug 1330512 (part 2) - DocGroup labelling in nsGlobalWindow.cpp. r=billm.
Comment 15•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e9850399c17e
https://hg.mozilla.org/mozilla-central/rev/63a2802a6c15
https://hg.mozilla.org/mozilla-central/rev/456e087c0df8
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox53:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•