[LayoutNG] Add NGLineBoxFragment

[LayoutNG] Add NGLineBoxFragment

This patch adds NGLineBoxFragment, along with:
- NGPhysicalLineBoxFragment
- NGLineBoxFragmentBuilder
- NGTextFragmentBuilder for consistency

Part of code in NGLineBuilder was extracted to new classes, in
preparation of renaming it to NGLineLayoutAlgorithm.

NGFragment::Overflow was moved to NGBoxFragment, since neither text nor
linebox has layout overflow. All types of fragments have ink overflow,
but this is not included in this CL.

Some of NGFragmentBuilders could be shared as super classes. This is not
in this CL but to be discussed further.

This patch moves one step towards computing baseline position from
NGFragment, but its implementation is not included in this CL.

BUG=636993
Review-Url: https://codereview.chromium.org/2764753007
Cr-Commit-Position: refs/heads/master@{#459446}
Committed: https://chromium.googlesource.com/chromium/src/+/9872e64c2d620fe4c01d7fd1e3776954b162fc9b

[kojii, 2017-03-22 14:36:48]

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

[commit-bot: I haz the power, 2017-03-22 14:37:01]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[kojii, 2017-03-22 14:48:37]

Description was changed from

==========
WIP: nglineboxfragment

BUG=
==========

to

==========
[LayoutNG] Add NGLineboxFragment

This patch adds NGLineboxFragment, along with:
- NGPhysicalLineboxFragment
- NGLineboxFragmentBuilder
- NGTextFragmentBuilder for consistency

Part of code in NGLineBuilder was extracted to new classes, in
preparation of renaming it to NGLineLayoutAlgorithm.

NGFragment::Overflow was moved to NGBoxFragment, since neither text nor
linebox has layout overflow. All types of fragments have ink overflow,
but this is not included in this CL.

Some of NGFragmentBuilders could be shared as super classes. This is not
in this CL but to be discussed further.

This patch moves one step towards computing baseline position from
NGFragment, but its implementation is not included in this CL.

BUG=636993
==========

[kojii, 2017-03-22 14:54:35]

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

[kojii, 2017-03-22 14:54:36]

Ian, Emil, PTAL. I hope I understand Ian's comment in the other CL correctly,
please point out if any.

Sorry this gets a bit long, but mostly cleanup, moving around, and new files.
Logic do not change.

[commit-bot: I haz the power, 2017-03-22 16:13:57]

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

[commit-bot: I haz the power, 2017-03-22 16:13:58]

Dry run: This issue passed the CQ dry run.

[ikilpatrick, 2017-03-23 16:47:26]

this looks really good - should it be line_box however?

https://codereview.chromium.org/2764753007/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/ng/ng_line_builder.cc (right):

https://codereview.chromium.org/2764753007/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/ng_line_builder.cc:268:
NGLineboxFragmentBuilder linebox(inline_box_);
This is going to be incredibly annoying but shouldn't this be:

NGLineBoxFragmentBuilder & line_box ?

i.e. line box is two sep words?

(and elsewhere obviously).

:P

[eae, 2017-03-23 17:15:20]

This is great, LGTM

[kojii, 2017-03-23 17:53:35]

Description was changed from

==========
[LayoutNG] Add NGLineboxFragment

This patch adds NGLineboxFragment, along with:
- NGPhysicalLineboxFragment
- NGLineboxFragmentBuilder
- NGTextFragmentBuilder for consistency

Part of code in NGLineBuilder was extracted to new classes, in
preparation of renaming it to NGLineLayoutAlgorithm.

NGFragment::Overflow was moved to NGBoxFragment, since neither text nor
linebox has layout overflow. All types of fragments have ink overflow,
but this is not included in this CL.

Some of NGFragmentBuilders could be shared as super classes. This is not
in this CL but to be discussed further.

This patch moves one step towards computing baseline position from
NGFragment, but its implementation is not included in this CL.

BUG=636993
==========

to

==========
[LayoutNG] Add NGLineBoxFragment

This patch adds NGLineBoxFragment, along with:
- NGPhysicalLineBoxFragment
- NGLineBoxFragmentBuilder
- NGTextFragmentBuilder for consistency

Part of code in NGLineBuilder was extracted to new classes, in
preparation of renaming it to NGLineLayoutAlgorithm.

NGFragment::Overflow was moved to NGBoxFragment, since neither text nor
linebox has layout overflow. All types of fragments have ink overflow,
but this is not included in this CL.

Some of NGFragmentBuilders could be shared as super classes. This is not
in this CL but to be discussed further.

This patch moves one step towards computing baseline position from
NGFragment, but its implementation is not included in this CL.

BUG=636993
==========

[kojii, 2017-03-23 17:55:24]

Thank you both for prompt review on this long CL.

On 2017/03/23 at 16:47:26, ikilpatrick wrote:
> this looks really good - should it be line_box however?

You're right! I was referring old spec where both were used, but recent specs
use two words. I'll rename all of them.

[ikilpatrick, 2017-03-23 17:57:52]

On 2017/03/23 17:55:24, kojii wrote:
> Thank you both for prompt review on this long CL.
> 
> On 2017/03/23 at 16:47:26, ikilpatrick wrote:
> > this looks really good - should it be line_box however?
> 
> You're right! I was referring old spec where both were used, but recent specs
> use two words. I'll rename all of them.

Thanks, sorry :)

[kojii, 2017-03-23 17:59:10]

On 2017/03/23 at 17:57:52, ikilpatrick wrote:
> 
> Thanks, sorry :)

I meant thank you for pointing out, don't be sorry ;)

[kojii, 2017-03-23 18:16:33]

The CQ bit was checked by kojii@chromium.org

[kojii, 2017-03-23 18:16:33]

The patchset sent to the CQ was uploaded after l-g-t-m from eae@chromium.org
Link to the patchset: https://codereview.chromium.org/2764753007/#ps80001
(title: "Line box as two separate words")

[commit-bot: I haz the power, 2017-03-23 18:17:31]

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

[ikilpatrick, 2017-03-23 18:26:50]

lgtm, thanks :)

[commit-bot: I haz the power, 2017-03-23 18:36:27]

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

[commit-bot: I haz the power, 2017-03-23 18:36:28]

Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[kojii, 2017-03-24 02:41:46]

The CQ bit was checked by kojii@chromium.org

[kojii, 2017-03-24 02:41:47]

The patchset sent to the CQ was uploaded after l-g-t-m from
ikilpatrick@chromium.org, eae@chromium.org
Link to the patchset: https://codereview.chromium.org/2764753007/#ps100001
(title: "Resolve merge conflict")

[commit-bot: I haz the power, 2017-03-24 02:42:04]

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-03-24 05:04:13]

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

[commit-bot: I haz the power, 2017-03-24 05:04:14]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[kojii, 2017-03-24 06:25:12]

The CQ bit was checked by kojii@chromium.org

[commit-bot: I haz the power, 2017-03-24 06:25:30]

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-03-24 10:08:01]

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

[commit-bot: I haz the power, 2017-03-24 10:08:03]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[kojii, 2017-03-24 13:11:26]

The CQ bit was checked by kojii@chromium.org

[commit-bot: I haz the power, 2017-03-24 13:11:39]

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-03-24 14:12:12]

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

[commit-bot: I haz the power, 2017-03-24 14:12:15]

Failed to apply patch for
third_party/WebKit/Source/core/layout/ng/ng_text_layout_algorithm_test.cc:
While running git apply --index -p1;
  error: patch failed:
third_party/WebKit/Source/core/layout/ng/ng_text_layout_algorithm_test.cc:65
  error:
third_party/WebKit/Source/core/layout/ng/ng_text_layout_algorithm_test.cc: patch
does not apply

Patch:      
third_party/WebKit/Source/core/layout/ng/ng_text_layout_algorithm_test.cc
Index: third_party/WebKit/Source/core/layout/ng/ng_text_layout_algorithm_test.cc
diff --git
a/third_party/WebKit/Source/core/layout/ng/ng_text_layout_algorithm_test.cc
b/third_party/WebKit/Source/core/layout/ng/ng_text_layout_algorithm_test.cc
index
af6aa3cc09ba16609ec502c45fc22d28bf3e8f8f..d42310e66535fe568a8b5f512b8de46d128bb49d
100644
--- a/third_party/WebKit/Source/core/layout/ng/ng_text_layout_algorithm_test.cc
+++ b/third_party/WebKit/Source/core/layout/ng/ng_text_layout_algorithm_test.cc
@@ -7,6 +7,7 @@
 #include "core/dom/TagCollection.h"
 #include "core/layout/line/InlineTextBox.h"
 #include "core/layout/ng/ng_inline_node.h"
+#include "core/layout/ng/ng_physical_line_box_fragment.h"
 #include "core/layout/ng/ng_physical_text_fragment.h"
 #include "platform/geometry/LayoutPoint.h"
 #include "platform/geometry/LayoutRect.h"
@@ -56,8 +57,15 @@ TEST_F(NGTextLayoutAlgorithmTest,
TextFloatsAroundFloatsBefore) {
       toNGPhysicalBoxFragment(html_fragment->Children()[0].get());
   auto* container_fragment =
       toNGPhysicalBoxFragment(body_fragment->Children()[0].get());
-  auto* text_fragments_wrapper =
+  auto* line_box_fragments_wrapper =
       toNGPhysicalBoxFragment(container_fragment->Children()[0].get());
+  Vector<NGPhysicalTextFragment*> text_fragments;
+  for (const auto& child : line_box_fragments_wrapper->Children()) {
+    auto* line_box = toNGPhysicalLineBoxFragment(child.get());
+    EXPECT_EQ(1u, line_box->Children().size());
+    for (const auto& text : line_box->Children())
+      text_fragments.push_back(toNGPhysicalTextFragment(text.get()));
+  }
 
   LayoutText* layout_text =
       toLayoutText(getLayoutObjectByElementId("text")->slowFirstChild());
@@ -65,10 +73,9 @@ TEST_F(NGTextLayoutAlgorithmTest,
TextFloatsAroundFloatsBefore) {
 
   // TODO(glebl): Should have only 3 text fragments. For some reason we have a
   // left over fragment with text == "dog".
-  ASSERT_EQ(4UL, text_fragments_wrapper->Children().size());
+  ASSERT_EQ(4UL, text_fragments.size());
 
-  auto* text_fragment1 =
-      toNGPhysicalTextFragment(text_fragments_wrapper->Children()[0].get());
+  auto* text_fragment1 = text_fragments[0];
   auto* text_node = text_fragment1->Node();
   // 40 = #left-float1' width 30 + #left-float2 10
   EXPECT_EQ(LayoutUnit(40), text_fragment1->LeftOffset());
@@ -77,8 +84,7 @@ TEST_F(NGTextLayoutAlgorithmTest,
TextFloatsAroundFloatsBefore) {
   InlineTextBox* inline_text_box1 = layout_text->firstTextBox();
   EXPECT_EQ(LayoutUnit(40), inline_text_box1->x());
 
-  auto* text_fragment2 =
-      toNGPhysicalTextFragment(text_fragments_wrapper->Children()[1].get());
+  auto* text_fragment2 = text_fragments[1];
   // 40 = #left-float1' width 30
   EXPECT_EQ(LayoutUnit(30), text_fragment2->LeftOffset());
   EXPECT_EQ("brown fox jumps over",
@@ -87,8 +93,7 @@ TEST_F(NGTextLayoutAlgorithmTest,
TextFloatsAroundFloatsBefore) {
   InlineTextBox* inline_text_box2 = inline_text_box1->nextTextBox();
   EXPECT_EQ(LayoutUnit(30), inline_text_box2->x());
 
-  auto* text_fragment3 =
-      toNGPhysicalTextFragment(text_fragments_wrapper->Children()[2].get());
+  auto* text_fragment3 = text_fragments[2];
   EXPECT_EQ(LayoutUnit(), text_fragment3->LeftOffset());
   EXPECT_EQ("jumps over the lazy dog",
             text_node->Text(text_fragment3->StartOffset(),

[kojii, 2017-03-24 14:28:30]

The CQ bit was checked by kojii@chromium.org

[kojii, 2017-03-24 14:28:30]

The patchset sent to the CQ was uploaded after l-g-t-m from
ikilpatrick@chromium.org, eae@chromium.org
Link to the patchset: https://codereview.chromium.org/2764753007/#ps120001
(title: "Rebase again as other CLs landed faster")

[commit-bot: I haz the power, 2017-03-24 14:28:52]

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-03-24 16:18:28]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1490365710041110,
"parent_rev": "8f57f071dcdc75056365050f11a5e5673cb51d10", "commit_rev":
"9872e64c2d620fe4c01d7fd1e3776954b162fc9b"}

[commit-bot: I haz the power, 2017-03-24 16:19:08]

Description was changed from

==========
[LayoutNG] Add NGLineBoxFragment

This patch adds NGLineBoxFragment, along with:
- NGPhysicalLineBoxFragment
- NGLineBoxFragmentBuilder
- NGTextFragmentBuilder for consistency

Part of code in NGLineBuilder was extracted to new classes, in
preparation of renaming it to NGLineLayoutAlgorithm.

NGFragment::Overflow was moved to NGBoxFragment, since neither text nor
linebox has layout overflow. All types of fragments have ink overflow,
but this is not included in this CL.

Some of NGFragmentBuilders could be shared as super classes. This is not
in this CL but to be discussed further.

This patch moves one step towards computing baseline position from
NGFragment, but its implementation is not included in this CL.

BUG=636993
==========

to

==========
[LayoutNG] Add NGLineBoxFragment

This patch adds NGLineBoxFragment, along with:
- NGPhysicalLineBoxFragment
- NGLineBoxFragmentBuilder
- NGTextFragmentBuilder for consistency

Part of code in NGLineBuilder was extracted to new classes, in
preparation of renaming it to NGLineLayoutAlgorithm.

NGFragment::Overflow was moved to NGBoxFragment, since neither text nor
linebox has layout overflow. All types of fragments have ink overflow,
but this is not included in this CL.

Some of NGFragmentBuilders could be shared as super classes. This is not
in this CL but to be discussed further.

This patch moves one step towards computing baseline position from
NGFragment, but its implementation is not included in this CL.

BUG=636993

Review-Url: https://codereview.chromium.org/2764753007
Cr-Commit-Position: refs/heads/master@{#459446}
Committed:
https://chromium.googlesource.com/chromium/src/+/9872e64c2d620fe4c01d7fd1e377...
==========

[commit-bot: I haz the power, 2017-03-24 16:19:12]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/9872e64c2d620fe4c01d7fd1e377...