Closed Bug 392698 Opened 17 years ago Closed 17 years ago

ASSERTION: can't mark frame dirty during reflow: '!mIsReflowing', file layout/base/nsPresShell.cpp, line 3099 resizing given URL

Categories

(Core :: Layout: Positioned, defect, P3)

x86
macOS
defect

Tracking

()

RESOLVED FIXED

People

(Reporter: Waldo, Assigned: smaug)

References

(Depends on 1 open bug, )

Details

(Keywords: assertion)

Attachments

(4 files)

When I resize the page for the given URL, I get the following assertion gobs of times:

###!!! ASSERTION: can't mark frame dirty during reflow: '!mIsReflowing', file /Users/jwalden/moz/trunk/mozilla/layout/base/nsPresShell.cpp, line 3099

I'll make a minimal testcase sometime tomorrow.
So, it turns out the reduced testcase is dead simple: a div with position:fixed.  Eli says he's not seeing this on Windows, so it may be Mac-specific.

On the off chance it was an out-of-date tree, I rebuilt and tested -- this is present on an hour-old build.
Component: Layout → Layout: R & A Pos
QA Contact: layout → layout.r-and-a-pos
How bad is this assertion?
It basically means that some reflows that need to happen might not.  So the layout might end up wrong.  I don't _think_ that it's a memory-safety issue...
Attached file Stack
The issue seems this code in nsGfxScrollFrameInner::LayoutScrollbars right around the comment

  2478       // XXX Will this work given where we currently are in reflow?

which does:

  2470   if (aOldScrollArea.Size() != aScrollArea.Size() && 
  2471       !(mOuter->GetStateBits() & NS_FRAME_IS_DIRTY) &&
  2472       mIsRoot) {
          ....
  2479       aState.PresContext()->PresShell()->
  2480         FrameNeedsReflow(fixedChild, nsIPresShell::eResize,
  2481                          NS_FRAME_HAS_DIRTY_CHILDREN);

We should either adjust the assertion or move this to a post-reflow callback, or something...
Flags: blocking1.9?
Setting to blockign and P3 based on comment 4 - if you think there should be a different priority please comment
Flags: blocking1.9? → blocking1.9+
Priority: -- → P3
We could move that to a post-reflow callback. This would be easy for Olli...
Assignee: nobody → Olli.Pettay
Attached patch like this?Splinter Review
Attachment #293817 - Flags: review?(roc)
Comment on attachment 293817 [details] [diff] [review]
like this?

+      mIsRoot && mOuter->GetParent()->GetFirstChild(nsGkAtoms::fixedList)) {

This aditional check of GetFirstChild is not needed. Other than that, looks good.
Attachment #293817 - Flags: superreview+
Attachment #293817 - Flags: review?(roc)
Attachment #293817 - Flags: review+
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Depends on: 409406
data:text/html,<script>onload%20=%20function()%20{%20window.resizeTo(500,%20500);%20};%20</script>%20<div%20id="e"%20style="position:fixed"></div>

can automatically reproduce the assertion, but I remember skimming the list of reftests recently and seeing that window resizes somehow break subsequent reftests:

http://mxr.mozilla.org/mozilla/source/layout/reftests/svg/sizing/dynamic--inline-resize-window-height.xhtml

Bug 409406 covers the sizing problem, which probably needs to be fixed before we can have a test for this bug.
Flags: in-testsuite?
Can the assertion be reproduced by resizing an iframe instead of resizing a top-level window?
Attached patch CrashtestSplinter Review
Well-reasoned!  The attachment should work as an automated testcase, although as we don't run with fatal assertions yet it won't actually test anything other than not-crashing for now.  I'll check it in tomorrow morning when I have time to watch the tree and am not half-asleep.
Test checked in, now we just need tinderboxen running it with fatal assertions...
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: