Implement collection of out-of-flow descendants

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

Index: third_party/WebKit/Source/core/layout/ng/ng_fragment_builder.h
diff --git a/third_party/WebKit/Source/core/layout/ng/ng_fragment_builder.h b/third_party/WebKit/Source/core/layout/ng/ng_fragment_builder.h
index 5b1a1f14a45407523f45b47e291a92e618ff25d8..a54e4b8bf1ecf18b61850195678cc7e6fd53e185 100644
--- a/third_party/WebKit/Source/core/layout/ng/ng_fragment_builder.h
+++ b/third_party/WebKit/Source/core/layout/ng/ng_fragment_builder.h
@@ -28,8 +28,31 @@ class CORE_EXPORT NGFragmentBuilder final
 
   NGFragmentBuilder& AddChild(NGFragmentBase*, NGLogicalOffset);
 
-  NGFragmentBuilder& SetOutOfFlowDescendants(WeakBoxList&,
-                                             Vector<NGLogicalOffset>&);
+  // Builder is not dumb when handling out of flow descendants.

[ikilpatrick, 2016/12/02 17:47:06]

Maybe:

The builder isn't "simple" when handling out of flow (OOF) descendants. It will
collect OOF descendants from:
 - AddChild (will add all descendants from the appended fragment).
 - AddOutOfFlowDescendantCandidate (directs adds a descendant).

[atotic, 2016/12/02 19:55:15]

entire comment revised for clarity

+  // Out of flow descendants API should be used like this:
+  // for (child : children)
+  //   if (child->position == (Absolute or Fixed)

[ikilpatrick, 2016/12/02 17:47:06]

missing closing ')'

[atotic, 2016/12/02 19:55:15]

entire comment revised for clarity

+  //     builder->AddOutOfFlowCandidateChild(child);
+  //   else
+  //     child->Layout()

[ikilpatrick, 2016/12/02 17:47:06]

i'd write:
fragment = child->Layout()

instead. (make clear that not adding input node).

[atotic, 2016/12/02 19:55:16]

done

+  //     builder_->AddChild(child)
+  // end
+  // builder->SetInlineSize/SetBlockSize
+  // while (builder->GetOutOfFlowDescendantCandidates(oof_candidates)

[ikilpatrick, 2016/12/02 17:47:06]

while loop bad?

[atotic, 2016/12/02 19:55:16]

entire comment revised for clarity

+  //    && oof_candidates.size() > 0)
+  // {
+  //   for (candidate : oof_candidates)
+  //     if (CanPosition(candidate)
+  //        candidate->Layout();
+  //        builder_->AddChild();
+  //     else
+  //        builder_->AddOutOfFlowDescendant();
+  // }

[ikilpatrick, 2016/12/02 17:47:06]

There are two cases:
 - The descendant came from a child fragment.
 - The descendant is a direct child of the current fragment.

[atotic, 2016/12/02 19:55:15]

entire comment revised for clarity

+  NGFragmentBuilder& AddOutOfFlowCandidateChild(NGBlockNode*, NGLogicalOffset);

[ikilpatrick, 2016/12/02 17:47:06]

AddOutOfFlowDescendantCandidate for consistency.

[atotic, 2016/12/02 19:55:15]

It can only be a child, so will not rename to Descendant.

Did rename to AddOutOfFlowChildCandidate for consistency with 
AddOutOfFlowDescendant, AddChild

+
+  void GetOutOfFlowDescendantCandidates(WeakBoxList&, Vector<NGCorner>&);
+
+  NGFragmentBuilder& AddOutOfFlowDescendant(NGBlockNode*, const NGCorner&);
 
   // Sets MarginStrut for the resultant fragment.
   NGFragmentBuilder& SetMarginStrutBlockStart(const NGMarginStrut& from);
@@ -44,6 +67,12 @@ class CORE_EXPORT NGFragmentBuilder final
   DECLARE_VIRTUAL_TRACE();
 
  private:
+  // Descendant offset information

[ikilpatrick, 2016/12/02 17:47:06]

Maybe:

Descendant offset information. We need information from two places to calculate
the NGCorner of the descendant correctly.
 - The inner physical offset (child_offset). This is set if the descendant came
from a child fragment. It is the offset within that fragment.
 - The outer logical offset (descendant_corner). This is set in both cases (see
above). It is the offset to the descendant, or the child fragment as
appropriate. We store the outer logical offset here separately as it is
impossible to convert to physical without the fragment size.  

[atotic, 2016/12/02 19:55:16]

Renamed OutOfFlowOffset to OutOfFlowPlacement for clarity, and wrote a lengthy
comment.

+  struct OutOfFlowOffset {
+    NGLogicalOffset child_offset;
+    NGCorner descendant_corner;
+  };
+
   NGPhysicalFragmentBase::NGFragmentType type_;
   NGWritingMode writing_mode_;
   TextDirection direction_;
@@ -55,8 +84,12 @@ class CORE_EXPORT NGFragmentBuilder final
 
   HeapVector<Member<NGPhysicalFragmentBase>> children_;
   Vector<NGLogicalOffset> offsets_;
+
+  WeakBoxList out_of_flow_descendant_candidates_;
+  Vector<OutOfFlowOffset> oof_candidate_offsets_;

[ikilpatrick, 2016/12/02 17:47:06]

s/oof/out_of_flow/

to be consistent with rest of variable naming.

[atotic, 2016/12/02 19:55:15]

Will do.

Note: this is an example where "explicit variable naming" and "80 cols"
guidelines produce less readable code. out_of_flow_of_candidate_placements_ is
34 characters. This leaves only 47 characters left for other code, causing
lots and 
lots of
line 
wrapping

+
   WeakBoxList out_of_flow_descendants_;
-  Vector<NGLogicalOffset> out_of_flow_offsets_;
+  Vector<NGCorner> out_of_flow_corners_;
 };
 
 }  // namespace blink