Closed Bug 735943 Opened 12 years ago Closed 12 years ago

Crash @ nsCSSFrameConstructor::ProcessPendingRestyles


(Core :: Layout, defect)

Not set



Tracking Status
firefox12 + verified
firefox13 + verified
firefox14 --- verified
firefox-esr10 12+ verified
status1.9.2 --- unaffected


(Reporter: jruderman, Assigned: MatsPalmgren_bugz)



(Keywords: crash, testcase, Whiteboard: [sg:critical][qa!])

Crash Data


(6 files)

About 50% of the time, this testcase causes a crash in nsCSSFrameConstructor::ProcessPendingRestyles.
Assignee: nobody → matspal
Component: DOM: Core & HTML → Layout
OS: Mac OS X → All
QA Contact: general → layout
Hardware: x86_64 → All
Attached file stack
Attached patch fix+testSplinter Review
FYI, the document has a strong pointer to the nsSMILAnimationController
object, which keeps a raw pointer to it (mDocument).

Try results pending:

Filed bug 735966 on the assertion when running the crash test.
Attachment #606076 - Flags: review?(bzbarsky)
Again, with just this change this time (there's a jsreftest crash in the one above):
My testcase (reduced from what the fuzzer created) is an insane tangle of setTimeouts. It might be possible to create a more robust regression test ;)
I hit bp-d0df45dc-63db-44d8-a6d8-f76722120315 (popups blocked) and bp-3d521fe3-cc02-44fc-8058-d85972120315 (popups allowed).
Crash Signature: [@ nsCSSFrameConstructor::ProcessPendingRestyles()] [@ nsCSSFrameConstructor::ProcessPendingRestyles] [@ PresShell::FlushPendingNotifications(mozFlushType)]
Summary: Crash [@ nsCSSFrameConstructor::ProcessPendingRestyles] → Crash @ nsCSSFrameConstructor::ProcessPendingRestyles
Hmm, there's a couple of "ASSERTION: This is unsafe! Fix the caller!: 'Error', content/events/src/nsEventDispatcher.cpp, line 558" on the new crash test.
Comment on attachment 606076 [details] [diff] [review]

Document (probably in the header and at the presshell check for mIsDestroying) that FlushResampleRequests can trigger style flushes, and r=me
Attachment #606076 - Flags: review?(bzbarsky) → review+
This crash fix seems worth taking on branches after baking it for a while...
Target Milestone: --- → mozilla14
Closed: 12 years ago
Resolution: --- → FIXED
(In reply to Mats Palmgren [:mats] from comment #11)
> This crash fix seems worth taking on branches after baking it for a while...

Do we know what the regressing bug is or whether this is a pain point for users? If not, this can ride the trains.
I would guess the addition of the FlushResampleRequests() call in
is what caused the regression (bug 547333).

The reason I recommend merging this fix to branches is that without it we
run script from inside a destroyed pres shell, which I think is unsafe.
The fix is very low-risk.
Comment on attachment 606076 [details] [diff] [review]

See comment 14.
Attachment #606076 - Flags: approval-mozilla-beta?
Attachment #606076 - Flags: approval-mozilla-aurora?
Group: core-security
Whiteboard: [sg:critical]
Comment on attachment 606076 [details] [diff] [review]

[Triage Comment]
Low risk fix, and both Mats and the security team agree that this is something we should fix all the way up to FF12. Please land asap, as we'll go to build with Beta 4 tomorrow (4/3) at 5PM PT.

Please make sure to prepare a patch for the ESR branch as well.
Attachment #606076 - Flags: approval-mozilla-beta?
Attachment #606076 - Flags: approval-mozilla-beta+
Attachment #606076 - Flags: approval-mozilla-aurora?
Attachment #606076 - Flags: approval-mozilla-aurora+
Attached patch for esr10Splinter Review
The fix applies also to esr10, but without the test since the pref()
reftest feature doesn't exist there, afaict.
Attachment #611648 - Flags: approval-mozilla-esr10?
Verified fixed in nightly Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:14.0) Gecko/20120402 Firefox/14.0a1. The testcase pretty reliably crashes my Aurora build.
Comment on attachment 611648 [details] [diff] [review]
for esr10

Thanks, please go ahead and land as per
Attachment #611648 - Flags: approval-mozilla-esr10? → approval-mozilla-esr10+
Whiteboard: [sg:critical] → [sg:critical][qa+]
Whiteboard: [sg:critical][qa+] → [sg:critical][qa!]
Group: core-security
You need to log in before you can comment on or make changes to this bug.