Open Bug 386142 Opened 13 years ago Updated 10 years ago

fantasai's li'l Need More Comments bug

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

REOPENED

People

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

Details

Attachments

(3 files, 1 obsolete file)

Splitting out some comment fixes from bug 379349.
Attached patch patch A (obsolete) — Splinter Review
Assignee: nobody → fantasai.bugs
Status: NEW → ASSIGNED
Attachment #270120 - Flags: review?(roc)
Well, there's a couple things that aren't quite comment/whitespace fixes. One removal of an unused variable, one removal of a duplicated 3 lines of code....

I forgot the context on that, here it is:

  // If we have a next-in-flow, then we're not complete
  // XXXldb This used to be done only for the incremental reflow codepath.
  if (GetNextInFlow()) {
    aStatus = NS_FRAME_NOT_COMPLETE;
  }

  SetHasStyleHeight((NS_UNCONSTRAINEDSIZE != aReflowState.mComputedHeight) &&
                    (aReflowState.mComputedHeight > 0)); 
  
  // just set our width to what was available. The table will calculate the width and not use our value.
  aDesiredSize.width = aReflowState.availableWidth;

  // if we have a nextinflow we are not complete
  if (GetNextInFlow()) {
    aStatus |= NS_FRAME_NOT_COMPLETE;
  }
  aDesiredSize.mOverflowArea.UnionRect(aDesiredSize.mOverflowArea, nsRect(0, 0, aDesiredSize.width,
Attachment #270120 - Attachment is obsolete: true
Attachment #270122 - Flags: review?(roc)
Attachment #270120 - Flags: review?(roc)
> -  NS_ASSERTION(mFrameCount == 0, "Some frame destructors were not called");
> +  NS_ASSERTION(mFrameCount == 0, "Some frame or linebox destructors were not called");

While you're trying to make my assertion message from bug 334514 more accurate, you might be interested in bug 365909 comment 5 and bug 365909 comment 15.
In that case, I suggest
"Some frames or other AllocateFrame'd objects not destroyed"
Checked in.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Comment on attachment 270122 [details] [diff] [review]
patch A (with context)

>Index: layout/base/nsPresShell.cpp
>-  NS_ASSERTION(mFrameCount == 0, "Some frame destructors were not called");
>+  NS_ASSERTION(mFrameCount == 0, "Some frame or linebox destructors were not called");

A bunch of other things are allocated in the frame arena.  It would be clearer not to mention line boxes specifically, but make it more general.
(In reply to comment #7)
> (From update of attachment 270122 [details] [diff] [review])
> >Index: layout/base/nsPresShell.cpp
> >-  NS_ASSERTION(mFrameCount == 0, "Some frame destructors were not called");
> >+  NS_ASSERTION(mFrameCount == 0, "Some frame or linebox destructors were not called");
> 
> A bunch of other things are allocated in the frame arena.  It would be clearer
> not to mention line boxes specifically, but make it more general.

I generalized it to "Some objects allocated with AllocateFrame were not freed" when I checked it in.
Couldn't find anything about an AppendToFlow function, but I did check that nsSplittableFrame::Init sets up both sides of the two-way link.
Attachment #272850 - Flags: superreview?(roc)
Attachment #272850 - Flags: review?(roc)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #272850 - Flags: superreview?(roc)
Attachment #272850 - Flags: superreview+
Attachment #272850 - Flags: review?(roc)
Attachment #272850 - Flags: review+
Keywords: checkin-needed
Whiteboard: checkin needed for patch to nsIFrame.h::Init comments
nsIFrame::Init patch committed.
Keywords: checkin-needed
Whiteboard: checkin needed for patch to nsIFrame.h::Init comments
Attachment #404391 - Flags: review?(chris.double)
Attachment #404391 - Attachment is patch: true
Attachment #404391 - Attachment mime type: application/octet-stream → text/plain
Attachment #404391 - Flags: review?(chris.double) → review+
carriage returns patch pushed to mozilla-central
  http://hg.mozilla.org/mozilla-central/rev/90d2c483697b
You need to log in before you can comment on or make changes to this bug.