Bug 1393816 - part2: Selection::SetBaseAndExtent() should use mCachedRange if it's available r=smaug
authorMasayuki Nakano <masayuki@d-toybox.com>
Sat, 26 Aug 2017 00:12:38 +0900
changeset 377166 fbc63e299cd0ee4a40a804d35ebc6105ae53ae41
parent 377165 d311ad8f8c764904c7b56a9cf9726e942257367e
child 377167 08e729b3651719ff260cb3651539fff11a322b2f
push id32402
push userarchaeopteryx@coole-files.de
push dateMon, 28 Aug 2017 14:47:04 +0000
treeherdermozilla-central@d5b6d113cf17 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1393816
milestone57.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 1393816 - part2: Selection::SetBaseAndExtent() should use mCachedRange if it's available r=smaug Similar to Selection::Collapse(), if mCachedRange is available, Selection::SetBaseAndExtent() should use it rather than creating new nsRange instance. Then, it can reduce the allocation cost and may reduce some other cost, e.g., adding it to mutation observer. MozReview-Commit-ID: InQQusw2KMc
dom/base/Selection.cpp
dom/base/Selection.h
--- a/dom/base/Selection.cpp
+++ b/dom/base/Selection.cpp
@@ -4074,49 +4074,54 @@ Selection::SetBaseAndExtent(nsINode& aAn
   }
 
   if (!HasSameRoot(aAnchorNode) ||
       !HasSameRoot(aFocusNode)) {
     // Return with no error
     return;
   }
 
-  // Since a range will be created, we don't need the cached range because
-  // Collapse() won't use it.
-  // TODO: We should use the cache.
-  mCachedRange = nullptr;
-
   SelectionBatcher batch(this);
 
   int32_t relativePosition =
     nsContentUtils::ComparePoints(&aAnchorNode, aAnchorOffset,
                                   &aFocusNode, aFocusOffset);
   nsINode* start = &aAnchorNode;
   nsINode* end = &aFocusNode;
   uint32_t startOffset = aAnchorOffset;
   uint32_t endOffset = aFocusOffset;
   if (relativePosition > 0) {
     start = &aFocusNode;
     end = &aAnchorNode;
     startOffset = aFocusOffset;
     endOffset = aAnchorOffset;
   }
 
-  RefPtr<nsRange> newRange;
-  nsresult rv = nsRange::CreateRange(start, startOffset, end, endOffset,
-                                     getter_AddRefs(newRange));
-  // CreateRange returns IndexSizeError if any offset is out of bounds.
+  // If there is cached range, we should reuse it for saving the allocation
+  // const (and some other cost in nsRange::DoSetRange().
+  RefPtr<nsRange> newRange = Move(mCachedRange);
+
+  nsresult rv = NS_OK;
+  if (newRange) {
+    rv = newRange->SetStartAndEnd(start, startOffset, end, endOffset);
+  } else {
+    rv = nsRange::CreateRange(start, startOffset, end, endOffset,
+                              getter_AddRefs(newRange));
+  }
+
+  // nsRange::SetStartAndEnd() and nsRange::CreateRange() returns
+  // IndexSizeError if any offset is out of bounds.
   if (NS_FAILED(rv)) {
     aRv.Throw(rv);
     return;
   }
 
-  rv = RemoveAllRanges();
-  if (NS_FAILED(rv)) {
-    aRv.Throw(rv);
+  // Use non-virtual method instead of nsISelection::RemoveAllRanges().
+  RemoveAllRanges(aRv);
+  if (aRv.Failed()) {
     return;
   }
 
   rv = AddRange(newRange);
   if (NS_FAILED(rv)) {
     aRv.Throw(rv);
     return;
   }
--- a/dom/base/Selection.h
+++ b/dom/base/Selection.h
@@ -477,20 +477,20 @@ private:
   // two binary searches plus up to an additional 6 DOM comparisons. If this
   // proves to be a performance concern, then an interval tree may be a
   // possible solution, allowing the calculation of the overlap interval in
   // O(log n) time, though this would require rebalancing and other overhead.
   AutoTArray<RangeData, 1> mRanges;
 
   RefPtr<nsRange> mAnchorFocusRange;
   // mCachedRange is set by RemoveAllRangesTemporarily() and used by
-  // Collapse().  If there is a range which will be released by Clear(),
-  // RemoveAllRangesTemporarily() stores it with this.  If Collapse() is
-  // called without existing ranges, it'll reuse this range for saving the
-  // creating cost.
+  // Collapse() and SetBaseAndExtent().  If there is a range which will be
+  // released by Clear(), RemoveAllRangesTemporarily() stores it with this.
+  // If Collapse() is called without existing ranges, it'll reuse this range
+  // for saving the creation cost.
   RefPtr<nsRange> mCachedRange;
   RefPtr<nsFrameSelection> mFrameSelection;
   RefPtr<nsAutoScrollTimer> mAutoScrollTimer;
   FallibleTArray<nsCOMPtr<nsISelectionListener>> mSelectionListeners;
   nsRevocableEventPtr<ScrollSelectionIntoViewEvent> mScrollEvent;
   CachedOffsetForFrame* mCachedOffsetForFrame;
   nsDirection mDirection;
   SelectionType mSelectionType;