Introduce range-based for loop in LayoutSelection.

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 56 matching lines @@
 LayoutObject* SelectionPaintRange::EndLayoutObject() const {
   DCHECK(!IsNull());
   return end_layout_object_;
 }
 
 int SelectionPaintRange::EndOffset() const {
   DCHECK(!IsNull());
   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),
       paint_range_(SelectionPaintRange()) {}
 
 SelectionInFlatTree LayoutSelection::CalcVisibleSelection(
     const VisibleSelectionInFlatTree& original_selection) const {
   const PositionInFlatTree& start = original_selection.Start();
   const PositionInFlatTree& end = original_selection.end();
   SelectionType selection_type = original_selection.GetSelectionType();
@@ skipping 36 matching lines @@
                ? TextAffinity::kUpstream
                : affinity);
   if (visible_end.IsNull())
     return SelectionInFlatTree();
   SelectionInFlatTree::Builder builder;
   builder.Collapse(visible_start.ToPositionWithAffinity());
   builder.Extend(visible_end.DeepEquivalent());
   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>;
 // 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>;
 struct SelectedMap {
@@ skipping 16 matching lines @@
   kNotCollectBlock,
 };
 
 static SelectedMap CollectSelectedMap(const SelectionPaintRange& range,
                                       CollectSelectedMapOption option) {
   if (range.IsNull())
     return SelectedMap();
 
   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;
 
     // 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 (option == CollectSelectedMapOption::kCollectBlock) {
       LayoutBlock* containing_block = runner->ContainingBlock();
       while (containing_block && !containing_block->IsLayoutView()) {
         SelectedBlockMap::AddResult result = selected_map.block_map.insert(
@@ skipping 14 matching lines @@
 
   if (range.StartLayoutObject() == range.EndLayoutObject()) {
     range.StartLayoutObject()->SetSelectionStateIfNeeded(
         SelectionState::kStartAndEnd);
   } else {
     range.StartLayoutObject()->SetSelectionStateIfNeeded(
         SelectionState::kStart);
     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);
   }
 }
 
 // Set SetSelectionState and ShouldInvalidateSelection flag of LayoutObjects
 // comparing them in |new_range| and |old_range|.
 static void UpdateLayoutObjectState(const SelectionPaintRange& new_range,
                                     const SelectionPaintRange& old_range) {
@@ skipping 160 matching lines @@
   // Now create a single bounding box rect that encloses the whole selection.
   LayoutRect sel_rect;
 
   typedef HashSet<const LayoutBlock*> VisitedContainingBlockSet;
   VisitedContainingBlockSet visited_containing_blocks;
 
   Commit();
   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;
+
+    // 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();
     }
-
-    os = os->NextInPreOrder();
   }
 
   return PixelSnappedIntRect(sel_rect);
 }
 
 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();
   }
 }
 
 DEFINE_TRACE(LayoutSelection) {
   visitor->Trace(frame_selection_);
 }
 
 }  // namespace blink