[LayoutNG] PODify NGLayoutInputNode and sub-classes.

[LayoutNG] PODify NGLayoutInputNode and sub-classes.

This converts NGLayoutInputNode into a POD-like class.

The interesting parts of this patch are:
 - The lifecycle of the NGInlineNodeData.
 - The manual v-table inside of NGLayoutInputNode.

BUG=635619
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_layout_ng

Review-Url: https://codereview.chromium.org/2921463004
Cr-Commit-Position: refs/heads/master@{#477023}
Committed: https://chromium.googlesource.com/chromium/src/+/1656845883fb809150e98b3eb0748ea1bb663992

[ikilpatrick, 2017-06-01 19:14:46]

Description was changed from

==========
[LayoutNG] PODify NGLayoutInputNode and sub-classes.

This converts NGLayoutInputNode into a POD-like class.

The interesting parts of this patch are:
 - The lifecycle of the NGInlineNodeData.
 - The manual v-table inside of NGLayoutInputNode.

BUG=635619
==========

to

==========
[LayoutNG] PODify NGLayoutInputNode and sub-classes.

This converts NGLayoutInputNode into a POD-like class.

The interesting parts of this patch are:
 - The lifecycle of the NGInlineNodeData.
 - The manual v-table inside of NGLayoutInputNode.

BUG=635619
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_layout_ng
==========

[ikilpatrick, 2017-06-01 19:14:59]

The CQ bit was checked by ikilpatrick@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-01 19:15:57]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ikilpatrick, 2017-06-01 19:23:17]

ikilpatrick@chromium.org changed reviewers:
+ cbiesinger@chromium.org, eae@chromium.org, kojii@chromium.org

[ikilpatrick, 2017-06-01 19:23:19]

There's still some failing tests but will probably just rebase them as its a
pretty complicated and large patch.

[ikilpatrick, 2017-06-01 19:23:35]

The CQ bit was checked by ikilpatrick@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-01 19:24:50]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[eae, 2017-06-01 20:24:41]

LGTM

[commit-bot: I haz the power, 2017-06-01 20:32:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-01 20:32:09]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[cbiesinger, 2017-06-01 20:47:45]

lgtm

https://codereview.chromium.org/2921463004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/ng/inline/ng_inline_node_data.h
(right):

https://codereview.chromium.org/2921463004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/inline/ng_inline_node_data.h:27:
LayoutBox* next_sibling_;
I think it would be good to add a comment here describing how we ensure that
these stay up-to-date (ie. non-dangling)

https://codereview.chromium.org/2921463004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/ng/layout_ng_block_flow.cc (right):

https://codereview.chromium.org/2921463004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/layout_ng_block_flow.cc:27: //
TODO(layout-dev): This should be created in the constructor once instead.
This comment is now not needed anymore, you can delete it.

[cbiesinger, 2017-06-01 21:47:59]

Also I have to mention, these aren't technically PODs due to use of inheritance
and constructors, though I don't think that really matters.

[kojii, 2017-06-02 11:51:41]

lgtm

On 2017/06/01 at 19:23:19, ikilpatrick wrote:
> There's still some failing tests but will probably just rebase them as its a
pretty complicated and large patch.

Agreed, looked at them, they were probably working only because of magic,
marking as failure look reasonable.

[ikilpatrick, 2017-06-02 15:48:05]

The CQ bit was checked by ikilpatrick@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-02 15:48:28]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ikilpatrick, 2017-06-02 15:49:25]

Yeah they were similar to the local failures i get on my mac so wasn't too
worried.

https://codereview.chromium.org/2921463004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/ng/inline/ng_inline_node_data.h
(right):

https://codereview.chromium.org/2921463004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/inline/ng_inline_node_data.h:27:
LayoutBox* next_sibling_;
On 2017/06/01 20:47:44, cbiesinger wrote:
> I think it would be good to add a comment here describing how we ensure that
> these stay up-to-date (ie. non-dangling)

Done.

https://codereview.chromium.org/2921463004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/ng/layout_ng_block_flow.cc (right):

https://codereview.chromium.org/2921463004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/layout_ng_block_flow.cc:27: //
TODO(layout-dev): This should be created in the constructor once instead.
On 2017/06/01 20:47:45, cbiesinger wrote:
> This comment is now not needed anymore, you can delete it.

Done.

[commit-bot: I haz the power, 2017-06-02 22:01:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-02 22:01:46]

Dry run: Try jobs failed on following builders:
  linux_layout_tests_layout_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_layout_te...)

[ikilpatrick, 2017-06-05 13:30:21]

The CQ bit was checked by ikilpatrick@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-05 13:30:30]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-05 15:30:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-05 15:30:26]

Dry run: Try jobs failed on following builders:
  linux_layout_tests_layout_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_layout_te...)

[ikilpatrick, 2017-06-05 15:58:20]

The CQ bit was checked by ikilpatrick@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-05 15:58:32]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ikilpatrick, 2017-06-05 17:11:59]

The CQ bit was unchecked by ikilpatrick@chromium.org

[ikilpatrick, 2017-06-05 17:12:20]

The CQ bit was checked by ikilpatrick@chromium.org

[ikilpatrick, 2017-06-05 17:12:21]

The patchset sent to the CQ was uploaded after l-g-t-m from
cbiesinger@chromium.org, kojii@chromium.org, eae@chromium.org
Link to the patchset: https://codereview.chromium.org/2921463004/#ps80001
(title: "new ng-bot expectations")

[commit-bot: I haz the power, 2017-06-05 17:12:45]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-05 18:00:05]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1496682740689670,
"parent_rev": "4544e34e0534fb77aa77cad8ce76b665dbf229b2", "commit_rev":
"1656845883fb809150e98b3eb0748ea1bb663992"}

[commit-bot: I haz the power, 2017-06-05 18:00:18]

Description was changed from

==========
[LayoutNG] PODify NGLayoutInputNode and sub-classes.

This converts NGLayoutInputNode into a POD-like class.

The interesting parts of this patch are:
 - The lifecycle of the NGInlineNodeData.
 - The manual v-table inside of NGLayoutInputNode.

BUG=635619
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_layout_ng
==========

to

==========
[LayoutNG] PODify NGLayoutInputNode and sub-classes.

This converts NGLayoutInputNode into a POD-like class.

The interesting parts of this patch are:
 - The lifecycle of the NGInlineNodeData.
 - The manual v-table inside of NGLayoutInputNode.

BUG=635619
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_layout_ng

Review-Url: https://codereview.chromium.org/2921463004
Cr-Commit-Position: refs/heads/master@{#477023}
Committed:
https://chromium.googlesource.com/chromium/src/+/1656845883fb809150e98b3eb074...
==========

[commit-bot: I haz the power, 2017-06-05 18:00:19]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/1656845883fb809150e98b3eb074...