Bug 1438996. Don't construct a unneeded temporary range object in RangeUtils::CompareNodeToRange. r=masayuki
authorTimothy Nikkel <tnikkel@gmail.com>
Thu, 15 Aug 2019 21:13:54 +0000
changeset 488359 2fce06fd642bada80669049de12f9b15924c979b
parent 488358 0dce79612a6d823e56c09724b9c02f1420cb50d3
child 488360 673ebee1e845f5fd65c7ae95c2ea4372f10fc037
push id36440
push userncsoregi@mozilla.com
push dateFri, 16 Aug 2019 03:57:48 +0000
treeherdermozilla-central@a58b7dc85887 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmasayuki
bugs1438996
milestone70.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 1438996. Don't construct a unneeded temporary range object in RangeUtils::CompareNodeToRange. r=masayuki nsContentUtils::ComparePoints just immediately deconstructs the range boundary into container and offset and calls a version of nsContentUtils::ComparePoints that takes containers/offsets instead of range boundaries. This also avoids a warning in the RangeBoundary constructor. Differential Revision: https://phabricator.services.mozilla.com/D42071
dom/base/RangeUtils.cpp
--- a/dom/base/RangeUtils.cpp
+++ b/dom/base/RangeUtils.cpp
@@ -137,30 +137,39 @@ nsresult RangeUtils::CompareNodeToRange(
     nodeEnd = nodeStart + 1;
     MOZ_ASSERT(nodeStart < nodeEnd, "nodeStart shouldn't be INT32_MAX");
   }
 
   // XXX nsContentUtils::ComparePoints() may be expensive.  If some callers
   //     just want one of aNodeIsBeforeRange or aNodeIsAfterRange, we can
   //     skip the other comparison.
 
+  // In the ComparePoints calls below we use a container & offset instead of
+  // a range boundary because the range boundary constructor warns if you pass
+  // in a -1 offset and the ComputeIndexOf call above can return -1 if aNode
+  // is native anonymous content. ComparePoints has comments about offsets
+  // being -1 and it seems to deal with it, or at least we aren't aware of any
+  // problems arising because of it. We don't have a better idea how to get
+  // rid of the warning without much larger changes so we do this just to
+  // silence the warning. (Bug 1438996)
+
   // is RANGE(start) <= NODE(start) ?
   bool disconnected = false;
   *aNodeIsBeforeRange =
-      nsContentUtils::ComparePoints(aAbstractRange->StartRef(),
-                                    RawRangeBoundary(parent, nodeStart),
-                                    &disconnected) > 0;
+      nsContentUtils::ComparePoints(aAbstractRange->StartRef().Container(),
+                                    aAbstractRange->StartRef().Offset(), parent,
+                                    nodeStart, &disconnected) > 0;
   if (NS_WARN_IF(disconnected)) {
     return NS_ERROR_DOM_WRONG_DOCUMENT_ERR;
   }
 
   // is RANGE(end) >= NODE(end) ?
   *aNodeIsAfterRange =
-      nsContentUtils::ComparePoints(aAbstractRange->EndRef(),
-                                    RawRangeBoundary(parent, nodeEnd),
-                                    &disconnected) < 0;
+      nsContentUtils::ComparePoints(aAbstractRange->EndRef().Container(),
+                                    aAbstractRange->EndRef().Offset(), parent,
+                                    nodeEnd, &disconnected) < 0;
   if (NS_WARN_IF(disconnected)) {
     return NS_ERROR_DOM_WRONG_DOCUMENT_ERR;
   }
   return NS_OK;
 }
 
 }  // namespace mozilla