[LayoutNG] Skip positioning list-markers on empty line-boxes.

This fixes floats-do-not-fit-on-line.html which was two bugs combined.

1) List-markers shouldn't be positioned next to empty line boxes
   (unless point 2 occurs). This occurs in particular with floats,
   the list marker should follow the text down (which was the
   primary bug here).

2) List-markers should only be positioned next to empty line boxes if
   we are at the "last" line. I believe this includes forced \n breaks.
   We should produce a line box in these circumstances.

This patch also cleans up the "is empty line" logic inside:
NGInlineLayoutAlgorithm. Which was previously inconsistent :).

Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I9bdc76a5cc40bf8c15651916594c44fd2383097c
Bug: 636993
Reviewed-on: https://chromium-review.googlesource.com/1136799Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575425}

third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG

@@ -490,7 +490,6 @@ crbug.com/591099 fast/block/float/negative-margin-on-element-avoiding-floats.htm
 crbug.com/591099 fast/block/float/nopaint-after-layer-destruction.html [ Failure ]
 crbug.com/591099 fast/block/float/nopaint-after-layer-destruction2.html [ Failure ]
 crbug.com/591099 fast/block/float/overlapping-floats-paint-hittest-order-1.html [ Failure ]
-crbug.com/591099 fast/block/line-layout/floats-do-not-fit-on-line.html [ Failure ]
 crbug.com/591099 fast/block/positioning/positioned-child-inside-relative-positioned-anonymous-block.html [ Failure ]
 crbug.com/591099 fast/borders/bidi-002.html [ Failure ]
 crbug.com/859497 fast/borders/bidi-009a.html [ Failure ]

third_party/blink/renderer/core/layout/ng/inline/ng_inline_layout_algorithm.cc

@@ -251,9 +251,10 @@ void NGInlineLayoutAlgorithm::CreateLine(NGLineInfo* line_info,
                           IsLtr(line_info->BaseDirection()) ? 0 : 1, box);
   }
 
-  if (line_box_.IsEmpty() && !container_builder_.UnpositionedListMarker()) {
-    return;  // The line was empty.
-  }
+  // We can return early if we don't have any children (and don't need to
+  // create a line-box for a list marker, etc).
+  if (line_box_.IsEmpty() && line_info->IsEmptyLine())
+    return;
 
   box_states_->OnEndPlaceItems(&line_box_, baseline_type_);
 
@@ -284,12 +285,7 @@ void NGInlineLayoutAlgorithm::CreateLine(NGLineInfo* line_info,
 
   // Handle out-of-flow positioned objects. They need inline offsets for their
   // static positions.
-  if (!PlaceOutOfFlowObjects(*line_info, line_box_metrics) &&
-      !container_builder_.UnpositionedListMarker()) {
-    // If we have out-of-flow objects but nothing else, we don't have line box
-    // metrics nor BFC offset. Exit early.
-    return;
-  }
+  PlaceOutOfFlowObjects(*line_info, line_box_metrics);
 
   // Even if we have something in-flow, it may just be empty items that
   // shouldn't trigger creation of a line. Exit now if that's the case.
@@ -438,10 +434,9 @@ void NGInlineLayoutAlgorithm::PlaceLayoutResult(NGInlineItemResult* item_result,
 
 // Place all out-of-flow objects in |line_box_| and clear them.
 // @return whether |line_box_| has any in-flow fragments.
-bool NGInlineLayoutAlgorithm::PlaceOutOfFlowObjects(
+void NGInlineLayoutAlgorithm::PlaceOutOfFlowObjects(
     const NGLineInfo& line_info,
     const NGLineHeightMetrics& line_box_metrics) {
-  bool has_fragments = false;
   for (NGLineBoxFragmentBuilder::Child& child : line_box_) {
     if (LayoutObject* box = child.out_of_flow_positioned_box) {
       // The static position is at the line-top. Ignore the block_offset.
@@ -471,11 +466,8 @@ bool NGInlineLayoutAlgorithm::PlaceOutOfFlowObjects(
 
       child.out_of_flow_positioned_box = child.out_of_flow_containing_box =
           nullptr;
-    } else if (!has_fragments) {
-      has_fragments = child.HasFragment();
     }
   }
-  return has_fragments;
 }
 
 // Place a list marker.

third_party/blink/renderer/core/layout/ng/inline/ng_inline_layout_algorithm.h

@@ -72,7 +72,7 @@ class CORE_EXPORT NGInlineLayoutAlgorithm final
   void PlaceLayoutResult(NGInlineItemResult*,
                          NGInlineBoxState*,
                          LayoutUnit inline_offset = LayoutUnit());
-  bool PlaceOutOfFlowObjects(const NGLineInfo&, const NGLineHeightMetrics&);
+  void PlaceOutOfFlowObjects(const NGLineInfo&, const NGLineHeightMetrics&);
   void PlaceListMarker(const NGInlineItem&,
                        NGInlineItemResult*,
                        const NGLineInfo&);

third_party/blink/renderer/core/layout/ng/inline/ng_line_breaker.cc

@@ -202,9 +202,16 @@ void NGLineBreaker::NextLine(const NGLineLayoutOpportunity& line_opportunity,
     result.CheckConsistency();
 #endif
 
+  // We should create a line-box when:
+  //  - We have an item which needs a line box (text, etc).
+  //  - A list-marker is present, and it would be the last line or last line
+  //    before a forced new-line.
+  //  - During min/max content sizing (to correctly determine the line width).
+  //
   // TODO(kojii): There are cases where we need to PlaceItems() without creating
   // line boxes. These cases need to be reviewed.
   if (ShouldCreateLineBox(line_info->Results()) ||
+      (has_list_marker_ && line_info->IsLastLine()) ||
       mode_ != NGLineBreakerMode::kContent)
     ComputeLineLocation(line_info);
   else
@@ -284,7 +291,7 @@ void NGLineBreaker::BreakLine(NGLineInfo* line_info) {
       MoveToNextOf(item);
     } else if (item.Type() == NGInlineItem::kListMarker) {
       NGInlineItemResult* item_result = AddItem(item, item_results);
-      item_result->should_create_line_box = true;
+      has_list_marker_ = true;
       DCHECK(!item_result->can_break_after);
       MoveToNextOf(item);
     } else {

third_party/blink/renderer/core/layout/ng/inline/ng_line_breaker.h

@@ -221,6 +221,9 @@ class CORE_EXPORT NGLineBreaker {
   // https://quirks.spec.whatwg.org/#the-line-height-calculation-quirk
   bool in_line_height_quirks_mode_ = false;
 
+  // True when the line we are breaking has a list marker.
+  bool has_list_marker_ = false;
+
   bool ignore_floats_ = false;
 };
 

third_party/blink/renderer/core/layout/ng/list/ng_unpositioned_list_marker.cc

@@ -64,7 +64,16 @@ bool NGUnpositionedListMarker::AddToBox(
   // Compute the baseline of the child content.
   NGLineHeightMetrics content_metrics;
   if (content.IsLineBox()) {
-    content_metrics = ToNGPhysicalLineBoxFragment(content).Metrics();
+    const NGPhysicalLineBoxFragment& line_box =
+        ToNGPhysicalLineBoxFragment(content);
+
+    // If this child is an empty line-box, the list marker should be aligned
+    // with the next non-empty line box produced. (This can occur with floats
+    // producing empty line-boxes).
+    if (line_box.Children().IsEmpty() && !line_box.BreakToken()->IsFinished())
+      return false;
+
+    content_metrics = line_box.Metrics();
   } else {
     NGBoxFragment content_fragment(space.GetWritingMode(),
                                    ToNGPhysicalBoxFragment(content));