Introduce SelectionPaintRange in LayoutSelection

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

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  * Copyright (C) 2006 Apple Computer, Inc.
  *
  * 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.
  *
  * This library is distributed in the hope that it will be useful,
@@ skipping 12 matching lines @@
 #define LayoutSelection_h
 
 #include "core/editing/Position.h"
 #include "core/editing/VisibleSelection.h"
 #include "platform/heap/Handle.h"
 
 namespace blink {
 
 class FrameSelection;
 
+struct SelectionPaintRange {

[yosin_UTC9, 2017/05/24 08:38:46]

I think it is better to use |class| and make this object as *virtual* immutable.
Once we constructor this object, we don't need to change members.

[yoichio, 2017/05/24 09:09:29]

Done.

+  DISALLOW_NEW();
+  // The current selection represented as 2 boundaries.
+  // Selection boundaries are represented in LayoutView by a tuple
+  // (LayoutObject, DOM node offset).
+  // See http://www.w3.org/TR/dom/#range for more information.
+  //
+  // |start_layout_object_| and |end_layout_object_| are only valid for
+  // |Text| node without 'transform' or 'first-letter'.
+  //
+  // Those are used for selection painting and paint invalidation upon
+  // selection change.
+  // TODO(editing-dev): Clarify the meaning of |start_offset_| and
+  // |end_offset_|. editing/ passes them as offsets in the DOM tree  but layout
+  // uses them as offset in the layout tree.
+  LayoutObject* start_layout_object_;

[yosin_UTC9, 2017/05/24 08:38:46]

nit: Member variable of |struct| should not end with "_".

[yoichio, 2017/05/24 09:09:29]

Acknowledged.

+  int start_offset_;
+  LayoutObject* end_layout_object_;
+  int end_offset_;
+
+  SelectionPaintRange()
+      : start_layout_object_(nullptr),

[yosin_UTC9, 2017/05/24 08:38:46]

nit: Let's use inline initialization, e.g.

LayoutObject* start_layout_object_ = nullptr;
...

Then we can use |= default;|

[yoichio, 2017/05/24 09:09:29]

Done.

+        start_offset_(-1),
+        end_layout_object_(nullptr),
+        end_offset_(-1) {}
+  SelectionPaintRange(LayoutObject* start_layout_object,
+                      int start_offset,
+                      LayoutObject* end_layout_object,
+                      int end_offset)
+      : start_layout_object_(start_layout_object),
+        start_offset_(start_offset),
+        end_layout_object_(end_layout_object),
+        end_offset_(end_offset){};

[yosin_UTC9, 2017/05/24 08:38:46]

nit: s/{}/ {}/

We should have DCHECK for parameters.
DCHECK(start_layout_object);
DCHECK(end_layout_object);
DCHECK_GE(start_offset, 0);
DCHECK_GE(end_offset, 0);
if (start_layout_object == end_layout_object)
  DCHECK_LT(start_offset, end_offset);

This ctor should be ".cpp" file.

[yoichio, 2017/05/24 09:09:29]

Done.

+
+  bool operator==(const SelectionPaintRange& other) {

[yosin_UTC9, 2017/05/24 08:38:46]

nit: s/{/ const {/

This function should be ".cpp" file.

[yoichio, 2017/05/24 09:09:29]

Done.

+    return start_layout_object_ == other.start_layout_object_ &&
+           start_offset_ == other.start_offset_ &&
+           end_layout_object_ == other.end_layout_object_ &&
+           end_offset_ == other.end_offset_;
+  }
+};
+
 class LayoutSelection final : public GarbageCollected<LayoutSelection> {
  public:
   static LayoutSelection* Create(FrameSelection& frame_selection) {
     return new LayoutSelection(frame_selection);
   }
 
   bool HasPendingSelection() const { return has_pending_selection_; }
   void SetHasPendingSelection() { has_pending_selection_ = true; }
   void Commit();
 
   IntRect SelectionBounds();
   void InvalidatePaintForSelection();
   enum SelectionPaintInvalidationMode {
     kPaintInvalidationNewXOROld,
     kPaintInvalidationNewMinusOld
   };
   void SetSelection(
-      LayoutObject* start,
-      int start_pos,
-      LayoutObject*,
-      int end_pos,
+      const SelectionPaintRange&,
       SelectionPaintInvalidationMode = kPaintInvalidationNewXOROld);
   void ClearSelection();
   std::pair<int, int> SelectionStartEnd();
   void OnDocumentShutdown();
 
   DECLARE_TRACE();
 
  private:
   LayoutSelection(FrameSelection&);
 
   SelectionInFlatTree CalcVisibleSelection(
       const VisibleSelectionInFlatTree&) const;
 
   Member<FrameSelection> frame_selection_;
   bool has_pending_selection_ : 1;
 
-  // The current selection represented as 2 boundaries.
-  // Selection boundaries are represented in LayoutView by a tuple
-  // (LayoutObject, DOM node offset).
-  // See http://www.w3.org/TR/dom/#range for more information.
-  //
-  // |m_selectionStartPos| and |m_selectionEndPos| are only valid for
-  // |Text| node without 'transform' or 'first-letter'.
-  //
-  // Those are used for selection painting and paint invalidation upon
-  // selection change.
-  LayoutObject* selection_start_;
-  LayoutObject* selection_end_;
-
-  // TODO(yosin): Clarify the meaning of these variables. editing/ passes
-  // them as offsets in the DOM tree  but layout uses them as offset in the
-  // layout tree.
-  int selection_start_pos_;
-  int selection_end_pos_;
+  SelectionPaintRange paint_range_;
 };
 
 }  // namespace blink
 
 #endif