Split LayoutSelecton::ClearSelection() out from SetSelection()

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 0b628863e698c6c3fa22fe9b1db0b81bcce8e299..82f1eca9a4e9e14ac7232effecbc345278aeec66 100644
--- a/third_party/WebKit/Source/core/editing/LayoutSelection.cpp
+++ b/third_party/WebKit/Source/core/editing/LayoutSelection.cpp
@@ -213,16 +213,8 @@ void LayoutSelection::SetSelection(
     LayoutObject* end,
     int end_pos,
     SelectionPaintInvalidationMode block_paint_invalidation_mode) {
-  // This code makes no assumptions as to if the layout tree is up to date or
-  // not and will not try to update it. Currently clearSelection calls this
-  // (intentionally) without updating the layout tree as it doesn't care.
-  // Other callers may want to force recalc style before calling this.
-
-  // Make sure both our start and end objects are defined.
-  // Check www.msnbc.com and try clicking around to find the case where this
-  // happened.
-  if ((start && !end) || (end && !start))
-    return;
+  DCHECK(start);

[yosin_UTC9, 2017/05/23 05:29:05]

Nice! (^_^)b

+  DCHECK(end);
 
   // Just return if the selection hasn't changed.
   if (selection_start_ == start && selection_start_pos_ == start_pos &&
@@ -230,6 +222,7 @@ void LayoutSelection::SetSelection(
     return;
 
   DCHECK(frame_selection_->GetDocument().GetLayoutView()->GetFrameView());
+  DCHECK(!frame_selection_->GetDocument().NeedsLayoutTreeUpdate());
 
   SelectedMap old_selected_map =
       CollectSelectedMap(selection_start_, selection_end_, selection_end_pos_,
@@ -301,7 +294,30 @@ void LayoutSelection::ClearSelection() {
   // invalidations.
   DisableCompositingQueryAsserts disabler;
 
-  SetSelection(0, -1, 0, -1, kPaintInvalidationNewMinusOld);
+  // 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);
+    return;
+  }
+
+  SelectedMap old_selected_map =

[yosin_UTC9, 2017/05/23 05:29:05]

nit: s/SelectedMap/const SelectedMap&/

[yoichio, 2017/05/23 05:43:07]

Done.

+      CollectSelectedMap(selection_start_, selection_end_, selection_end_pos_,
+                         kPaintInvalidationNewMinusOld);
+  // Clear SelectionState and invalidation.
+  for (auto layout_object : old_selected_map.object_map.Keys()) {
+    const SelectionState old_state = layout_object->GetSelectionState();
+    layout_object->SetSelectionStateIfNeeded(SelectionNone);
+    if (layout_object->GetSelectionState() != old_state)

[yosin_UTC9, 2017/05/23 05:29:05]

nit: early-continue style is better.

if (layoutObject->GetSelectionState() == old_state)
  continue;
layout_object->SetShouldInvalidateSelection();

[yoichio, 2017/05/23 05:43:07]

Done.

+      layout_object->SetShouldInvalidateSelection();
+  }
+
+  // Reset selection start and end.
+  selection_start_ = nullptr;

[yosin_UTC9, 2017/05/23 05:29:05]

Q: do you have a plan to introduce object representing LayoutTreeRange or
another name
to represent range in Layout tree?

struct LayoutTreeRange {
  LayoutObject* start = nullptr;
  LayoutObject* end = nullptr;
  int start_offset = -1;
  int end_offset = -1;

  LayoutTreeRange() {}
};

Is |class| better?

[yoichio, 2017/05/23 05:43:07]

I consider having SelectionInFlatTree instead of having LayoutObjects.

+  selection_start_pos_ = -1;
+  selection_end_ = nullptr;
+  selection_end_pos_ = -1;
 }
 
 void LayoutSelection::Commit() {