Last Comment Bug 322436 - WARNING: nsBlockFrame::CheckFloats: Explicit float list is out of sync with float cache
: WARNING: nsBlockFrame::CheckFloats: Explicit float list is out of sync with f...
Status: RESOLVED FIXED
required by 366128
: assertion, fixed1.8.0.13, fixed1.8.1.5, testcase
Product: Core
Classification: Components
Component: Layout: Misc Code (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Robert O'Callahan (:roc) (email my personal email if necessary)
:
Mentors:
Depends on: 282173
Blocks: randomstyles 366128 373919 384344
  Show dependency treegraph
 
Reported: 2006-01-04 20:38 PST by Jesse Ruderman
Modified: 2007-12-16 18:40 PST (History)
10 users (show)
roc: blocking1.9+
jruderman: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (341 bytes, text/html)
2006-01-04 20:39 PST, Jesse Ruderman
no flags Details
What I think should be an even smaller testcase (201 bytes, text/html)
2006-01-04 21:20 PST, Boris Zbarsky [:bz]
no flags Details
Patch to mute the frame dumps (1.29 KB, patch)
2006-06-06 09:11 PDT, Mats Palmgren (:mats)
roc: review+
roc: superreview+
Details | Diff | Splinter Review
fix (1.55 KB, patch)
2007-05-31 21:26 PDT, Robert O'Callahan (:roc) (email my personal email if necessary)
bzbarsky: review-
bzbarsky: superreview-
Details | Diff | Splinter Review
fix v2 (2.58 KB, patch)
2007-06-01 02:10 PDT, Robert O'Callahan (:roc) (email my personal email if necessary)
bzbarsky: review+
bzbarsky: superreview+
Details | Diff | Splinter Review

Description Jesse Ruderman 2006-01-04 20:38:39 PST
Steps to reproduce:
1. Load the testcase in a trunk debug build.

Result:

###!!! ASSERTION: not in child list: 'nsFrameList(aChildFrame->GetParent()->GetFirstChild(listName)) .ContainsFrame(aChildFrame)', file /Users/admin/trunk/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 1888
Comment 1 Jesse Ruderman 2006-01-04 20:39:41 PST
Created attachment 207582 [details]
testcase
Comment 2 Jesse Ruderman 2006-01-04 20:42:45 PST
bz, here's one of the testcases you asked for in bug 319280 comment 13.
Comment 3 Boris Zbarsky [:bz] 2006-01-04 21:20:15 PST
Created attachment 207583 [details]
What I think should be an even smaller testcase

This testcase shows what I think is the underlying problem -- the float being dropped from the frame tree altogether.  I'll bet money the reason is that BuildFloatList doesn't walk into boxes and that the answer is to nuke BuildFloatList.
Comment 4 Jesse Ruderman 2006-04-10 09:28:48 PDT
I don't see the assertion any more (after BuildFloatList removal), but I do see

WARNING: nsBlockFrame::CheckFloats: Explicit float list is out of sync with float cache: file /Users/admin/trunk/mozilla/layout/generic/nsBlockFrame.cpp, line 7068

and the second testcase's "This text should show up" text still doesn't show up.
Comment 5 Robert O'Callahan (:roc) (email my personal email if necessary) 2006-04-10 13:49:38 PDT
Yeah, we're still not reflowing the float. However, I believe we have changed the error from something potentially catastrophic to something merely incorrect.
Comment 6 Jesse Ruderman 2006-04-10 19:36:13 PDT
That's good to know.

Each warning is accompanied by a screenful of frame dumps (?), so if I see the warning often while fuzzing, I'll ask you to make at least the frame dump be DEBUG_roc or something.
Comment 7 Mats Palmgren (:mats) 2006-06-06 09:11:44 PDT
Created attachment 224570 [details] [diff] [review]
Patch to mute the frame dumps

(In reply to comment #6)
> I'll ask you to make at least the frame dump be DEBUG_roc or something.

I'll second that request.
Comment 8 Mats Palmgren (:mats) 2006-06-06 19:18:41 PDT
Comment on attachment 224570 [details] [diff] [review]
Patch to mute the frame dumps

Landed on trunk.
Comment 9 Mats Palmgren (:mats) 2006-08-16 15:42:15 PDT
A float inside a Box where the float containing block is an ancestor
of the Box is going to trigger this warning.
So, what is the real fix to this bug? do we want to add the float
of the nearest block reflow state? or wrap it in a anonymous block
during frame construction? or something else?

In the meantime, we could make CheckFloats() be silent for this
particular case, since it's a known problem (allowing us to see any
remaining issues with the float cache).
Comment 10 Robert O'Callahan (:roc) (email my personal email if necessary) 2006-08-17 15:15:11 PDT
You mean a XUL box?
Comment 11 Mats Palmgren (:mats) 2006-08-17 16:42:16 PDT
Yes - anything that uses nsFrame::BoxReflow()
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsFrame.cpp&rev=3.667&root=/cvsroot&mark=5534#5532
It doesn't seem to add the float to the block reflow state (which eventually
adds it to the float cache for the line), like nsLineLayout::ReflowFrame does:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsLineLayout.cpp&rev=3.238&root=/cvsroot&mark=1013-1018#992

There is also this block:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsFrame.cpp&rev=3.667&root=/cvsroot&mark=5657-5659#5648
which made me think it could be intentional.
Comment 12 Robert O'Callahan (:roc) (email my personal email if necessary) 2006-08-21 21:03:08 PDT
I don't know how that should be handled. Perhaps we should just ignore the float property on elements that aren't in a block formatting context?
Comment 13 Robert O'Callahan (:roc) (email my personal email if necessary) 2006-09-06 14:30:00 PDT
A fix for this could be backported to the 1.0.x branch, which might be useful. I'll try to work on it.
Comment 14 Jesse Ruderman 2007-03-20 16:53:42 PDT
Nominating because this blocks an [sg:critical] crash bug.
Comment 15 chris hofmann 2007-05-17 13:14:51 PDT
Is this ready for check-in on the trunk?  the problem blocks some of jesse's testing.

also do we know if it really does fix bug 366128, or will more work be needed for that one.
Comment 16 Jesse Ruderman 2007-05-17 13:26:44 PDT
The patch in this bug was just to reduce the amount of console output when this bug occurs.  There isn't a patch for this bug yet afaik.
Comment 17 Robert O'Callahan (:roc) (email my personal email if necessary) 2007-05-17 18:07:31 PDT
This is basically the same as bug 366128. We should fix it by pushing a null float containing block for XUL box frames, as Boris suggests:
https://bugzilla.mozilla.org/show_bug.cgi?id=366128#c10
Comment 18 Robert O'Callahan (:roc) (email my personal email if necessary) 2007-05-31 21:26:24 PDT
Created attachment 266862 [details] [diff] [review]
fix

It should be this simple.
Comment 19 Boris Zbarsky [:bz] 2007-05-31 23:38:00 PDT
Comment on attachment 266862 [details] [diff] [review]
fix

Would that it were so.  GetFloatContainingBlock also needs to be changed.
Comment 20 Robert O'Callahan (:roc) (email my personal email if necessary) 2007-06-01 02:10:56 PDT
Created attachment 266892 [details] [diff] [review]
fix v2

Indeed.
Comment 21 Boris Zbarsky [:bz] 2007-06-03 13:30:52 PDT
Comment on attachment 266892 [details] [diff] [review]
fix v2

I assume you checked that IsBoxFrame() is true if and only if the frame is created by ConstructXULFrame, right?  If so, r+sr=bzbarsky.
Comment 22 Robert O'Callahan (:roc) (email my personal email if necessary) 2007-06-06 20:17:48 PDT
There was one issue --- ConstructXULFrame creates non-box blocks for <label>, <text> and <description>. So I added an IsBoxFrame check here:

+    nsFrameConstructorSaveState floatSaveState;
+    if (newFrame->IsBoxFrame()) {
+      aState.PushFloatContainingBlock(nsnull, floatSaveState, PR_FALSE,
+                                      PR_FALSE);
+    }

and checked it in.
Comment 23 Daniel Veditz [:dveditz] 2007-07-12 18:37:03 PDT
Mats incorporated this into a branch fix for bug 384344
Comment 24 Jesse Ruderman 2007-12-16 18:40:01 PST
I committed my testcase as a crashtest.

I also committed bz's testcase as a reftest.  I assumed the current rendering is correct.

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