Make LayoutSelection::SetSelection() local

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 81174f1e27f92bd38e33634a79b251a30c2ca537..e10a33d27d3f1bfeb9af69ea21aad86bb10e0c63 100644
--- a/third_party/WebKit/Source/core/editing/LayoutSelection.cpp
+++ b/third_party/WebKit/Source/core/editing/LayoutSelection.cpp
@@ -162,10 +162,12 @@ struct SelectedMap {
   DISALLOW_COPY_AND_ASSIGN(SelectedMap);
 };
 
-static SelectedMap CollectSelectedMap(
-    const SelectionPaintRange& range,
-    LayoutSelection::SelectionPaintInvalidationMode
-        block_paint_invalidation_mode) {
+enum class CollectSelectedMapOption {
+  kCollectBlock,
+  kNotCollectBlock,
+};
+static SelectedMap CollectSelectedMap(const SelectionPaintRange& range,

[yosin_UTC9, 2017/05/29 07:59:37]

nit: Could you add a blank line between enum and function?

[yoichio, 2017/05/30 04:15:03]

Done.

+                                      CollectSelectedMapOption option) {
   if (range.IsNull())
     return SelectedMap();
 
@@ -184,8 +186,7 @@ static SelectedMap CollectSelectedMap(
     // Blocks are responsible for painting line gaps and margin gaps.  They
     // must be examined as well.
     selected_map.object_map.Set(runner, runner->GetSelectionState());
-    if (block_paint_invalidation_mode ==
-        LayoutSelection::kPaintInvalidationNewXOROld) {
+    if (option == CollectSelectedMapOption::kCollectBlock) {
       LayoutBlock* containing_block = runner->ContainingBlock();
       while (containing_block && !containing_block->IsLayoutView()) {
         SelectedBlockMap::AddResult result = selected_map.block_map.insert(
@@ -221,20 +222,11 @@ static void SetSelectionState(const SelectionPaintRange& range) {
   }
 }
 
-void LayoutSelection::SetSelection(
+static void SetSelectionStateAndShouldInvalidateSelection(

[yosin_UTC9, 2017/05/29 07:59:37]

nit:
s/SetSelectionStateAndShouldInvalidateSelection/SetSelectionStateAndInvalidate/
or better name, we don't need to have |Should| in function name.

[yoichio, 2017/05/29 08:36:26]

Fmm, actually we don't invalidate LayoutObject yet.
Just turn |ShouldInvalidateSelection| flag on.

[yosin_UTC9, 2017/05/29 09:38:15]

How about SetSelectionStateAndPaintInvalidationFlag?
LayoutObject::ClearPaintInvalidationFlags() has
bitfiels_.SetShouldInvalidateSelection(false);

[yoichio, 2017/05/30 04:15:03]

Sine this is a local function and used only once, exact descriptive naming is 
 not needed.
Added some comment.

     const SelectionPaintRange& new_range,
-    SelectionPaintInvalidationMode block_paint_invalidation_mode) {
-  DCHECK(!new_range.IsNull());
-
-  // Just return if the selection hasn't changed.
-  if (paint_range_ == new_range)
-    return;
-
-  DCHECK(frame_selection_->GetDocument().GetLayoutView()->GetFrameView());
-  DCHECK(!frame_selection_->GetDocument().NeedsLayoutTreeUpdate());
-
+    const SelectionPaintRange& old_range) {
   SelectedMap old_selected_map =
-      CollectSelectedMap(paint_range_, block_paint_invalidation_mode);
+      CollectSelectedMap(old_range, CollectSelectedMapOption::kCollectBlock);
 
   // Now clear the selection.
   for (auto layout_object : old_selected_map.object_map.Keys())
@@ -248,7 +240,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(new_range, kPaintInvalidationNewXOROld);
+      CollectSelectedMap(new_range, CollectSelectedMapOption::kCollectBlock);
 
   // Have any of the old selected objects changed compared to the new selection?
   for (const auto& pair : old_selected_map.object_map) {
@@ -257,9 +249,9 @@ void LayoutSelection::SetSelection(
     SelectionState old_selection_state = pair.value;
     if (new_selection_state != old_selection_state ||
         (new_range.StartLayoutObject() == obj &&
-         new_range.StartOffset() != paint_range_.StartOffset()) ||
+         new_range.StartOffset() != old_range.StartOffset()) ||
         (new_range.EndLayoutObject() == obj &&
-         new_range.EndOffset() != paint_range_.EndOffset())) {
+         new_range.EndOffset() != old_range.EndOffset())) {
       obj->SetShouldInvalidateSelection();
       new_selected_map.object_map.erase(obj);
     }
@@ -285,8 +277,6 @@ void LayoutSelection::SetSelection(
   // they need to be updated.
   for (auto layout_object : new_selected_map.block_map.Keys())
     layout_object->SetShouldInvalidateSelection();
-
-  paint_range_ = new_range;
 }
 
 std::pair<int, int> LayoutSelection::SelectionStartEnd() {
@@ -306,8 +296,8 @@ void LayoutSelection::ClearSelection() {
   if (paint_range_.IsNull())
     return;
 
-  const SelectedMap& old_selected_map =
-      CollectSelectedMap(paint_range_, kPaintInvalidationNewMinusOld);
+  const SelectedMap& old_selected_map = CollectSelectedMap(
+      paint_range_, CollectSelectedMapOption::kNotCollectBlock);
   // Clear SelectionState and invalidation.
   for (auto layout_object : old_selected_map.object_map.Keys()) {
     const SelectionState old_state = layout_object->GetSelectionState();
@@ -369,9 +359,18 @@ void LayoutSelection::Commit() {
   if (!start_layout_object || !end_layout_object)
     return;
   DCHECK(start_layout_object->View() == end_layout_object->View());
-  SetSelection(
-      SelectionPaintRange(start_layout_object, start_pos.ComputeEditingOffset(),
-                          end_layout_object, end_pos.ComputeEditingOffset()));
+
+  SelectionPaintRange new_range(
+      start_layout_object, start_pos.ComputeEditingOffset(), end_layout_object,
+      end_pos.ComputeEditingOffset());
+  // Just return if the selection hasn't changed.
+  if (paint_range_ == new_range)
+    return;
+
+  DCHECK(frame_selection_->GetDocument().GetLayoutView()->GetFrameView());
+  DCHECK(!frame_selection_->GetDocument().NeedsLayoutTreeUpdate());
+  SetSelectionStateAndShouldInvalidateSelection(new_range, paint_range_);
+  paint_range_ = new_range;
 }
 
 void LayoutSelection::OnDocumentShutdown() {