[LayoutNG] Cleanup NGBaseline

This is a follow up and minor cleanups after the initial NGBaseline
patch[1].

1. Addresses cbiesinger reviews.
2. NGBaseline has NGBaselineRequest as a member.
3. Some arguments are changed to use struct, not each member.

[1] https://codereview.chromium.org/2867293002/

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

Change-Id: Ib781a08f3b85be6519b70f07413719d0a76db7e7
Reviewed-on: https://chromium-review.googlesource.com/571680
Commit-Queue: Koji Ishii <kojii@chromium.org>
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487006}

third_party/WebKit/Source/core/layout/ng/inline/ng_baseline.cc

@@ -10,6 +10,15 @@
 
 namespace blink {
 
+bool operator==(const NGBaselineRequest& lhs, const NGBaselineRequest& rhs) {
+  return lhs.algorithm_type == rhs.algorithm_type &&
+         lhs.baseline_type == rhs.baseline_type;
+}
+
+bool operator!=(const NGBaselineRequest& lhs, const NGBaselineRequest& rhs) {
+  return !(lhs == rhs);
+}
+
 bool NGBaseline::ShouldPropagateBaselines(const NGLayoutInputNode node) {
   if (node.IsInline())
     return true;

third_party/WebKit/Source/core/layout/ng/inline/ng_baseline.h

@@ -29,10 +29,12 @@ struct NGBaselineRequest {
   FontBaseline baseline_type;
 };
 
+bool operator==(const NGBaselineRequest&, const NGBaselineRequest&);
+bool operator!=(const NGBaselineRequest&, const NGBaselineRequest&);
+
 // Represents a computed baseline position.
 struct NGBaseline {
-  NGBaselineAlgorithmType algorithm_type;
-  FontBaseline baseline_type;
+  NGBaselineRequest request;
   LayoutUnit offset;
 
   // @return if the node needs to propagate baseline requests/results.

third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.cc

@@ -396,21 +396,18 @@ LayoutUnit NGInlineLayoutAlgorithm::ComputeContentSize(
   return content_size;
 }
 
-// Add a baseline from a child line box.
+// Add a baseline from a child line box fragment.
 // @return false if the specified child is not a line box.
 bool NGInlineLayoutAlgorithm::AddBaseline(const NGBaselineRequest& request,
-                                          unsigned child_index) {
-  const NGPhysicalFragment* child =
-      container_builder_.Children()[child_index].Get();
+                                          const NGPhysicalFragment* child,
+                                          LayoutUnit child_offset) {
   if (!child->IsLineBox())
     return false;
 
   const NGPhysicalLineBoxFragment* line_box =
       ToNGPhysicalLineBoxFragment(child);
   LayoutUnit offset = line_box->BaselinePosition(request.baseline_type);
-  container_builder_.AddBaseline(
-      request.algorithm_type, request.baseline_type,
-      offset + container_builder_.Offsets()[child_index].block_offset);
+  container_builder_.AddBaseline(request, offset + child_offset);
   return true;
 }
 
@@ -426,13 +423,15 @@ void NGInlineLayoutAlgorithm::PropagateBaselinesFromChildren() {
       case NGBaselineAlgorithmType::kAtomicInline:
       case NGBaselineAlgorithmType::kAtomicInlineForFirstLine:
         for (unsigned i = container_builder_.Children().size(); i--;) {
-          if (AddBaseline(request, i))
+          if (AddBaseline(request, container_builder_.Children()[i].Get(),
+                          container_builder_.Offsets()[i].block_offset))
             break;
         }
         break;
       case NGBaselineAlgorithmType::kFirstLine:
         for (unsigned i = 0; i < container_builder_.Children().size(); i++) {
-          if (AddBaseline(request, i))
+          if (AddBaseline(request, container_builder_.Children()[i].Get(),
+                          container_builder_.Offsets()[i].block_offset))
             break;
         }
         break;

third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.h

@@ -68,7 +68,9 @@ class CORE_EXPORT NGInlineLayoutAlgorithm final
   LayoutUnit ComputeContentSize(const NGLineInfo&, LayoutUnit line_bottom);
 
   void PropagateBaselinesFromChildren();
-  bool AddBaseline(const NGBaselineRequest&, unsigned);
+  bool AddBaseline(const NGBaselineRequest&,
+                   const NGPhysicalFragment*,
+                   LayoutUnit child_offset);
 
   NGInlineLayoutStateStack box_states_;
   LayoutUnit content_size_;

third_party/WebKit/Source/core/layout/ng/inline/ng_line_breaker.cc

@@ -366,12 +366,12 @@ NGLineBreaker::LineBreakState NGLineBreaker::HandleAtomicInline(
   // would synthesize box-baseline.
   if (NGBaseline::ShouldPropagateBaselines(layout_box)) {
     constraint_space_builder.AddBaselineRequest(
-        line_info.UseFirstLineStyle()
-            ? NGBaselineAlgorithmType::kAtomicInlineForFirstLine
-            : NGBaselineAlgorithmType::kAtomicInline,
-        IsHorizontalWritingMode(constraint_space_->WritingMode())
-            ? FontBaseline::kAlphabeticBaseline
-            : FontBaseline::kIdeographicBaseline);
+        {line_info.UseFirstLineStyle()
+             ? NGBaselineAlgorithmType::kAtomicInlineForFirstLine
+             : NGBaselineAlgorithmType::kAtomicInline,
+         IsHorizontalWritingMode(constraint_space_->WritingMode())
+             ? FontBaseline::kAlphabeticBaseline
+             : FontBaseline::kIdeographicBaseline});
   }
   RefPtr<NGConstraintSpace> constraint_space =
       constraint_space_builder.SetIsNewFormattingContext(true)

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

@@ -871,10 +871,11 @@ RefPtr<NGConstraintSpace> NGBlockLayoutAlgorithm::CreateConstraintSpaceForChild(
       FromPlatformWritingMode(child_style.GetWritingMode()));
 }
 
+// Add a baseline from a child box fragment.
+// @return false if the specified child is not a box or is OOF.
 bool NGBlockLayoutAlgorithm::AddBaseline(const NGBaselineRequest& request,
-                                         unsigned child_index) {
-  const NGPhysicalFragment* child =
-      container_builder_.Children()[child_index].Get();
+                                         const NGPhysicalFragment* child,
+                                         LayoutUnit child_offset) {
   if (!child->IsBox())
     return false;
   LayoutObject* layout_object = child->GetLayoutObject();
@@ -883,10 +884,7 @@ bool NGBlockLayoutAlgorithm::AddBaseline(const NGBaselineRequest& request,
 
   const NGPhysicalBoxFragment* box = ToNGPhysicalBoxFragment(child);
   if (const NGBaseline* baseline = box->Baseline(request)) {
-    container_builder_.AddBaseline(
-        request.algorithm_type, request.baseline_type,
-        baseline->offset +
-            container_builder_.Offsets()[child_index].block_offset);
+    container_builder_.AddBaseline(request, baseline->offset + child_offset);
     return true;
   }
   return false;
@@ -905,13 +903,15 @@ void NGBlockLayoutAlgorithm::PropagateBaselinesFromChildren() {
       case NGBaselineAlgorithmType::kAtomicInline:
       case NGBaselineAlgorithmType::kAtomicInlineForFirstLine:
         for (unsigned i = container_builder_.Children().size(); i--;) {
-          if (AddBaseline(request, i))
+          if (AddBaseline(request, container_builder_.Children()[i].Get(),
+                          container_builder_.Offsets()[i].block_offset))
             break;
         }
         break;
       case NGBaselineAlgorithmType::kFirstLine:
         for (unsigned i = 0; i < container_builder_.Children().size(); i++) {
-          if (AddBaseline(request, i))
+          if (AddBaseline(request, container_builder_.Children()[i].Get(),
+                          container_builder_.Offsets()[i].block_offset))
             break;
         }
         break;

third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.h

@@ -148,7 +148,9 @@ class CORE_EXPORT NGBlockLayoutAlgorithm
   void FinalizeForFragmentation();
 
   void PropagateBaselinesFromChildren();
-  bool AddBaseline(const NGBaselineRequest&, unsigned);
+  bool AddBaseline(const NGBaselineRequest&,
+                   const NGPhysicalFragment*,
+                   LayoutUnit child_offset);
 
   // Calculates logical offset for the current fragment using either
   // {@code content_size_} when the fragment doesn't know it's offset

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

@@ -392,10 +392,8 @@ void NGBlockNode::CopyBaselinesFromOldLayout(
         break;
       case NGBaselineAlgorithmType::kFirstLine: {
         int position = box_->FirstLineBoxBaseline();
-        if (position != -1) {
-          builder->AddBaseline(request.algorithm_type, request.baseline_type,
-                               LayoutUnit(position));
-        }
+        if (position != -1)
+          builder->AddBaseline(request, LayoutUnit(position));
         break;
       }
     }
@@ -425,7 +423,7 @@ void NGBlockNode::AddAtomicInlineBaselineFromOldLayout(
   if (box_->IsAtomicInlineLevel())
     position -= box_->MarginOver();
 
-  builder->AddBaseline(request.algorithm_type, request.baseline_type, position);
+  builder->AddBaseline(request, position);
 }
 
 void NGBlockNode::UseOldOutOfFlowPositioning() {

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

@@ -153,14 +153,17 @@ NGConstraintSpaceBuilder& NGConstraintSpaceBuilder::SetUnpositionedFloats(
 
 void NGConstraintSpaceBuilder::AddBaselineRequests(
     const Vector<NGBaselineRequest>& requests) {
+  DCHECK(baseline_requests_.IsEmpty());
   baseline_requests_.AppendVector(requests);
 }
 
 NGConstraintSpaceBuilder& NGConstraintSpaceBuilder::AddBaselineRequest(
-    NGBaselineAlgorithmType algorithm_type,
-    FontBaseline baseline_type) {
-  baseline_requests_.push_back(
-      NGBaselineRequest{algorithm_type, baseline_type});
+    const NGBaselineRequest& request) {
+  for (const auto& existing : baseline_requests_) {
+    if (existing == request)
+      return *this;
+  }
+  baseline_requests_.push_back(request);
   return *this;
 }
 

third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.h

@@ -61,8 +61,7 @@ class CORE_EXPORT NGConstraintSpaceBuilder final {
       const WTF::Optional<LayoutUnit>& clearance_offset);
 
   void AddBaselineRequests(const Vector<NGBaselineRequest>&);
-  NGConstraintSpaceBuilder& AddBaselineRequest(NGBaselineAlgorithmType,
-                                               FontBaseline);
+  NGConstraintSpaceBuilder& AddBaselineRequest(const NGBaselineRequest&);
 
   // Creates a new constraint space. This may be called multiple times, for
   // example the constraint space will be different for a child which:

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

@@ -187,10 +187,13 @@ NGFragmentBuilder& NGFragmentBuilder::AddOutOfFlowDescendant(
   return *this;
 }
 
-void NGFragmentBuilder::AddBaseline(NGBaselineAlgorithmType algorithm_type,
-                                    FontBaseline baseline_type,
+void NGFragmentBuilder::AddBaseline(NGBaselineRequest request,
                                     LayoutUnit offset) {
-  baselines_.push_back(NGBaseline{algorithm_type, baseline_type, offset});
+#if DCHECK_IS_ON()
+  for (const auto& baseline : baselines_)
+    DCHECK(baseline.request != request);
+#endif
+  baselines_.push_back(NGBaseline{request, offset});
 }
 
 RefPtr<NGLayoutResult> NGFragmentBuilder::ToBoxFragment() {

third_party/WebKit/Source/core/layout/ng/ng_fragment_builder.h

@@ -106,6 +106,8 @@ class CORE_EXPORT NGFragmentBuilder final {
 
   RefPtr<NGLayoutResult> Abort(NGLayoutResult::NGLayoutResultStatus);
 
+  // A vector of child offsets. Initially set by AddChild().
+  const Vector<NGLogicalOffset>& Offsets() const { return offsets_; }
   Vector<NGLogicalOffset>& MutableOffsets() { return offsets_; }
 
   void SwapUnpositionedFloats(
@@ -121,8 +123,6 @@ class CORE_EXPORT NGFragmentBuilder final {
     return children_;
   }
 
-  const Vector<NGLogicalOffset>& Offsets() const { return offsets_; }
-
   bool DidBreak() const { return did_break_; }
 
   NGFragmentBuilder& SetBorderEdges(NGBorderEdges border_edges) {
@@ -130,7 +130,15 @@ class CORE_EXPORT NGFragmentBuilder final {
     return *this;
   }
 
-  void AddBaseline(NGBaselineAlgorithmType, FontBaseline, LayoutUnit);
+  // Layout algorithms should call this function for each baseline request in
+  // the constraint space.
+  //
+  // If a request should use a synthesized baseline from the box rectangle,
+  // algorithms can omit the call.
+  //
+  // This function should be called at most once for a given algorithm/baseline
+  // type pair.
+  void AddBaseline(NGBaselineRequest, LayoutUnit);
 
  private:
   // An out-of-flow positioned-candidate is a temporary data structure used

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

@@ -31,8 +31,7 @@ NGPhysicalBoxFragment::NGPhysicalBoxFragment(
 const NGBaseline* NGPhysicalBoxFragment::Baseline(
     const NGBaselineRequest& request) const {
   for (const auto& baseline : baselines_) {
-    if (baseline.algorithm_type == request.algorithm_type &&
-        baseline.baseline_type == request.baseline_type)
+    if (baseline.request == request)
       return &baseline;
   }
   return nullptr;