Range: node offsets should be unsigned.

third_party/WebKit/Source/core/dom/RangeBoundaryPoint.h

Index: third_party/WebKit/Source/core/dom/RangeBoundaryPoint.h
diff --git a/third_party/WebKit/Source/core/dom/RangeBoundaryPoint.h b/third_party/WebKit/Source/core/dom/RangeBoundaryPoint.h
index 7b2dd719d8d1843ef53e23c69eb569d297ceaf31..32d2681fea6b9042c17366bdf35f5b4a1d17d74b 100644
--- a/third_party/WebKit/Source/core/dom/RangeBoundaryPoint.h
+++ b/third_party/WebKit/Source/core/dom/RangeBoundaryPoint.h
@@ -44,13 +44,13 @@ class RangeBoundaryPoint {
   const Position toPosition() const;
 
   Node* container() const;
-  int offset() const;
+  unsigned offset() const;
   Node* childBefore() const;
 
   void clear();
 
-  void set(Node* container, int offset, Node* childBefore);
-  void setOffset(int);
+  void set(Node* container, unsigned offset, Node* childBefore);
+  void setOffset(unsigned);
 
   void setToBeforeChild(Node&);
   void setToStartOfNode(Node&);
@@ -70,12 +70,12 @@ class RangeBoundaryPoint {
   void ensureOffsetIsValid() const;
   bool isOffsetValid() const;
 
-  static const int invalidOffset = -1;
+  static const unsigned invalidOffset = static_cast<unsigned>(-1);

[Sunny, 2017/02/21 12:32:05]

Hi tkent@, I've tried working on this issue before and have a question.

In this file do you think it's acceptable to cast -1 to unsigned as a indicator
of invalid offset?

My solution is use a boolean as indicator:
https://codereview.chromium.org/2673303002/diff/20001/third_party/WebKit/Sour...

Do you think it's unnecessary?

[tkent, 2017/02/22 02:08:43]

Adding a boolean flag is a more graceful way.
However, static_cast<unsigned>(-1) as an invalid value is more efficient than
the boolean flag in terms of memory consumption.
With the static_cast<unsigned>(-1), we can't handle offset=4294967295 correctly.
 But it's not a practical problem because Blink can't handle such large nodes
anyway.

 
   Member<Node> m_containerNode;
   Member<Node> m_childBeforeBoundary;
   mutable uint64_t m_domTreeVersion;
-  mutable int m_offsetInContainer;
+  mutable unsigned m_offsetInContainer;
 };
 
 inline RangeBoundaryPoint::RangeBoundaryPoint(Node* container)
@@ -135,7 +135,7 @@ inline const Position RangeBoundaryPoint::toPosition() const {
                                      m_offsetInContainer);
 }
 
-inline int RangeBoundaryPoint::offset() const {
+inline unsigned RangeBoundaryPoint::offset() const {
   ensureOffsetIsValid();
   return m_offsetInContainer;
 }
@@ -148,10 +148,10 @@ inline void RangeBoundaryPoint::clear() {
 }
 
 inline void RangeBoundaryPoint::set(Node* container,
-                                    int offset,
+                                    unsigned offset,
                                     Node* childBefore) {
   DCHECK(container);
-  DCHECK_GE(offset, 0);
+  DCHECK_GE(offset, 0u);
   DCHECK_EQ(childBefore,
             offset ? NodeTraversal::childAt(*container, offset - 1) : 0);
   m_containerNode = container;
@@ -160,10 +160,10 @@ inline void RangeBoundaryPoint::set(Node* container,
   markValid();
 }
 
-inline void RangeBoundaryPoint::setOffset(int offset) {
+inline void RangeBoundaryPoint::setOffset(unsigned offset) {
   DCHECK(m_containerNode);
   DCHECK(m_containerNode->isCharacterDataNode());
-  DCHECK_GE(m_offsetInContainer, 0);
+  DCHECK_GE(m_offsetInContainer, 0u);
   DCHECK(!m_childBeforeBoundary);
   m_offsetInContainer = offset;
   markValid();
@@ -200,7 +200,7 @@ inline void RangeBoundaryPoint::childBeforeWillBeRemoved() {
   m_childBeforeBoundary = m_childBeforeBoundary->previousSibling();
   if (!isOffsetValid())
     return;
-  DCHECK_GT(m_offsetInContainer, 0);
+  DCHECK_GT(m_offsetInContainer, 0u);
   if (!m_childBeforeBoundary)
     m_offsetInContainer = 0;
   else if (m_offsetInContainer > 0)