Ensure we remove floats from descendants even if they have become inline

Ensure we remove floats from descendants even if they have become inline

This is a second go at https://codereview.chromium.org/828163002/. My solution
there was wrong. The real problem was that we were failing to remove the float
from the float-lists of all its parents descendants. The reason this has started
happening is because we can now make children inline while leaving their float
lists in place: this was introduced in https://codereview.chromium.org/253313005.

So if we are trying to remove a float from float-lists, inspect its children
even if they are inline.

BUG=448067, 445285
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=189049

[rhogan, 2015-01-19 20:27:45]

robhogan@gmail.com changed reviewers:
+ jchaffraix@chromium.org

[Julien - ping for review, 2015-01-22 14:34:46]

jchaffraix@chromium.org changed reviewers:
+ leviw@chromium.org

[Julien - ping for review, 2015-01-22 14:34:47]

Levi is a way better reviewer for float changes. I didn't find anything crazy
but the change is more complicated than last time.

Also shouldn't we add an extra test for the rollout?

https://codereview.chromium.org/850143002/diff/1/Source/core/rendering/Render...
File Source/core/rendering/RenderBlockFlow.cpp (right):

https://codereview.chromium.org/850143002/diff/1/Source/core/rendering/Render...
Source/core/rendering/RenderBlockFlow.cpp:1829: if (!childrenInline() ||
floatToRemove) {
Does the childrenInline check still make sense?

https://codereview.chromium.org/850143002/diff/1/Source/core/rendering/Render...
File Source/core/rendering/RenderHTMLCanvas.cpp (right):

https://codereview.chromium.org/850143002/diff/1/Source/core/rendering/Render...
Source/core/rendering/RenderHTMLCanvas.cpp:84: CompositingReasons
RenderHTMLCanvas::additionalCompositingReasons() const
What happened to the test from https://codereview.chromium.org/828163002/ ?

[rhogan, 2015-01-27 18:41:29]

On 2015/01/22 at 14:34:47, jchaffraix wrote:
> Levi is a way better reviewer for float changes. I didn't find anything crazy
but the change is more complicated than last time.
> 
> Also shouldn't we add an extra test for the rollout?

No need..

>
https://codereview.chromium.org/850143002/diff/1/Source/core/rendering/Render...
> Source/core/rendering/RenderHTMLCanvas.cpp:84: CompositingReasons
RenderHTMLCanvas::additionalCompositingReasons() const
> What happened to the test from https://codereview.chromium.org/828163002/ ?

I left it in. So it is testing the revised fix.

> 
>
https://codereview.chromium.org/850143002/diff/1/Source/core/rendering/Render...
> File Source/core/rendering/RenderBlockFlow.cpp (right):
> 
>
https://codereview.chromium.org/850143002/diff/1/Source/core/rendering/Render...
> Source/core/rendering/RenderBlockFlow.cpp:1829: if (!childrenInline() ||
floatToRemove) {
> Does the childrenInline check still make sense?

Generally floats will only need to be tracked in more than one renderer if block
flow allows a float in one to intrude into another. Where the children are all
inline this can't happen as the only children in an inline flow that can contain
floats do not let floats from other renderers intrude into them; they create a
new formatting context.

So yes, I believe this check still makes sense as an optimization when we're not
removing a float.

[leviw_travelin_and_unemployed, 2015-01-27 19:08:26]

The CQ bit was checked by leviw@chromium.org

[leviw_travelin_and_unemployed, 2015-01-27 19:08:26]

LGTM.

[commit-bot: I haz the power, 2015-01-27 19:09:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/850143002/1

[commit-bot: I haz the power, 2015-01-27 19:13:07]

Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=189049