Closed Bug 417116 Opened 12 years ago Closed 12 years ago

switch GetOverflowAreaProperty users to GetOverflowRect

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9beta4

People

(Reporter: fantasai.bugs, Assigned: fantasai.bugs)

Details

Attachments

(2 files, 1 obsolete file)

We've got two methods on nsIFrame for getting the overflow rect:
  nsRect GetOverflowRect() const;
and
  nsRect* GetOverflowAreaProperty(PRBool aCreateIfNecessary = PR_FALSE);

Most users of GetOverflowAreaProperty should be using GetOverflowRect, which is easier to use. Consolidating the two will also make it easier to add e.g. ymost data for bug 404215.
Attached patch patch v0 (obsolete) — Splinter Review
Patch, v0. I added an assertion to make sure we don't grab mRect during reflow (to catch callers that use GetOverflowRect after the overflow rect's been set but before mRect's been set), and it's firing in nsBox.cpp with this backtrace:

#1  0xb7cf4abc in NS_DebugBreak_P (aSeverity=1, 
    aStr=0xb541a8a4 "Don't have accurate size data until reflow is finished!", 
    aExpr=0xb541a83c "!(GetStateBits() & NS_FRAME_IN_REFLOW) || (PresContext()->LayoutPhaseCount(eLayoutPhase_Reflow) == 0)", 
    aFile=0xb54193bc "/home/fantasai/moz/mozilla/layout/generic/nsFrame.cpp", aLine=3667)
    at /home/fantasai/moz/mozilla/xpcom/base/nsDebugImpl.cpp:358
#2  0xb4c3d477 in nsIFrame::GetOverflowRect 
    at /home/fantasai/moz/mozilla/layout/generic/nsFrame.cpp:3665
#3  0xb4ddefc5 in nsIFrame::Redraw
    at /home/fantasai/moz/mozilla/layout/xul/base/src/nsBox.cpp:657
#4  0xb4ddf06f in nsBox::SyncLayout
    at /home/fantasai/moz/mozilla/layout/xul/base/src/nsBox.cpp:592
#5  0xb4ddf2c8 in nsBox::EndLayout 
    at /home/fantasai/moz/mozilla/layout/xul/base/src/nsBox.cpp:204
#6  0xb4ddf37a in nsIFrame::Layout
#7  0xb4de2ca5 in nsBoxFrame::Reflow
    at /home/fantasai/moz/mozilla/layout/xul/base/src/nsBoxFrame.cpp:756
#8  0xb4ddd529 in nsRootBoxFrame::Reflow
    at /home/fantasai/moz/mozilla/layout/xul/base/src/nsRootBoxFrame.cpp:236

So I'll need to figure out whether there's a bug here or what.
Assignee: nobody → fantasai.bugs
Status: NEW → ASSIGNED
Attached patch path v1Splinter Review
So I concluded that it's not possible to add that assertion without spending a lot of time tweaking xul frames. Maybe we should be setting the overflow rect and mRect's size at the same time so we don't have this problem, but that's out-of-scope here.
Attachment #303470 - Flags: superreview?(dbaron)
Attachment #303470 - Flags: review?(roc)
Comment on attachment 303470 [details] [diff] [review]
path v1

+  if (GetStateBits() & NS_FRAME_OUTSIDE_CHILDREN) {
+    return *const_cast<nsIFrame*>(this)->GetOverflowAreaProperty(PR_FALSE);
+  }
+  else {

Don't do else after return.
Attachment #303470 - Flags: superreview?(dbaron)
Attachment #303470 - Flags: superreview+
Attachment #303470 - Flags: review?(roc)
Attachment #303470 - Flags: review+
Attached patch patch v1.1Splinter Review
dropped else after return. Requesting approval for 1.9, to make it easier to deal with the ymost approach discussed in bug 404215 comment 30.
Attachment #303173 - Attachment is obsolete: true
Attachment #304153 - Flags: approval1.9?
Attachment #304153 - Flags: approval1.9? → approval1.9+
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9beta4
Version: unspecified → Trunk
You need to log in before you can comment on or make changes to this bug.