crash in nsAnimationManager::DispatchEvents()

RESOLVED FIXED in Firefox 43

Status

()

defect
--
critical
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: lizzard, Assigned: birtles)

Tracking

({crash, topcrash})

43 Branch
mozilla44
x86
Windows NT
Points:
---

Firefox Tracking Flags

(firefox42 unaffected, firefox43+ fixed, firefox44+ fixed)

Details

(crash signature)

Attachments

(1 attachment)

This bug was filed from the Socorro interface and is 
report bp-2a4ef110-4669-45bf-8d90-f1d422151003.
=============================================================

Newly showing up as a topcrash for 43. The crash signature has also been reported for 44.

Crashing thread:
0 	xul.dll 	nsAnimationManager::DispatchEvents() 	layout/style/nsAnimationManager.h
1 	xul.dll 	DispatchAnimationEventsOnSubDocuments 	layout/base/nsRefreshDriver.cpp
2 	xul.dll 	nsRefreshDriver::Tick(__int64, mozilla::TimeStamp) 	layout/base/nsRefreshDriver.cpp
3 	xul.dll 	nsRefreshDriver::DoTick() 	layout/base/nsRefreshDriver.cpp
4 	xul.dll 	nsRefreshDriver::FinishedWaitingForTransaction() 	layout/base/nsRefreshDriver.cpp
5 	xul.dll 	nsRefreshDriver::NotifyTransactionCompleted(unsigned __int64) 	layout/base/nsRefreshDriver.cpp
6 	xul.dll 	mozilla::layers::ClientLayerManager::DidComposite(unsigned __int64, mozilla::TimeStamp const&, mozilla::TimeStamp const&) 	gfx/layers/client/ClientLayerManager.cpp
7 	xul.dll 	mozilla::layers::PCompositorChild::OnMessageReceived(IPC::Message const&) 	obj-firefox/ipc/ipdl/PCompositorChild.cpp
8 	kernel32.dll 	GetCurrentThreadId 	
9 		@0x1de938af 	
10 	mozglue.dll 	je_free 	memory/mozjemalloc/jemalloc.c
Looks like "context->AnimationManager()" is null there.
I guess the "context->TransitionManager()->DispatchEvents()" that precedes it
destroyed the pres shell somehow, and then nsPresContext::SetShell nulled out
this member.
http://hg.mozilla.org/releases/mozilla-aurora/annotate/0bb47dc85ea8/layout/base/nsRefreshDriver.cpp#l1343
We should probably add some check after each of these calls and do
an early return.  Not sure what the right check is though,
context->GetPresShell() perhaps?
Flags: needinfo?(bbirtles)
(Assignee)

Comment 2

4 years ago
(In reply to Mats Palmgren (:mats) from comment #1)
> Looks like "context->AnimationManager()" is null there.
> I guess the "context->TransitionManager()->DispatchEvents()" that precedes it
> destroyed the pres shell somehow, and then nsPresContext::SetShell nulled out
> this member.
> http://hg.mozilla.org/releases/mozilla-aurora/annotate/0bb47dc85ea8/layout/
> base/nsRefreshDriver.cpp#l1343
> We should probably add some check after each of these calls and do
> an early return.  Not sure what the right check is though,
> context->GetPresShell() perhaps?

Yes, that sounds right. Do you want to right the patch since you did the work of analyzing it? Otherwise, I'm happy to knock it up.
Flags: needinfo?(bbirtles)
> Otherwise, I'm happy to knock it up.

Yes, please do so.  I think you know this code much better, in case there are
other places with similar problems etc.

Also, I missed one issue: the |kungFuDeathGrip| is holding on to the document.
I misread that as holding on to |context|, so I think we need a strong ref
on that too before these Dispatch calls.  We should just change the declaration
to a nsCOMPtr I think.
> We should just change the declaration to a nsCOMPtr I think.

By that I mean a nsRefPtr of course. :-)
(Assignee)

Updated

4 years ago
Assignee: nobody → bbirtles
Status: NEW → ASSIGNED
Comment on attachment 8670634 [details] [diff] [review]
Make sure presshell is still available after dispatching transition events

r=mats
Attachment #8670634 - Flags: review?(mats) → review+
https://hg.mozilla.org/mozilla-central/rev/ad1571ec7ecd
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
(Assignee)

Comment 10

4 years ago
Comment on attachment 8670634 [details] [diff] [review]
Make sure presshell is still available after dispatching transition events

Approval Request Comment
[Feature/regressing bug #]: bug 1183461
[User impact if declined]: Crasher
[Describe test coverage new/current, TreeHerder]: Baked on m-c for 1 day
[Risks and why]: None
[String/UUID change made/needed]: None
Attachment #8670634 - Flags: approval-mozilla-aurora?
Comment on attachment 8670634 [details] [diff] [review]
Make sure presshell is still available after dispatching transition events

Looks fixed on Nightly past the 2015100703 build. Great. Let's uplift this fix!
Attachment #8670634 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.