When accessing line boxes, use RenderBlockFlow instead of RenderBlock.

When accessing line boxes, use RenderBlockFlow instead of RenderBlock.

BUG=393945, 302024
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=178516

[jbroman, 2014-07-15 13:36:09]

Only a RenderBlockFlow can actually have line boxes. Many call sites of
RenderBlock's remaining line box methods already know that they have a
RenderBlockFlow, so it's better for them to use that type.

Hopefully some of these can be moved down out of RenderBlock in the future.

[Julien - ping for review, 2014-07-15 23:23:22]

> Only a RenderBlockFlow can actually have line boxes. Many call sites of
RenderBlock's remaining line box methods already know that they have a
RenderBlockFlow, so it's better for them to use that type.
> 
> Hopefully some of these can be moved down out of RenderBlock in the future.

The change is fine but I would rather see us move firstLineBox() / lastLineBox()
and associated code until RenderBlockFlow (Levi says this has been forgotten).
That would be a better change as it will prevent people from adding more calls.

[jbroman, 2014-07-15 23:26:03]

On 2014/07/15 23:23:22, Julien Chaffraix - PST wrote:
> > Only a RenderBlockFlow can actually have line boxes. Many call sites of
> RenderBlock's remaining line box methods already know that they have a
> RenderBlockFlow, so it's better for them to use that type.
> > 
> > Hopefully some of these can be moved down out of RenderBlock in the future.
> 
> The change is fine but I would rather see us move firstLineBox() /
lastLineBox()
> and associated code until RenderBlockFlow (Levi says this has been forgotten).
> That would be a better change as it will prevent people from adding more
calls.

I agree that it would be nice to move those. They have a lot of existing calls
that have to be cleaned up before that is possible; these are some of them.

[jbroman, 2014-07-18 14:13:03]

Alright; how does this look? I've made some of the methods in question protected
in RenderBlock (but public in RenderBlockFlow) so that other classes have to
call via a RenderBlockFlow (but the call sites within RenderBlock itself can be
moved later on).

[jbroman, 2014-07-18 14:14:10]

On 2014/07/18 14:13:03, jbroman wrote:
> Alright; how does this look? I've made some of the methods in question
protected
> in RenderBlock (but public in RenderBlockFlow) so that other classes have to
> call via a RenderBlockFlow (but the call sites within RenderBlock itself can
be
> moved later on).

Err, this managed to drop the previous changes. Please hold.

[jbroman, 2014-07-18 14:17:33]

On 2014/07/18 14:14:10, jbroman wrote:
> On 2014/07/18 14:13:03, jbroman wrote:
> > Alright; how does this look? I've made some of the methods in question
> protected
> > in RenderBlock (but public in RenderBlockFlow) so that other classes have to
> > call via a RenderBlockFlow (but the call sites within RenderBlock itself can
> be
> > moved later on).
> 
> Err, this managed to drop the previous changes. Please hold.

Alright; PTAL. I think I managed to accidentally merge the previous changes in
another CL, so this CL now *only* changes the access modifier on these methods.

[Julien - ping for review, 2014-07-18 21:10:50]

That's a good trade-off (unfortunately won't prevent abuse in RenderBlock but I
assume you will be following up), lgtm.

https://codereview.chromium.org/390173003/diff/60001/Source/core/rendering/Re...
File Source/core/rendering/RenderBlockFlow.h (right):

https://codereview.chromium.org/390173003/diff/60001/Source/core/rendering/Re...
Source/core/rendering/RenderBlockFlow.h:89: // FIXME-BLOCKFLOW: Move this into
RenderBlockFlow once there are no calls
I think we should tie that to a bug if we have one.

[jbroman, 2014-07-19 13:52:46]

The CQ bit was checked by jbroman@chromium.org

[commit-bot: I haz the power, 2014-07-19 13:53:21]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jbroman@chromium.org/390173003/80001

[commit-bot: I haz the power, 2014-07-19 14:52:33]

Change committed as 178516