Move line/continuation specific parts of willBeDestroyed() into LayoutBlockFlow.

Move line/continuation specific parts of willBeDestroyed() into LayoutBlockFlow.

Also moved beingDestroyed() down to LayoutBlockFlow, since it was no longer
called on other types.

Note that dirtyLinesFromChangedChild() is now called regardless of the object
having line boxes or not at the time of destruction. This should be safer and
more correct. If we're an inline-block, for instance, we definitely want to
notify our parent that we're going away, since that will affect the line box
tree in the parent. I assume that the reason why this hasn't been a problem
(use-after-free crashes, typically), is that the condition that previously
could block this from happening is never true. Looks like line boxes are always
deleted before we reach willBeDestroyed(). Added a TODO to investigate further.
We hopefully don't need that code.

BUG=302024
Committed: https://crrev.com/2266cecd4b7b660f4d4a494c6e61eaa8a2f8dc51
Cr-Commit-Position: refs/heads/master@{#393939}

[mstensho (USE GERRIT), 2016-05-15 21:10:51]

mstensho@opera.com changed reviewers:
+ eae@chromium.org, szager@chromium.org, wkorman@chromium.org

[wkorman, 2016-05-15 21:16:59]

lgtm

[eae, 2016-05-16 16:51:08]

Very nice! LGTM

[eae, 2016-05-16 16:51:13]

The CQ bit was checked by eae@chromium.org

[commit-bot: I haz the power, 2016-05-16 16:51:53]

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

[szager1, 2016-05-16 16:56:40]

https://codereview.chromium.org/1977083002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutBlock.h (left):

https://codereview.chromium.org/1977083002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutBlock.h:127: bool beingDestroyed()
const { return m_beingDestroyed; }
Sorry to jump in here, but I don't like moving this method to LayoutBlockFlow
while the bitfield is still defined on LayoutBlock.  Can we leave it here?

[mstensho (USE GERRIT), 2016-05-16 17:57:27]

https://codereview.chromium.org/1977083002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutBlock.h (left):

https://codereview.chromium.org/1977083002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutBlock.h:127: bool beingDestroyed()
const { return m_beingDestroyed; }
On 2016/05/16 16:56:40, szager1 wrote:
> Sorry to jump in here, but I don't like moving this method to LayoutBlockFlow
> while the bitfield is still defined on LayoutBlock.  Can we leave it here?

I moved it because the flag is now only set by LayoutBlockFlow, so it would
always be false for other types of LayoutBlock - which kind of seemed misleading
and unuseful to me.

Also note that there's nothing unusual about having bitfields in LayoutBlock
that are only used in subclasses, though. Establishing a bit field typically
requires the size of an int, so keeping them all in LayoutBlock makes sense,
when there are so few of them.

See m_hasMarkupTruncation, m_isSelfCollapsing,
m_descendantsWithFloatsMarkedForLayout...

[commit-bot: I haz the power, 2016-05-16 21:35:41]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-05-16 21:37:09]

Description was changed from

==========
Move line/continuation specific parts of willBeDestroyed() into LayoutBlockFlow.

Also moved beingDestroyed() down to LayoutBlockFlow, since it was no longer
called on other types.

Note that dirtyLinesFromChangedChild() is now called regardless of the object
having line boxes or not at the time of destruction. This should be safer and
more correct. If we're an inline-block, for instance, we definitely want to
notify our parent that we're going away, since that will affect the line box
tree in the parent. I assume that the reason why this hasn't been a problem
(use-after-free crashes, typically), is that the condition that previously
could block this from happening is never true. Looks like line boxes are always
deleted before we reach willBeDestroyed(). Added a TODO to investigate further.
We hopefully don't need that code.

BUG=302024
==========

to

==========
Move line/continuation specific parts of willBeDestroyed() into LayoutBlockFlow.

Also moved beingDestroyed() down to LayoutBlockFlow, since it was no longer
called on other types.

Note that dirtyLinesFromChangedChild() is now called regardless of the object
having line boxes or not at the time of destruction. This should be safer and
more correct. If we're an inline-block, for instance, we definitely want to
notify our parent that we're going away, since that will affect the line box
tree in the parent. I assume that the reason why this hasn't been a problem
(use-after-free crashes, typically), is that the condition that previously
could block this from happening is never true. Looks like line boxes are always
deleted before we reach willBeDestroyed(). Added a TODO to investigate further.
We hopefully don't need that code.

BUG=302024

Committed: https://crrev.com/2266cecd4b7b660f4d4a494c6e61eaa8a2f8dc51
Cr-Commit-Position: refs/heads/master@{#393939}
==========

[commit-bot: I haz the power, 2016-05-16 21:37:10]

Patchset 1 (id:??) landed as
https://crrev.com/2266cecd4b7b660f4d4a494c6e61eaa8a2f8dc51
Cr-Commit-Position: refs/heads/master@{#393939}

[mstensho (USE GERRIT), 2016-05-17 21:55:17]

On 2016/05/16 17:57:27, mstensho wrote:
>
https://codereview.chromium.org/1977083002/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/layout/LayoutBlock.h (left):
> 
>
https://codereview.chromium.org/1977083002/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/layout/LayoutBlock.h:127: bool beingDestroyed()
> const { return m_beingDestroyed; }
> On 2016/05/16 16:56:40, szager1 wrote:
> > Sorry to jump in here, but I don't like moving this method to
LayoutBlockFlow
> > while the bitfield is still defined on LayoutBlock.  Can we leave it here?
> 
> I moved it because the flag is now only set by LayoutBlockFlow, so it would
> always be false for other types of LayoutBlock - which kind of seemed
misleading
> and unuseful to me.
> 
> Also note that there's nothing unusual about having bitfields in LayoutBlock
> that are only used in subclasses, though. Establishing a bit field typically
> requires the size of an int, so keeping them all in LayoutBlock makes sense,
> when there are so few of them.
> 
> See m_hasMarkupTruncation, m_isSelfCollapsing,
> m_descendantsWithFloatsMarkedForLayout...

Whoops, this landed. szager: please let me know if you want this part reverted.