Closed Bug 864289 Opened 7 years ago Closed 7 years ago

rename nsLineLayout::GetLineContainer(Frame|RS) to LineContainer$1

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla23

People

(Reporter: heycam, Assigned: heycam)

Details

Attachments

(1 file)

As Daniel points out in bug 861593 comment 9, nsHTMLReflowState::GetLineContainerFrame() will never return null, so it would be good to indicate this by having the function called LineContainerFrame.
Sorry, that's nsLineLayout::GetLineContainerFrame().
Summary: rename nsHTMLReflowState::GetLineContainerFrame to LineContainerFrame → rename nsLineLayout::GetLineContainerFrame to LineContainerFrame
GetLineContainerRS should be renamed too.
Summary: rename nsLineLayout::GetLineContainerFrame to LineContainerFrame → rename nsLineLayout::GetLineContainer(Frame|RS) to LineContainer$1
Attached patch patchSplinter Review
Assignee: nobody → cam
Status: NEW → ASSIGNED
Attachment #740259 - Flags: review?(dholbert)
Comment on attachment 740259 [details] [diff] [review]
patch

>+++ b/layout/generic/nsLineLayout.cpp
>@@ -78,16 +78,17 @@ nsLineLayout::nsLineLayout(nsPresContext
> {
>   NS_ASSERTION(aFloatManager || aOuterReflowState->frame->GetType() ==
>                                   nsGkAtoms::letterFrame,
>                "float manager should be present");
>+  MOZ_ASSERT(aOuterReflowState, "aOuterReflowState must not be null");
>   MOZ_COUNT_CTOR(nsLineLayout);

Bump the new assertion up to be the first thing we do.  (Right now, it's a bit backwards, because the preexisting NS_ASSERTION dereferences aOuterReflowState, and *then* you assert that aOuterReflowState is non-null).

>@@ -183,34 +184,34 @@ nsLineLayout::BeginLineReflow(nscoord aX
>   // If we're in a constrained height frame, then we don't allow a
>   // max line box width to take effect.
>-  if (!(GetLineContainerFrame()->GetStateBits() &
>+  if (!(LineContainerFrame()->GetStateBits() &
>       NS_FRAME_IN_CONSTRAINED_HEIGHT)) {

Optional: while you're here, add a space before NS_FRAME_IN_CONSTRAINED_HEIGHT to make it line up correctly, parenthesization-wise.

r=me
Attachment #740259 - Flags: review?(dholbert) → review+
OS: Mac OS X → All
Hardware: x86 → All
(In reply to Daniel Holbert [:dholbert] from comment #4)
> Bump the new assertion up to be the first thing we do.  (Right now, it's a
> bit backwards, because the preexisting NS_ASSERTION dereferences
> aOuterReflowState, and *then* you assert that aOuterReflowState is non-null).

Good catch.

https://hg.mozilla.org/integration/mozilla-inbound/rev/32503b49921e
https://hg.mozilla.org/mozilla-central/rev/32503b49921e
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.