Use Opportunity Iterator to position text fragments in NGLineBuilder

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

Index: third_party/WebKit/Source/core/layout/ng/ng_text_layout_algorithm.cc
diff --git a/third_party/WebKit/Source/core/layout/ng/ng_text_layout_algorithm.cc b/third_party/WebKit/Source/core/layout/ng/ng_text_layout_algorithm.cc
index ab37fecc93bdd4b124e68615ef51c9937c414861..5c9366d0b2f2e04e0f3643f42a053c7f7b0773b6 100644
--- a/third_party/WebKit/Source/core/layout/ng/ng_text_layout_algorithm.cc
+++ b/third_party/WebKit/Source/core/layout/ng/ng_text_layout_algorithm.cc
@@ -9,6 +9,7 @@
 #include "core/layout/ng/ng_constraint_space.h"
 #include "core/layout/ng/ng_fragment_builder.h"
 #include "core/layout/ng/ng_inline_node.h"
+#include "core/layout/ng/ng_layout_opportunity_iterator.h"
 #include "core/layout/ng/ng_line_builder.h"
 #include "core/layout/ng/ng_text_fragment.h"
 #include "core/style/ComputedStyle.h"
@@ -44,8 +45,10 @@ void NGTextLayoutAlgorithm::LayoutInline(NGLineBuilder* line_builder) {
   DCHECK(!text_content.isEmpty());
   // TODO(kojii): Give the locale to LazyLineBreakIterator.
   LazyLineBreakIterator line_break_iterator(text_content);
+
   unsigned current_offset = 0;
   line_builder->SetStart(0, current_offset);
+  line_builder->FindNextLayoutOpportunity();

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

It looks more natural to call this at the end of |SetStart|.

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

done. Although I think it adds an additional side effect to SetStart and
CreateLineUpToLastBreakOpportunity and violates the single responsibility
principle.

may be we can to change the line builder API a bit
1) rename SetStart -> Initialize
2) Move the logic starting from line 91-107 to line_builder->Finalize or
line_builder->CreateLine or line_builder->BuildLine
Feel free to ignore this if you already have a solution for this. 

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

Yeah, I was thinking of this, eae@ misunderstood this before, and I'm seeing a
need of "SetStart without initialize". But let's do this later, we'll probably
have large refactoring + renaming sometime soon, and currently eae@ has a WIP to
re-write this line breaker.
Finalize isn't probably appropriate here though, since the call flow would be:
  SetStart w/Initialize
  CreateLine
  (may need to call SetStart without initialize)
  CreateLine
  CreateLine
  CreateFragments <= so maybe this is finalize
but this may change as we support BreakToken.

Thank you for the nice advice anyway.

   const unsigned end_offset = text_content.length();
   while (current_offset < end_offset) {
     // Find the next break opportunity.
@@ -102,6 +105,7 @@ void NGTextLayoutAlgorithm::LayoutInline(NGLineBuilder* line_builder) {
           line_builder->SetBreakOpportunity();
       }
     }
+    line_builder->FindNextLayoutOpportunity();

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

and at the end of |CreateLineUpToLastBreakOpportunity|.

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

Done.

   }
 
   // If inline children ended with items left in the line builder, create a line