563837 - large numbers of frame construction assertions on reftests with reftest-print and position:fixed

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

layout/reftests/bugs/200774-1.html causes a very large number of assertions (30 on Linux, 35 on Windows and Mac):

The following five assertions repeat six or seven times:

^G###!!! ASSERTION: Why did we get called?: '!aChildContent->GetPrimaryFrame() || aChildContent->GetPrimaryFrame()->GetContent() != aChildContent', file /builds/slave/mozilla-central-macosx64-debug/build/layout/base/nsCSSFrameConstructor.cpp, line 2178
^G###!!! ASSERTION: Why did we get called?: '!aChildContent->GetPrimaryFrame() || aChildContent->GetPrimaryFrame()->GetContent() != aChildContent', file /builds/slave/mozilla-central-macosx64-debug/build/layout/base/nsCSSFrameConstructor.cpp, line 2178
^G###!!! ASSERTION: How did that happen?: '!mContent->GetPrimaryFrame()', file /builds/slave/mozilla-central-macosx64-debug/build/layout/base/nsCSSFrameConstructor.cpp, line 11846
^G###!!! ASSERTION: Losing track of existing primary frame: '!aFrame || !mPrimaryFrame || aFrame == mPrimaryFrame', file ../../dist/include/nsIContent.h, line 893
^G###!!! ASSERTION: Losing track of existing primary frame: '!aFrame || !mPrimaryFrame || aFrame == mPrimaryFrame', file ../../dist/include/nsIContent.h, line 893

Comment 1

[Timothy Nikkel (:tnikkel)]

Without looking in detail, it's reftest-print with a fixed element, seems like that would be the cause.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Also 0-10 on layout/reftests/bugs/417676.html

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

actually, it's strictly 10 on 417676.html and/or its -ref

also 15 on 427017-1.xhtml and 5 on 381497-f.html

Comment 4

[Boris Zbarsky [:bzbarsky]]

Yeah, if we're calling into frame construction over and over again for this node to create the fixed frames on every page these asserts would fire.  Looks like ReplicateFixedFrames does just that.  There's similar code in table-land; see bug 553972.

Let me see if I can find the patch I wrote while debugging that; we should just land it.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed fix

I'm not entirely sure about adding the state to all those methods, but most of them should inline and optimize it away in opt builds...  and I'd rather like to keep the asserts in debug builds instead of just removing them.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 443545 [details] [diff] [review]
Proposed fix

+  PRPackedBool              mSetPrimaryFrames;

Add a comment explaining what this means?

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

When you land this, you should also remove the asserts() annotations referencing this bug that I added in http://hg.mozilla.org/mozilla-central/rev/39aa13ebf607

Comment 8

[Boris Zbarsky [:bzbarsky]]

Pushed  http://hg.mozilla.org/mozilla-central/rev/994569c16070 with the comment fix, but forgot the asserts() fix.  Working on that now.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/bdf8167b2c18 to fix those.