Introduce range-based for loop 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 2467fa86c10c5a12a722b27791feae251433bad8..e6cad8bf8b691c01a34a4b6ae10920eb8429e739 100644
--- a/third_party/WebKit/Source/core/editing/LayoutSelection.cpp
+++ b/third_party/WebKit/Source/core/editing/LayoutSelection.cpp
@@ -74,6 +74,31 @@ int SelectionPaintRange::EndOffset() const {
   return end_offset_;
 }
 
+SelectionPaintRange::Iterator::Iterator(const SelectionPaintRange* range) {
+  if (!range) {
+    current_ = nullptr;
+    return;
+  }
+  current_ = range->StartLayoutObject();
+  included_end_ = range->EndLayoutObject();
+  stop_ = range->EndLayoutObject()->ChildAt(range->EndOffset());
+  if (!stop_)

[yosin_UTC9, 2017/06/02 08:35:44]

nit: prefer early-retrun

if (stop_)
  return;
stop_ = range->EndLayoutObject()->NextInPreOrderAfterChildren();

[yoichio, 2017/06/12 06:12:51]

Done.

+    stop_ = range->EndLayoutObject()->NextInPreOrderAfterChildren();
+}
+
+SelectionPaintRange::Iterator& SelectionPaintRange::Iterator::operator++() {
+  DCHECK(current_);
+  current_ = current_->NextInPreOrder();

[yosin_UTC9, 2017/06/02 08:35:44]

Let's use for-statement

for(current_ = current_->NextInPreOrder();
    current_ && current_ != stop;
    current_ = current_->NextInPreOrder()) {
    if (current_ == included_end_ || current_->CanBeSelectionLeaf())
      return *this;
}

[yoichio, 2017/06/12 06:12:51]

Done.

+  while (current_ && current_ != stop_) {
+    if (current_ == included_end_ || current_->CanBeSelectionLeaf())
+      return *this;
+    current_ = current_->NextInPreOrder();
+  }
+
+  current_ = nullptr;
+  return *this;
+}
+
 LayoutSelection::LayoutSelection(FrameSelection& frame_selection)
     : frame_selection_(&frame_selection),
       has_pending_selection_(false),
@@ -130,14 +155,7 @@ SelectionInFlatTree LayoutSelection::CalcVisibleSelection(
   return builder.Build();
 }
 
-static LayoutObject* LayoutObjectAfterPosition(LayoutObject* object,
-                                               unsigned offset) {
-  if (!object)
-    return nullptr;
 
-  LayoutObject* child = object->ChildAt(offset);
-  return child ? child : object->NextInPreOrderAfterChildren();
-}
 
 // Objects each have a single selection rect to examine.
 using SelectedObjectMap = HashMap<LayoutObject*, SelectionState>;
@@ -174,13 +192,7 @@ static SelectedMap CollectSelectedMap(const SelectionPaintRange& range,
 
   SelectedMap selected_map;
 
-  LayoutObject* const stop =
-      LayoutObjectAfterPosition(range.EndLayoutObject(), range.EndOffset());
-  for (LayoutObject* runner = range.StartLayoutObject();
-       runner && (runner != stop); runner = runner->NextInPreOrder()) {
-    if (!runner->CanBeSelectionLeaf() && runner != range.StartLayoutObject() &&
-        runner != range.EndLayoutObject())
-      continue;
+  for (LayoutObject* runner : range) {
     if (runner->GetSelectionState() == SelectionState::kNone)
       continue;
 
@@ -215,10 +227,7 @@ static void SetSelectionState(const SelectionPaintRange& range) {
     range.EndLayoutObject()->SetSelectionStateIfNeeded(SelectionState::kEnd);
   }
 
-  LayoutObject* const stop =
-      LayoutObjectAfterPosition(range.EndLayoutObject(), range.EndOffset());
-  for (LayoutObject* runner = range.StartLayoutObject();
-       runner && runner != stop; runner = runner->NextInPreOrder()) {
+  for (LayoutObject* runner : range) {
     if (runner != range.StartLayoutObject() &&
         runner != range.EndLayoutObject() && runner->CanBeSelectionLeaf())
       runner->SetSelectionStateIfNeeded(SelectionState::kInside);
@@ -399,28 +408,22 @@ IntRect LayoutSelection::SelectionBounds() {
   if (paint_range_.IsNull())
     return IntRect();
 
-  LayoutObject* os = paint_range_.StartLayoutObject();
-  LayoutObject* stop = LayoutObjectAfterPosition(paint_range_.EndLayoutObject(),
-                                                 paint_range_.EndOffset());
-  while (os && os != stop) {
-    if ((os->CanBeSelectionLeaf() || os == paint_range_.StartLayoutObject() ||
-         os == paint_range_.EndLayoutObject()) &&
-        os->GetSelectionState() != SelectionState::kNone) {
-      // Blocks are responsible for painting line gaps and margin gaps. They
-      // must be examined as well.
-      sel_rect.Unite(SelectionRectForLayoutObject(os));
-      const LayoutBlock* cb = os->ContainingBlock();
-      while (cb && !cb->IsLayoutView()) {
-        sel_rect.Unite(SelectionRectForLayoutObject(cb));
-        VisitedContainingBlockSet::AddResult add_result =
-            visited_containing_blocks.insert(cb);
-        if (!add_result.is_new_entry)
-          break;
-        cb = cb->ContainingBlock();
-      }
-    }
+  for (LayoutObject* runner : paint_range_) {
+    if (runner->GetSelectionState() == SelectionState::kNone)
+      continue;
 
-    os = os->NextInPreOrder();
+    // Blocks are responsible for painting line gaps and margin gaps. They
+    // must be examined as well.
+    sel_rect.Unite(SelectionRectForLayoutObject(runner));
+    const LayoutBlock* cb = runner->ContainingBlock();

[yosin_UTC9, 2017/06/02 08:35:44]

Let't use for-statement

for (const LayoutBlock* cb = runner->ContainingBlock();
     cb && !cb->IsLayoutView();
     cb = cb->ContainingBlock()) {
  sel_rect.Unite(SelectionRectForLayoutObject(cb));
  VisitedContainingBlockSet::AddResult add_result =
visited_containing_blocks.insert(cb);
  if (!add_result.is_new_entry)
    break;
}

Could you use descriptive name instead of |cb|?
Blink doesn't use abbreviation.

[yoichio, 2017/06/12 06:12:51]

I will use CollectSelectedMap and remove these lines.

+    while (cb && !cb->IsLayoutView()) {
+      sel_rect.Unite(SelectionRectForLayoutObject(cb));
+      VisitedContainingBlockSet::AddResult add_result =
+          visited_containing_blocks.insert(cb);
+      if (!add_result.is_new_entry)
+        break;
+      cb = cb->ContainingBlock();
+    }
   }
 
   return PixelSnappedIntRect(sel_rect);
@@ -430,17 +433,11 @@ void LayoutSelection::InvalidatePaintForSelection() {
   if (paint_range_.IsNull())
     return;
 
-  LayoutObject* end = LayoutObjectAfterPosition(paint_range_.EndLayoutObject(),
-                                                paint_range_.EndOffset());
-  for (LayoutObject* o = paint_range_.StartLayoutObject(); o && o != end;
-       o = o->NextInPreOrder()) {
-    if (!o->CanBeSelectionLeaf() && o != paint_range_.StartLayoutObject() &&
-        o != paint_range_.EndLayoutObject())
-      continue;
-    if (o->GetSelectionState() == SelectionState::kNone)
+  for (LayoutObject* runner : paint_range_) {
+    if (runner->GetSelectionState() == SelectionState::kNone)
       continue;
 
-    o->SetShouldInvalidateSelection();
+    runner->SetShouldInvalidateSelection();
   }
 }