Introduce CollectSelectedMap() in LayoutSelection::SetSelection()

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

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009 Apple Inc. All rights
  * reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
@@ skipping 115 matching lines @@
 }
 
 // Objects each have a single selection rect to examine.
 using SelectedObjectMap = HashMap<LayoutObject*, SelectionState>;
 // Blocks contain selected objects and fill gaps between them, either on the
 // left, right, or in between lines and blocks.
 // In order to get the visual rect right, we have to examine left, middle, and
 // right rects individually, since otherwise the union of those rects might
 // remain the same even when changes have occurred.
 using SelectedBlockMap = HashMap<LayoutBlock*, SelectionState>;
-using SelectedMap = std::pair<SelectedObjectMap, SelectedBlockMap>;
+struct SelectedMap {
+  STACK_ALLOCATED();
+
+  SelectedMap() {}

[yosin_UTC9, 2017/05/12 05:16:11]

nit: s/{}/= default/

[yoichio, 2017/05/12 05:46:43]

Done.

+  SelectedMap(SelectedMap&& other) {
+    object_map = std::move(other.object_map);
+    block_map = std::move(other.block_map);
+  }
+  SelectedObjectMap object_map;

[yosin_UTC9, 2017/05/12 05:16:11]

Please move member variable declaration before ctor's.

e.g. From "Node.h"

 struct AttachContext {
    STACK_ALLOCATED();
    ComputedStyle* resolved_style = nullptr;
    bool performing_reattach = false;
    bool clear_invalidation = false;

    AttachContext() {}
  };

[yoichio, 2017/05/12 05:46:43]

Done.

+  SelectedBlockMap block_map;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(SelectedMap);
+};
+
+static SelectedMap CollectSelectedMap(
+    LayoutObject* selection_start,
+    LayoutObject* selection_end,
+    int selection_end_pos,
+    LayoutSelection::SelectionPaintInvalidationMode
+        block_paint_invalidation_mode =
+            LayoutSelection::kPaintInvalidationNewXOROld) {

[yosin_UTC9, 2017/05/12 05:16:11]

Since |CollectSelectedMap()| is used in two places, 
please avoid to use default parameter for ease of reading
e.g. when we see a call site using default parameter, we
may not know |CollectSelectedMap()| does XOR.

[yoichio, 2017/05/12 05:46:43]

Done.

+  SelectedMap selected_map;
+  LayoutObject* iter = selection_start;

[yosin_UTC9, 2017/05/12 05:16:11]

nit: s/iter/runner/ or s/iter/layout_object/
Blink doesn't prefer abbreviation

[yoichio, 2017/05/12 05:46:43]

Done.

+  LayoutObject* const stop =
+      LayoutObjectAfterPosition(selection_end, selection_end_pos);
+  bool exploring_backwards = false;
+  bool continue_exploring = iter && (iter != stop);
+  while (continue_exploring) {
+    if ((iter->CanBeSelectionLeaf() || iter == selection_start ||
+         iter == selection_end) &&
+        iter->GetSelectionState() != SelectionNone) {
+      // Blocks are responsible for painting line gaps and margin gaps.  They
+      // must be examined as well.
+      selected_map.object_map.Set(iter, iter->GetSelectionState());
+      if (block_paint_invalidation_mode ==
+          LayoutSelection::kPaintInvalidationNewXOROld) {
+        LayoutBlock* containing_block = iter->ContainingBlock();
+        while (containing_block && !containing_block->IsLayoutView()) {
+          SelectedBlockMap::AddResult result = selected_map.block_map.insert(
+              containing_block, containing_block->GetSelectionState());
+          if (!result.is_new_entry)
+            break;
+          containing_block = containing_block->ContainingBlock();
+        }
+      }
+    }
+
+    iter = GetNextOrPrevLayoutObjectBasedOnDirection(
+        iter, stop, continue_exploring, exploring_backwards);
+  }
+  return selected_map;
+}
 
 void LayoutSelection::SetSelection(
     LayoutObject* start,
     int start_pos,
     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.
@@ skipping 10 matching lines @@
       selection_end_ == end && selection_end_pos_ == end_pos)
     return;
 
   DCHECK(frame_selection_->GetDocument().GetLayoutView()->GetFrameView());
 
   // Record the old selected objects. These will be used later when we compare
   // against the new selected objects.
   int old_start_pos = selection_start_pos_;
   int old_end_pos = selection_end_pos_;
 
-  SelectedMap old_selected_map;
-  LayoutObject* os = selection_start_;
-  LayoutObject* stop =
-      LayoutObjectAfterPosition(selection_end_, selection_end_pos_);
-  bool exploring_backwards = false;
-  bool continue_exploring = os && (os != stop);
-  while (continue_exploring) {
-    if ((os->CanBeSelectionLeaf() || os == selection_start_ ||
-         os == selection_end_) &&
-        os->GetSelectionState() != SelectionNone) {
-      // Blocks are responsible for painting line gaps and margin gaps.  They
-      // must be examined as well.
-      old_selected_map.first.Set(os, os->GetSelectionState());
-      if (block_paint_invalidation_mode == kPaintInvalidationNewXOROld) {
-        LayoutBlock* cb = os->ContainingBlock();
-        while (cb && !cb->IsLayoutView()) {
-          SelectedBlockMap::AddResult result =
-              old_selected_map.second.insert(cb, cb->GetSelectionState());
-          if (!result.is_new_entry)
-            break;
-          cb = cb->ContainingBlock();
-        }
-      }
-    }
-
-    os = GetNextOrPrevLayoutObjectBasedOnDirection(os, stop, continue_exploring,
-                                                   exploring_backwards);
-  }
+  SelectedMap old_selected_map =
+      CollectSelectedMap(selection_start_, selection_end_, selection_end_pos_,
+                         block_paint_invalidation_mode);
 
   // Now clear the selection.
-  SelectedObjectMap::iterator old_objects_end = old_selected_map.first.end();
-  for (SelectedObjectMap::iterator i = old_selected_map.first.begin();
+  SelectedObjectMap::iterator old_objects_end =
+      old_selected_map.object_map.end();
+  for (SelectedObjectMap::iterator i = old_selected_map.object_map.begin();
        i != old_objects_end; ++i)
     i->key->SetSelectionStateIfNeeded(SelectionNone);
 
   // set selection start and end
   selection_start_ = start;
   selection_start_pos_ = start_pos;
   selection_end_ = end;
   selection_end_pos_ = end_pos;
 
   // Update the selection status of all objects between m_selectionStart and
   // m_selectionEnd
   if (start && start == end) {
     start->SetSelectionStateIfNeeded(SelectionBoth);
   } else {
     if (start)
       start->SetSelectionStateIfNeeded(SelectionStart);
     if (end)
       end->SetSelectionStateIfNeeded(SelectionEnd);
   }
 
   LayoutObject* o = start;
-  stop = LayoutObjectAfterPosition(end, end_pos);
+  LayoutObject* const stop = LayoutObjectAfterPosition(end, end_pos);
 
   while (o && o != stop) {
     if (o != start && o != end && o->CanBeSelectionLeaf())
       o->SetSelectionStateIfNeeded(SelectionInside);
     o = o->NextInPreOrder();
   }
 
   // Now that the selection state has been updated for the new objects, walk
   // them again and put them in the new objects list.
-  // FIXME: |new_selected_map| doesn't really need to store the
+  // TODO(editing-dev): |new_selected_map| doesn't really need to store the
   // SelectionState, it's just more convenient to have it use the same data
   // structure as |old_selected_map|.
-  SelectedMap new_selected_map;
-  o = start;
-  exploring_backwards = false;
-  continue_exploring = o && (o != stop);
-  while (continue_exploring) {
-    if ((o->CanBeSelectionLeaf() || o == start || o == end) &&
-        o->GetSelectionState() != SelectionNone) {
-      new_selected_map.first.Set(o, o->GetSelectionState());
-      LayoutBlock* cb = o->ContainingBlock();
-      while (cb && !cb->IsLayoutView()) {
-        SelectedBlockMap::AddResult result =
-            new_selected_map.second.insert(cb, cb->GetSelectionState());
-        if (!result.is_new_entry)
-          break;
-        cb = cb->ContainingBlock();
-      }
-    }
-
-    o = GetNextOrPrevLayoutObjectBasedOnDirection(o, stop, continue_exploring,
-                                                  exploring_backwards);
-  }
+  SelectedMap new_selected_map = CollectSelectedMap(start, end, end_pos);
 
   // Have any of the old selected objects changed compared to the new selection?
-  for (SelectedObjectMap::iterator i = old_selected_map.first.begin();
+  for (SelectedObjectMap::iterator i = old_selected_map.object_map.begin();
        i != old_objects_end; ++i) {
     LayoutObject* obj = i->key;
     SelectionState new_selection_state = obj->GetSelectionState();
     SelectionState old_selection_state = i->value;
     if (new_selection_state != old_selection_state ||
         (selection_start_ == obj && old_start_pos != selection_start_pos_) ||
         (selection_end_ == obj && old_end_pos != selection_end_pos_)) {
       obj->SetShouldInvalidateSelection();
-      new_selected_map.first.erase(obj);
+      new_selected_map.object_map.erase(obj);
     }
   }
 
   // Any new objects that remain were not found in the old objects dict, and so
   // they need to be updated.
-  SelectedObjectMap::iterator new_objects_end = new_selected_map.first.end();
-  for (SelectedObjectMap::iterator i = new_selected_map.first.begin();
+  SelectedObjectMap::iterator new_objects_end =
+      new_selected_map.object_map.end();
+  for (SelectedObjectMap::iterator i = new_selected_map.object_map.begin();
        i != new_objects_end; ++i)
     i->key->SetShouldInvalidateSelection();
 
   // Have any of the old blocks changed?
-  SelectedBlockMap::iterator old_blocks_end = old_selected_map.second.end();
-  for (SelectedBlockMap::iterator i = old_selected_map.second.begin();
+  SelectedBlockMap::iterator old_blocks_end = old_selected_map.block_map.end();
+  for (SelectedBlockMap::iterator i = old_selected_map.block_map.begin();
        i != old_blocks_end; ++i) {
     LayoutBlock* block = i->key;
     SelectionState new_selection_state = block->GetSelectionState();
     SelectionState old_selection_state = i->value;
     if (new_selection_state != old_selection_state) {
       block->SetShouldInvalidateSelection();
-      new_selected_map.second.erase(block);
+      new_selected_map.block_map.erase(block);
     }
   }
 
   // Any new blocks that remain were not found in the old blocks dict, and so
   // they need to be updated.
-  SelectedBlockMap::iterator new_blocks_end = new_selected_map.second.end();
-  for (SelectedBlockMap::iterator i = new_selected_map.second.begin();
+  SelectedBlockMap::iterator new_blocks_end = new_selected_map.block_map.end();
+  for (SelectedBlockMap::iterator i = new_selected_map.block_map.begin();
        i != new_blocks_end; ++i)
     i->key->SetShouldInvalidateSelection();
 }
 
 std::pair<int, int> LayoutSelection::SelectionStartEnd() {
   Commit();
   return std::make_pair(selection_start_pos_, selection_end_pos_);
 }
 
 void LayoutSelection::ClearSelection() {
@@ skipping 122 matching lines @@
 
     o->SetShouldInvalidateSelection();
   }
 }
 
 DEFINE_TRACE(LayoutSelection) {
   visitor->Trace(frame_selection_);
 }
 
 }  // namespace blink