[LayoutNG] Fix offset in inline context in NGBoxFragmentPainter

Following CL:780821, this patch fixes paint_offset for inline objects.

In inline context, LayoutBox::Location() is relative to its block
container, while LayoutNG fragment has offset relative to its parent.
Painters for LayoutObject follow the same convention. Boxes in inline
context, such as replaced elements or inline blocks, require
different offset due to this difference.

To simplify this, this patch separates code path for block children
and inline children.

There are still some cases we clip incorrectly, but offset
accumulation looks correct. I will look into the clipping issue in
following patches. Local tests for this change is CL:776334, until
bots are enabled.

Bug: 714962
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I75386dcf5271cc6b9884cf6449ff0f6549c79df9
Reviewed-on: https://chromium-review.googlesource.com/792834
Commit-Queue: Koji Ishii <kojii@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520887}

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

@@ -127,17 +127,19 @@ void CopyFragmentDataToLayoutBoxForInlineChildren(
     const NGPhysicalContainerFragment& container,
     NGPhysicalOffset offset = {}) {
   for (const auto& child : container.Children()) {
-    // If the child is painted by non-NG painters, they need Location() set
-    // correctly.
-    LayoutObject* layout_object = child->GetLayoutObject();
-    if (layout_object && layout_object->IsLayoutReplaced()) {
-      LayoutBox& layout_box = ToLayoutBox(*layout_object);
-      layout_box.SetLocation({child->Offset().left, child->Offset().top});
-    }
-
     if (child->IsContainer()) {
+      NGPhysicalOffset child_offset = offset + child->Offset();
+
+      // Replaced elements and inline blocks need Location() set relative to
+      // their block container.
+      LayoutObject* layout_object = child->GetLayoutObject();
+      if (layout_object && layout_object->IsBox()) {
+        LayoutBox& layout_box = ToLayoutBox(*layout_object);
+        layout_box.SetLocation(child_offset.ToLayoutPoint());
+      }
+
       CopyFragmentDataToLayoutBoxForInlineChildren(
-          ToNGPhysicalContainerFragment(*child), offset + child->Offset());
+          ToNGPhysicalContainerFragment(*child), child_offset);
     }
   }
 }

third_party/WebKit/Source/core/paint/ng/ng_box_fragment_painter.cc

@@ -11,10 +11,12 @@
 #include "core/layout/ng/geometry/ng_box_strut.h"
 #include "core/layout/ng/inline/ng_physical_line_box_fragment.h"
 #include "core/layout/ng/inline/ng_physical_text_fragment.h"
+#include "core/layout/ng/layout_ng_mixin.h"
 #include "core/layout/ng/ng_physical_box_fragment.h"
 #include "core/paint/AdjustPaintOffsetScope.h"
 #include "core/paint/BackgroundImageGeometry.h"
 #include "core/paint/BoxDecorationData.h"
+#include "core/paint/ObjectPainter.h"
 #include "core/paint/PaintInfo.h"
 #include "core/paint/PaintLayer.h"
 #include "core/paint/PaintPhase.h"
@@ -45,7 +47,8 @@ NGBoxFragmentPainter::NGBoxFragmentPainter(const NGPaintFragment& box)
           box.GetLayoutObject()->GeneratingNode(),
           BoxStrutToLayoutRectOutsets(box.PhysicalFragment().BorderWidths()),
           LayoutRectOutsets()),
-      box_fragment_(box) {
+      box_fragment_(box),
+      is_inline_(false) {
   DCHECK(box.PhysicalFragment().IsBox());
 }
 
@@ -53,17 +56,7 @@ void NGBoxFragmentPainter::Paint(const PaintInfo& paint_info,
                                  const LayoutPoint& paint_offset) {
   const NGPhysicalFragment& fragment = box_fragment_.PhysicalFragment();
   const LayoutObject* layout_object = fragment.GetLayoutObject();
-  DCHECK(layout_object && layout_object->IsBoxModelObject());
-  if (!layout_object->IsBox()) {
-    // A box fragment can be either a block box or an inline box.
-    // Use AdjustPaintOffsetScope only for block boxes.
-    DCHECK(layout_object->IsLayoutInline());
-    PaintInfo info(paint_info);
-    PaintWithAdjustedOffset(
-        info, paint_offset + box_fragment_.Offset().ToLayoutPoint());
-    return;
-  }
-
+  DCHECK(layout_object && layout_object->IsBox());
   if (!fragment.IsPlacedByLayoutNG()) {
     // |fragment.Offset()| is valid only when it is placed by LayoutNG parent.
     // Use LayoutBox::Location() if not. crbug.com/788590
@@ -79,6 +72,19 @@ void NGBoxFragmentPainter::Paint(const PaintInfo& paint_info,
                           adjustment.AdjustedPaintOffset());
 }
 
+void NGBoxFragmentPainter::PaintInlineBox(
+    const PaintInfo& paint_info,
+    const LayoutPoint& paint_offset,
+    const LayoutPoint& block_paint_offset) {
+  DCHECK(box_fragment_.GetLayoutObject() &&
+         box_fragment_.GetLayoutObject()->IsLayoutInline());
+  is_inline_ = true;
+  block_paint_offset_ = block_paint_offset;
+  PaintInfo info(paint_info);
+  PaintWithAdjustedOffset(
+      info, paint_offset + box_fragment_.Offset().ToLayoutPoint());
+}
+
 void NGBoxFragmentPainter::PaintWithAdjustedOffset(
     PaintInfo& info,
     const LayoutPoint& paint_offset) {
@@ -226,6 +232,11 @@ void NGBoxFragmentPainter::PaintInlineObject(const PaintInfo& paint_info,
 void NGBoxFragmentPainter::PaintContents(const PaintInfo& paint_info,
                                          const LayoutPoint& paint_offset) {
   PaintInfo descendants_info = paint_info.ForDescendants();
+  if (is_inline_) {
+    PaintInlineChildren(box_fragment_.Children(), descendants_info,
+                        paint_offset);
+    return;
+  }
   PaintChildren(box_fragment_.Children(), descendants_info, paint_offset);
 }
 
@@ -329,6 +340,33 @@ static bool RequiresLegacyFallback(const NGPhysicalFragment& fragment) {
   return layout_object->IsLayoutReplaced();
 }
 
+void NGBoxFragmentPainter::PaintInlineChildBoxUsingLegacyFallback(
+    const NGPhysicalFragment& fragment,
+    const PaintInfo& paint_info,
+    const LayoutPoint& paint_offset) {
+  LayoutObject* layout_object = fragment.GetLayoutObject();
+  DCHECK(layout_object);
+  if (layout_object->IsLayoutNGMixin() &&
+      ToLayoutBlockFlow(layout_object)->PaintFragment()) {
+    // This object will use NGBoxFragmentPainter. NGBoxFragmentPainter expects
+    // |paint_offset| relative to the parent, even when in inline context.
+    layout_object->Paint(paint_info, paint_offset);
+    return;
+  }
+
+  // When in inline context, pre-NG painters expect |paint_offset| of their
+  // block container.
+  if (layout_object->IsAtomicInlineLevel()) {
+    // Pre-NG painters also expect callers to use |PaintAllPhasesAtomically()|
+    // for atomic inlines.
+    ObjectPainter(*layout_object)
+        .PaintAllPhasesAtomically(paint_info, block_paint_offset_);
+    return;
+  }
+
+  layout_object->Paint(paint_info, block_paint_offset_);
+}
+
 void NGBoxFragmentPainter::PaintAllPhasesAtomically(
     const PaintInfo& paint_info,
     const LayoutPoint& paint_offset) {
@@ -371,8 +409,28 @@ void NGBoxFragmentPainter::PaintChildren(
         NGBoxFragmentPainter(*child).Paint(paint_info, paint_offset);
     } else if (fragment.Type() == NGPhysicalFragment::kFragmentLineBox) {
       PaintLineBox(*child, paint_info, paint_offset);
-    } else if (fragment.Type() == NGPhysicalFragment::kFragmentText) {
+    }
+  }
+}
+
+void NGBoxFragmentPainter::PaintInlineChildren(
+    const Vector<std::unique_ptr<NGPaintFragment>>& children,
+    const PaintInfo& paint_info,
+    const LayoutPoint& paint_offset) {
+  for (const auto& child : children) {
+    const NGPhysicalFragment& fragment = child->PhysicalFragment();
+    if (fragment.Type() == NGPhysicalFragment::kFragmentText) {
       PaintText(*child, paint_info, paint_offset);
+    } else if (fragment.Type() == NGPhysicalFragment::kFragmentBox) {
+      if (child->HasSelfPaintingLayer())
+        continue;
+      if (RequiresLegacyFallback(fragment)) {
+        PaintInlineChildBoxUsingLegacyFallback(fragment, paint_info,
+                                               paint_offset);
+      } else {
+        NGBoxFragmentPainter(*child).PaintInlineBox(paint_info, paint_offset,
+                                                    block_paint_offset_);
+      }
     }
   }
 }
@@ -390,8 +448,10 @@ void NGBoxFragmentPainter::PaintLineBox(
 
   // Line box fragments don't have LayoutObject and nothing to paint. Accumulate
   // its offset and paint children.
-  PaintChildren(line_box_fragment.Children(), paint_info,
-                paint_offset + line_box_fragment.Offset().ToLayoutPoint());
+  block_paint_offset_ = paint_offset;
+  PaintInlineChildren(
+      line_box_fragment.Children(), paint_info,
+      paint_offset + line_box_fragment.Offset().ToLayoutPoint());
 }
 
 void NGBoxFragmentPainter::PaintInlineBlock(const PaintInfo& paint_info,

third_party/WebKit/Source/core/paint/ng/ng_box_fragment_painter.h

@@ -8,6 +8,7 @@
 #include "core/layout/BackgroundBleedAvoidance.h"
 #include "core/layout/api/HitTestAction.h"
 #include "core/paint/BoxPainterBase.h"
+#include "platform/geometry/LayoutPoint.h"
 #include "platform/geometry/LayoutSize.h"
 #include "platform/wtf/Allocator.h"
 
@@ -62,6 +63,9 @@ class NGBoxFragmentPainter : public BoxPainterBase {
       const PaintInfo&);
   bool IntersectsPaintRect(const PaintInfo&, const LayoutPoint&) const;
 
+  void PaintInlineBox(const PaintInfo&,
+                      const LayoutPoint&,
+                      const LayoutPoint& block_paint_offset);
   void PaintWithAdjustedOffset(PaintInfo&, const LayoutPoint&);
   void PaintBoxDecorationBackground(const PaintInfo&, const LayoutPoint&);
   void PaintBoxDecorationBackgroundWithRect(const PaintInfo&,
@@ -71,6 +75,12 @@ class NGBoxFragmentPainter : public BoxPainterBase {
   void PaintChildren(const Vector<std::unique_ptr<NGPaintFragment>>&,
                      const PaintInfo&,
                      const LayoutPoint&);
+  void PaintInlineChildren(const Vector<std::unique_ptr<NGPaintFragment>>&,
+                           const PaintInfo&,
+                           const LayoutPoint&);
+  void PaintInlineChildBoxUsingLegacyFallback(const NGPhysicalFragment&,
+                                              const PaintInfo&,
+                                              const LayoutPoint&);
   void PaintText(const NGPaintFragment&,
                  const PaintInfo&,
                  const LayoutPoint& paint_offset);
@@ -102,6 +112,12 @@ class NGBoxFragmentPainter : public BoxPainterBase {
                            const LayoutPoint& accumulated_offset);
 
   const NGPaintFragment& box_fragment_;
+
+  // True when this is an inline box.
+  bool is_inline_;
+
+  // The paint offset of the container block when painting inline children.
+  LayoutPoint block_paint_offset_;
 };
 
 }  // namespace blink