Bug 1587433: part 3) Strengthen conditions for `AbstractRange::mIsPositioned`. r=smaug
☠☠ backed out by c0c22dbdd5b8 ☠ ☠
authorMirko Brodesser <mbrodesser@mozilla.com>
Wed, 11 Dec 2019 12:24:28 +0000
changeset 507068 0e1577031addefed6aeaa5df8a724b73edb690a0
parent 507067 3cff413b1897dbc48924cae9435dde5015f2a68b
child 507069 4032df295a67aca4366d33ebb0ba9a57305d1bcc
push id36922
push userncsoregi@mozilla.com
push dateMon, 16 Dec 2019 17:21:47 +0000
treeherdermozilla-central@27d0d6cc2131 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1587433
milestone73.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1587433: part 3) Strengthen conditions for `AbstractRange::mIsPositioned`. r=smaug Makes it less mysterious. Differential Revision: https://phabricator.services.mozilla.com/D54276
dom/base/AbstractRange.h
dom/base/StaticRange.cpp
dom/base/nsRange.cpp
--- a/dom/base/AbstractRange.h
+++ b/dom/base/AbstractRange.h
@@ -72,18 +72,18 @@ class AbstractRange : public nsISupports
             typename RangeType>
   static nsresult SetStartAndEndInternal(
       const RangeBoundaryBase<SPT, SRT>& aStartBoundary,
       const RangeBoundaryBase<EPT, ERT>& aEndBoundary, RangeType* aRange);
 
   RefPtr<Document> mOwner;
   RangeBoundary mStart;
   RangeBoundary mEnd;
-  // `true` if `mStart` has a container and potentially other conditions are
-  // fulfilled.
+  // `true` if `mStart` and `mEnd` are set for StaticRange or set and valid
+  // for nsRange.
   bool mIsPositioned;
 
   // Used by nsRange, but this should have this for minimizing the size.
   bool mIsGenerated;
   // Used by nsRange, but this should have this for minimizing the size.
   bool mCalledByJS;
 };
 
--- a/dom/base/StaticRange.cpp
+++ b/dom/base/StaticRange.cpp
@@ -77,17 +77,18 @@ already_AddRefed<StaticRange> StaticRang
 }
 
 template <typename SPT, typename SRT, typename EPT, typename ERT>
 void StaticRange::DoSetRange(const RangeBoundaryBase<SPT, SRT>& aStartBoundary,
                              const RangeBoundaryBase<EPT, ERT>& aEndBoundary,
                              nsINode* aRootNode) {
   mStart = aStartBoundary;
   mEnd = aEndBoundary;
-  mIsPositioned = mStart.IsSet();
+  MOZ_ASSERT(mStart.IsSet() == mEnd.IsSet());
+  mIsPositioned = mStart.IsSet() && mEnd.IsSet();
 }
 
 /* static */
 already_AddRefed<StaticRange> StaticRange::Constructor(
     const GlobalObject& global, const StaticRangeInit& init, ErrorResult& aRv) {
   if (init.mStartContainer->NodeType() == nsINode::DOCUMENT_TYPE_NODE ||
       init.mStartContainer->NodeType() == nsINode::ATTRIBUTE_NODE ||
       init.mEndContainer->NodeType() == nsINode::DOCUMENT_TYPE_NODE ||
--- a/dom/base/nsRange.cpp
+++ b/dom/base/nsRange.cpp
@@ -911,19 +911,21 @@ void nsRange::NotifySelectionListenersAf
 // for content notification of range ownership.
 // Calling DoSetRange with either parent argument null will collapse
 // the range to have both endpoints point to the other node
 template <typename SPT, typename SRT, typename EPT, typename ERT>
 void nsRange::DoSetRange(const RangeBoundaryBase<SPT, SRT>& aStartBoundary,
                          const RangeBoundaryBase<EPT, ERT>& aEndBoundary,
                          nsINode* aRootNode,
                          bool aNotInsertedYet /* = false */) {
-  MOZ_ASSERT((aStartBoundary.IsSet() && aEndBoundary.IsSet() && aRootNode) ||
-                 (!aStartBoundary.IsSet() && !aEndBoundary.IsSet()),
-             "Set all or none");
+  mIsPositioned = aStartBoundary.IsSetAndValid() &&
+                  aEndBoundary.IsSetAndValid() && aRootNode;
+  MOZ_ASSERT(
+      mIsPositioned || (!aStartBoundary.IsSet() && !aEndBoundary.IsSet()),
+      "Set all or none");
 
   MOZ_ASSERT(
       !aRootNode || (!aStartBoundary.IsSet() && !aEndBoundary.IsSet()) ||
           aNotInsertedYet ||
           (aStartBoundary.Container()->IsInclusiveDescendantOf(aRootNode) &&
            aEndBoundary.Container()->IsInclusiveDescendantOf(aRootNode) &&
            aRootNode ==
                RangeUtils::ComputeRootNode(aStartBoundary.Container()) &&
@@ -958,17 +960,16 @@ void nsRange::DoSetRange(const RangeBoun
       IsInSelection() && !aNotInsertedYet;
 
   // GetCommonAncestor is unreliable while we're unlinking (could
   // return null if our start/end have already been unlinked), so make
   // sure to not use it here to determine our "old" current ancestor.
   mStart = aStartBoundary;
   mEnd = aEndBoundary;
 
-  mIsPositioned = !!mStart.Container();
   if (checkCommonAncestor) {
     nsINode* oldCommonAncestor = mRegisteredCommonAncestor;
     nsINode* newCommonAncestor = GetCommonAncestor();
     if (newCommonAncestor != oldCommonAncestor) {
       if (oldCommonAncestor) {
         UnregisterCommonAncestor(oldCommonAncestor, false);
       }
       if (newCommonAncestor) {