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

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Cameron McCormack (:heycam)]

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.

Comment 1

[Cameron McCormack (:heycam)]

Sorry, that's nsLineLayout::GetLineContainerFrame().

Comment 2

[Cameron McCormack (:heycam)]

GetLineContainerRS should be renamed too.

Comment 3

[Cameron McCormack (:heycam)]

Attachment: patch

Comment 4

[Daniel Holbert [: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

Comment 5

[Cameron McCormack (:heycam)]

(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

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/32503b49921e