Bug 1415409 - Make == operator of RangeBoundaryBase compare mRef and mOffset more carefully r=catalinb

Currently, RangeBoundaryBase can store either only mRef or mOffset.  However,
== operator of RangeBoudaryBase still compares mRef simply.  However, if one
has only mRef and the other has only mOffset, it returns false.

This patch makes == operator checks if both mOffset have been set.  If so,
this checks both mOffset.value() and mRef are matched.  However, if mRef of
only one of them is nullptr, this doesn't make it check mRef because computing
mRef needs some cost and initializing mRef from the other fixes the referring
child stronger.  If the user of the operator sets only mOffset and wait DOM
tree changes, computing mRef may break such callers.

If one has only mRef and the other has only mOffset, this patch makes it
compute mRef.  This is not the best behavior, perhaps.  However, there is no
way to compare these instances.  If this becomes a problem, we should make it
create temporary instance, but it'll waste runtime cost.  So, let's avoid using
this approach for now.

Finally, making it check both mRef simply.

MozReview-Commit-ID: 4nsW5SYDTiZ

--HG--
extra : rebase_source : b16ec01bdefc8fd847fc43513581aeb0efbf4e21

dom/base/RangeBoundary.h

@@ -409,8 +409,54 @@ public:
   template<typename A, typename B>
   bool operator==(const RangeBoundaryBase<A, B>& aOther) const
   {
-    return mParent == aOther.mParent &&
-      (mRef ? mRef == aOther.mRef : mOffset == aOther.mOffset);
+    if (mParent != aOther.mParent) {
+      return false;
+    }
+
+    if (mOffset.isSome() && aOther.mOffset.isSome()) {
+      // If both mOffset are set, we need to compare both mRef too because
+      // the relation of mRef and mOffset have already broken by DOM tree
+      // changes.
+      if (mOffset != aOther.mOffset) {
+        return false;
+      }
+      if (mRef == aOther.mRef) {
+        return true;
+      }
+      if (NS_WARN_IF(mRef && aOther.mRef)) {
+        // In this case, relation between mRef and mOffset of one of or both of
+        // them doesn't match with current DOM tree since the DOM tree might
+        // have been changed after computing mRef or mOffset.
+        return false;
+      }
+      // If one of mRef hasn't been computed yet, we should compare them only
+      // with mOffset.  Perhaps, we shouldn't copy mRef from non-nullptr one to
+      // nullptr one since if we copy it here, it may be unexpected behavior for
+      // some callers.
+      return true;
+    }
+
+    if (mOffset.isSome() && !mRef &&
+        !aOther.mOffset.isSome() && aOther.mRef) {
+      // If this has only mOffset and the other has only mRef, this needs to
+      // compute mRef now.
+      EnsureRef();
+      return mRef == aOther.mRef;
+    }
+
+    if (!mOffset.isSome() && mRef &&
+        aOther.mOffset.isSome() && !aOther.mRef) {
+      // If this has only mRef and the other has only mOffset, the other needs
+      // to compute mRef now.
+      aOther.EnsureRef();
+      return mRef == aOther.mRef;
+    }
+
+    // If mOffset of one of them hasn't been computed from mRef yet, we should
+    // compare only with mRef.  Perhaps, we shouldn't copy mOffset from being
+    // some one to not being some one since if we copy it here, it may be
+    // unexpected behavior for some callers.
+    return mRef == aOther.mRef;
   }
 
   template<typename A, typename B>