Introduce SelectionPaintRange in LayoutSelection

third_party/WebKit/Source/core/editing/LayoutSelection.cpp

Index: third_party/WebKit/Source/core/editing/LayoutSelection.cpp
diff --git a/third_party/WebKit/Source/core/editing/LayoutSelection.cpp b/third_party/WebKit/Source/core/editing/LayoutSelection.cpp
index 7fe3f6b27259f3321a664818cd3c5e9ff835272c..168c0aeec098756b65a716293c167b0dbc0b07c2 100644
--- a/third_party/WebKit/Source/core/editing/LayoutSelection.cpp
+++ b/third_party/WebKit/Source/core/editing/LayoutSelection.cpp
@@ -35,10 +35,7 @@ namespace blink {
 LayoutSelection::LayoutSelection(FrameSelection& frame_selection)
     : frame_selection_(&frame_selection),
       has_pending_selection_(false),
-      selection_start_(nullptr),
-      selection_end_(nullptr),
-      selection_start_pos_(-1),
-      selection_end_pos_(-1) {}
+      paint_range_(SelectionPaintRange()) {}
 
 SelectionInFlatTree LayoutSelection::CalcVisibleSelection(
     const VisibleSelectionInFlatTree& original_selection) const {
@@ -124,19 +121,17 @@ struct SelectedMap {
 };
 
 static SelectedMap CollectSelectedMap(
-    LayoutObject* selection_start,
-    LayoutObject* selection_end,
-    int selection_end_pos,
+    const SelectionPaintRange& range,
     LayoutSelection::SelectionPaintInvalidationMode
         block_paint_invalidation_mode) {
   SelectedMap selected_map;
 
   LayoutObject* const stop =
-      LayoutObjectAfterPosition(selection_end, selection_end_pos);
-  for (LayoutObject* runner = selection_start; runner && (runner != stop);
-       runner = runner->NextInPreOrder()) {
-    if (!runner->CanBeSelectionLeaf() && runner != selection_start &&
-        runner != selection_end)
+      LayoutObjectAfterPosition(range.end_layout_object_, range.end_offset_);
+  for (LayoutObject* runner = range.start_layout_object_;
+       runner && (runner != stop); runner = runner->NextInPreOrder()) {
+    if (!runner->CanBeSelectionLeaf() && runner != range.start_layout_object_ &&
+        runner != range.end_layout_object_)
       continue;
     if (runner->GetSelectionState() == SelectionNone)
       continue;
@@ -160,52 +155,47 @@ static SelectedMap CollectSelectedMap(
 }
 
 // Update the selection status of all LayoutObjects between |start| and |end|.
-static void SetSelectionState(LayoutObject* start,
-                              LayoutObject* end,
-                              int end_pos) {
-  if (start && start == end) {
-    start->SetSelectionStateIfNeeded(SelectionBoth);
+static void SetSelectionState(const SelectionPaintRange& range) {
+  DCHECK(range.start_layout_object_);
+  DCHECK(range.end_layout_object_);
+  if (range.start_layout_object_ == range.end_layout_object_) {
+    range.start_layout_object_->SetSelectionStateIfNeeded(SelectionBoth);
   } else {
-    if (start)
-      start->SetSelectionStateIfNeeded(SelectionStart);
-    if (end)
-      end->SetSelectionStateIfNeeded(SelectionEnd);
+    range.start_layout_object_->SetSelectionStateIfNeeded(SelectionStart);
+    range.end_layout_object_->SetSelectionStateIfNeeded(SelectionEnd);
   }
 
-  LayoutObject* const stop = LayoutObjectAfterPosition(end, end_pos);
-  for (LayoutObject* runner = start; runner && runner != stop;
-       runner = runner->NextInPreOrder()) {
-    if (runner != start && runner != end && runner->CanBeSelectionLeaf())
+  LayoutObject* const stop =
+      LayoutObjectAfterPosition(range.end_layout_object_, range.end_offset_);
+  for (LayoutObject* runner = range.start_layout_object_;
+       runner && runner != stop; runner = runner->NextInPreOrder()) {
+    if (runner != range.start_layout_object_ &&
+        runner != range.end_layout_object_ && runner->CanBeSelectionLeaf())
       runner->SetSelectionStateIfNeeded(SelectionInside);
   }
 }
 
 void LayoutSelection::SetSelection(
-    LayoutObject* start,
-    int start_pos,
-    LayoutObject* end,
-    int end_pos,
+    const SelectionPaintRange& new_range,
     SelectionPaintInvalidationMode block_paint_invalidation_mode) {
-  DCHECK(start);
-  DCHECK(end);
+  DCHECK(new_range.start_layout_object_);

[yosin_UTC9, 2017/05/24 08:38:46]

Once ctor has DCHECK's for members, we don't need to check here.

[yoichio, 2017/05/24 09:09:29]

How about null SelectionPaintRange?

[yosin_UTC9, 2017/05/24 09:31:18]

We need to check null SelectionPaintRange.
Is it better to have |SelectionPaintRange::IsNull()| as |EphemeralRange|.

+  DCHECK(new_range.end_layout_object_);
 
   // Just return if the selection hasn't changed.
-  if (selection_start_ == start && selection_start_pos_ == start_pos &&
-      selection_end_ == end && selection_end_pos_ == end_pos)
+  if (paint_range_ == new_range)
     return;
 
   DCHECK(frame_selection_->GetDocument().GetLayoutView()->GetFrameView());
   DCHECK(!frame_selection_->GetDocument().NeedsLayoutTreeUpdate());
 
   SelectedMap old_selected_map =
-      CollectSelectedMap(selection_start_, selection_end_, selection_end_pos_,
-                         block_paint_invalidation_mode);
+      CollectSelectedMap(paint_range_, block_paint_invalidation_mode);
 
   // Now clear the selection.
   for (auto layout_object : old_selected_map.object_map.Keys())
     layout_object->SetSelectionStateIfNeeded(SelectionNone);
 
-  SetSelectionState(start, end, end_pos);
+  SetSelectionState(new_range);
 
   // Now that the selection state has been updated for the new objects, walk
   // them again and put them in the new objects list.
@@ -213,7 +203,7 @@ void LayoutSelection::SetSelection(
   // SelectionState, it's just more convenient to have it use the same data
   // structure as |old_selected_map|.
   SelectedMap new_selected_map =
-      CollectSelectedMap(start, end, end_pos, kPaintInvalidationNewXOROld);
+      CollectSelectedMap(new_range, kPaintInvalidationNewXOROld);
 
   // Have any of the old selected objects changed compared to the new selection?
   for (const auto& pair : old_selected_map.object_map) {
@@ -221,8 +211,10 @@ void LayoutSelection::SetSelection(
     SelectionState new_selection_state = obj->GetSelectionState();
     SelectionState old_selection_state = pair.value;
     if (new_selection_state != old_selection_state ||
-        (start == obj && start_pos != selection_start_pos_) ||
-        (end == obj && end_pos != selection_end_pos_)) {
+        (new_range.start_layout_object_ == obj &&

[yosin_UTC9, 2017/05/24 08:38:46]

Can we use SelectionPaintRange::operator!=()?
e.g. new_range != paint_range_

[yoichio, 2017/05/24 09:09:29]

This is not SelectionPaintRange compare.

+         new_range.start_offset_ != paint_range_.start_offset_) ||
+        (new_range.end_layout_object_ == obj &&
+         new_range.end_offset_ != paint_range_.end_offset_)) {
       obj->SetShouldInvalidateSelection();
       new_selected_map.object_map.erase(obj);
     }
@@ -249,16 +241,12 @@ void LayoutSelection::SetSelection(
   for (auto layout_object : new_selected_map.block_map.Keys())
     layout_object->SetShouldInvalidateSelection();
 
-  // set selection start and end
-  selection_start_ = start;
-  selection_start_pos_ = start_pos;
-  selection_end_ = end;
-  selection_end_pos_ = end_pos;
+  paint_range_ = new_range;
 }
 
 std::pair<int, int> LayoutSelection::SelectionStartEnd() {
   Commit();
-  return std::make_pair(selection_start_pos_, selection_end_pos_);
+  return std::make_pair(paint_range_.start_offset_, paint_range_.end_offset_);
 }
 
 void LayoutSelection::ClearSelection() {
@@ -267,17 +255,12 @@ void LayoutSelection::ClearSelection() {
   // invalidations.
   DisableCompositingQueryAsserts disabler;
 
-  // Just return if the selection hasn't changed.
-  if (!selection_start_) {
-    DCHECK_EQ(selection_end_, nullptr);
-    DCHECK_EQ(selection_start_pos_, -1);
-    DCHECK_EQ(selection_end_pos_, -1);
+  // Just return if the selection is already empty.
+  if (paint_range_ == SelectionPaintRange())
     return;
-  }
 
   const SelectedMap& old_selected_map =
-      CollectSelectedMap(selection_start_, selection_end_, selection_end_pos_,
-                         kPaintInvalidationNewMinusOld);
+      CollectSelectedMap(paint_range_, kPaintInvalidationNewMinusOld);
   // Clear SelectionState and invalidation.
   for (auto layout_object : old_selected_map.object_map.Keys()) {
     const SelectionState old_state = layout_object->GetSelectionState();
@@ -288,10 +271,7 @@ void LayoutSelection::ClearSelection() {
   }
 
   // Reset selection start and end.
-  selection_start_ = nullptr;
-  selection_start_pos_ = -1;
-  selection_end_ = nullptr;
-  selection_end_pos_ = -1;
+  paint_range_ = SelectionPaintRange();
 }
 
 void LayoutSelection::Commit() {
@@ -342,16 +322,14 @@ void LayoutSelection::Commit() {
   if (!start_layout_object || !end_layout_object)
     return;
   DCHECK(start_layout_object->View() == end_layout_object->View());
-  SetSelection(start_layout_object, start_pos.ComputeEditingOffset(),
-               end_layout_object, end_pos.ComputeEditingOffset());
+  SetSelection(
+      SelectionPaintRange(start_layout_object, start_pos.ComputeEditingOffset(),
+                          end_layout_object, end_pos.ComputeEditingOffset()));
 }
 
 void LayoutSelection::OnDocumentShutdown() {
   has_pending_selection_ = false;
-  selection_start_ = nullptr;
-  selection_end_ = nullptr;
-  selection_start_pos_ = -1;
-  selection_end_pos_ = -1;
+  paint_range_ = SelectionPaintRange();
 }
 
 static LayoutRect SelectionRectForLayoutObject(const LayoutObject* object) {
@@ -372,12 +350,12 @@ IntRect LayoutSelection::SelectionBounds() {
   VisitedContainingBlockSet visited_containing_blocks;
 
   Commit();
-  LayoutObject* os = selection_start_;
-  LayoutObject* stop =
-      LayoutObjectAfterPosition(selection_end_, selection_end_pos_);
+  LayoutObject* os = paint_range_.start_layout_object_;
+  LayoutObject* stop = LayoutObjectAfterPosition(
+      paint_range_.end_layout_object_, paint_range_.end_offset_);
   while (os && os != stop) {
-    if ((os->CanBeSelectionLeaf() || os == selection_start_ ||
-         os == selection_end_) &&
+    if ((os->CanBeSelectionLeaf() || os == paint_range_.start_layout_object_ ||
+         os == paint_range_.end_layout_object_) &&
         os->GetSelectionState() != SelectionNone) {
       // Blocks are responsible for painting line gaps and margin gaps. They
       // must be examined as well.
@@ -400,12 +378,12 @@ IntRect LayoutSelection::SelectionBounds() {
 }
 
 void LayoutSelection::InvalidatePaintForSelection() {
-  LayoutObject* end =
-      LayoutObjectAfterPosition(selection_end_, selection_end_pos_);
-  for (LayoutObject* o = selection_start_; o && o != end;
+  LayoutObject* end = LayoutObjectAfterPosition(paint_range_.end_layout_object_,
+                                                paint_range_.end_offset_);
+  for (LayoutObject* o = paint_range_.start_layout_object_; o && o != end;
        o = o->NextInPreOrder()) {
-    if (!o->CanBeSelectionLeaf() && o != selection_start_ &&
-        o != selection_end_)
+    if (!o->CanBeSelectionLeaf() && o != paint_range_.start_layout_object_ &&
+        o != paint_range_.end_layout_object_)
       continue;
     if (o->GetSelectionState() == SelectionNone)
       continue;