Don't pass semi-detached renderers to the flow thread (or anyone else).

Don't pass semi-detached renderers to the flow thread (or anyone else).

Clean up RenderBlock::removeLeftoverAnonymousBlock(), and make sure that it
doesn't pass weird semi-detached renderers to anyone.

Promote the children of the leftover block (like before), then keep the
leftover block right in front of them (this is new; although it actually used
to be exactly there, it used to be semi-detached, confusing anyone who wanted
to traverse the tree), then notify the flow thread (and the grid code, that
doesn't seem to care either way), then manually detach the leftover block (like
before, only that it's a one-liner now) and finally destroy it.

The leftover block used to be partially detached in the sense that it had its
nextSibling reset, but still had its parent and previousSibling left intact.
Missing a nextSibling could make the flow thread code incorrectly believe that
we were removing the last piece of multicol content, and thus trigger a
deletion of a column set. This would in turn trigger an assertion failure when
attempting to insert more content later on, because the flow thread wouldn't
find any suitable column set.

BUG=457461
R=dsinclair@chromium.org,jchaffraix@chromium.org

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=191096

[dsinclair, 2015-02-17 18:25:02]

lgtm with nit. Please give jchaffraix@ a chance to chime in before landing.

https://codereview.chromium.org/922173002/diff/20001/Source/core/rendering/Re...
File Source/core/rendering/RenderBlock.cpp (right):

https://codereview.chromium.org/922173002/diff/20001/Source/core/rendering/Re...
Source/core/rendering/RenderBlock.cpp:1043: child->children()->setLastChild(0);
nit: change the 0's to nullptrs

[mstensho (USE GERRIT), 2015-02-17 18:28:57]

I hadn't published this review yet (wanted to write something about other
solutions), but fine. Thanks for reviewing! :)

[dsinclair, 2015-02-17 18:30:37]

On 2015/02/17 at 18:28:57, mstensho wrote:
> I hadn't published this review yet (wanted to write something about other
solutions), but fine. Thanks for reviewing! :)

Hah, sorry. It showed up in my codereview list and I just assumed I'd missed the
email.

dan

[mstensho (USE GERRIT), 2015-02-18 20:01:48]

New patchsets have been uploaded after l-g-t-m from dsinclair@chromium.org

[mstensho (USE GERRIT), 2015-02-18 20:02:26]

@jchaffraix - do you have anything to add here?

https://codereview.chromium.org/922173002/diff/20001/Source/core/rendering/Re...
File Source/core/rendering/RenderBlock.cpp (right):

https://codereview.chromium.org/922173002/diff/20001/Source/core/rendering/Re...
Source/core/rendering/RenderBlock.cpp:1043: child->children()->setLastChild(0);
On 2015/02/17 18:25:01, dsinclair wrote:
> nit: change the 0's to nullptrs

Done.

[Julien - ping for review, 2015-02-26 18:29:47]

lgtm too!

> @jchaffraix - do you have anything to add here?

I missed that you were waiting on me on this one. As a rule, you should assume
that if I don't comment in a timely manner, it's fine to just commit.

https://codereview.chromium.org/922173002/diff/40001/Source/core/rendering/Re...
File Source/core/rendering/RenderBlock.cpp (right):

https://codereview.chromium.org/922173002/diff/40001/Source/core/rendering/Re...
Source/core/rendering/RenderBlock.cpp:1050:
children()->setLastChild(lastPromotee);
This really looks like it could be a helper function. No other part of the code
does this though so it's a nit.

[mstensho (USE GERRIT), 2015-03-02 10:25:01]

The CQ bit was checked by mstensho@opera.com

[mstensho (USE GERRIT), 2015-03-02 10:25:02]

The patchset sent to the CQ was uploaded after l-g-t-m from
dsinclair@chromium.org, jchaffraix@chromium.org
Link to the patchset: https://codereview.chromium.org/922173002/#ps80001 (title:
"Code review - make a helper method.")

[commit-bot: I haz the power, 2015-03-02 10:25:56]

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

[commit-bot: I haz the power, 2015-03-02 11:49:13]

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

[mstensho (USE GERRIT), 2015-03-02 21:59:21]

It appears that I forgot to publish this. Did push the code changes before the
CL landed, though. :)

https://codereview.chromium.org/922173002/diff/40001/Source/core/rendering/Re...
File Source/core/rendering/RenderBlock.cpp (right):

https://codereview.chromium.org/922173002/diff/40001/Source/core/rendering/Re...
Source/core/rendering/RenderBlock.cpp:1050:
children()->setLastChild(lastPromotee);
On 2015/02/26 18:29:47, Julien Chaffraix - PST wrote:
> This really looks like it could be a helper function. No other part of the
code
> does this though so it's a nit.

Done.