374167 - [FIX]ASSERTION: wasDirty lied: 'mDirtyRoots.IndexOf(f) == -1'

Status: RESOLVED FIXED

(Core :: Layout, defect, P3)

Description

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

A current CVS Firefox debug build asserts opening a new window via
javascript:window.open("about:blank").  This is a reflow branch landing
regression.

This is a followup bug from bug 363253 which resolved some of these assertions by moving scrollframe attribute-setting out of reflow.

Salient comment from bz about the remaining work:
https://bugzilla.mozilla.org/show_bug.cgi?id=363253#c11

"For the rest, it seems to me that PresShell::StyleChangeReflow and
PresShell::ResizeReflow should clear mDirtyRoots at some point, no?

I also think we should add an assert that the frame we're adding to mDirtyRoots
is not currently in reflow; that should let us catch the scrollbar sort of
problem earlier."

Comment 1

[Boris Zbarsky [:bzbarsky]]

Might as well wait till bug 370952 lands, then revisit this.

Comment 2

[Eli Friedman]

At this point, it's asserting because ResizeReflow reflows the root frame but doesn't remove it from mDirtyRoots.  We should be doing that somewhere.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Yes, indeed

Comment 4

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

Comment on attachment 262807 [details] [diff] [review]
Yes, indeed

You can't do it this way, since resizes get optimized away if there's no size change, so there could be dirty roots that don't get reached.

You should just remove the root frame from mDirtyRoots.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Yeah, indeed

I missed us not setting the DIRTY flag on the root...

Comment 6

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

Comment on attachment 262822 [details] [diff] [review]
Yeah, indeed

r+sr=dbaron

Comment 7

[Boris Zbarsky [:bzbarsky]]

Fixed.