[LayoutNG] Update LayoutSelectionStartEndForNG signature

This patch updates LayoutSelectionStartEndForNG signature by
1. Introducing LayoutSelectionStatus structure for return type consisting of
start/end.
2. Change argument type from NGPhysicalTextFragment to NGPaintFragment.

This is a preparation for line break selection painting:
crbug.com/c/896780

Bug: 708452
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_layout_ng;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Idca785e841a577b1a591ae10cf3d9771d21c6ce1
Reviewed-on: https://chromium-review.googlesource.com/1029563
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553962}

third_party/blink/renderer/core/editing/frame_selection.cc

@@ -1220,9 +1220,9 @@ void FrameSelection::ClearLayoutSelection() {
   layout_selection_->ClearSelection();
 }
 
-std::pair<unsigned, unsigned> FrameSelection::LayoutSelectionStartEndForNG(
-    const NGPhysicalTextFragment& text_fragment) const {
-  return layout_selection_->SelectionStartEndForNG(text_fragment);
+LayoutSelectionStatus FrameSelection::ComputeLayoutSelectionStatus(
+    const NGPaintFragment& text_fragment) const {
+  return layout_selection_->ComputeSelectionStatus(text_fragment);
 }
 
 bool FrameSelection::IsDirectional() const {

third_party/blink/renderer/core/editing/frame_selection.h

@@ -49,12 +49,13 @@ class LocalFrame;
 class FrameCaret;
 class GranularityStrategy;
 class GraphicsContext;
-class NGPhysicalTextFragment;
+class NGPaintFragment;
 class Range;
 class SelectionEditor;
 class LayoutSelection;
 enum class SelectionModifyAlteration;
 enum class SelectionModifyDirection;
+enum class SelectionState;
 class TextIteratorBehavior;
 struct PaintInvalidatorContext;
 
@@ -64,6 +65,27 @@ enum class CaretVisibility;
 
 enum class HandleVisibility { kNotVisible, kVisible };
 
+// This is return type of ComputeLayoutSelectionStatus(paintfragment).
+// This structure represents how the fragment is selected.
+// |start|, |end| : Selection start/end offset. This offset is based on
+//   the text of NGInlineNode of a parent block thus
+//   |fragemnt.StartOffset <= start <= end <= fragment.EndOffset|.
+// |start| == |end| means this fragment is not selected.
+struct LayoutSelectionStatus {
+  STACK_ALLOCATED();
+
+  LayoutSelectionStatus(unsigned passed_start, unsigned passed_end)
+      : start(passed_start), end(passed_end) {
+    DCHECK_LE(start, end);
+  }
+  bool operator==(const LayoutSelectionStatus& other) const {
+    return start == other.start && end == other.end;
+  }
+
+  unsigned start;
+  unsigned end;
+};
+
 class CORE_EXPORT FrameSelection final
     : public GarbageCollectedFinalized<FrameSelection>,
       public SynchronousMutationObserver {
@@ -220,8 +242,8 @@ class CORE_EXPORT FrameSelection final
   base::Optional<unsigned> LayoutSelectionStart() const;
   base::Optional<unsigned> LayoutSelectionEnd() const;
   void ClearLayoutSelection();
-  std::pair<unsigned, unsigned> LayoutSelectionStartEndForNG(
-      const NGPhysicalTextFragment&) const;
+  LayoutSelectionStatus ComputeLayoutSelectionStatus(
+      const NGPaintFragment&) const;
 
   void Trace(blink::Visitor*) override;
 

third_party/blink/renderer/core/editing/layout_selection.cc

@@ -34,6 +34,7 @@
 #include "third_party/blink/renderer/core/layout/layout_view.h"
 #include "third_party/blink/renderer/core/layout/ng/inline/ng_offset_mapping.h"
 #include "third_party/blink/renderer/core/layout/ng/inline/ng_physical_text_fragment.h"
+#include "third_party/blink/renderer/core/paint/ng/ng_paint_fragment.h"
 #include "third_party/blink/renderer/core/paint/paint_layer.h"
 
 namespace blink {
@@ -657,8 +658,10 @@ static unsigned ClampOffset(unsigned offset,
                   text_fragment.EndOffset());
 }
 
-std::pair<unsigned, unsigned> LayoutSelection::SelectionStartEndForNG(
-    const NGPhysicalTextFragment& text_fragment) const {
+LayoutSelectionStatus LayoutSelection::ComputeSelectionStatus(
+    const NGPaintFragment& fragment) const {
+  const NGPhysicalTextFragment& text_fragment =
+      ToNGPhysicalTextFragmentOrDie(fragment.PhysicalFragment());
   // FrameSelection holds selection offsets in layout block flow at
   // LayoutSelection::Commit() if selection starts/ends within Text that
   // each LayoutObject::SelectionState indicates.

third_party/blink/renderer/core/editing/layout_selection.h

@@ -31,9 +31,9 @@ namespace blink {
 
 class IntRect;
 class LayoutObject;
-class NGPhysicalTextFragment;
+class NGPaintFragment;
 class FrameSelection;
-
+struct LayoutSelectionStatus;
 // This class represents a selection range in layout tree for painting and
 // paint invalidation.
 // The current selection to be painted is represented as 2 pairs of
@@ -105,12 +105,7 @@ class LayoutSelection final : public GarbageCollected<LayoutSelection> {
   void ClearSelection();
   base::Optional<unsigned> SelectionStart() const;
   base::Optional<unsigned> SelectionEnd() const;
-  // This function returns selected part of |text_fragment|.
-  // Returned pair is a partial range of
-  // (text_fragment.StartOffset(), text_fragment.EndOffset()).
-  // If first equals second, it indicates "no selection in fragment".
-  std::pair<unsigned, unsigned> SelectionStartEndForNG(
-      const NGPhysicalTextFragment&) const;
+  LayoutSelectionStatus ComputeSelectionStatus(const NGPaintFragment&) const;
 
   void OnDocumentShutdown();
 

third_party/blink/renderer/core/editing/layout_selection_test.cc

@@ -696,6 +696,11 @@ class NGLayoutSelectionTest
         ScopedPaintUnderInvalidationCheckingForTest(true) {}
 };
 
+std::ostream& operator<<(std::ostream& ostream,
+                         const LayoutSelectionStatus& status) {
+  return ostream << status.start << ", " << status.end;
+}
+
 static const NGPaintFragment* FindNGPaintFragmentInternal(
     const NGPaintFragment* paint,
     const LayoutObject* layout_object) {
@@ -709,7 +714,7 @@ static const NGPaintFragment* FindNGPaintFragmentInternal(
   return nullptr;
 }
 
-static const NGPhysicalTextFragment& GetNGPhysicalTextFragment(
+static const NGPaintFragment& GetNGPaintFragment(
     const LayoutObject* layout_object) {
   DCHECK(layout_object->IsText());
   LayoutBlockFlow* block_flow = layout_object->EnclosingNGBlockFlow();
@@ -718,9 +723,8 @@ static const NGPhysicalTextFragment& GetNGPhysicalTextFragment(
   LayoutNGBlockFlow* layout_ng = ToLayoutNGBlockFlow(block_flow);
   const NGPaintFragment* paint_fragment =
       FindNGPaintFragmentInternal(layout_ng->PaintFragment(), layout_object);
-  const NGPhysicalFragment& physical_fragment =
-      paint_fragment->PhysicalFragment();
-  return ToNGPhysicalTextFragment(physical_fragment);
+  DCHECK(paint_fragment);
+  return *paint_fragment;
 }
 
 TEST_F(NGLayoutSelectionTest, SelectOnOneText) {
@@ -740,16 +744,16 @@ TEST_F(NGLayoutSelectionTest, SelectOnOneText) {
 
   LayoutObject* const foo =
       GetDocument().body()->firstChild()->GetLayoutObject();
-  EXPECT_EQ(std::make_pair(0u, 0u), Selection().LayoutSelectionStartEndForNG(
-                                        GetNGPhysicalTextFragment(foo)));
+  EXPECT_EQ(LayoutSelectionStatus(0u, 0u),
+            Selection().ComputeLayoutSelectionStatus(GetNGPaintFragment(foo)));
   LayoutObject* const bar = GetDocument()
                                 .body()
                                 ->firstChild()
                                 ->nextSibling()
                                 ->firstChild()
                                 ->GetLayoutObject();
-  EXPECT_EQ(std::make_pair(4u, 5u), Selection().LayoutSelectionStartEndForNG(
-                                        GetNGPhysicalTextFragment(bar)));
+  EXPECT_EQ(LayoutSelectionStatus(4u, 5u),
+            Selection().ComputeLayoutSelectionStatus(GetNGPaintFragment(bar)));
 }
 
 TEST_F(NGLayoutSelectionTest, FirstLetterInAnotherBlockFlow) {
@@ -765,12 +769,14 @@ TEST_F(NGLayoutSelectionTest, FirstLetterInAnotherBlockFlow) {
   Node* const foo = GetDocument().body()->firstChild()->nextSibling();
   const LayoutTextFragment* const foo_f =
       ToLayoutTextFragment(AssociatedLayoutObjectOf(*foo, 0));
-  EXPECT_EQ(std::make_pair(0u, 1u), Selection().LayoutSelectionStartEndForNG(
-                                        GetNGPhysicalTextFragment(foo_f)));
+  EXPECT_EQ(
+      LayoutSelectionStatus(0u, 1u),
+      Selection().ComputeLayoutSelectionStatus(GetNGPaintFragment(foo_f)));
   const LayoutTextFragment* const foo_oo =
       ToLayoutTextFragment(AssociatedLayoutObjectOf(*foo, 1));
-  EXPECT_EQ(std::make_pair(1u, 2u), Selection().LayoutSelectionStartEndForNG(
-                                        GetNGPhysicalTextFragment(foo_oo)));
+  EXPECT_EQ(
+      LayoutSelectionStatus(1u, 2u),
+      Selection().ComputeLayoutSelectionStatus(GetNGPaintFragment(foo_oo)));
 }
 
 TEST_F(NGLayoutSelectionTest, TwoNGBlockFlows) {
@@ -786,16 +792,16 @@ TEST_F(NGLayoutSelectionTest, TwoNGBlockFlows) {
   TEST_NO_NEXT_LAYOUT_OBJECT();
   LayoutObject* const foo =
       GetDocument().body()->firstChild()->firstChild()->GetLayoutObject();
-  EXPECT_EQ(std::make_pair(1u, 3u), Selection().LayoutSelectionStartEndForNG(
-                                        GetNGPhysicalTextFragment(foo)));
+  EXPECT_EQ(LayoutSelectionStatus(1u, 3u),
+            Selection().ComputeLayoutSelectionStatus(GetNGPaintFragment(foo)));
   LayoutObject* const bar = GetDocument()
                                 .body()
                                 ->firstChild()
                                 ->nextSibling()
                                 ->firstChild()
                                 ->GetLayoutObject();
-  EXPECT_EQ(std::make_pair(0u, 2u), Selection().LayoutSelectionStartEndForNG(
-                                        GetNGPhysicalTextFragment(bar)));
+  EXPECT_EQ(LayoutSelectionStatus(0u, 2u),
+            Selection().ComputeLayoutSelectionStatus(GetNGPaintFragment(bar)));
 }
 
 TEST_F(NGLayoutSelectionTest, MixedBlockFlowsAsSibling) {
@@ -812,8 +818,8 @@ TEST_F(NGLayoutSelectionTest, MixedBlockFlowsAsSibling) {
   TEST_NO_NEXT_LAYOUT_OBJECT();
   LayoutObject* const foo =
       GetDocument().body()->firstChild()->firstChild()->GetLayoutObject();
-  EXPECT_EQ(std::make_pair(1u, 3u), Selection().LayoutSelectionStartEndForNG(
-                                        GetNGPhysicalTextFragment(foo)));
+  EXPECT_EQ(LayoutSelectionStatus(1u, 3u),
+            Selection().ComputeLayoutSelectionStatus(GetNGPaintFragment(foo)));
   EXPECT_EQ(2u, Selection().LayoutSelectionEnd().value());
 }
 
@@ -849,8 +855,8 @@ TEST_F(NGLayoutSelectionTest, MixedBlockFlowsDecendant) {
   TEST_NO_NEXT_LAYOUT_OBJECT();
   LayoutObject* const foo =
       GetDocument().body()->firstChild()->firstChild()->GetLayoutObject();
-  EXPECT_EQ(std::make_pair(1u, 3u), Selection().LayoutSelectionStartEndForNG(
-                                        GetNGPhysicalTextFragment(foo)));
+  EXPECT_EQ(LayoutSelectionStatus(1u, 3u),
+            Selection().ComputeLayoutSelectionStatus(GetNGPaintFragment(foo)));
   EXPECT_EQ(2u, Selection().LayoutSelectionEnd().value());
 }
 

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

@@ -79,13 +79,14 @@ NGTextFragmentPainter::NGTextFragmentPainter(
 // |selection_start| and |selection_end| should be between
 // [text_fragment.StartOffset(), text_fragment.EndOffset()].
 static void PaintSelection(GraphicsContext& context,
-                           const NGPhysicalTextFragment& text_fragment,
+                           const NGPaintFragment& paint_fragment,
                            const Document& document,
                            const ComputedStyle& style,
                            Color text_color,
                            const LayoutRect& box_rect,
-                           unsigned int selection_start,
-                           unsigned int selection_end) {
+                           const LayoutSelectionStatus& selection_status) {
+  const NGPhysicalTextFragment& text_fragment =
+      ToNGPhysicalTextFragment(paint_fragment.PhysicalFragment());
   const Color color =
       SelectionBackgroundColor(document, style, text_fragment, text_color);
   if (!color.Alpha())
@@ -93,14 +94,15 @@ static void PaintSelection(GraphicsContext& context,
 
   GraphicsContextStateSaver state_saver(context);
 
-  DCHECK_LE(text_fragment.StartOffset(), selection_start);
-  DCHECK_LE(text_fragment.StartOffset(), selection_end);
-  DCHECK_GE(text_fragment.EndOffset(), selection_start);
-  DCHECK_GE(text_fragment.EndOffset(), selection_end);
+  DCHECK_LE(text_fragment.StartOffset(), selection_status.start);
+  DCHECK_LE(text_fragment.StartOffset(), selection_status.end);
+  DCHECK_GE(text_fragment.EndOffset(), selection_status.start);
+  DCHECK_GE(text_fragment.EndOffset(), selection_status.end);
   const ShapeResult* shape_result = text_fragment.TextShapeResult();
+  DCHECK(shape_result);
   const CharacterRange& range = shape_result->GetCharacterRange(
-      selection_start - text_fragment.StartOffset(),
-      selection_end - text_fragment.StartOffset());
+      selection_status.start - text_fragment.StartOffset(),
+      selection_status.end - text_fragment.StartOffset());
   const FloatRect& selection_rect = PixelSnappedSelectionRect(
       FloatRect(box_rect.Location().X() + range.start, box_rect.Location().Y(),
                 range.Width(), box_rect.Height().ToFloat()));
@@ -135,13 +137,10 @@ void NGTextFragmentPainter::Paint(const PaintInfo& paint_info,
   bool is_printing = paint_info.IsPrinting();
 
   // Determine whether or not we're selected.
-  int selection_start;
-  int selection_end;
-  std::tie(selection_start, selection_end) =
-      document.GetFrame()->Selection().LayoutSelectionStartEndForNG(
-          text_fragment);
-  DCHECK_LE(selection_start, selection_end);
-  const bool have_selection = selection_start < selection_end;
+  const LayoutSelectionStatus& selection_status =
+      document.GetFrame()->Selection().ComputeLayoutSelectionStatus(fragment_);
+  DCHECK_LE(selection_status.start, selection_status.end);
+  const bool have_selection = selection_status.start < selection_status.end;
   if (!have_selection && paint_info.phase == PaintPhase::kSelection) {
     // When only painting the selection, don't bother to paint if there is none.
     return;
@@ -182,9 +181,8 @@ void NGTextFragmentPainter::Paint(const PaintInfo& paint_info,
   if (have_selection && paint_info.phase != PaintPhase::kSelection &&
       paint_info.phase != PaintPhase::kTextClip && !is_printing) {
     // TODO(yoichio): Implement composition highlights.
-    PaintSelection(context, text_fragment, document, style,
-                   selection_style.fill_color, box_rect, selection_start,
-                   selection_end);
+    PaintSelection(context, fragment_, document, style,
+                   selection_style.fill_color, box_rect, selection_status);
   }
 
   // 2. Now paint the foreground, including text and decorations.
@@ -227,15 +225,19 @@ void NGTextFragmentPainter::Paint(const PaintInfo& paint_info,
           &has_line_through_decoration);
     }
 
-    int start_offset = text_fragment.StartOffset();
-    int end_offset = start_offset + length;
+    unsigned start_offset = text_fragment.StartOffset();
+    unsigned end_offset = start_offset + length;
 
     if (have_selection && paint_selected_text_separately) {
       // Paint only the text that is not selected.
-      if (start_offset < selection_start)
-        text_painter.Paint(start_offset, selection_start, length, text_style);
-      if (selection_end < end_offset)
-        text_painter.Paint(selection_end, end_offset, length, text_style);
+      if (start_offset < selection_status.start) {
+        text_painter.Paint(start_offset, selection_status.start, length,
+                           text_style);
+      }
+      if (selection_status.end < end_offset) {
+        text_painter.Paint(selection_status.end, end_offset, length,
+                           text_style);
+      }
     } else {
       text_painter.Paint(start_offset, end_offset, length, text_style);
     }
@@ -251,7 +253,8 @@ void NGTextFragmentPainter::Paint(const PaintInfo& paint_info,
   if (have_selection &&
       (paint_selected_text_only || paint_selected_text_separately)) {
     // Paint only the text that is selected.
-    text_painter.Paint(selection_start, selection_end, length, selection_style);
+    text_painter.Paint(selection_status.start, selection_status.end, length,
+                       selection_style);
   }
 }