Commit f92b9734 authored by ager@chromium.org's avatar ager@chromium.org

Remove usages of temporary Range objects that end up registered on the

Document for no reason.

This is important for Oilpan because the Ranges do not die until the
next GC. This gives a 10% improvement on DOM/textarea-edit in the
oilpan build.

R=yosin@chromium.org
BUG=388681

Review URL: https://codereview.chromium.org/565613002

git-svn-id: svn://svn.chromium.org/blink/trunk@181889 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 27ebfb94
...@@ -147,9 +147,9 @@ void DocumentMarkerController::prepareForDestruction() ...@@ -147,9 +147,9 @@ void DocumentMarkerController::prepareForDestruction()
clear(); clear();
} }
void DocumentMarkerController::removeMarkers(Range* range, DocumentMarker::MarkerTypes markerTypes, RemovePartiallyOverlappingMarkerOrNot shouldRemovePartiallyOverlappingMarker) void DocumentMarkerController::removeMarkers(TextIterator& markedText, DocumentMarker::MarkerTypes markerTypes, RemovePartiallyOverlappingMarkerOrNot shouldRemovePartiallyOverlappingMarker)
{ {
for (TextIterator markedText(range); !markedText.atEnd(); markedText.advance()) { for (; !markedText.atEnd(); markedText.advance()) {
if (!possiblyHasMarkers(markerTypes)) if (!possiblyHasMarkers(markerTypes))
return; return;
ASSERT(!m_markers.isEmpty()); ASSERT(!m_markers.isEmpty());
...@@ -160,6 +160,18 @@ void DocumentMarkerController::removeMarkers(Range* range, DocumentMarker::Marke ...@@ -160,6 +160,18 @@ void DocumentMarkerController::removeMarkers(Range* range, DocumentMarker::Marke
} }
} }
void DocumentMarkerController::removeMarkers(Range* range, DocumentMarker::MarkerTypes markerTypes, RemovePartiallyOverlappingMarkerOrNot shouldRemovePartiallyOverlappingMarker)
{
TextIterator markedText(range);
DocumentMarkerController::removeMarkers(markedText, markerTypes, shouldRemovePartiallyOverlappingMarker);
}
void DocumentMarkerController::removeMarkers(const Position& start, const Position& end, DocumentMarker::MarkerTypes markerTypes, RemovePartiallyOverlappingMarkerOrNot shouldRemovePartiallyOverlappingMarker)
{
TextIterator markedText(start, end);
DocumentMarkerController::removeMarkers(markedText, markerTypes, shouldRemovePartiallyOverlappingMarker);
}
static bool startsFurther(const OwnPtrWillBeMember<RenderedDocumentMarker>& lhv, const DocumentMarker* rhv) static bool startsFurther(const OwnPtrWillBeMember<RenderedDocumentMarker>& lhv, const DocumentMarker* rhv)
{ {
return lhv->startOffset() < rhv->startOffset(); return lhv->startOffset() < rhv->startOffset();
......
...@@ -38,9 +38,11 @@ namespace blink { ...@@ -38,9 +38,11 @@ namespace blink {
class LayoutPoint; class LayoutPoint;
class LayoutRect; class LayoutRect;
class Node; class Node;
class Position;
class Range; class Range;
class RenderedDocumentMarker; class RenderedDocumentMarker;
class Text; class Text;
class TextIterator;
class MarkerRemoverPredicate FINAL { class MarkerRemoverPredicate FINAL {
public: public:
...@@ -73,6 +75,7 @@ public: ...@@ -73,6 +75,7 @@ public:
// the portion that is outside of the range. // the portion that is outside of the range.
enum RemovePartiallyOverlappingMarkerOrNot { DoNotRemovePartiallyOverlappingMarker, RemovePartiallyOverlappingMarker }; enum RemovePartiallyOverlappingMarkerOrNot { DoNotRemovePartiallyOverlappingMarker, RemovePartiallyOverlappingMarker };
void removeMarkers(Range*, DocumentMarker::MarkerTypes = DocumentMarker::AllMarkers(), RemovePartiallyOverlappingMarkerOrNot = DoNotRemovePartiallyOverlappingMarker); void removeMarkers(Range*, DocumentMarker::MarkerTypes = DocumentMarker::AllMarkers(), RemovePartiallyOverlappingMarkerOrNot = DoNotRemovePartiallyOverlappingMarker);
void removeMarkers(const Position& start, const Position& end, DocumentMarker::MarkerTypes = DocumentMarker::AllMarkers(), RemovePartiallyOverlappingMarkerOrNot = DoNotRemovePartiallyOverlappingMarker);
void removeMarkers(Node*, unsigned startOffset, int length, DocumentMarker::MarkerTypes = DocumentMarker::AllMarkers(), RemovePartiallyOverlappingMarkerOrNot = DoNotRemovePartiallyOverlappingMarker); void removeMarkers(Node*, unsigned startOffset, int length, DocumentMarker::MarkerTypes = DocumentMarker::AllMarkers(), RemovePartiallyOverlappingMarkerOrNot = DoNotRemovePartiallyOverlappingMarker);
void removeMarkers(DocumentMarker::MarkerTypes = DocumentMarker::AllMarkers()); void removeMarkers(DocumentMarker::MarkerTypes = DocumentMarker::AllMarkers());
...@@ -105,6 +108,7 @@ private: ...@@ -105,6 +108,7 @@ private:
void mergeOverlapping(MarkerList*, DocumentMarker&); void mergeOverlapping(MarkerList*, DocumentMarker&);
bool possiblyHasMarkers(DocumentMarker::MarkerTypes); bool possiblyHasMarkers(DocumentMarker::MarkerTypes);
void removeMarkersFromList(MarkerMap::iterator, DocumentMarker::MarkerTypes); void removeMarkersFromList(MarkerMap::iterator, DocumentMarker::MarkerTypes);
void removeMarkers(TextIterator&, DocumentMarker::MarkerTypes, RemovePartiallyOverlappingMarkerOrNot);
MarkerMap m_markers; MarkerMap m_markers;
// Provide a quick way to determine whether a particular marker type is absent without going through the map. // Provide a quick way to determine whether a particular marker type is absent without going through the map.
......
...@@ -803,12 +803,14 @@ void SpellChecker::respondToChangedSelection(const VisibleSelection& oldSelectio ...@@ -803,12 +803,14 @@ void SpellChecker::respondToChangedSelection(const VisibleSelection& oldSelectio
// shouldEraseMarkersAfterChangeSelection is true, we cause synchronous // shouldEraseMarkersAfterChangeSelection is true, we cause synchronous
// layout. // layout.
if (textChecker().shouldEraseMarkersAfterChangeSelection(TextCheckingTypeSpelling)) { if (textChecker().shouldEraseMarkersAfterChangeSelection(TextCheckingTypeSpelling)) {
if (RefPtrWillBeRawPtr<Range> wordRange = newAdjacentWords.toNormalizedRange()) Position start, end;
m_frame.document()->markers().removeMarkers(wordRange.get(), DocumentMarker::Spelling); if (newAdjacentWords.toNormalizedPositions(start, end))
m_frame.document()->markers().removeMarkers(start, end, DocumentMarker::Spelling);
} }
if (textChecker().shouldEraseMarkersAfterChangeSelection(TextCheckingTypeGrammar)) { if (textChecker().shouldEraseMarkersAfterChangeSelection(TextCheckingTypeGrammar)) {
if (RefPtrWillBeRawPtr<Range> sentenceRange = newSelectedSentence.toNormalizedRange()) Position start, end;
m_frame.document()->markers().removeMarkers(sentenceRange.get(), DocumentMarker::Grammar); if (newSelectedSentence.toNormalizedPositions(start, end))
m_frame.document()->markers().removeMarkers(start, end, DocumentMarker::Grammar);
} }
} }
......
...@@ -1289,6 +1289,46 @@ SimplifiedBackwardsTextIterator::SimplifiedBackwardsTextIterator(const Range* r, ...@@ -1289,6 +1289,46 @@ SimplifiedBackwardsTextIterator::SimplifiedBackwardsTextIterator(const Range* r,
int startOffset = r->startOffset(); int startOffset = r->startOffset();
int endOffset = r->endOffset(); int endOffset = r->endOffset();
init(startNode, endNode, startOffset, endOffset);
}
SimplifiedBackwardsTextIterator::SimplifiedBackwardsTextIterator(const Position& start, const Position& end, TextIteratorBehaviorFlags behavior)
: m_node(nullptr)
, m_offset(0)
, m_handledNode(false)
, m_handledChildren(false)
, m_startNode(nullptr)
, m_startOffset(0)
, m_endNode(nullptr)
, m_endOffset(0)
, m_positionNode(nullptr)
, m_positionStartOffset(0)
, m_positionEndOffset(0)
, m_textOffset(0)
, m_textLength(0)
, m_lastTextNode(nullptr)
, m_lastCharacter(0)
, m_singleCharacterBuffer(0)
, m_havePassedStartNode(false)
, m_shouldHandleFirstLetter(false)
, m_stopsOnFormControls(behavior & TextIteratorStopsOnFormControls)
, m_shouldStop(false)
, m_emitsOriginalText(false)
{
ASSERT(behavior == TextIteratorDefaultBehavior || behavior == TextIteratorStopsOnFormControls);
Node* startNode = start.deprecatedNode();
if (!startNode)
return;
Node* endNode = end.deprecatedNode();
int startOffset = start.deprecatedEditingOffset();
int endOffset = end.deprecatedEditingOffset();
init(startNode, endNode, startOffset, endOffset);
}
void SimplifiedBackwardsTextIterator::init(Node* startNode, Node* endNode, int startOffset, int endOffset)
{
if (!startNode->offsetInCharacters() && startOffset >= 0) { if (!startNode->offsetInCharacters() && startOffset >= 0) {
// NodeTraversal::childAt() will return 0 if the offset is out of range. We rely on this behavior // NodeTraversal::childAt() will return 0 if the offset is out of range. We rely on this behavior
// instead of calling countChildren() to avoid traversing the children twice. // instead of calling countChildren() to avoid traversing the children twice.
...@@ -1713,6 +1753,16 @@ BackwardsCharacterIterator::BackwardsCharacterIterator(const Range* range, TextI ...@@ -1713,6 +1753,16 @@ BackwardsCharacterIterator::BackwardsCharacterIterator(const Range* range, TextI
m_textIterator.advance(); m_textIterator.advance();
} }
BackwardsCharacterIterator::BackwardsCharacterIterator(const Position& start, const Position& end, TextIteratorBehaviorFlags behavior)
: m_offset(0)
, m_runOffset(0)
, m_atBreak(true)
, m_textIterator(start, end, behavior)
{
while (!atEnd() && !m_textIterator.length())
m_textIterator.advance();
}
PassRefPtrWillBeRawPtr<Range> BackwardsCharacterIterator::range() const PassRefPtrWillBeRawPtr<Range> BackwardsCharacterIterator::range() const
{ {
RefPtrWillBeRawPtr<Range> r = m_textIterator.range(); RefPtrWillBeRawPtr<Range> r = m_textIterator.range();
......
...@@ -232,6 +232,7 @@ class SimplifiedBackwardsTextIterator { ...@@ -232,6 +232,7 @@ class SimplifiedBackwardsTextIterator {
STACK_ALLOCATED(); STACK_ALLOCATED();
public: public:
explicit SimplifiedBackwardsTextIterator(const Range*, TextIteratorBehaviorFlags = TextIteratorDefaultBehavior); explicit SimplifiedBackwardsTextIterator(const Range*, TextIteratorBehaviorFlags = TextIteratorDefaultBehavior);
SimplifiedBackwardsTextIterator(const Position& start, const Position& end, TextIteratorBehaviorFlags = TextIteratorDefaultBehavior);
bool atEnd() const { return !m_positionNode || m_shouldStop; } bool atEnd() const { return !m_positionNode || m_shouldStop; }
void advance(); void advance();
...@@ -258,6 +259,7 @@ public: ...@@ -258,6 +259,7 @@ public:
Position startPosition() const; Position startPosition() const;
private: private:
void init(Node* startNode, Node* endNode, int startOffset, int endOffset);
void exitNode(); void exitNode();
bool handleTextNode(); bool handleTextNode();
RenderText* handleFirstLetter(int& startOffset, int& offsetInNode); RenderText* handleFirstLetter(int& startOffset, int& offsetInNode);
...@@ -354,6 +356,7 @@ class BackwardsCharacterIterator { ...@@ -354,6 +356,7 @@ class BackwardsCharacterIterator {
STACK_ALLOCATED(); STACK_ALLOCATED();
public: public:
explicit BackwardsCharacterIterator(const Range*, TextIteratorBehaviorFlags = TextIteratorDefaultBehavior); explicit BackwardsCharacterIterator(const Range*, TextIteratorBehaviorFlags = TextIteratorDefaultBehavior);
BackwardsCharacterIterator(const Position&, const Position&, TextIteratorBehaviorFlags = TextIteratorDefaultBehavior);
void advance(int); void advance(int);
......
...@@ -203,9 +203,17 @@ bool VisibleSelection::intersectsNode(Node* node) const ...@@ -203,9 +203,17 @@ bool VisibleSelection::intersectsNode(Node* node) const
} }
PassRefPtrWillBeRawPtr<Range> VisibleSelection::toNormalizedRange() const PassRefPtrWillBeRawPtr<Range> VisibleSelection::toNormalizedRange() const
{
Position start, end;
if (toNormalizedPositions(start, end))
return Range::create(*start.document(), start, end);
return nullptr;
}
bool VisibleSelection::toNormalizedPositions(Position& start, Position& end) const
{ {
if (isNone()) if (isNone())
return nullptr; return false;
// Make sure we have an updated layout since this function is called // Make sure we have an updated layout since this function is called
// in the course of running edit commands which modify the DOM. // in the course of running edit commands which modify the DOM.
...@@ -215,15 +223,14 @@ PassRefPtrWillBeRawPtr<Range> VisibleSelection::toNormalizedRange() const ...@@ -215,15 +223,14 @@ PassRefPtrWillBeRawPtr<Range> VisibleSelection::toNormalizedRange() const
// Check again, because updating layout can clear the selection. // Check again, because updating layout can clear the selection.
if (isNone()) if (isNone())
return nullptr; return false;
Position s, e;
if (isCaret()) { if (isCaret()) {
// If the selection is a caret, move the range start upstream. This helps us match // If the selection is a caret, move the range start upstream. This helps us match
// the conventions of text editors tested, which make style determinations based // the conventions of text editors tested, which make style determinations based
// on the character before the caret, if any. // on the character before the caret, if any.
s = m_start.upstream().parentAnchoredEquivalent(); start = m_start.upstream().parentAnchoredEquivalent();
e = s; end = start;
} else { } else {
// If the selection is a range, select the minimum range that encompasses the selection. // If the selection is a range, select the minimum range that encompasses the selection.
// Again, this is to match the conventions of text editors tested, which make style // Again, this is to match the conventions of text editors tested, which make style
...@@ -237,25 +244,23 @@ PassRefPtrWillBeRawPtr<Range> VisibleSelection::toNormalizedRange() const ...@@ -237,25 +244,23 @@ PassRefPtrWillBeRawPtr<Range> VisibleSelection::toNormalizedRange() const
// ^ selected // ^ selected
// //
ASSERT(isRange()); ASSERT(isRange());
s = m_start.downstream(); start = m_start.downstream();
e = m_end.upstream(); end = m_end.upstream();
if (comparePositions(s, e) > 0) { if (comparePositions(start, end) > 0) {
// Make sure the start is before the end. // Make sure the start is before the end.
// The end can wind up before the start if collapsed whitespace is the only thing selected. // The end can wind up before the start if collapsed whitespace is the only thing selected.
Position tmp = s; Position tmp = start;
s = e; start = end;
e = tmp; end = tmp;
} }
s = s.parentAnchoredEquivalent(); start = start.parentAnchoredEquivalent();
e = e.parentAnchoredEquivalent(); end = end.parentAnchoredEquivalent();
} }
if (!s.containerNode() || !e.containerNode()) if (!start.containerNode() || !end.containerNode())
return nullptr; return false;
// VisibleSelections are supposed to always be valid. This constructor will ASSERT return true;
// if a valid range could not be created, which is fine for this callsite.
return Range::create(*s.document(), s, e);
} }
bool VisibleSelection::expandUsingGranularity(TextGranularity granularity) bool VisibleSelection::expandUsingGranularity(TextGranularity granularity)
......
...@@ -98,10 +98,12 @@ public: ...@@ -98,10 +98,12 @@ public:
bool intersectsNode(Node*) const; bool intersectsNode(Node*) const;
// FIXME: Most callers probably don't want this function, but are using it // FIXME: Most callers probably don't want these functions, but
// for historical reasons. toNormalizedRange contracts the range around // are using them for historical reasons. toNormalizedRange and
// text, and moves the caret upstream before returning the range. // toNormalizedPositions contracts the range around text, and
// moves the caret upstream before returning the range/positions.
PassRefPtrWillBeRawPtr<Range> toNormalizedRange() const; PassRefPtrWillBeRawPtr<Range> toNormalizedRange() const;
bool toNormalizedPositions(Position& start, Position& end) const;
Element* rootEditableElement() const; Element* rootEditableElement() const;
bool isContentEditable() const; bool isContentEditable() const;
......
...@@ -457,7 +457,6 @@ static VisiblePosition previousBoundary(const VisiblePosition& c, BoundarySearch ...@@ -457,7 +457,6 @@ static VisiblePosition previousBoundary(const VisiblePosition& c, BoundarySearch
Document& d = boundary->document(); Document& d = boundary->document();
Position start = createLegacyEditingPosition(boundary, 0).parentAnchoredEquivalent(); Position start = createLegacyEditingPosition(boundary, 0).parentAnchoredEquivalent();
Position end = pos.parentAnchoredEquivalent(); Position end = pos.parentAnchoredEquivalent();
RefPtrWillBeRawPtr<Range> searchRange = Range::create(d);
Vector<UChar, 1024> string; Vector<UChar, 1024> string;
unsigned suffixLength = 0; unsigned suffixLength = 0;
...@@ -480,14 +479,11 @@ static VisiblePosition previousBoundary(const VisiblePosition& c, BoundarySearch ...@@ -480,14 +479,11 @@ static VisiblePosition previousBoundary(const VisiblePosition& c, BoundarySearch
} }
} }
searchRange->setStart(start.deprecatedNode(), start.deprecatedEditingOffset(), exceptionState);
searchRange->setEnd(end.deprecatedNode(), end.deprecatedEditingOffset(), exceptionState);
ASSERT(!exceptionState.hadException()); ASSERT(!exceptionState.hadException());
if (exceptionState.hadException()) if (exceptionState.hadException())
return VisiblePosition(); return VisiblePosition();
SimplifiedBackwardsTextIterator it(searchRange.get()); SimplifiedBackwardsTextIterator it(start, end);
unsigned next = 0; unsigned next = 0;
bool needMoreContext = false; bool needMoreContext = false;
while (!it.atEnd()) { while (!it.atEnd()) {
...@@ -522,7 +518,7 @@ static VisiblePosition previousBoundary(const VisiblePosition& c, BoundarySearch ...@@ -522,7 +518,7 @@ static VisiblePosition previousBoundary(const VisiblePosition& c, BoundarySearch
return VisiblePosition(createLegacyEditingPosition(node, next), DOWNSTREAM); return VisiblePosition(createLegacyEditingPosition(node, next), DOWNSTREAM);
// Use the character iterator to translate the next value into a DOM position. // Use the character iterator to translate the next value into a DOM position.
BackwardsCharacterIterator charIt(searchRange.get()); BackwardsCharacterIterator charIt(start, end);
charIt.advance(string.size() - suffixLength - next); charIt.advance(string.size() - suffixLength - next);
// FIXME: charIt can get out of shadow host. // FIXME: charIt can get out of shadow host.
return VisiblePosition(charIt.endPosition(), DOWNSTREAM); return VisiblePosition(charIt.endPosition(), DOWNSTREAM);
...@@ -536,7 +532,6 @@ static VisiblePosition nextBoundary(const VisiblePosition& c, BoundarySearchFunc ...@@ -536,7 +532,6 @@ static VisiblePosition nextBoundary(const VisiblePosition& c, BoundarySearchFunc
return VisiblePosition(); return VisiblePosition();
Document& d = boundary->document(); Document& d = boundary->document();
RefPtrWillBeRawPtr<Range> searchRange(d.createRange());
Position start(pos.parentAnchoredEquivalent()); Position start(pos.parentAnchoredEquivalent());
Vector<UChar, 1024> string; Vector<UChar, 1024> string;
...@@ -559,9 +554,11 @@ static VisiblePosition nextBoundary(const VisiblePosition& c, BoundarySearchFunc ...@@ -559,9 +554,11 @@ static VisiblePosition nextBoundary(const VisiblePosition& c, BoundarySearchFunc
} }
} }
searchRange->selectNodeContents(boundary, IGNORE_EXCEPTION); Position searchStart = createLegacyEditingPosition(start.deprecatedNode(), start.deprecatedEditingOffset());
searchRange->setStart(start.deprecatedNode(), start.deprecatedEditingOffset(), IGNORE_EXCEPTION); RangeBoundaryPoint searchEndPoint(boundary);
TextIterator it(searchRange.get(), TextIteratorEmitsCharactersBetweenAllVisiblePositions); searchEndPoint.setToEndOfNode(*boundary);
Position searchEnd = searchEndPoint.toPosition();
TextIterator it(searchStart, searchEnd, TextIteratorEmitsCharactersBetweenAllVisiblePositions);
const unsigned invalidOffset = static_cast<unsigned>(-1); const unsigned invalidOffset = static_cast<unsigned>(-1);
unsigned next = invalidOffset; unsigned next = invalidOffset;
bool needMoreContext = false; bool needMoreContext = false;
...@@ -593,7 +590,7 @@ static VisiblePosition nextBoundary(const VisiblePosition& c, BoundarySearchFunc ...@@ -593,7 +590,7 @@ static VisiblePosition nextBoundary(const VisiblePosition& c, BoundarySearchFunc
pos = it.startPosition(); pos = it.startPosition();
} else if (next != invalidOffset && next != prefixLength) { } else if (next != invalidOffset && next != prefixLength) {
// Use the character iterator to translate the next value into a DOM position. // Use the character iterator to translate the next value into a DOM position.
CharacterIterator charIt(searchRange.get(), TextIteratorEmitsCharactersBetweenAllVisiblePositions); CharacterIterator charIt(searchStart, searchEnd, TextIteratorEmitsCharactersBetweenAllVisiblePositions);
charIt.advance(next - prefixLength - 1); charIt.advance(next - prefixLength - 1);
pos = charIt.endPosition(); pos = charIt.endPosition();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment