[LayoutNG] Fix 'vertical-align' not to include metrics before alignment

This patch fixes not to include metrics before alignment when
'vertical-align' is one of values to be computed by ancestors; i.e.,
top, bottom, text-top, or text-bottom.

Also undoes a temporary fix introduced in CL[1].

[1] https://codereview.chromium.org/2840883002#msg14

BUG=636993

Review-Url: https://codereview.chromium.org/2852883003
Cr-Commit-Position: refs/heads/master@{#468559}

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

@@ -102,21 +102,22 @@ void NGInlineLayoutStateStack::OnEndPlaceItems(
 
 void NGInlineLayoutStateStack::EndBoxState(NGInlineBoxState* box,
                                            NGLineBoxFragmentBuilder* line_box) {
-  ApplyBaselineShift(box, line_box);
+  PositionPending position_pending = ApplyBaselineShift(box, line_box);
 
   // Unite the metrics to the parent box.
-  if (box != stack_.begin()) {
+  if (position_pending == kPositionNotPending && box != stack_.begin()) {
     box[-1].metrics.Unite(box->metrics);
   }
 }
 
-void NGInlineLayoutStateStack::ApplyBaselineShift(
+NGInlineLayoutStateStack::PositionPending
+NGInlineLayoutStateStack::ApplyBaselineShift(
     NGInlineBoxState* box,
     NGLineBoxFragmentBuilder* line_box) {
   // Compute descendants that depend on the layout size of this box if any.
   LayoutUnit baseline_shift;
   if (!box->pending_descendants.IsEmpty()) {
-    for (const auto& child : box->pending_descendants) {
+    for (auto& child : box->pending_descendants) {
       switch (child.vertical_align) {
         case EVerticalAlign::kTextTop:
         case EVerticalAlign::kTop:
@@ -130,6 +131,8 @@ void NGInlineLayoutStateStack::ApplyBaselineShift(
           NOTREACHED();
           continue;
       }
+      child.metrics.Move(baseline_shift);
+      box->metrics.Unite(child.metrics);
       line_box->MoveChildrenInBlockDirection(
           baseline_shift, child.fragment_start, child.fragment_end);
     }
@@ -139,16 +142,16 @@ void NGInlineLayoutStateStack::ApplyBaselineShift(
   const ComputedStyle& style = *box->style;
   EVerticalAlign vertical_align = style.VerticalAlign();
   if (vertical_align == EVerticalAlign::kBaseline)
-    return;
+    return kPositionNotPending;
 
   // 'vertical-align' aplies only to inline-level elements.
   if (box == stack_.begin())
-    return;
+    return kPositionNotPending;
 
   // Check if there are any fragments to move.
   unsigned fragment_end = line_box->Children().size();
   if (box->fragment_start == fragment_end)
-    return;
+    return kPositionNotPending;
 
   switch (vertical_align) {
     case EVerticalAlign::kSub:
@@ -182,17 +185,18 @@ void NGInlineLayoutStateStack::ApplyBaselineShift(
       // 'top' and 'bottom' require the layout size of the line box.
       stack_.front().pending_descendants.push_back(NGPendingPositions{
           box->fragment_start, fragment_end, box->metrics, vertical_align});
-      return;
+      return kPositionPending;
     default:
       // Other values require the layout size of the parent box.
       SECURITY_CHECK(box != stack_.begin());
       box[-1].pending_descendants.push_back(NGPendingPositions{
           box->fragment_start, fragment_end, box->metrics, vertical_align});
-      return;
+      return kPositionPending;
   }
   box->metrics.Move(baseline_shift);
   line_box->MoveChildrenInBlockDirection(baseline_shift, box->fragment_start,
                                          fragment_end);
+  return kPositionNotPending;
 }
 
 }  // namespace blink

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

@@ -78,13 +78,16 @@ class NGInlineLayoutStateStack {
   // end of a line.
   void EndBoxState(NGInlineBoxState*, NGLineBoxFragmentBuilder*);
 
+  enum PositionPending { kPositionNotPending, kPositionPending };
+
   // Compute vertical position for the 'vertical-align' property.
   // The timing to apply varies by values; some values apply at the layout of
   // the box was computed. Other values apply when the layout of the parent or
   // the line box was computed.
   // https://www.w3.org/TR/CSS22/visudet.html#propdef-vertical-align
   // https://www.w3.org/TR/css-inline-3/#propdef-vertical-align
-  void ApplyBaselineShift(NGInlineBoxState*, NGLineBoxFragmentBuilder*);
+  PositionPending ApplyBaselineShift(NGInlineBoxState*,
+                                     NGLineBoxFragmentBuilder*);
 
   Vector<NGInlineBoxState, 4> stack_;
 };

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

@@ -50,10 +50,6 @@ NGLogicalOffset GetOriginPointForFloats(const NGConstraintSpace& space,
   return origin_point;
 }
 
-inline bool IsObjectReplacementCharacter(UChar character) {
-  return character == kObjectReplacementCharacter;
-}
-
 }  // namespace
 
 NGInlineLayoutAlgorithm::NGInlineLayoutAlgorithm(
@@ -485,11 +481,7 @@ bool NGInlineLayoutAlgorithm::PlaceItems(
   baseline = LayoutUnit(baseline.Round());
 
   // Check if the line fits into the constraint space in block direction.
-  LayoutUnit line_bottom = baseline;
-
-  // See http://crrev.com/2840883002
-  if (!Node()->Text().IsAllSpecialCharacters<IsObjectReplacementCharacter>())
-    line_bottom += line_box.Metrics().descent;
+  LayoutUnit line_bottom = baseline + line_box.Metrics().descent;
 
   if (!container_builder_.Children().IsEmpty() &&
       ConstraintSpace().AvailableSize().block_size != NGSizeIndefinite &&

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

@@ -79,6 +79,31 @@ TEST_F(NGInlineLayoutAlgorithmTest, BreakToken) {
   EXPECT_EQ(0u, wrapper2_break_token->ChildBreakTokens().size());
 }
 
+TEST_F(NGInlineLayoutAlgorithmTest, VerticalAlignBottomReplaced) {
+  SetBodyInnerHTML(R"HTML(
+    <!DOCTYPE html>
+    <style>
+    html { font-size: 10px; }
+    img { vertical-align: bottom; }
+    </style>
+    <div id=container><img src="#" width="96" height="96"></div>
+  )HTML");
+  LayoutNGBlockFlow* block_flow =
+      ToLayoutNGBlockFlow(GetLayoutObjectByElementId("container"));
+  NGInlineNode* inline_node =
+      new NGInlineNode(block_flow->FirstChild(), block_flow);
+  RefPtr<NGConstraintSpace> space =
+      NGConstraintSpace::CreateFromLayoutObject(*block_flow);
+  RefPtr<NGLayoutResult> layout_result = inline_node->Layout(space.Get());
+  auto* wrapper =
+      ToNGPhysicalBoxFragment(layout_result->PhysicalFragment().Get());
+  EXPECT_EQ(1u, wrapper->Children().size());
+  auto* line = ToNGPhysicalLineBoxFragment(wrapper->Children()[0].Get());
+  EXPECT_EQ(LayoutUnit(96), line->Height());
+  auto* img = line->Children()[0].Get();
+  EXPECT_EQ(LayoutUnit(0), img->TopOffset());
+}
+
 // Verifies that text can flow correctly around floats that were positioned
 // before the inline block.
 TEST_F(NGInlineLayoutAlgorithmTest, TextFloatsAroundFloatsBefore) {

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

@@ -37,7 +37,8 @@ class CORE_EXPORT NGInlineNode : public NGLayoutInputNode {
   const ComputedStyle& Style() const override { return block_->StyleRef(); }
   NGLayoutInputNode* NextSibling() override;
 
-  RefPtr<NGLayoutResult> Layout(NGConstraintSpace*, NGBreakToken*) override;
+  RefPtr<NGLayoutResult> Layout(NGConstraintSpace*,
+                                NGBreakToken* = nullptr) override;
   LayoutObject* GetLayoutObject() override;
 
   // Computes the value of min-content and max-content for this anonymous block