[CI] Refactoring BlockPainter and BlockFlowPainter

Previously, BlockPainter's PaintObject method:
 1) was rather uncommented, particularly as it related to its correspondence
    to the W3C "Appendix E" spec for stacking context painting (at
    https://www.w3.org/TR/CSS21/zindex.html).
 2) was a bit convoluted, in the parts that dealt with painting LayoutBlockFlow
    objects. BlockPainter handled painting most parts of painting both
    LayoutBlocks and LayoutBlockFlows, except for in two places. In those
    places, a BlockFlowPainter was constructed and called, and then
    at one point in BlockFlowPainter::PaintContents, a BlockPainter
    was constructed and PaintContents called on that. Functional but confusing.

With this CL, comments are added to link BlockPainter to Appendix E, and
BlockFlowPainter is eliminated. That code is folded back into BlockPainter,
inline where it was called.

Bug: 873387
Change-Id: Idc041eacbcfadd1df1a610f81a2a98de024b7d49
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Reviewed-on: https://chromium-review.googlesource.com/1175226
Commit-Queue: Mason Freed <masonfreed@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585366}

third_party/blink/renderer/core/paint/BUILD.gn

@@ -13,8 +13,6 @@ blink_core_sources("paint") {
     "background_image_geometry.h",
     "block_flow_paint_invalidator.cc",
     "block_flow_paint_invalidator.h",
-    "block_flow_painter.cc",
-    "block_flow_painter.h",
     "block_paint_invalidator.cc",
     "block_paint_invalidator.h",
     "block_painter.cc",

third_party/blink/renderer/core/paint/block_flow_painter.cc

-// Copyright 2014 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "third_party/blink/renderer/core/paint/block_flow_painter.h"
-
-#include "third_party/blink/renderer/core/layout/floating_objects.h"
-#include "third_party/blink/renderer/core/layout/layout_block_flow.h"
-#include "third_party/blink/renderer/core/paint/block_painter.h"
-#include "third_party/blink/renderer/core/paint/line_box_list_painter.h"
-#include "third_party/blink/renderer/core/paint/object_painter.h"
-#include "third_party/blink/renderer/core/paint/paint_info.h"
-
-namespace blink {
-
-void BlockFlowPainter::PaintContents(const PaintInfo& paint_info,
-                                     const LayoutPoint& paint_offset) {
-  if (paint_info.SuppressPaintingDescendants() &&
-      !layout_block_flow_.IsLayoutView()) {
-    return;
-  }
-
-  if (!layout_block_flow_.ChildrenInline()) {
-    BlockPainter(layout_block_flow_).PaintContents(paint_info, paint_offset);
-    return;
-  }
-  if (ShouldPaintDescendantOutlines(paint_info.phase)) {
-    ObjectPainter(layout_block_flow_).PaintInlineChildrenOutlines(paint_info);
-  } else {
-    LineBoxListPainter(layout_block_flow_.LineBoxes())
-        .Paint(layout_block_flow_, paint_info, paint_offset);
-  }
-}
-
-void BlockFlowPainter::PaintFloats(const PaintInfo& paint_info) {
-  if (!layout_block_flow_.GetFloatingObjects())
-    return;
-
-  DCHECK(paint_info.phase == PaintPhase::kFloat ||
-         paint_info.phase == PaintPhase::kSelection ||
-         paint_info.phase == PaintPhase::kTextClip);
-  PaintInfo float_paint_info(paint_info);
-  if (paint_info.phase == PaintPhase::kFloat)
-    float_paint_info.phase = PaintPhase::kForeground;
-
-  for (const auto& floating_object :
-       layout_block_flow_.GetFloatingObjects()->Set()) {
-    if (!floating_object->ShouldPaint())
-      continue;
-
-    const LayoutBox* floating_layout_object =
-        floating_object->GetLayoutObject();
-    // TODO(wangxianzhu): Should this be a DCHECK?
-    if (floating_layout_object->HasSelfPaintingLayer())
-      continue;
-
-    ObjectPainter(*floating_layout_object)
-        .PaintAllPhasesAtomically(float_paint_info);
-  }
-}
-
-}  // namespace blink

third_party/blink/renderer/core/paint/block_flow_painter.h

-// Copyright 2014 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef THIRD_PARTY_BLINK_RENDERER_CORE_PAINT_BLOCK_FLOW_PAINTER_H_
-#define THIRD_PARTY_BLINK_RENDERER_CORE_PAINT_BLOCK_FLOW_PAINTER_H_
-
-#include "third_party/blink/renderer/platform/wtf/allocator.h"
-
-namespace blink {
-
-struct PaintInfo;
-class LayoutBlockFlow;
-class LayoutPoint;
-
-class BlockFlowPainter {
-  STACK_ALLOCATED();
-
- public:
-  BlockFlowPainter(const LayoutBlockFlow& layout_block_flow)
-      : layout_block_flow_(layout_block_flow) {}
-  void PaintContents(const PaintInfo&, const LayoutPoint& paint_offset);
-  void PaintFloats(const PaintInfo&);
-
- private:
-  const LayoutBlockFlow& layout_block_flow_;
-};
-
-}  // namespace blink
-
-#endif  // THIRD_PARTY_BLINK_RENDERER_CORE_PAINT_BLOCK_FLOW_PAINTER_H_

third_party/blink/renderer/core/paint/block_painter.cc

@@ -12,8 +12,8 @@
 #include "third_party/blink/renderer/core/layout/layout_flexible_box.h"
 #include "third_party/blink/renderer/core/layout/layout_inline.h"
 #include "third_party/blink/renderer/core/page/page.h"
-#include "third_party/blink/renderer/core/paint/block_flow_painter.h"
 #include "third_party/blink/renderer/core/paint/box_painter.h"
+#include "third_party/blink/renderer/core/paint/line_box_list_painter.h"
 #include "third_party/blink/renderer/core/paint/object_painter.h"
 #include "third_party/blink/renderer/core/paint/paint_info.h"
 #include "third_party/blink/renderer/core/paint/paint_info_with_offset.h"
@@ -185,15 +185,36 @@ void BlockPainter::RecordHitTestData(const PaintInfo& paint_info,
 DISABLE_CFI_PERF
 void BlockPainter::PaintObject(const PaintInfo& paint_info,
                                const LayoutPoint& paint_offset) {
+  const PaintPhase paint_phase = paint_info.phase;
+  // This function implements some of the painting order algorithm (described
+  // within the description of stacking context, here
+  // https://www.w3.org/TR/css-position-3/#det-stacking-context). References are
+  // made below to the step numbers described in that document.
+
+  // If this block has been truncated, early-out here, because it will not be
+  // displayed. A truncated block occurs when text-overflow: ellipsis is set on
+  // a block, and there is not enough room to display all elements. The elements
+  // that don't get shown are "Truncated".
   if (layout_block_.IsTruncated())
     return;
 
-  const PaintPhase paint_phase = paint_info.phase;
+  // If we're *printing* the foreground, paint the URL.
+  if (paint_phase == PaintPhase::kForeground && paint_info.IsPrinting()) {
+    ObjectPainter(layout_block_)
+        .AddPDFURLRectIfNeeded(paint_info, paint_offset);
+  }
 
+  // If we're painting our background (either 1. kBlockBackground - background
+  // of the current object and non-self-painting descendants, or 2.
+  // kSelfBlockBackgroundOnly -  Paint background of the current object only),
+  // paint those now. This is steps #1, 2, and 4 of the CSS spec (see above).
   if (ShouldPaintSelfBlockBackground(paint_phase)) {
+    // Paint the background if we're visible and this block has a box decoration
+    // (background, border, appearance, or box shadow).
     if (layout_block_.StyleRef().Visibility() == EVisibility::kVisible &&
-        layout_block_.HasBoxDecorationBackground())
+        layout_block_.HasBoxDecorationBackground()) {
       layout_block_.PaintBoxDecorationBackground(paint_info, paint_offset);
+    }
     if (RuntimeEnabledFeatures::PaintTouchActionRectsEnabled())
       RecordHitTestData(paint_info, paint_offset);
     // Record the scroll hit test after the background so background squashing
@@ -201,22 +222,15 @@ void BlockPainter::PaintObject(const PaintInfo& paint_info,
     // immediately before the background.
     if (RuntimeEnabledFeatures::SlimmingPaintV2Enabled())
       PaintScrollHitTestDisplayItem(paint_info);
-    // We're done. We don't bother painting any children.
-    if (paint_phase == PaintPhase::kSelfBlockBackgroundOnly)
-      return;
   }
 
-  if (paint_phase == PaintPhase::kMask &&
-      layout_block_.StyleRef().Visibility() == EVisibility::kVisible) {
-    layout_block_.PaintMask(paint_info, paint_offset);
-    return;
-  }
-
-  if (paint_phase == PaintPhase::kForeground && paint_info.IsPrinting())
-    ObjectPainter(layout_block_)
-        .AddPDFURLRectIfNeeded(paint_info, paint_offset);
-
-  if (paint_phase != PaintPhase::kSelfOutlineOnly) {
+  // If we're in any phase except *just* the self (outline or background) or a
+  // mask, paint children now. This is step #5, 7, 8, and 9 of the CSS spec (see
+  // above).
+  if (paint_phase != PaintPhase::kSelfOutlineOnly &&
+      paint_phase != PaintPhase::kSelfBlockBackgroundOnly &&
+      paint_phase != PaintPhase::kMask) {
+    // Handle scrolling translation.
     base::Optional<ScopedPaintChunkProperties> scoped_scroll_property;
     base::Optional<PaintInfo> scrolled_paint_info;
     if (const auto* fragment = paint_info.FragmentToPaint(layout_block_)) {
@@ -238,30 +252,85 @@ void BlockPainter::PaintObject(const PaintInfo& paint_info,
         }
       }
     }
-
     const PaintInfo& contents_paint_info =
         scrolled_paint_info ? *scrolled_paint_info : paint_info;
 
+    // Actually paint the contents.
     if (layout_block_.IsLayoutBlockFlow()) {
-      BlockFlowPainter block_flow_painter(ToLayoutBlockFlow(layout_block_));
-      block_flow_painter.PaintContents(contents_paint_info, paint_offset);
-      if (paint_phase == PaintPhase::kFloat ||
-          paint_phase == PaintPhase::kSelection ||
-          paint_phase == PaintPhase::kTextClip)
-        block_flow_painter.PaintFloats(contents_paint_info);
+      // All floating descendants will be LayoutBlockFlow objects, and will get
+      // painted here. That is step #5 of the CSS spec (see above).
+      PaintBlockFlowContents(contents_paint_info, paint_offset);
     } else {
       PaintContents(contents_paint_info, paint_offset);
     }
   }
 
+  // If we're painting the outline, paint it now. This is step #10 of the CSS
+  // spec (see above).
   if (ShouldPaintSelfOutline(paint_phase))
     ObjectPainter(layout_block_).PaintOutline(paint_info, paint_offset);
 
+  // If we're painting a visible mask, paint it now. (This does not correspond
+  // to any painting order steps within the CSS spec.)
+  if (paint_phase == PaintPhase::kMask &&
+      layout_block_.StyleRef().Visibility() == EVisibility::kVisible) {
+    layout_block_.PaintMask(paint_info, paint_offset);
+  }
+
   // If the caret's node's layout object's containing block is this block, and
-  // the paint action is PaintPhaseForeground, then paint the caret.
+  // the paint action is PaintPhaseForeground, then paint the caret (cursor or
+  // drag caret). (This does not correspond to any painting order steps within
+  // the CSS spec.)
   if (paint_phase == PaintPhase::kForeground &&
-      layout_block_.ShouldPaintCarets())
+      layout_block_.ShouldPaintCarets()) {
     PaintCarets(paint_info, paint_offset);
+  }
+}
+
+void BlockPainter::PaintBlockFlowContents(const PaintInfo& paint_info,
+                                          const LayoutPoint& paint_offset) {
+  DCHECK(layout_block_.IsLayoutBlockFlow());
+  if (layout_block_.IsLayoutView() ||
+      !paint_info.SuppressPaintingDescendants()) {
+    if (!layout_block_.ChildrenInline()) {
+      PaintContents(paint_info, paint_offset);
+    } else if (ShouldPaintDescendantOutlines(paint_info.phase)) {
+      ObjectPainter(layout_block_).PaintInlineChildrenOutlines(paint_info);
+    } else {
+      LineBoxListPainter(ToLayoutBlockFlow(layout_block_).LineBoxes())
+          .Paint(layout_block_, paint_info, paint_offset);
+    }
+  }
+
+  // If we don't have any floats to paint, or we're in the wrong paint phase,
+  // then we're done for now.
+  auto* floating_objects =
+      ToLayoutBlockFlow(layout_block_).GetFloatingObjects();
+  const PaintPhase paint_phase = paint_info.phase;
+  if (!floating_objects || !(paint_phase == PaintPhase::kFloat ||
+                             paint_phase == PaintPhase::kSelection ||
+                             paint_phase == PaintPhase::kTextClip)) {
+    return;
+  }
+
+  // If we're painting floats (not selections or textclips), change
+  // the paint phase to foreground.
+  PaintInfo float_paint_info(paint_info);
+  if (paint_info.phase == PaintPhase::kFloat)
+    float_paint_info.phase = PaintPhase::kForeground;
+
+  // Paint all floats.
+  for (const auto& floating_object : floating_objects->Set()) {
+    if (!floating_object->ShouldPaint())
+      continue;
+    const LayoutBox* floating_layout_object =
+        floating_object->GetLayoutObject();
+    // TODO(wangxianzhu): Should this be a DCHECK?
+    if (floating_layout_object->HasSelfPaintingLayer())
+      continue;
+    ObjectPainter(*floating_layout_object)
+        .PaintAllPhasesAtomically(float_paint_info);
+  }
 }
 
 void BlockPainter::PaintCarets(const PaintInfo& paint_info,

third_party/blink/renderer/core/paint/block_painter.h

@@ -47,6 +47,7 @@ class BlockPainter {
   // called in the background paint phase even if there is no other painted
   // content.
   void RecordHitTestData(const PaintInfo&, const LayoutPoint& paint_offset);
+  void PaintBlockFlowContents(const PaintInfo&, const LayoutPoint&);
   void PaintCarets(const PaintInfo&, const LayoutPoint& paint_offset);
 
   const LayoutBlock& layout_block_;

third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.h

@@ -531,7 +531,7 @@ class CORE_EXPORT CompositedLayerMapping final : public GraphicsLayerClient {
   //      + overflow_controls_ancestor_clipping_layer_ [OPTIONAL]
   //      | + overflow_controls_host_layer_ [OPTIONAL]
   //      |   + layer_for_vertical_scrollbar_ [OPTIONAL]
-  //      |   + layer_for_vertical_scrollbar_ [OPTIONAL]
+  //      |   + layer_for_horizontal_scrollbar_ [OPTIONAL]
   //      |   + layer_for_scroll_corner_ [OPTIONAL]
   //      + decoration_outline_layer_ [OPTIONAL]
   // The overflow controls may need to be repositioned in the graphics layer

third_party/blink/renderer/core/paint/object_painter.cc

@@ -113,7 +113,7 @@ void ObjectPainter::AddPDFURLRectIfNeeded(const PaintInfo& paint_info,
 }
 
 void ObjectPainter::PaintAllPhasesAtomically(const PaintInfo& paint_info) {
-  // Pass PaintPhaseSelection and PaintPhaseTextClip to the descendants so that
+  // Pass kSelection and kTextClip to the descendants so that
   // they will paint for selection and text clip respectively. We don't need
   // complete painting for these phases.
   if (paint_info.phase == PaintPhase::kSelection ||

third_party/blink/renderer/core/paint/object_painter.h

@@ -39,9 +39,8 @@ class ObjectPainter : public ObjectPainterBase {
   //
   // It is expected that the caller will call this function independent of the
   // value of paintInfo.phase, and this function will do atomic paint (for
-  // PaintPhaseForeground), normal paint (for PaintPhaseSelection and
-  // PaintPhaseTextClip) or nothing (other paint phases) according to
-  // paintInfo.phase.
+  // kForeground), normal paint (for kSelection and kTextClip) or nothing (other
+  // paint phases) according to paintInfo.phase.
   void PaintAllPhasesAtomically(const PaintInfo&);
 
   const LayoutObject& layout_object_;

third_party/blink/renderer/core/paint/paint_phase.h

@@ -49,7 +49,7 @@ enum class PaintPhase {
   kBlockBackground = 0,
   //
   // The following two values are added besides the normal
-  // PaintPhaseBlockBackground to distinguish backgrounds for the object itself
+  // kBlockBackground to distinguish backgrounds for the object itself
   // and for descendants, because the two backgrounds are often painted with
   // different scroll offsets and clips.
   //
@@ -57,8 +57,8 @@ enum class PaintPhase {
   kSelfBlockBackgroundOnly = 1,
   // Paint backgrounds of non-self-painting descendants only. The painter should
   // call each non-self-painting child's paint method by passing
-  // paintInfo.forDescendants() which converts
-  // PaintPhaseDescendantsBlockBackgroundsOnly to PaintPhaseBlockBackground.
+  // paintInfo.forDescendants() which converts kDescendantBlockBackgroundsOnly
+  // to kBlockBackground.
   kDescendantBlockBackgroundsOnly = 2,
 
   // Float phase
@@ -79,8 +79,8 @@ enum class PaintPhase {
   kSelfOutlineOnly = 6,
   // Paint outlines of non-self-painting descendants only. The painter should
   // call each non-self-painting child's paint method by passing
-  // paintInfo.forDescendants() which
-  // converts PaintPhaseDescendantsOutlinesOnly to PaintPhaseBlockOutline.
+  // paintInfo.forDescendants() which converts kDescendantOutlinesOnly to
+  // kOutline.
   kDescendantOutlinesOnly = 7,
 
   // The below are auxiliary phases which are used to paint special effects.