Bug 1218032 part.1 nsContentIterator should warn if unexpected case occurs r=smaug
authorMasayuki Nakano <masayuki@d-toybox.com>
Tue, 10 Nov 2015 11:49:04 +0900
changeset 305993 1cda7977a8f1b346c9334b096a94389ff09d8381
parent 305992 48f644f1c41e1e1d65b5a05c7bfcda4865b4288f
child 305994 2cb75cbd2d6b2c3668cdc5b51a59c131ce1434a1
push idunknown
push userunknown
push dateunknown
reviewerssmaug
bugs1218032
milestone45.0a1
Bug 1218032 part.1 nsContentIterator should warn if unexpected case occurs r=smaug
dom/base/nsContentIterator.cpp
--- a/dom/base/nsContentIterator.cpp
+++ b/dom/base/nsContentIterator.cpp
@@ -25,31 +25,32 @@ static nsINode*
 NodeToParentOffset(nsINode* aNode, int32_t* aOffset)
 {
   *aOffset = 0;
 
   nsINode* parent = aNode->GetParentNode();
 
   if (parent) {
     *aOffset = parent->IndexOf(aNode);
+    NS_WARN_IF(*aOffset < 0);
   }
 
   return parent;
 }
 
 ///////////////////////////////////////////////////////////////////////////
 // NodeIsInTraversalRange: returns true if content is visited during
 // the traversal of the range in the specified mode.
 //
 static bool
 NodeIsInTraversalRange(nsINode* aNode, bool aIsPreMode,
                        nsINode* aStartNode, int32_t aStartOffset,
                        nsINode* aEndNode, int32_t aEndOffset)
 {
-  if (!aStartNode || !aEndNode || !aNode) {
+  if (NS_WARN_IF(!aStartNode) || NS_WARN_IF(!aEndNode) || NS_WARN_IF(!aNode)) {
     return false;
   }
 
   // If a leaf node contains an end point of the traversal range, it is
   // always in the traversal range.
   if (aNode == aStartNode || aNode == aEndNode) {
     if (aNode->IsNodeOfType(nsINode::eDATA_NODE)) {
       return true; // text node or something
@@ -66,16 +67,17 @@ NodeIsInTraversalRange(nsINode* aNode, b
   }
 
   nsINode* parent = aNode->GetParentNode();
   if (!parent) {
     return false;
   }
 
   int32_t indx = parent->IndexOf(aNode);
+  NS_WARN_IF(indx == -1);
 
   if (!aIsPreMode) {
     ++indx;
   }
 
   return nsContentUtils::ComparePoints(aStartNode, aStartOffset,
                                        parent, indx) <= 0 &&
          nsContentUtils::ComparePoints(aEndNode, aEndOffset,
@@ -257,58 +259,70 @@ nsContentIterator::~nsContentIterator()
 /******************************************************
  * Init routines
  ******************************************************/
 
 
 nsresult
 nsContentIterator::Init(nsINode* aRoot)
 {
-  if (!aRoot) {
+  if (NS_WARN_IF(!aRoot)) {
     return NS_ERROR_NULL_POINTER;
   }
 
   mIsDone = false;
   mIndexes.Clear();
 
   if (mPre) {
     mFirst = aRoot;
     mLast  = GetDeepLastChild(aRoot);
+    NS_WARN_IF(!mLast);
   } else {
     mFirst = GetDeepFirstChild(aRoot);
+    NS_WARN_IF(!mFirst);
     mLast  = aRoot;
   }
 
   mCommonParent = aRoot;
   mCurNode = mFirst;
   RebuildIndexStack();
   return NS_OK;
 }
 
 nsresult
 nsContentIterator::Init(nsIDOMRange* aDOMRange)
 {
-  NS_ENSURE_ARG_POINTER(aDOMRange);
+  if (NS_WARN_IF(!aDOMRange)) {
+    return NS_ERROR_INVALID_ARG;
+  }
   nsRange* range = static_cast<nsRange*>(aDOMRange);
 
   mIsDone = false;
 
   // get common content parent
   mCommonParent = range->GetCommonAncestor();
-  NS_ENSURE_TRUE(mCommonParent, NS_ERROR_FAILURE);
+  if (NS_WARN_IF(!mCommonParent)) {
+    return NS_ERROR_FAILURE;
+  }
 
   // get the start node and offset
   int32_t startIndx = range->StartOffset();
+  NS_WARN_IF(startIndx < 0);
   nsINode* startNode = range->GetStartParent();
-  NS_ENSURE_TRUE(startNode, NS_ERROR_FAILURE);
+  if (NS_WARN_IF(!startNode)) {
+    return NS_ERROR_FAILURE;
+  }
 
   // get the end node and offset
   int32_t endIndx = range->EndOffset();
+  NS_WARN_IF(endIndx < 0);
   nsINode* endNode = range->GetEndParent();
-  NS_ENSURE_TRUE(endNode, NS_ERROR_FAILURE);
+  if (NS_WARN_IF(!endNode)) {
+    return NS_ERROR_FAILURE;
+  }
 
   bool startIsData = startNode->IsNodeOfType(nsINode::eDATA_NODE);
 
   // short circuit when start node == end node
   if (startNode == endNode) {
     // Check to see if we have a collapsed range, if so, there is nothing to
     // iterate over.
     //
@@ -322,27 +336,29 @@ nsContentIterator::Init(nsIDOMRange* aDO
     }
 
     if (startIsData) {
       // It's a character data node.
       mFirst   = startNode->AsContent();
       mLast    = mFirst;
       mCurNode = mFirst;
 
-      RebuildIndexStack();
+      nsresult rv = RebuildIndexStack();
+      NS_WARN_IF(NS_FAILED(rv));
       return NS_OK;
     }
   }
 
   // Find first node in range.
 
   nsIContent* cChild = nullptr;
 
   if (!startIsData && startNode->HasChildren()) {
     cChild = startNode->GetChildAt(startIndx);
+    NS_WARN_IF(!cChild);
   }
 
   if (!cChild) {
     // no children, must be a text node
     //
     // XXXbz no children might also just mean no children.  So I'm not
     // sure what that comment above is talking about.
 
@@ -351,131 +367,142 @@ nsContentIterator::Init(nsIDOMRange* aDO
       //      character in the cdata node, should we set mFirst to
       //      the next sibling?
 
       // If the node has no child, the child may be <br> or something.
       // So, we shouldn't skip the empty node if the start offset is 0.
       // In other words, if the offset is 1, the node should be ignored.
       if (!startIsData && startIndx) {
         mFirst = GetNextSibling(startNode);
+        NS_WARN_IF(!mFirst);
 
         // Does mFirst node really intersect the range?  The range could be
         // 'degenerate', i.e., not collapsed but still contain no content.
-        if (mFirst && !NodeIsInTraversalRange(mFirst, mPre, startNode,
-                                              startIndx, endNode, endIndx)) {
+        if (mFirst &&
+            NS_WARN_IF(!NodeIsInTraversalRange(mFirst, mPre, startNode,
+                                               startIndx, endNode, endIndx))) {
           mFirst = nullptr;
         }
       } else {
         mFirst = startNode->AsContent();
       }
     } else {
       // post-order
-      if (startNode->IsContent()) {
-        mFirst = startNode->AsContent();
-      } else {
+      if (NS_WARN_IF(!startNode->IsContent())) {
         // What else can we do?
         mFirst = nullptr;
+      } else {
+        mFirst = startNode->AsContent();
       }
     }
   } else {
     if (mPre) {
       mFirst = cChild;
     } else {
       // post-order
       mFirst = GetDeepFirstChild(cChild);
+      NS_WARN_IF(!mFirst);
 
       // Does mFirst node really intersect the range?  The range could be
       // 'degenerate', i.e., not collapsed but still contain no content.
 
-      if (mFirst && !NodeIsInTraversalRange(mFirst, mPre, startNode, startIndx,
-                                            endNode, endIndx)) {
+      if (mFirst &&
+          NS_WARN_IF(!NodeIsInTraversalRange(mFirst, mPre, startNode, startIndx,
+                                             endNode, endIndx))) {
         mFirst = nullptr;
       }
     }
   }
 
 
   // Find last node in range.
 
   bool endIsData = endNode->IsNodeOfType(nsINode::eDATA_NODE);
 
   if (endIsData || !endNode->HasChildren() || endIndx == 0) {
     if (mPre) {
-      if (endNode->IsContent()) {
+      if (NS_WARN_IF(!endNode->IsContent())) {
+        // Not much else to do here...
+        mLast = nullptr;
+      } else {
         // If the end node is an empty element and the end offset is 0,
         // the last element should be the previous node (i.e., shouldn't
         // include the end node in the range).
         if (!endIsData && !endNode->HasChildren() && !endIndx) {
           mLast = GetPrevSibling(endNode);
-          if (!NodeIsInTraversalRange(mLast, mPre, startNode, startIndx,
-                                      endNode, endIndx)) {
+          NS_WARN_IF(!mLast);
+          if (NS_WARN_IF(!NodeIsInTraversalRange(mLast, mPre,
+                                                 startNode, startIndx,
+                                                 endNode, endIndx))) {
             mLast = nullptr;
           }
         } else {
           mLast = endNode->AsContent();
         }
-      } else {
-        // Not much else to do here...
-        mLast = nullptr;
       }
     } else {
       // post-order
       //
       // XXX: In the future, if end offset is before the first character in the
       //      cdata node, should we set mLast to the prev sibling?
 
       if (!endIsData) {
         mLast = GetPrevSibling(endNode);
+        NS_WARN_IF(!mLast);
 
-        if (!NodeIsInTraversalRange(mLast, mPre, startNode, startIndx,
-                                    endNode, endIndx)) {
+        if (NS_WARN_IF(!NodeIsInTraversalRange(mLast, mPre,
+                                               startNode, startIndx,
+                                               endNode, endIndx))) {
           mLast = nullptr;
         }
       } else {
         mLast = endNode->AsContent();
       }
     }
   } else {
     int32_t indx = endIndx;
 
     cChild = endNode->GetChildAt(--indx);
 
-    if (!cChild) {
+    if (NS_WARN_IF(!cChild)) {
       // No child at offset!
       NS_NOTREACHED("nsContentIterator::nsContentIterator");
       return NS_ERROR_FAILURE;
     }
 
     if (mPre) {
       mLast  = GetDeepLastChild(cChild);
+      NS_WARN_IF(!mLast);
 
-      if (!NodeIsInTraversalRange(mLast, mPre, startNode, startIndx,
-                                  endNode, endIndx)) {
+      if (NS_WARN_IF(!NodeIsInTraversalRange(mLast, mPre,
+                                             startNode, startIndx,
+                                             endNode, endIndx))) {
         mLast = nullptr;
       }
     } else {
       // post-order
       mLast = cChild;
     }
   }
 
   // If either first or last is null, they both have to be null!
 
-  if (!mFirst || !mLast) {
+  if (NS_WARN_IF(!mFirst) || NS_WARN_IF(!mLast)) {
     mFirst = nullptr;
     mLast  = nullptr;
   }
 
   mCurNode = mFirst;
   mIsDone  = !mCurNode;
 
   if (!mCurNode) {
     mIndexes.Clear();
   } else {
-    RebuildIndexStack();
+    nsresult rv = RebuildIndexStack();
+    NS_WARN_IF(NS_FAILED(rv));
   }
 
   return NS_OK;
 }
 
 
 /******************************************************
  * Helper routines
@@ -494,17 +521,17 @@ nsContentIterator::RebuildIndexStack()
   current = mCurNode;
   if (!current) {
     return NS_OK;
   }
 
   while (current != mCommonParent) {
     parent = current->GetParentNode();
 
-    if (!parent) {
+    if (NS_WARN_IF(!parent)) {
       return NS_ERROR_FAILURE;
     }
 
     mIndexes.InsertElementAt(0, parent->IndexOf(current));
 
     current = parent;
   }
 
@@ -521,33 +548,33 @@ nsContentIterator::MakeEmpty()
   mIsDone       = true;
   mIndexes.Clear();
 }
 
 nsINode*
 nsContentIterator::GetDeepFirstChild(nsINode* aRoot,
                                      nsTArray<int32_t>* aIndexes)
 {
-  if (!aRoot || !aRoot->HasChildren()) {
+  if (NS_WARN_IF(!aRoot) || !aRoot->HasChildren()) {
     return aRoot;
   }
   // We can't pass aRoot itself to the full GetDeepFirstChild, because that
   // will only take nsIContent and aRoot might be a document.  Pass aRoot's
   // child, but be sure to preserve aIndexes.
   if (aIndexes) {
     aIndexes->AppendElement(0);
   }
   return GetDeepFirstChild(aRoot->GetFirstChild(), aIndexes);
 }
 
 nsIContent*
 nsContentIterator::GetDeepFirstChild(nsIContent* aRoot,
                                      nsTArray<int32_t>* aIndexes)
 {
-  if (!aRoot) {
+  if (NS_WARN_IF(!aRoot)) {
     return nullptr;
   }
 
   nsIContent* node = aRoot;
   nsIContent* child = node->GetFirstChild();
 
   while (child) {
     if (aIndexes) {
@@ -560,33 +587,33 @@ nsContentIterator::GetDeepFirstChild(nsI
 
   return node;
 }
 
 nsINode*
 nsContentIterator::GetDeepLastChild(nsINode* aRoot,
                                     nsTArray<int32_t>* aIndexes)
 {
-  if (!aRoot || !aRoot->HasChildren()) {
+  if (NS_WARN_IF(!aRoot) || !aRoot->HasChildren()) {
     return aRoot;
   }
   // We can't pass aRoot itself to the full GetDeepLastChild, because that will
   // only take nsIContent and aRoot might be a document.  Pass aRoot's child,
   // but be sure to preserve aIndexes.
   if (aIndexes) {
     aIndexes->AppendElement(aRoot->GetChildCount() - 1);
   }
   return GetDeepLastChild(aRoot->GetLastChild(), aIndexes);
 }
 
 nsIContent*
 nsContentIterator::GetDeepLastChild(nsIContent* aRoot,
                                     nsTArray<int32_t>* aIndexes)
 {
-  if (!aRoot) {
+  if (NS_WARN_IF(!aRoot)) {
     return nullptr;
   }
 
   nsIContent* node = aRoot;
   int32_t numChildren = node->GetChildCount();
 
   while (numChildren) {
     nsIContent* child = node->GetChildAt(--numChildren);
@@ -602,43 +629,45 @@ nsContentIterator::GetDeepLastChild(nsIC
   return node;
 }
 
 // Get the next sibling, or parent's next sibling, or grandpa's next sibling...
 nsIContent*
 nsContentIterator::GetNextSibling(nsINode* aNode,
                                   nsTArray<int32_t>* aIndexes)
 {
-  if (!aNode) {
+  if (NS_WARN_IF(!aNode)) {
     return nullptr;
   }
 
   nsINode* parent = aNode->GetParentNode();
-  if (!parent) {
+  if (NS_WARN_IF(!parent)) {
     return nullptr;
   }
 
   int32_t indx = 0;
 
   NS_ASSERTION(!aIndexes || !aIndexes->IsEmpty(),
                "ContentIterator stack underflow");
   if (aIndexes && !aIndexes->IsEmpty()) {
     // use the last entry on the Indexes array for the current index
     indx = (*aIndexes)[aIndexes->Length()-1];
   } else {
     indx = mCachedIndex;
   }
+  NS_WARN_IF(indx < 0);
 
   // reverify that the index of the current node hasn't changed.
   // not super cheap, but a lot cheaper than IndexOf(), and still O(1).
   // ignore result this time - the index may now be out of range.
   nsIContent* sib = parent->GetChildAt(indx);
   if (sib != aNode) {
     // someone changed our index - find the new index the painful way
     indx = parent->IndexOf(aNode);
+    NS_WARN_IF(indx < 0);
   }
 
   // indx is now canonically correct
   if ((sib = parent->GetChildAt(++indx))) {
     // update index cache
     if (aIndexes && !aIndexes->IsEmpty()) {
       aIndexes->ElementAt(aIndexes->Length()-1) = indx;
     } else {
@@ -663,22 +692,22 @@ nsContentIterator::GetNextSibling(nsINod
   return sib;
 }
 
 // Get the prev sibling, or parent's prev sibling, or grandpa's prev sibling...
 nsIContent*
 nsContentIterator::GetPrevSibling(nsINode* aNode,
                                   nsTArray<int32_t>* aIndexes)
 {
-  if (!aNode) {
+  if (NS_WARN_IF(!aNode)) {
     return nullptr;
   }
 
   nsINode* parent = aNode->GetParentNode();
-  if (!parent) {
+  if (NS_WARN_IF(!parent)) {
     return nullptr;
   }
 
   int32_t indx = 0;
 
   NS_ASSERTION(!aIndexes || !aIndexes->IsEmpty(),
                "ContentIterator stack underflow");
   if (aIndexes && !aIndexes->IsEmpty()) {
@@ -689,16 +718,17 @@ nsContentIterator::GetPrevSibling(nsINod
   }
 
   // reverify that the index of the current node hasn't changed
   // ignore result this time - the index may now be out of range.
   nsIContent* sib = parent->GetChildAt(indx);
   if (sib != aNode) {
     // someone changed our index - find the new index the painful way
     indx = parent->IndexOf(aNode);
+    NS_WARN_IF(indx < 0);
   }
 
   // indx is now canonically correct
   if (indx > 0 && (sib = parent->GetChildAt(--indx))) {
     // update index cache
     if (aIndexes && !aIndexes->IsEmpty()) {
       aIndexes->ElementAt(aIndexes->Length()-1) = indx;
     } else {
@@ -720,16 +750,17 @@ nsContentIterator::NextNode(nsINode* aNo
 {
   nsINode* node = aNode;
 
   // if we are a Pre-order iterator, use pre-order
   if (mPre) {
     // if it has children then next node is first child
     if (node->HasChildren()) {
       nsIContent* firstChild = node->GetFirstChild();
+      MOZ_ASSERT(firstChild);
 
       // update cache
       if (aIndexes) {
         // push an entry on the index stack
         aIndexes->AppendElement(0);
       } else {
         mCachedIndex = 0;
       }
@@ -738,16 +769,17 @@ nsContentIterator::NextNode(nsINode* aNo
     }
 
     // else next sibling is next
     return GetNextSibling(node, aIndexes);
   }
 
   // post-order
   nsINode* parent = node->GetParentNode();
+  NS_WARN_IF(!parent);
   nsIContent* sibling = nullptr;
   int32_t indx = 0;
 
   // get the cached index
   NS_ASSERTION(!aIndexes || !aIndexes->IsEmpty(),
                "ContentIterator stack underflow");
   if (aIndexes && !aIndexes->IsEmpty()) {
     // use the last entry on the Indexes array for the current index
@@ -760,16 +792,17 @@ nsContentIterator::NextNode(nsINode* aNo
   // cheap, but a lot cheaper than IndexOf(), and still O(1).  ignore result
   // this time - the index may now be out of range.
   if (indx >= 0) {
     sibling = parent->GetChildAt(indx);
   }
   if (sibling != node) {
     // someone changed our index - find the new index the painful way
     indx = parent->IndexOf(node);
+    NS_WARN_IF(indx < 0);
   }
 
   // indx is now canonically correct
   sibling = parent->GetChildAt(++indx);
   if (sibling) {
     // update cache
     if (aIndexes && !aIndexes->IsEmpty()) {
       // replace an entry on the index stack
@@ -801,16 +834,17 @@ nsContentIterator::NextNode(nsINode* aNo
 nsINode*
 nsContentIterator::PrevNode(nsINode* aNode, nsTArray<int32_t>* aIndexes)
 {
   nsINode* node = aNode;
 
   // if we are a Pre-order iterator, use pre-order
   if (mPre) {
     nsINode* parent = node->GetParentNode();
+    NS_WARN_IF(!parent);
     nsIContent* sibling = nullptr;
     int32_t indx = 0;
 
     // get the cached index
     NS_ASSERTION(!aIndexes || !aIndexes->IsEmpty(),
                  "ContentIterator stack underflow");
     if (aIndexes && !aIndexes->IsEmpty()) {
       // use the last entry on the Indexes array for the current index
@@ -819,21 +853,23 @@ nsContentIterator::PrevNode(nsINode* aNo
       indx = mCachedIndex;
     }
 
     // reverify that the index of the current node hasn't changed.  not super
     // cheap, but a lot cheaper than IndexOf(), and still O(1).  ignore result
     // this time - the index may now be out of range.
     if (indx >= 0) {
       sibling = parent->GetChildAt(indx);
+      NS_WARN_IF(!sibling);
     }
 
     if (sibling != node) {
       // someone changed our index - find the new index the painful way
       indx = parent->IndexOf(node);
+      NS_WARN_IF(indx < 0);
     }
 
     // indx is now canonically correct
     if (indx && (sibling = parent->GetChildAt(--indx))) {
       // update cache
       if (aIndexes && !aIndexes->IsEmpty()) {
         // replace an entry on the index stack
         aIndexes->ElementAt(aIndexes->Length()-1) = indx;
@@ -853,20 +889,22 @@ nsContentIterator::PrevNode(nsINode* aNo
       // this might be wrong, but we are better off guessing
       mCachedIndex = 0;
     }
     return parent;
   }
 
   // post-order
   int32_t numChildren = node->GetChildCount();
+  NS_WARN_IF(numChildren < 0);
 
   // if it has children then prev node is last child
   if (numChildren) {
     nsIContent* lastChild = node->GetLastChild();
+    NS_WARN_IF(!lastChild);
     numChildren--;
 
     // update cache
     if (aIndexes) {
       // push an entry on the index stack
       aIndexes->AppendElement(numChildren);
     } else {
       mCachedIndex = numChildren;
@@ -882,66 +920,58 @@ nsContentIterator::PrevNode(nsINode* aNo
 /******************************************************
  * ContentIterator routines
  ******************************************************/
 
 void
 nsContentIterator::First()
 {
   if (mFirst) {
-#ifdef DEBUG
-    nsresult rv =
-#endif
-    PositionAt(mFirst);
-
+    DebugOnly<nsresult> rv = PositionAt(mFirst);
     NS_ASSERTION(NS_SUCCEEDED(rv), "Failed to position iterator!");
   }
 
   mIsDone = mFirst == nullptr;
 }
 
 
 void
 nsContentIterator::Last()
 {
   NS_ASSERTION(mLast, "No last node!");
 
   if (mLast) {
-#ifdef DEBUG
-    nsresult rv =
-#endif
-    PositionAt(mLast);
-
+    DebugOnly<nsresult> rv = PositionAt(mLast);
     NS_ASSERTION(NS_SUCCEEDED(rv), "Failed to position iterator!");
   }
 
   mIsDone = mLast == nullptr;
 }
 
 
 void
 nsContentIterator::Next()
 {
-  if (mIsDone || !mCurNode) {
+  if (mIsDone || NS_WARN_IF(!mCurNode)) {
     return;
   }
 
   if (mCurNode == mLast) {
     mIsDone = true;
     return;
   }
 
   mCurNode = NextNode(mCurNode, &mIndexes);
 }
 
 
 void
 nsContentIterator::Prev()
 {
-  if (mIsDone || !mCurNode) {
+  if (NS_WARN_IF(mIsDone) || NS_WARN_IF(!mCurNode)) {
     return;
   }
 
   if (mCurNode == mFirst) {
     mIsDone = true;
     return;
   }
 
@@ -956,17 +986,17 @@ nsContentIterator::IsDone()
 }
 
 
 // Keeping arrays of indexes for the stack of nodes makes PositionAt
 // interesting...
 nsresult
 nsContentIterator::PositionAt(nsINode* aCurNode)
 {
-  if (!aCurNode) {
+  if (NS_WARN_IF(!aCurNode)) {
     return NS_ERROR_NULL_POINTER;
   }
 
   nsINode* newCurNode = aCurNode;
   nsINode* tempNode = mCurNode;
 
   mCurNode = aCurNode;
   // take an early out if this doesn't actually change the position
@@ -979,44 +1009,54 @@ nsContentIterator::PositionAt(nsINode* a
 
   nsINode* firstNode = mFirst;
   nsINode* lastNode = mLast;
   int32_t firstOffset = 0, lastOffset = 0;
 
   if (firstNode && lastNode) {
     if (mPre) {
       firstNode = NodeToParentOffset(mFirst, &firstOffset);
+      NS_WARN_IF(!firstNode);
+      NS_WARN_IF(firstOffset < 0);
 
       if (lastNode->GetChildCount()) {
         lastOffset = 0;
       } else {
         lastNode = NodeToParentOffset(mLast, &lastOffset);
+        NS_WARN_IF(!lastNode);
+        NS_WARN_IF(lastOffset < 0);
         ++lastOffset;
       }
     } else {
       uint32_t numChildren = firstNode->GetChildCount();
 
       if (numChildren) {
         firstOffset = numChildren;
+        NS_WARN_IF(firstOffset < 0);
       } else {
         firstNode = NodeToParentOffset(mFirst, &firstOffset);
+        NS_WARN_IF(!firstNode);
+        NS_WARN_IF(firstOffset < 0);
       }
 
       lastNode = NodeToParentOffset(mLast, &lastOffset);
+      NS_WARN_IF(!lastNode);
+      NS_WARN_IF(lastOffset < 0);
       ++lastOffset;
     }
   }
 
   // The end positions are always in the range even if it has no parent.  We
   // need to allow that or 'iter->Init(root)' would assert in Last() or First()
   // for example, bug 327694.
   if (mFirst != mCurNode && mLast != mCurNode &&
-      (!firstNode || !lastNode ||
-       !NodeIsInTraversalRange(mCurNode, mPre, firstNode, firstOffset,
-                               lastNode, lastOffset))) {
+      (NS_WARN_IF(!firstNode) || NS_WARN_IF(!lastNode) ||
+       NS_WARN_IF(!NodeIsInTraversalRange(mCurNode, mPre,
+                                          firstNode, firstOffset,
+                                          lastNode, lastOffset)))) {
     mIsDone = true;
     return NS_ERROR_FAILURE;
   }
 
   // We can be at ANY node in the sequence.  Need to regenerate the array of
   // indexes back to the root or common parent!
   nsAutoTArray<nsINode*, 8>     oldParentStack;
   nsAutoTArray<int32_t, 8>      newIndexes;
@@ -1035,17 +1075,17 @@ nsContentIterator::PositionAt(nsINode* a
   // for loop, and mIndexes only has entries for nodes from tempNode up through
   // an ancestor of tempNode that's a child of mCommonParent.
   for (int32_t i = mIndexes.Length() + 1; i > 0 && tempNode; i--) {
     // Insert at head since we're walking up
     oldParentStack.InsertElementAt(0, tempNode);
 
     nsINode* parent = tempNode->GetParentNode();
 
-    if (!parent) {
+    if (NS_WARN_IF(!parent)) {
       // this node has no parent, and thus no index
       break;
     }
 
     if (parent == mCurNode) {
       // The position was moved to a parent of the current position.  All we
       // need to do is drop some indexes.  Shortcut here.
       mIndexes.RemoveElementsAt(mIndexes.Length() - oldParentStack.Length(),
@@ -1055,22 +1095,23 @@ nsContentIterator::PositionAt(nsINode* a
     }
     tempNode = parent;
   }
 
   // Ok.  We have the array of old parents.  Look for a match.
   while (newCurNode) {
     nsINode* parent = newCurNode->GetParentNode();
 
-    if (!parent) {
+    if (NS_WARN_IF(!parent)) {
       // this node has no parent, and thus no index
       break;
     }
 
     int32_t indx = parent->IndexOf(newCurNode);
+    NS_WARN_IF(indx < 0);
 
     // insert at the head!
     newIndexes.InsertElementAt(0, indx);
 
     // look to see if the parent is in the stack
     indx = oldParentStack.IndexOf(parent);
     if (indx >= 0) {
       // ok, the parent IS on the old stack!  Rework things.  We want