Bug 1367740 Selection::Collapse() should reuse old nsRange instance if it's not referred by anybody r?mats draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 25 May 2017 18:04:55 +0900
changeset 586318 ccb38b1973cba7d501aff917c082c912bdbd928c
parent 586317 407d8616611f1e4261ce55ebb6d1abedea154716
child 586417 cfd069e00bd6f3296baeb85af343f33c9622d4ff
child 586419 de87645143103d274870a0b8b773cdff22a08595
push id61367
push usermasayuki@d-toybox.com
push dateTue, 30 May 2017 07:46:00 +0000
reviewersmats
bugs1367740
milestone55.0a1
Bug 1367740 Selection::Collapse() should reuse old nsRange instance if it's not referred by anybody r?mats Selection::Collapse() removes all ranges first, then, adds a collapsed range which is a new instance of nsRange. However, new nsRange's initialize cost isn't cheap. It needs to add itself to mutation observer and computes common ancestor. However, Selection::Collapse() doesn't move the range to different document. If old range is reusable, we can avoid to remove old range from mutation observer and add new range to mutation observer, and also we can avoid to recompute common ancestor if the node is not changed, e.g., only offset is changed in selected node. MozReview-Commit-ID: BoCBod7WVr5
dom/base/nsRange.h
layout/generic/nsSelection.cpp
--- a/dom/base/nsRange.h
+++ b/dom/base/nsRange.h
@@ -55,16 +55,21 @@ public:
                               nsIDOMRange** aRange);
   static nsresult CreateRange(nsINode* aStartParent, int32_t aStartOffset,
                               nsINode* aEndParent, int32_t aEndOffset,
                               nsRange** aRange);
 
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS_AMBIGUOUS(nsRange, nsIDOMRange)
 
+  nsrefcnt GetRefCount() const
+  {
+    return mRefCnt;
+  }
+
   /**
    * The DOM Range spec requires that when a node is removed from its parent,
    * and the node's subtree contains the start or end point of a range, that
    * start or end point is moved up to where the node was removed from its
    * parent.
    * For some internal uses of Ranges it's useful to disable that behavior,
    * so that a range of children within a single parent is preserved even if
    * that parent is removed from the document tree.
--- a/layout/generic/nsSelection.cpp
+++ b/layout/generic/nsSelection.cpp
@@ -5245,16 +5245,19 @@ Selection::Collapse(nsINode& aParentNode
   nsresult result;
 
   RefPtr<nsPresContext> presContext = GetPresContext();
   if (!presContext || presContext->Document() != parentNode->OwnerDoc()) {
     aRv.Throw(NS_ERROR_FAILURE);
     return;
   }
 
+  // Cache current range is if there is because it may be reusable.
+  RefPtr<nsRange> oldRange = !mRanges.IsEmpty() ? mRanges[0].mRange : nullptr;
+
   // Delete all of the current ranges
   Clear(presContext);
 
   // Turn off signal for table selection
   frameSelection->ClearTableCellSelection();
 
   // Hack to display the caret on the right line (bug 1237236).
   if (frameSelection->GetHint() != CARET_ASSOCIATE_AFTER &&
@@ -5268,17 +5271,24 @@ Selection::Collapse(nsINode& aParentNode
            f->GetContentEnd() == int32_t(aOffset)) ||
           (parentNode == f->GetContent()->GetParentNode() &&
            parentNode->IndexOf(f->GetContent()) + 1 == int32_t(aOffset))) {
         frameSelection->SetHint(CARET_ASSOCIATE_AFTER);
       }
     }
   }
 
-  RefPtr<nsRange> range = new nsRange(parentNode);
+  RefPtr<nsRange> range;
+  // If the old range isn't referred by anybody other than this method,
+  // we should reuse it for reducing the recreation cost.
+  if (oldRange && oldRange->GetRefCount() == 1) {
+    range = oldRange;
+  } else {
+    range = new nsRange(parentNode);
+  }
   result = range->CollapseTo(parentNode, aOffset);
   if (NS_FAILED(result)) {
     aRv.Throw(result);
     return;
   }
 
 #ifdef DEBUG_SELECTION
   nsCOMPtr<nsIContent> content = do_QueryInterface(parentNode);