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)
Tracking
()
RESOLVED
FIXED
People
(Reporter: Waldo, Assigned: smaug)
References
(Depends on 1 open bug, )
Details
(Keywords: assertion)
Attachments
(4 files)
6.18 KB,
text/plain
|
Details | |
4.25 KB,
patch
|
roc
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
4.17 KB,
patch
|
Details | Diff | Splinter Review | |
1.45 KB,
patch
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•17 years ago
|
||
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
Comment 3•17 years ago
|
||
How bad is this assertion?
Comment 4•17 years ago
|
||
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...
Reporter | ||
Comment 5•17 years ago
|
||
Comment 6•17 years ago
|
||
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?
Comment 7•17 years ago
|
||
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
Assignee | ||
Comment 9•17 years ago
|
||
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+
Assignee | ||
Comment 11•17 years ago
|
||
Assignee | ||
Updated•17 years ago
|
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 12•17 years ago
|
||
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?
Comment 13•17 years ago
|
||
Can the assertion be reproduced by resizing an iframe instead of resizing a top-level window?
Reporter | ||
Comment 14•17 years ago
|
||
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.
Reporter | ||
Comment 15•17 years ago
|
||
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.
Description
•