Last Comment Bug 350370 - [FIX] Crash [@ nsStyleContext::FindChildWithRules] with ::first-line, appending rows and table-cells, etc
: [FIX] Crash [@ nsStyleContext::FindChildWithRules] with ::first-line, appendi...
[sg:critical][baking 8/29][reflow-ref...
: crash, fixed1.8.1, regression, testcase, verified1.8.0.8
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: x86 All
: -- critical (vote)
: ---
Assigned To: Mats Palmgren (:mats)
Depends on:
Blocks: stirtable
  Show dependency treegraph
Reported: 2006-08-27 05:21 PDT by Martijn Wargers [:mwargers] (not working for Mozilla)
Modified: 2013-01-26 16:45 PST (History)
7 users (show)
mtschrep: blocking1.8.1+
dveditz: blocking1.8.0.7-
dveditz: blocking1.8.0.8+
mats: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

testcase, crashes on load (1.29 KB, text/html)
2006-08-27 05:23 PDT, Martijn Wargers [:mwargers] (not working for Mozilla)
no flags Details
zip file of original testcase and all in-between testcases (18.30 KB, application/zip)
2006-08-27 05:26 PDT, Martijn Wargers [:mwargers] (not working for Mozilla)
no flags Details
Patch rev. 1 (2.08 KB, patch)
2006-08-27 20:46 PDT, Mats Palmgren (:mats)
bzbarsky: review+
bzbarsky: superreview+
dveditz: approval1.8.0.8+
mtschrep: approval1.8.1+
Details | Diff | Splinter Review

Description Martijn Wargers [:mwargers] (not working for Mozilla) 2006-08-27 05:21:26 PDT
See upcoming testcase, which crashes current branch (1.8.0.x and 1.8.1) and trunk builds.
It doesn't crash Mozilla1.7.13.
This seems to have regressed between 2005-01-03 and 2005-01-04:
Maybe a regression from bug 271422 somehow?

Talkback ID: TB22561642M
nsStyleContext::FindChildWithRules   nsStyleSet::GetContext   nsStyleSet::ReParentStyleContext   nsFrameManager::ReParentStyleContext   nsFirstLineFrame::PullOneFrame   nsLineLayout::ReflowFrame  

I have several in-between testcases, that crash in different areas, mostly in the style system. I can attach those, if wanted.
Comment 1 Martijn Wargers [:mwargers] (not working for Mozilla) 2006-08-27 05:23:54 PDT
Created attachment 235640 [details]
testcase, crashes on load
Comment 2 Martijn Wargers [:mwargers] (not working for Mozilla) 2006-08-27 05:26:11 PDT
Created attachment 235641 [details]
zip file of original testcase and all in-between testcases
Comment 3 Mats Palmgren (:mats) 2006-08-27 08:36:17 PDT
Patch coming up...
Comment 4 Mats Palmgren (:mats) 2006-08-27 20:46:35 PDT
Created attachment 235697 [details] [diff] [review]
Patch rev. 1

The problem is that we iterate past the beginning on a line iterator.
We actually have an assertion for this but still continue to execute
leading to a near certain crash on Windows (it crashes less often on
Linux for some reason).

Test results with the patch:
I ran all the testcases in the zip archive for at least 5 minutes,
up to an hour for some, on both Windows and Linux and there were
no crashes. There are plenty of nasty assertions though, especially
the table related ones looks interesting...

I wanted to have two separate assertions, the first indicates a broken
frame tree while the second could also happen for a valid frame tree.
Comment 5 Mats Palmgren (:mats) 2006-08-27 20:47:58 PDT
sg: we're using random memory and in some cases overwrite the stack.
Comment 6 Boris Zbarsky [:bz] 2006-08-27 22:25:38 PDT
Comment on attachment 235697 [details] [diff] [review]
Patch rev. 1

Looks ok for branches.  On trunk, I'd just as soon not touch this code -- it's completely gone on reflow branch, and changing it will just make merging the branch that much more work.
Comment 7 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2006-08-27 22:52:01 PDT
Landing it on the trunk is fine -- these pieces are the easy ones to merge.
Comment 8 Boris Zbarsky [:bz] 2006-08-27 23:02:47 PDT
If dbaron's ok with it, then go ahead and land on trunk too, sure.
Comment 9 Mats Palmgren (:mats) 2006-08-28 02:39:22 PDT
Landed on trunk at 2006-08-28 00:30 PDT.

Comment 10 Daniel Veditz [:dveditz] 2006-08-28 10:20:23 PDT
This will have to wait for
Comment 11 Daniel Veditz [:dveditz] 2006-08-28 10:37:23 PDT
Comment on attachment 235697 [details] [diff] [review]
Patch rev. 1

approved for 1.8.0 branch, a=dveditz for drivers

We are slipping the candidate build until tomorrow. Please land this today to make
Comment 12 Daniel Veditz [:dveditz] 2006-08-28 16:28:56 PDT
Comment on attachment 235697 [details] [diff] [review]
Patch rev. 1

we're building now, I guess this will have to be after all.
Comment 13 Mike Schroepfer 2006-08-29 10:34:03 PDT
Comment on attachment 235697 [details] [diff] [review]
Patch rev. 1

a=schrep for drivers.
Comment 14 Mats Palmgren (:mats) 2006-08-31 10:41:40 PDT
Landed on MOZILLA_1_8_BRANCH at 2006-08-31 09:57 PDT.
Comment 15 Daniel Veditz [:dveditz] 2006-09-19 15:38:23 PDT
Restoring lost blocking flag
Comment 16 Daniel Veditz [:dveditz] 2006-09-26 14:58:33 PDT
Comment on attachment 235697 [details] [diff] [review]
Patch rev. 1

approved for 1.8.0 branch, a=dveditz for drivers
Comment 17 Mats Palmgren (:mats) 2006-10-05 18:53:21 PDT
Landed on MOZILLA_1_8_0_BRANCH at 2006-10-05 16:56 PDT.
Comment 18 Jay Patel [:jay] 2006-10-20 14:27:17 PDT
v.fixed on 1.8.0 branch with Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv: Gecko/20061020 Firefox/, no crash.
Comment 19 Daniel Veditz [:dveditz] 2006-12-14 20:42:29 PST
Comment #0 mentions the in-between testcases that crash in "different areas". Does the fix in this bug fix all of those or should we have spun off different bugs?
Comment 20 Mats Palmgren (:mats) 2006-12-14 23:52:21 PST
Comment 4 claims all crashes were fixed.  There were a few "interesting"
assertions though that might be worth reporting if they still occur.
Comment 21 Mats Palmgren (:mats) 2013-01-26 05:57:22 PST
Comment 22 Ryan VanderMeulen [:RyanVM] 2013-01-26 16:45:34 PST

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