Closed
Bug 688044
Opened 13 years ago
Closed 13 years ago
ASSERTION: Why is the root in mDirtyRoots already?
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla10
People
(Reporter: jaws, Assigned: MatsPalmgren_bugz)
References
(Blocks 2 open bugs)
Details
(Keywords: assertion, testcase)
Attachments
(6 files)
7.61 KB,
patch
|
Details | Diff | Splinter Review | |
41.26 KB,
patch
|
Details | Diff | Splinter Review | |
7.27 KB,
text/plain
|
Details | |
9.49 KB,
text/html
|
Details | |
2.14 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
12.23 KB,
text/plain
|
Details |
With the changes applied from these two patches, this assertion is hit. To reproduce, apply patch 462117 and then apply patch 666306.
Reporter | ||
Comment 1•13 years ago
|
||
Reporter | ||
Comment 2•13 years ago
|
||
Comment 3•13 years ago
|
||
What's the concrete class of the root in this case?
Assignee | ||
Comment 4•13 years ago
|
||
Here's the stack for when the root frame currently in InitialReflow is added to 'mDirtyRoots'. (It's a ViewportFrame)
Assignee | ||
Comment 5•13 years ago
|
||
http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell.cpp#1951 Maybe InitialReflow should just deal with it?
Comment 6•13 years ago
|
||
Yeah, seems like it....
Assignee | ||
Comment 7•13 years ago
|
||
Maybe we could do the FrameNeedsReflow stuff before the XBL stuff? Then it wouldn't add the root frame twice because FrameNeedsReflow bails out of the ancestor loop on the 'wasDirty' test here: http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell.cpp#2840
Assignee | ||
Comment 8•13 years ago
|
||
Actually, that should just work if the following comment is right... // Note: Because the frame just got created, it has the NS_FRAME_IS_DIRTY but it's not (at that point). The bit is set when the frame is created but it's then removed somewhere during ProcessAttachedQueue()...
Assignee | ||
Comment 9•13 years ago
|
||
ProcessAttachedQueue runs some script that forces reflow of the root frame. I'm assuming this is where the bit is removed...
Assignee | ||
Comment 10•13 years ago
|
||
I think we can just skip the FrameNeedsReflow stuff if the bit was cleared. Try server results pending...
Assignee | ||
Comment 11•13 years ago
|
||
Comment on attachment 561394 [details] [diff] [review] fix Green on Try so far; Android still pending (with a long backlog).
Attachment #561394 -
Flags: review?(bzbarsky)
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → matspal
Comment 12•13 years ago
|
||
Hmm. How did that script manage to force reflow of the root frame if the frame already had the dirty bit but wasn't in the reflow root set?
Assignee | ||
Comment 13•13 years ago
|
||
Here's the stack to nsSubDocumentFrame::Reflow that does the PostReflowCallback which is what the earlier stack is processing that leads to the subdocument root frame being reflowed. (the UpdatePositionAndSize ...) There are no checks of frame bits or mDirtyRoots on the UpdatePositionAndSize path, nsViewManager::DoSetWindowDimensions just calls ResizeReflow directly. http://mxr.mozilla.org/mozilla-central/source/view/src/nsViewManager.cpp#261
Comment 14•13 years ago
|
||
Comment on attachment 561394 [details] [diff] [review] fix Ah, ok. This is probably fine, then. That said, this is a pretty scary change; can you land it after the next aurora branchpoint?
Attachment #561394 -
Flags: review?(bzbarsky) → review+
Comment 15•13 years ago
|
||
Try run for 2c6e454541bc is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=2c6e454541bc Results (out of 171 total builds): success: 160 warnings: 7 failure: 4 Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mpalmgren@mozilla.com-2c6e454541bc
Comment 16•13 years ago
|
||
Try run for f2d288a2639a is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=f2d288a2639a Results (out of 171 total builds): success: 148 warnings: 23 Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mpalmgren@mozilla.com-f2d288a2639a
Assignee | ||
Comment 17•13 years ago
|
||
Nothing to worry about in the test failures above... the first one is for
the reviewed patch, with only known Orange. The second is a different patch.
> this is a pretty scary change; can you land it after the next aurora branchpoint?
OK
Assignee | ||
Comment 18•13 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/2a89973d7656
Whiteboard: [inbound]
Target Milestone: --- → mozilla10
Comment 19•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/2a89973d7656
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
You need to log in
before you can comment on or make changes to this bug.
Description
•