"ASSERTION: cannot call on a dirty frame not currently being reflowed" with abs pos, <tr>, <xul:window>

RESOLVED WORKSFORME

Status

()

defect
RESOLVED WORKSFORME
13 years ago
11 years ago

People

(Reporter: jruderman, Unassigned)

Tracking

(Blocks 1 bug, {assertion, testcase})

Trunk
Points:
---
Dependency tree / graph
Bug Flags:
wanted-next +
blocking1.9 -
wanted1.9 -
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 4 obsolete attachments)

Reporter

Description

13 years ago
Loading this testcase triggers two of these assertions (with different line numbers), each twice:

###!!! ASSERTION: cannot call on a dirty frame not currently being reflowed: '!(GetStateBits() & (NS_FRAME_IS_DIRTY | NS_FRAME_HAS_DIRTY_CHILDREN)) || (GetStateBits() & NS_FRAME_IN_REFLOW)', file /Users/admin/trunk/mozilla/layout/generic/nsFrame.cpp, line 714
Reporter

Comment 1

13 years ago
Posted file testcase
Posted file stack trace
Also be aware of bug 369563, which triggers a similar assertion with a totally different stack trace and different steps to reproduce.
OS: Mac OS X → All
Hardware: Macintosh → All
Component: Layout → Layout: R & A Pos
QA Contact: layout → layout.r-and-a-pos
Reporter

Comment 4

12 years ago
I filed bug 387213 with another testcase that triggers this assertion.  The testcase in that bug is simpler than the one here.
Reporter

Updated

12 years ago
Flags: blocking1.9?
Flags: blocking1.9? → blocking1.9-
Whiteboard: [wanted-1.9]
Posted patch patch to fix (obsolete) — Splinter Review
This should fix this one and bug 369563
Assignee: nobody → jonas
Status: NEW → ASSIGNED
Attachment #295329 - Flags: superreview?(roc)
Attachment #295329 - Flags: review?(roc)
Not sure if I need to deal with recursion. If I don't the patch can be simplified to not use |disableAssert|
Posted patch Patch v2 (obsolete) — Splinter Review
Doh! Edited out too much of the previous patch before attaching.
Attachment #295329 - Attachment is obsolete: true
Attachment #295330 - Flags: superreview?(roc)
Attachment #295330 - Flags: review?(roc)
Attachment #295329 - Flags: superreview?(roc)
Attachment #295329 - Flags: review?(roc)
Posted patch Patch v3 (obsolete) — Splinter Review
Roc asked me to create a helperclass instead. Not that much less code, but it is more readable.
Attachment #295330 - Attachment is obsolete: true
Attachment #295332 - Flags: superreview?(roc)
Attachment #295332 - Flags: review?(roc)
Attachment #295330 - Flags: superreview?(roc)
Attachment #295330 - Flags: review?(roc)
Posted patch Patch v4 (obsolete) — Splinter Review
Ugh, manual patch editing failed again.
Attachment #295332 - Attachment is obsolete: true
Attachment #295335 - Flags: superreview?(roc)
Attachment #295335 - Flags: review?(roc)
Attachment #295332 - Flags: superreview?(roc)
Attachment #295332 - Flags: review?(roc)
Reporter

Updated

12 years ago
Blocks: 369563
Disabling during painting seems wrong.  I agree that we should disable it during event handling (there's another bug discussing that), but why does it need to be disabled during painting?

This bug, on the other hand, is about the assertion firing during reflow, which your patch wouldn't fix.  Maybe it belongs on a different bug?
(In reply to comment #10)
> Disabling during painting seems wrong.  I agree that we should disable it
> during event handling (there's another bug discussing that),

... bug 399352.
We aren't always able to fully flush reflows before painting. See
http://mxr.mozilla.org/seamonkey/source/layout/base/nsPresShell.cpp#5894

[This will still be true even in a post-compositor world, assuming we do interruptible reflow as well. For example if you have some translucent chrome floating over a content window and we need to repaint that chrome, we'll have to paint the content window without flushing its reflows, otherwise chrome event processing would be blocked by the reflow flush, basically eliminating the point of interruptible reflow. In general we need to be able to paint and handle events in a dirty frame tree.]

I'm not really sure what we should do about this. Perhaps we should just convert those assertions to warnings.
Well, we could make frames have two style context pointers, one reflecting what should be used in the next reflow, and one reflecting what was used in the last one.

But this discussion really belongs in bug 399352.
Flags: wanted1.9+
Whiteboard: [wanted-1.9]
Comment on attachment 295335 [details] [diff] [review]
Patch v4

Moved patch and discussion to bug 399352
Attachment #295335 - Attachment is obsolete: true
Attachment #295335 - Flags: superreview?(roc)
Attachment #295335 - Flags: review?(roc)
Reassigning to nobody. The patches above are for when this assertion fires under completely different circumstances. So they don't relate to this bug at all.

Sorry about the confusion :(

The stack in this bug looks very similar to bug 387213 though, so possibly a dupe.
Assignee: jonas → nobody
Status: ASSIGNED → NEW
Flags: wanted1.9-
Flags: wanted1.9+
Flags: wanted-next+
Reporter

Comment 16

11 years ago
WFM on trunk.  I can still reproduce bug 387213, though.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → WORKSFORME

Updated

11 years ago
Flags: in-testsuite?
Reporter

Updated

11 years ago
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.