[LayoutNG] Remove NGPaintFragment selection invalidation logic

Now we are (maybe temporarily) using the containing LayoutObject's
visual rect for NGPaintFragments, and we have included selection
into LayoutText's visual rect (crrev.com/c/1457812), so we can
remove the NGPaintFragment selection invalidation logic to avoid
the extra cost and complexity.

Bug: 636993
Change-Id: Ia87ee6a25b3ad78ddb4e365daaddeddf43bfbb70
Reviewed-on: https://chromium-review.googlesource.com/c/1461184
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Yoichi Osato <yoichio@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#631074}

third_party/blink/renderer/core/layout/layout_object.cc

@@ -1644,15 +1644,6 @@ void LayoutObject::ClearPreviousVisualRects() {
     fragment->SetSelectionVisualRect(LayoutRect());
   }
 
-  if (IsInline()) {
-    auto fragments = NGPaintFragment::InlineFragmentsFor(this);
-    if (fragments.IsInLayoutNGInlineFormattingContext()) {
-      for (auto* fragment : fragments) {
-        fragment->SetSelectionVisualRect(LayoutRect());
-      }
-    }
-  }
-
   // After clearing ("invalidating") the visual rects, mark this object as
   // needing to re-compute them.
   SetShouldDoFullPaintInvalidation();

third_party/blink/renderer/core/layout/layout_object.h

@@ -277,6 +277,10 @@ class CORE_EXPORT LayoutObject : public ImageResourceObserver,
     return AdjustVisualRectForInlineBox(VisualRect());
   }
 
+  LayoutRect PartialInvalidationVisualRectForInlineBox() const {
+    return AdjustVisualRectForInlineBox(PartialInvalidationVisualRect());
+  }
+
   String DebugName() const final;
 
   // End of DisplayItemClient methods.
@@ -2476,9 +2480,6 @@ class CORE_EXPORT LayoutObject : public ImageResourceObserver,
   void ApplyPseudoStyleChanges(const ComputedStyle* old_style);
   void ApplyFirstLineChanges(const ComputedStyle* old_style);
 
-  LayoutRect PartialInvalidationVisualRectForInlineBox() const {
-    return AdjustVisualRectForInlineBox(PartialInvalidationVisualRect());
-  }
   LayoutRect AdjustVisualRectForInlineBox(const LayoutRect&) const;
 
   // This is set by Set[Subtree]ShouldDoFullPaintInvalidation, and cleared

third_party/blink/renderer/core/paint/ng/ng_paint_fragment.cc

@@ -317,18 +317,6 @@ scoped_refptr<NGPaintFragment> NGPaintFragment::Create(
   return paint_fragment;
 }
 
-NGPaintFragment::RareData& NGPaintFragment::EnsureRareData() {
-  if (!rare_data_)
-    rare_data_ = std::make_unique<RareData>();
-  return *rare_data_;
-}
-
-const NGPaintFragment* NGPaintFragment::Next() const {
-  if (!rare_data_)
-    return nullptr;
-  return rare_data_->next_fragmented_.get();
-}
-
 const NGPaintFragment* NGPaintFragment::Last() const {
   for (const NGPaintFragment* fragment = this;;) {
     const NGPaintFragment* next = fragment->Next();
@@ -363,8 +351,7 @@ scoped_refptr<NGPaintFragment>* NGPaintFragment::Find(
     if (!*fragment)
       return fragment;
 
-    scoped_refptr<NGPaintFragment>* next =
-        &(*fragment)->EnsureRareData().next_fragmented_;
+    scoped_refptr<NGPaintFragment>* next = &(*fragment)->next_fragmented_;
     if ((*fragment)->PhysicalFragment().BreakToken() == break_token)
       return next;
     fragment = next;
@@ -373,9 +360,7 @@ scoped_refptr<NGPaintFragment>* NGPaintFragment::Find(
 }
 
 void NGPaintFragment::SetNext(scoped_refptr<NGPaintFragment> fragment) {
-  if (!rare_data_ && !fragment)
-    return;
-  EnsureRareData().next_fragmented_ = std::move(fragment);
+  next_fragmented_ = std::move(fragment);
 }
 
 bool NGPaintFragment::IsDescendantOfNotSelf(
@@ -403,18 +388,6 @@ bool NGPaintFragment::ShouldClipOverflow() const {
          ToNGPhysicalBoxFragment(*physical_fragment_).ShouldClipOverflow();
 }
 
-LayoutRect NGPaintFragment::SelectionVisualRect() const {
-  if (!rare_data_)
-    return LayoutRect();
-  return rare_data_->selection_visual_rect_;
-}
-
-void NGPaintFragment::SetSelectionVisualRect(const LayoutRect& rect) {
-  if (!rare_data_ && rect.IsEmpty())
-    return;
-  EnsureRareData().selection_visual_rect_ = rect;
-}
-
 LayoutRect NGPaintFragment::SelfInkOverflow() const {
   return physical_fragment_->InkOverflow().ToLayoutRect();
 }
@@ -536,9 +509,8 @@ NGPaintFragment* NGPaintFragment::LastForSameLayoutObject() {
   return fragment;
 }
 
-LayoutRect NGPaintFragment::VisualRect() const {
-  // VisualRect is computed from fragment tree and set to LayoutObject in
-  // pre-paint. Use the stored value in the LayoutObject.
+const LayoutObject& NGPaintFragment::VisualRectLayoutObject(
+    bool& this_as_inline_box) const {
   const NGPhysicalFragment& fragment = PhysicalFragment();
   if (const LayoutObject* layout_object = fragment.GetLayoutObject()) {
     // For inline fragments, InlineBox uses one united rect for the LayoutObject
@@ -549,20 +521,36 @@ LayoutRect NGPaintFragment::VisualRect() const {
     // formatting context and another as a child of the inline formatting
     // context it participates. |Parent()| can distinguish them because a tree
     // is created for each inline formatting context.
-    if (Parent())
-      return layout_object->VisualRectForInlineBox();
-
-    return static_cast<const DisplayItemClient*>(layout_object)->VisualRect();
+    this_as_inline_box = Parent();
+    return *layout_object;
   }
 
   // Line box does not have corresponding LayoutObject. Use VisualRect of the
   // containing LayoutBlockFlow as RootInlineBox does so.
+  this_as_inline_box = true;
   DCHECK(fragment.IsLineBox());
   // Line box is always a direct child of its containing block.
   NGPaintFragment* containing_block_fragment = Parent();
   DCHECK(containing_block_fragment);
   DCHECK(containing_block_fragment->GetLayoutObject());
-  return containing_block_fragment->GetLayoutObject()->VisualRectForInlineBox();
+  return *containing_block_fragment->GetLayoutObject();
+}
+
+LayoutRect NGPaintFragment::VisualRect() const {
+  // VisualRect is computed from fragment tree and set to LayoutObject in
+  // pre-paint. Use the stored value in the LayoutObject.
+  bool this_as_inline_box;
+  const auto& layout_object = VisualRectLayoutObject(this_as_inline_box);
+  return this_as_inline_box ? layout_object.VisualRectForInlineBox()
+                            : layout_object.FragmentsVisualRectBoundingBox();
+}
+
+LayoutRect NGPaintFragment::PartialInvalidationVisualRect() const {
+  bool this_as_inline_box;
+  const auto& layout_object = VisualRectLayoutObject(this_as_inline_box);
+  return this_as_inline_box
+             ? layout_object.PartialInvalidationVisualRectForInlineBox()
+             : layout_object.PartialInvalidationVisualRect();
 }
 
 bool NGPaintFragment::FlippedLocalVisualRectFor(

third_party/blink/renderer/core/paint/ng/ng_paint_fragment.h

@@ -55,7 +55,7 @@ class CORE_EXPORT NGPaintFragment : public RefCounted<NGPaintFragment>,
   }
 
   // Next/last fragment for  when this is fragmented.
-  const NGPaintFragment* Next() const;
+  const NGPaintFragment* Next() const { return next_fragmented_.get(); }
   void SetNext(scoped_refptr<NGPaintFragment>);
   const NGPaintFragment* Last() const;
   const NGPaintFragment* Last(const NGBreakToken&) const;
@@ -161,9 +161,7 @@ class CORE_EXPORT NGPaintFragment : public RefCounted<NGPaintFragment>,
   bool HasSelfPaintingLayer() const;
   // This is equivalent to LayoutObject::VisualRect
   LayoutRect VisualRect() const override;
-
-  LayoutRect SelectionVisualRect() const;
-  void SetSelectionVisualRect(const LayoutRect& rect);
+  LayoutRect PartialInvalidationVisualRect() const override;
 
   // CSS ink overflow https://www.w3.org/TR/css-overflow-3/#ink
   // Encloses all pixels painted by self + children.
@@ -316,6 +314,9 @@ class CORE_EXPORT NGPaintFragment : public RefCounted<NGPaintFragment>,
   // Dirty line boxes containing |layout_object|.
   static void MarkLineBoxesDirtyFor(const LayoutObject& layout_object);
 
+  // This fragment will use the layout object's visual rect.
+  const LayoutObject& VisualRectLayoutObject(bool& this_as_inline_box) const;
+
   //
   // Following fields are computed in the layout phase.
   //
@@ -327,18 +328,8 @@ class CORE_EXPORT NGPaintFragment : public RefCounted<NGPaintFragment>,
   scoped_refptr<NGPaintFragment> first_child_;
   scoped_refptr<NGPaintFragment> next_sibling_;
 
-  struct RareData {
-    USING_FAST_MALLOC(RareData);
-
-   public:
-    // The next fragment for when this is fragmented.
-    scoped_refptr<NGPaintFragment> next_fragmented_;
-
-    // Used for invalidating selected fragment.
-    LayoutRect selection_visual_rect_;
-  };
-  RareData& EnsureRareData();
-  std::unique_ptr<RareData> rare_data_;
+  // The next fragment for when this is fragmented.
+  scoped_refptr<NGPaintFragment> next_fragmented_;
 
   NGPaintFragment* next_for_same_layout_object_ = nullptr;
   NGPhysicalOffset inline_offset_to_container_box_;

third_party/blink/renderer/core/paint/ng/ng_paint_fragment_test.cc

@@ -218,7 +218,6 @@ TEST_F(NGPaintFragmentTest, InlineBlock) {
                           .ToString());
   // TODO(kojii): This is still incorrect.
   EXPECT_EQ(LayoutRect(0, 0, 60, 10), outer_text.VisualRect());
-  EXPECT_EQ(LayoutRect(), outer_text.SelectionVisualRect());
 
   // Test |InlineFragmentsFor| can find the outer text.
   LayoutObject* layout_outer_text =
@@ -235,7 +234,6 @@ TEST_F(NGPaintFragmentTest, InlineBlock) {
   EXPECT_EQ(NGPhysicalFragment::kAtomicInline,
             box1.PhysicalFragment().BoxType());
   EXPECT_EQ(LayoutRect(60, 0, 10, 10), box1.VisualRect());
-  EXPECT_EQ(LayoutRect(), box1.SelectionVisualRect());
 
   // Test |InlineFragmentsFor| can find "box1".
   LayoutObject* layout_box1 = GetLayoutObjectByElementId("box1");
@@ -256,7 +254,6 @@ TEST_F(NGPaintFragmentTest, InlineBlock) {
   EXPECT_EQ(NGPhysicalFragment::kFragmentText,
             inner_text.PhysicalFragment().Type());
   EXPECT_EQ(LayoutRect(60, 0, 10, 10), inner_text.VisualRect());
-  EXPECT_EQ(LayoutRect(), inner_text.SelectionVisualRect());
 
   // Test |InlineFragmentsFor| can find the inner text of "box1".
   LayoutObject* layout_inner_text = layout_box1->SlowFirstChild();
@@ -272,18 +269,13 @@ TEST_F(NGPaintFragmentTest, InlineBlock) {
   EXPECT_EQ(NGPhysicalFragment::kAtomicInline,
             box2.PhysicalFragment().BoxType());
   EXPECT_EQ(LayoutRect(70, 10, 10, 10), box2.VisualRect());
-  EXPECT_EQ(LayoutRect(), box2.SelectionVisualRect());
 
   GetDocument().GetFrame()->Selection().SelectAll();
   UpdateAllLifecyclePhasesForTest();
   EXPECT_EQ(LayoutRect(0, 0, 60, 10), outer_text.VisualRect());
-  EXPECT_EQ(LayoutRect(0, 0, 60, 10), outer_text.SelectionVisualRect());
   EXPECT_EQ(LayoutRect(60, 0, 10, 10), box1.VisualRect());
-  EXPECT_EQ(LayoutRect(), box1.SelectionVisualRect());
   EXPECT_EQ(LayoutRect(60, 0, 10, 10), inner_text.VisualRect());
-  EXPECT_EQ(LayoutRect(60, 0, 10, 10), inner_text.SelectionVisualRect());
   EXPECT_EQ(LayoutRect(70, 10, 10, 10), box2.VisualRect());
-  EXPECT_EQ(LayoutRect(), box2.SelectionVisualRect());
 }
 
 TEST_F(NGPaintFragmentTest, RelativeBlock) {

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

@@ -290,15 +290,6 @@ ObjectPaintInvalidatorWithContext::ComputePaintInvalidationReason() {
 DISABLE_CFI_PERF
 PaintInvalidationReason ObjectPaintInvalidatorWithContext::InvalidateSelection(
     PaintInvalidationReason reason) {
-  // In LayoutNG, if NGPaintFragment paints the selection, we invalidate for
-  // selection change in PaintInvalidator.
-  if (RuntimeEnabledFeatures::LayoutNGEnabled() && object_.IsInline() &&
-      // LayoutReplaced still paints selection tint by itself.
-      !object_.IsLayoutReplaced() &&
-      NGPaintFragment::InlineFragmentsFor(&object_)
-          .IsInLayoutNGInlineFormattingContext())
-    return reason;
-
   // Update selection rect when we are doing full invalidation with geometry
   // change (in case that the object is moved, composite status changed, etc.)
   // or shouldInvalidationSelection is set (in case that the selection itself

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

@@ -13,7 +13,6 @@
 #include "third_party/blink/renderer/core/layout/layout_table.h"
 #include "third_party/blink/renderer/core/layout/layout_table_section.h"
 #include "third_party/blink/renderer/core/layout/layout_view.h"
-#include "third_party/blink/renderer/core/layout/ng/geometry/ng_physical_offset_rect.h"
 #include "third_party/blink/renderer/core/layout/ng/legacy_layout_tree_walking.h"
 #include "third_party/blink/renderer/core/layout/svg/svg_layout_support.h"
 #include "third_party/blink/renderer/core/paint/clip_path_clipper.h"
@@ -160,31 +159,6 @@ LayoutRect PaintInvalidator::ComputeVisualRect(
                                                            context);
 }
 
-static LayoutRect ComputeFragmentLocalSelectionRect(
-    const NGPaintFragment& fragment) {
-  DCHECK(fragment.PhysicalFragment().IsText());
-  const FrameSelection& frame_selection =
-      fragment.GetLayoutObject()->GetFrame()->Selection();
-  const LayoutSelectionStatus status =
-      frame_selection.ComputeLayoutSelectionStatus(fragment);
-  if (status.start == status.end)
-    return LayoutRect();
-  return fragment.ComputeLocalSelectionRectForText(status).ToLayoutRect();
-}
-
-LayoutRect PaintInvalidator::MapFragmentLocalRectToVisualRect(
-    const LayoutRect& local_rect,
-    const LayoutObject& object,
-    const NGPaintFragment& fragment,
-    const PaintInvalidatorContext& context) {
-  LayoutRect rect = local_rect;
-  if (!object.IsBox())
-    rect.Move(fragment.InlineOffsetToContainerBox().ToLayoutSize());
-  bool disable_flip = true;
-  return MapLocalRectToVisualRect<LayoutRect, LayoutPoint>(
-      object, rect, context, disable_flip);
-}
-
 void PaintInvalidator::UpdatePaintingLayer(const LayoutObject& object,
                                            PaintInvalidatorContext& context) {
   if (object.HasLayer() &&
@@ -321,57 +295,6 @@ void PaintInvalidator::UpdateVisualRect(const LayoutObject& object,
   if (new_visual_rect.IsEmpty())
     new_visual_rect.SetLocation(fragment_data.PaintOffset());
   fragment_data.SetVisualRect(new_visual_rect);
-
-  // For LayoutNG, update NGPaintFragments.
-  // TODO(kojii): If we can compute SelectionVisualRect when needed, the
-  // following code path will not be needed.
-  if (!RuntimeEnabledFeatures::LayoutNGEnabled())
-    return;
-
-  if (object.IsInline()) {
-    // An inline LayoutObject can produce multiple NGPaintFragment. Compute
-    // VisualRect for each fragment from |new_visual_rect|.
-    auto fragments = NGPaintFragment::InlineFragmentsFor(&object);
-    if (fragments.IsInLayoutNGInlineFormattingContext()) {
-      bool has_selection_in_this_object =
-          object.IsText() && ToLayoutText(object).IsSelected();
-      if (!has_selection_in_this_object) {
-        for (NGPaintFragment* fragment : fragments) {
-          if (UNLIKELY(!fragment->SelectionVisualRect().IsEmpty())) {
-            context.painting_layer->SetNeedsRepaint();
-            ObjectPaintInvalidator(object).InvalidateDisplayItemClient(
-                *fragment, PaintInvalidationReason::kSelection);
-            fragment->SetSelectionVisualRect(LayoutRect());
-          }
-        }
-      } else {
-        // TODO(kojii): It's not clear why we need to pre-compute selection rect
-        // for all fragments when legacy can handle it as needed. yoichio will
-        // look into this.
-        for (NGPaintFragment* fragment : fragments) {
-          LayoutRect local_selection_rect =
-              ComputeFragmentLocalSelectionRect(*fragment);
-          LayoutRect selection_visual_rect = MapFragmentLocalRectToVisualRect(
-              local_selection_rect, object, *fragment, context);
-          new_visual_rect.Unite(selection_visual_rect);
-          const bool should_invalidate =
-              object.ShouldInvalidateSelection() ||
-              selection_visual_rect != fragment->SelectionVisualRect();
-          const bool rect_exists = !selection_visual_rect.IsEmpty() ||
-                                   !fragment->SelectionVisualRect().IsEmpty();
-          if (should_invalidate && rect_exists) {
-            context.painting_layer->SetNeedsRepaint();
-            ObjectPaintInvalidator(object).InvalidateDisplayItemClient(
-                *fragment, PaintInvalidationReason::kSelection);
-            fragment->SetSelectionVisualRect(selection_visual_rect);
-          }
-        }
-        // TODO(kojii): Not sure why do we need to include SelectionVisualRect
-        // to VisualRect only in NG, but this is needed to pass some tests.
-        fragment_data.SetVisualRect(new_visual_rect);
-      }
-    }
-  }
 }
 
 void PaintInvalidator::UpdateEmptyVisualRectFlag(

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

@@ -13,7 +13,6 @@
 
 namespace blink {
 
-class NGPaintFragment;
 class PrePaintTreeWalk;
 
 struct CORE_EXPORT PaintInvalidatorContext {
@@ -164,11 +163,6 @@ class PaintInvalidator {
 
   ALWAYS_INLINE LayoutRect ComputeVisualRect(const LayoutObject&,
                                              const PaintInvalidatorContext&);
-  ALWAYS_INLINE LayoutRect
-  MapFragmentLocalRectToVisualRect(const LayoutRect&,
-                                   const LayoutObject&,
-                                   const NGPaintFragment&,
-                                   const PaintInvalidatorContext&);
   ALWAYS_INLINE void UpdatePaintingLayer(const LayoutObject&,
                                          PaintInvalidatorContext&);
   ALWAYS_INLINE void UpdatePaintInvalidationContainer(const LayoutObject&,

third_party/blink/web_tests/FlagExpectations/enable-blink-features=LayoutNG

@@ -448,6 +448,7 @@ crbug.com/591099 fast/webgl/texImage-imageBitmap-from-canvas-resize.html [ Pass
 crbug.com/591099 fast/writing-mode/auto-sizing-orthogonal-flows.html [ Failure ]
 crbug.com/591099 fast/writing-mode/percentage-height-orthogonal-writing-modes.html [ Failure ]
 crbug.com/591099 fast/writing-mode/table-percent-width-quirk.html [ Pass ]
+crbug.com/930034 fast/writing-mode/vertical-rl-replaced-selection.html [ Failure ]
 crbug.com/591099 html/marquee/marquee-destroyed-without-removed-from-crash.html [ Pass ]
 crbug.com/591099 http/tests/activedomobject/media.html [ Crash Pass ]
 crbug.com/591099 http/tests/appcache/abort-cache-ondownloading.html [ Crash Pass ]