Use Opportunity Iterator to position text fragments in NGLineBuilder

third_party/WebKit/Source/core/layout/ng/ng_line_builder.cc

Index: third_party/WebKit/Source/core/layout/ng/ng_line_builder.cc
diff --git a/third_party/WebKit/Source/core/layout/ng/ng_line_builder.cc b/third_party/WebKit/Source/core/layout/ng/ng_line_builder.cc
index 40fcb427f5851961060478cd39480f445ed18596..a0789b7aef68e30fe049f8df5999ec588ea0fd20 100644
--- a/third_party/WebKit/Source/core/layout/ng/ng_line_builder.cc
+++ b/third_party/WebKit/Source/core/layout/ng/ng_line_builder.cc
@@ -20,7 +20,7 @@
 namespace blink {
 
 NGLineBuilder::NGLineBuilder(NGInlineNode* inline_box,
-                             const NGConstraintSpace* constraint_space)
+                             NGConstraintSpace* constraint_space)
     : inline_box_(inline_box),
       constraint_space_(constraint_space),
       baseline_type_(constraint_space->WritingMode() ==
@@ -35,7 +35,7 @@ NGLineBuilder::NGLineBuilder(NGInlineNode* inline_box,
 }
 
 bool NGLineBuilder::CanFitOnLine() const {
-  LayoutUnit available_size = constraint_space_->AvailableSize().inline_size;
+  LayoutUnit available_size = current_opportunity_.InlineSize();
   if (available_size == NGSizeIndefinite)
     return true;
   return end_position_ <= available_size;
@@ -189,7 +189,7 @@ void NGLineBuilder::PlaceItems(
 
   NGFragmentBuilder text_builder(NGPhysicalFragment::kFragmentText,
                                  inline_box_);
-  text_builder.SetWritingMode(constraint_space_->WritingMode());
+  text_builder.SetWritingMode(ConstraintSpace().WritingMode());
   line_box_data_list_.grow(line_box_data_list_.size() + 1);
   LineBoxData& line_box_data = line_box_data_list_.back();
 
@@ -238,7 +238,12 @@ void NGLineBuilder::PlaceItems(
         line_item_chunk.index, line_item_chunk.start_offset,
         line_item_chunk.end_offset);
     fragments_.push_back(std::move(text_fragment));
-    offsets_.push_back(NGLogicalOffset(line_box_data.inline_size, top));
+
+    NGLogicalOffset logical_offset(
+        current_opportunity_.InlineStartOffset() -
+            ConstraintSpace().BfcOffset().inline_offset,
+        top);

[kojii, 2017/03/09 03:01:54]

Don't we need to add |line_box_data.inline_size|, or is it included somewhere I
missed?

[Gleb Lanbin, 2017/03/09 22:26:45]

could you explain why you think we need to include it?
Here is how I understand it
1) LayoutOpportunityIterator returns a LayoutOpportunity(NGLogicalRect)
2) NGTextLayoutAlgorithm::LayoutInline tries to fit as much text as possible to
that opportunity and then calls
NGLineBuilder::CreateLineUpToLastBreakOpportunity which place line_item_chunks
that has inline_size. 
3) In order to calculate the logical offset for each line we need to subtract
the current layout opportunity's offset from ConstraintSpace().offset. 

[kojii, 2017/03/10 03:12:43]

Ok if tests pass, I'll need to debug to understand better, and we can fix
anytime later.

When you have multiple elements, or multiple bidi-items, this is where we flow
inline items horizontally, and |line_box_data.inline_size| is the offset from
the left edge. The original comment was that I was not sure either opportunity
or BfcOffset() incorporates it.

+    offsets_.push_back(logical_offset);
     line_box_data.inline_size += line_item_chunk.inline_size;
   }
   DCHECK_EQ(fragments_.size(), offsets_.size());
@@ -330,7 +335,6 @@ void NGLineBuilder::CreateFragments(NGFragmentBuilder* container_builder) {
   }
 
   // TODO(kojii): Check if the line box width should be content or available.
-  // TODO(kojii): Need to take constraint_space into account.
   container_builder->SetInlineSize(max_inline_size_)
       .SetInlineOverflow(max_inline_size_)
       .SetBlockSize(content_size_)
@@ -387,7 +391,7 @@ void NGLineBuilder::CopyFragmentDataToLayoutBlockFlow() {
     BidiRun* run = bidi_runs.firstRun();
     for (auto* physical_fragment : fragments_for_bidi_runs) {
       DCHECK(run);
-      NGTextFragment fragment(constraint_space_->WritingMode(),
+      NGTextFragment fragment(ConstraintSpace().WritingMode(),
                               toNGPhysicalTextFragment(physical_fragment));
       InlineBox* inline_box = run->m_box;
       inline_box->setLogicalWidth(fragment.InlineSize());
@@ -413,4 +417,13 @@ void NGLineBuilder::CopyFragmentDataToLayoutBlockFlow() {
   }
 }
 
+void NGLineBuilder::FindNextLayoutOpportunity() {
+  NGLogicalOffset iter_offset = constraint_space_->BfcOffset();
+  iter_offset.block_offset += content_size_;
+  auto* iter = constraint_space_->LayoutOpportunityIterator(iter_offset);
+  NGLayoutOpportunity opportunity = iter->Next();
+  if (!opportunity.IsEmpty())
+    current_opportunity_ = opportunity;
+}
+
 }  // namespace blink