Bug 1393816 - part1: Cache a range until new range is created in Selection r?smaug
When setting value of <input type="text">, nsTextEditorState removes all
ranges of normal selection first. Then, TextEditor sets the value. Finally,
TextEditor collapses the selection at the end of the text.
In
bug 1386471, we got that there are some problems to remove the call of
Selection::RemoveAllRanges() in nsTextEditorState. Therefore, we need another
approach to improve Selection::Collapse().
The approach of this patch is, when removing all ranges from normal selection,
Selection can cache an nsRange instance if there is an instance which is not
referenced from other than the Selection (i.e., it'll be removed when
Selection::Clear() is called). Then, Selection::Collapse() can reuse it. With
this fix, Selection::Collapse() can reduce allocation cost and may reduce some
other cost like adding it to mutation observer.
However, keeping nsRange instance may cause increasing mutation observer's cost
since nsRange will be adjusted its start node/offset and end node/offset with
mutation observer to guarantee that the range is always valid. So, we can
cache such range only when the caller (or its callee) will set selection range
later. Therefore, this patch adds Selection::RemoveAllRangesTemporarily()
and make only nsTextEditorState::SetValue() and
ContentEventHandler::OnSelectionEvent() use it.
MozReview-Commit-ID: FjWrbz4S1ld
--- a/dom/base/Selection.cpp
+++ b/dom/base/Selection.cpp
@@ -781,28 +781,30 @@ Selection::GetParentObject() const
NS_IMPL_CYCLE_COLLECTION_CLASS(Selection)
NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(Selection)
// Unlink the selection listeners *before* we do RemoveAllRanges since
// we don't want to notify the listeners during JS GC (they could be
// in JS!).
NS_IMPL_CYCLE_COLLECTION_UNLINK(mSelectionListeners)
+ NS_IMPL_CYCLE_COLLECTION_UNLINK(mCachedRange)
tmp->RemoveAllRanges();
NS_IMPL_CYCLE_COLLECTION_UNLINK(mFrameSelection)
NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER
NS_IMPL_CYCLE_COLLECTION_UNLINK_END
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(Selection)
{
uint32_t i, count = tmp->mRanges.Length();
for (i = 0; i < count; ++i) {
NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mRanges[i].mRange)
}
}
NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mAnchorFocusRange)
+ NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mCachedRange)
NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mFrameSelection)
NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mSelectionListeners)
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
NS_IMPL_CYCLE_COLLECTION_TRACE_WRAPPERCACHE(Selection)
// QueryInterface implementation for Selection
NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(Selection)
NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY
@@ -2285,16 +2287,42 @@ Selection::RemoveAllRanges(ErrorResult&
// Also need to notify the frames!
// PresShell::CharacterDataChanged should do that on DocumentChanged
if (NS_FAILED(result)) {
aRv.Throw(result);
}
}
+nsresult
+Selection::RemoveAllRangesTemporarily()
+{
+ if (!mCachedRange) {
+ // Look for a range which isn't referred by other than this instance.
+ // If there is, it'll be released by calling Clear(). So, we can reuse it
+ // when we need to create a range.
+ for (auto& rangeData : mRanges) {
+ auto& range = rangeData.mRange;
+ if (range->GetRefCount() == 1 ||
+ (range->GetRefCount() == 2 && range == mAnchorFocusRange)) {
+ mCachedRange = range;
+ break;
+ }
+ }
+ }
+
+ // Then, remove all ranges.
+ ErrorResult result;
+ RemoveAllRanges(result);
+ if (result.Failed()) {
+ mCachedRange = nullptr;
+ }
+ return result.StealNSResult();
+}
+
/** AddRange adds the specified range to the selection
* @param aRange is the range to be added
*/
NS_IMETHODIMP
Selection::AddRange(nsIDOMRange* aDOMRange)
{
if (!aDOMRange) {
return NS_ERROR_NULL_POINTER;
@@ -2327,16 +2355,20 @@ Selection::AddRangeInternal(nsRange& aRa
if (aDocument != rangeRoot && (!rangeRoot ||
aDocument != rangeRoot->GetComposedDoc())) {
// http://w3c.github.io/selection-api/#dom-selection-addrange
// "... if the root of the range's boundary points are the document
// associated with context object. Otherwise, this method must do nothing."
return;
}
+ // If a range is being added, we don't need cached range because Collapse()
+ // won't use it.
+ mCachedRange = nullptr;
+
// AddTableCellRange might flush frame.
RefPtr<Selection> kungFuDeathGrip(this);
// This inserts a table cell range in proper document order
// and returns NS_OK if range doesn't contain just one table cell
bool didAddRange;
int32_t rangeIndex;
nsresult result = AddTableCellRange(&aRange, &didAddRange, &rangeIndex);
@@ -2583,17 +2615,19 @@ Selection::Collapse(nsINode& aContainer,
}
}
}
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;
+ range = Move(oldRange);
+ } else if (mCachedRange) {
+ range = Move(mCachedRange);
} else {
range = new nsRange(container);
}
result = range->CollapseTo(container, aOffset);
if (NS_FAILED(result)) {
aRv.Throw(result);
return;
}
@@ -4040,16 +4074,21 @@ 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;
--- a/dom/base/Selection.h
+++ b/dom/base/Selection.h
@@ -210,16 +210,24 @@ public:
void GetType(nsAString& aOutType) const;
nsRange* GetRangeAt(uint32_t aIndex, mozilla::ErrorResult& aRv);
void AddRangeJS(nsRange& aRange, mozilla::ErrorResult& aRv);
void RemoveRange(nsRange& aRange, mozilla::ErrorResult& aRv);
void RemoveAllRanges(mozilla::ErrorResult& aRv);
+ /**
+ * RemoveAllRangesTemporarily() is useful if the caller will add one or more
+ * ranges later. This tries to cache a removing range if it's possible.
+ * If a range is not referred by anything else this selection, the range
+ * can be reused later. Otherwise, this works as same as RemoveAllRanges().
+ */
+ nsresult RemoveAllRangesTemporarily();
+
void Stringify(nsAString& aResult);
bool ContainsNode(nsINode& aNode, bool aPartlyContained, mozilla::ErrorResult& aRv);
/**
* Check to see if the given point is contained within the selection area. In
* particular, this iterates through all the rects that make up the selection,
* not just the bounding box, and checks to see if the given point is contained
@@ -468,16 +476,22 @@ private:
// Inserting a new range requires finding the overlapping interval, requiring
// 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.
+ RefPtr<nsRange> mCachedRange;
RefPtr<nsFrameSelection> mFrameSelection;
RefPtr<nsAutoScrollTimer> mAutoScrollTimer;
FallibleTArray<nsCOMPtr<nsISelectionListener>> mSelectionListeners;
nsRevocableEventPtr<ScrollSelectionIntoViewEvent> mScrollEvent;
CachedOffsetForFrame* mCachedOffsetForFrame;
nsDirection mDirection;
SelectionType mSelectionType;
UniquePtr<SelectionCustomColors> mCustomColors;
--- a/dom/events/ContentEventHandler.cpp
+++ b/dom/events/ContentEventHandler.cpp
@@ -3227,17 +3227,17 @@ ContentEventHandler::OnSelectionEvent(Wi
if (NS_WARN_IF(!startNode) || NS_WARN_IF(!endNode) ||
NS_WARN_IF(startNodeOffset < 0) || NS_WARN_IF(endNodeOffset < 0)) {
return NS_ERROR_UNEXPECTED;
}
mSelection->StartBatchChanges();
// Clear selection first before setting
- rv = mSelection->RemoveAllRanges();
+ rv = mSelection->RemoveAllRangesTemporarily();
// Need to call EndBatchChanges at the end even if call failed
if (NS_SUCCEEDED(rv)) {
if (aEvent->mReversed) {
rv = mSelection->Collapse(endNode, endNodeOffset);
} else {
rv = mSelection->Collapse(startNode, startNodeOffset);
}
if (NS_SUCCEEDED(rv) &&
--- a/dom/html/nsTextEditorState.cpp
+++ b/dom/html/nsTextEditorState.cpp
@@ -2687,17 +2687,19 @@ nsTextEditorState::SetValue(const nsAStr
} else {
textEditor->InsertText(insertValue);
}
} else {
AutoDisableUndo disableUndo(textEditor);
if (selection) {
// Since we don't use undo transaction, we don't need to store
// selection state. SetText will set selection to tail.
- selection->RemoveAllRanges();
+ // Note that textEditor will collapse selection to the end.
+ // Therefore, it's safe to use RemoveAllRangesTemporarily() here.
+ selection->RemoveAllRangesTemporarily();
}
textEditor->SetText(newValue);
// Call the listener's HandleValueChanged() callback manually, since
// we don't use the transaction manager in this path and it could be
// that the editor would bypass calling the listener for that reason.
mTextListener->HandleValueChanged();