User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b1) Gecko/20060814 BonEcho/2.0b1 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b1) Gecko/20060814 BonEcho/2.0b1 When I visit avrev.com with Adblock Plus enabled Bon Echo hangs or crashes. Disabling Adblock Plus allows the site to work correctly. Reproducible: Always Steps to Reproduce: 1. Visit avrev.com with Adblock Plus enabled. 2. If Bon Echo does not immediately hang or crash, try clicking a link. Using crash2 I have this Talkback ID: TB22100260K
Here's a Talkback ID with Minefield crashing on the same site with Adblock Plus. TB22101355E Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060814 Minefield/3.0a1 ID:2006081404 [cairo]
Adblock Plus 0.7.1.1 can be downloaded from here: https://addons.mozilla.org/firefox/1865/ I don't get a crash with 2006-07-28 branch build, I get the crash with 2006-07-30 branch build. http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=MOZILLA_1_8_BRANCH&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2006-07-28+05&maxdate=2006-07-30+10&cvsroot=%2Fcvsroot I don't get a crash with 2006-07-28 1.5.0.x build, I get the crash with 2006-08-14 1.5.0.x build. I don't get a crash at all with trunk builds. I think the fix for bug 337883 caused this somehow. It's the only patch, that also was checked in the 1.8.0.x tree. Getting a minimised testcase out of this would be very tough.
I can reproduce it in a Firefox trunk debug build on Linux. I'll have a look. (Marking Security-Sensitive because it uses freed memory.)
Well, this was widely discussed in http://forums.mozillazine.org/viewtopic.php?t=451790&start=45 and the bug report was mentioned in: http://forums.mozillazine.org/viewtopic.php?t=451790&start=45#2432005
Created attachment 233774 [details] Testcase #1 When we remove a float inside an inline we fail to update the float containing block's (FCB) float cache which will contain the destroyed placeholder frame. Removing another frame directly in the FCB will crash in nsLineBox::RemovePlaceholderDescendantsOf. Making nsInlineFrame::RemoveFrame() also update the float cache of the nearest FCB fixes the crash for me... patch soon, need to investigate if there are other cases and the performance impact first...
This is more or less the same as bug 337883. Both are in fact regressions from bug 310638, this line to be precise: http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/base/nsCSSFrameConstructor.cpp&rev=1.1249&root=/cvsroot&mark=9775#9745 Nulling out mOutOfFlowFrame this early breaks the float cache which depends on it to find out which FC entry is associated with the float: http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsBlockFrame.cpp&rev=3.786&root=/cvsroot&mark=5564,5575#5557 http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsLineBox.cpp&rev=1.112&root=/cvsroot&mark=513,517#507 http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsLineBox.cpp&rev=1.112&root=/cvsroot&mark=913#907 I think the correct fix is to remove the SetOutOfFlowFrame(nsnull) line and backout bug 337883. I will run some more tests on this...
184.108.40.206 code freeze is coming up soon (Aug 26 I think). Is it reasonable to expect a patch by then?
Created attachment 234014 [details] [diff] [review] Patch rev. 1 The real fix for this bug and bug 337883 is to remove that SetOutOfFlowFrame(nsnull) line (the rest of the patch just backs out bug 337883). I wasn't aware that the float cache depends on that pointer being intact during frame destruction, mea culpa. I have tested this patch on 1.8/1.8.0 branches as well.
I'm a bit surprised that the various testing tools we have does not trigger this crash. The testcase in this bug for example is fairly simple and looks to me as if should be triggered, especially if you waste many, many CPU-hours on it ;-)
Bug 339651 seems related.
Comment on attachment 234014 [details] [diff] [review] Patch rev. 1 >Index: layout/base/nsCSSFrameConstructor.cpp >- ((nsPlaceholderFrame*)childFrame)->SetOutOfFlowFrame(nsnull); Add a comment here that we should not SetOutOfFlowFrame(nsnull), possibly with a pointer to either this bug or the float cache code or both? r=bzbarsky with that.
Checked in to trunk at 2006-08-17 06:15 PDT. (Will ask for branch approvals in a few days...) -> FIXED
Comment on attachment 234014 [details] [diff] [review] Patch rev. 1 approved for 1.8.0 branch, a=dveditz for drivers
Checked in to MOZILLA_1_8_0_BRANCH at 2006-08-21 13:34 PDT.
Comment on attachment 234014 [details] [diff] [review] Patch rev. 1 a=schrep for drivers - approving all [181approval pending] bugs now that tree is open.
Has this for certain been checked in on the Branch for 2.0? I no longer see crashes on the trunk, but I believe I just got the same crash on the same site with Adblock Plus 0.7.1.2. TB22441403X Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b2) Gecko/20060823 BonEcho/2.0b2 ID:2006082303
> Has this for certain been checked in on the Branch for 2.0? It has for certain NOT been checked in. When it is, the "fixed1.8.1" keyword will be added.
Checked in to MOZILLA_1_8_BRANCH at 2006-08-23 21:16 PDT.
v.fixed on both branches with 8/24 nightly builds, no crashes with AdBlock Plus and testcase in this bug. Weston: Are you still crashing with today's (20060824) Bon Echo builds?
Neither avrev.com or the testcase crash with today's build (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b2) Gecko/20060824 BonEcho/2.0b2 ID:2006082403). This has been fixed.
I checked in Mats Palmgren's testcase as a crashtest.