Last Comment Bug 690990 - "ABORT: should not get marked modified during parsing"
: "ABORT: should not get marked modified during parsing"
Status: VERIFIED FIXED
[qa!]
: assertion, testcase, verified-aurora, verified-beta
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: x86_64 Mac OS X
: P1 critical (vote)
: mozilla10
Assigned To: Boris Zbarsky [:bz] (Out June 25-July 6)
:
Mentors:
Depends on:
Blocks: stirdom 185236
  Show dependency treegraph
 
Reported: 2011-09-30 19:53 PDT by Jesse Ruderman
Modified: 2011-12-07 05:09 PST (History)
6 users (show)
bzbarsky: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
testcase (asserts fatally when loaded) (302 bytes, text/html)
2011-09-30 19:53 PDT, Jesse Ruderman
no flags Details
stack trace (1.09 KB, text/plain)
2011-09-30 19:53 PDT, Jesse Ruderman
no flags Details
Don't assert that sheets are not modified when it would be just fine for them to be modified. (2.61 KB, patch)
2011-10-02 21:33 PDT, Boris Zbarsky [:bz] (Out June 25-July 6)
peterv: review+
christian: approval‑mozilla‑aurora+
Details | Diff | Review

Description Jesse Ruderman 2011-09-30 19:53:21 PDT
Created attachment 563920 [details]
testcase (asserts fatally when loaded)

###!!! ABORT: should not get marked modified during parsing: '!data->mSheet->IsModified()', file layout/style/Loader.cpp, line 1748

Could this be a regression from bug 185236?
Comment 1 Jesse Ruderman 2011-09-30 19:53:38 PDT
Created attachment 563921 [details]
stack trace
Comment 2 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-10-02 21:25:34 PDT
Yes.  The bug is in the assertion itself, though.

The issue is that the appendChild call in this case will go ahead and create a new stylesheet which is all set to go.  It will also post an async SheetComplete for it.  Then the very next line of script will modify the sheet.

The assertion just needs to move into the !mSheetAlreadyComplete block.
Comment 3 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-10-02 21:33:33 PDT
Created attachment 564116 [details] [diff] [review]
Don't assert that sheets are not modified when it would be just fine for them to be modified.
Comment 4 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-10-03 12:15:22 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/f5aa42137fbc
Comment 5 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-10-03 12:16:37 PDT
Comment on attachment 564116 [details] [diff] [review]
Don't assert that sheets are not modified when it would be just fine for them to be modified.

I believe we should take this on aurora.  It's a very safe change (debug-only code moving around), and will keep this bug from getting in the way of Jesse's fuzzers.
Comment 6 Matt Brubeck (:mbrubeck) 2011-10-03 16:52:27 PDT
https://hg.mozilla.org/mozilla-central/rev/f5aa42137fbc
Comment 7 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-10-06 19:46:56 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/625b0a5109eb
Comment 8 Vlad [QA] 2011-11-28 02:54:46 PST
Hi guys.
In order to verify this bug, it's enough if I load the test case from the attachments and observe that Firefox isn't crashing?
Thanks
Comment 9 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-11-28 05:19:05 PST
In a debug build, yes.
Comment 10 Vlad [QA] 2011-12-07 05:09:06 PST
Setting resolution to Verified Fixed on debug builds:

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:9.0) Gecko/20111201 Firefox/9.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:10.0a2) Gecko/20111202 Firefox/10.0a2
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:11.0a1) Gecko/20111205 Firefox/11.0a1

Based on comment8 and comment9, I have loaded the test case from the description and Firefox did not crash.

Note You need to log in before you can comment on or make changes to this bug.