Bug 1367690 Optimize Selection::selectFrames() r?mats
Selection::selectFrames() always creates two content iterators. However, if the range is in a node and it doesn't have children, we don't need walk the DOM tree. So, it should create content iterators only when they are necessary.
Additionally, both Selection::selectFrames() and Selection::SelectAllFramesForContent() handle first content twice, but it's not necessary.
Finally, they query interface to retrieve nsIContent* from nsINode* a lot.
This patch fixes those issues too.
MozReview-Commit-ID: 5bfYz6Zuqkg
--- a/layout/generic/Selection.h
+++ b/layout/generic/Selection.h
@@ -337,16 +337,17 @@ private:
nsIPresShell::ScrollAxis mVerticalScroll;
nsIPresShell::ScrollAxis mHorizontalScroll;
int32_t mFlags;
};
void setAnchorFocusRange(int32_t aIndex); // pass in index into mRanges;
// negative value clears
// mAnchorFocusRange
+ void SelectFramesForContent(nsIContent* aContent, bool aSelected);
nsresult SelectAllFramesForContent(nsIContentIterator *aInnerIter,
nsIContent *aContent,
bool aSelected);
nsresult selectFrames(nsPresContext* aPresContext, nsRange *aRange, bool aSelect);
nsresult getTableCellLocationFromRange(nsRange *aRange, int32_t *aSelectionType, int32_t *aRow, int32_t *aCol);
nsresult addTableCellRange(nsRange *aRange, bool *aDidAddRange, int32_t *aOutIndex);
nsresult FindInsertionPoint(
--- a/layout/generic/nsSelection.cpp
+++ b/layout/generic/nsSelection.cpp
@@ -4456,56 +4456,63 @@ Selection::GetPrimaryFrameForFocusNode(n
GetFrameForNodeOffset(content, FocusOffset(),
hint, aOffsetUsed);
if (!*aReturnFrame)
return NS_ERROR_FAILURE;
return NS_OK;
}
+void
+Selection::SelectFramesForContent(nsIContent* aContent,
+ bool aSelected)
+{
+ nsIFrame* frame = aContent->GetPrimaryFrame();
+ if (!frame) {
+ return;
+ }
+ // The frame could be an SVG text frame, in which case we don't treat it
+ // as a text frame.
+ if (frame->IsTextFrame()) {
+ nsTextFrame* textFrame = static_cast<nsTextFrame*>(frame);
+ textFrame->SetSelectedRange(0, aContent->GetText()->GetLength(),
+ aSelected, mSelectionType);
+ } else {
+ frame->InvalidateFrameSubtree(); // frame continuations?
+ }
+}
+
//select all content children of aContent
nsresult
Selection::SelectAllFramesForContent(nsIContentIterator* aInnerIter,
nsIContent* aContent,
bool aSelected)
{
- nsresult result = aInnerIter->Init(aContent);
- nsIFrame *frame;
- if (NS_SUCCEEDED(result))
- {
- // First select frame of content passed in
- frame = aContent->GetPrimaryFrame();
- if (frame && frame->IsTextFrame()) {
- nsTextFrame* textFrame = static_cast<nsTextFrame*>(frame);
- textFrame->SetSelectedRange(0, aContent->GetText()->GetLength(),
- aSelected, mSelectionType);
- }
- // Now iterated through the child frames and set them
- while (!aInnerIter->IsDone()) {
- nsCOMPtr<nsIContent> innercontent =
- do_QueryInterface(aInnerIter->GetCurrentNode());
-
- frame = innercontent->GetPrimaryFrame();
- if (frame) {
- if (frame->IsTextFrame()) {
- nsTextFrame* textFrame = static_cast<nsTextFrame*>(frame);
- textFrame->SetSelectedRange(0, innercontent->GetText()->GetLength(),
- aSelected, mSelectionType);
- } else {
- frame->InvalidateFrameSubtree(); // frame continuations?
- }
- }
-
- aInnerIter->Next();
- }
-
+ // If aContent doesn't have children, we should avoid to use the content
+ // iterator for performance reason.
+ if (!aContent->HasChildren()) {
+ SelectFramesForContent(aContent, aSelected);
return NS_OK;
}
- return NS_ERROR_FAILURE;
+ if (NS_WARN_IF(NS_FAILED(aInnerIter->Init(aContent)))) {
+ return NS_ERROR_FAILURE;
+ }
+
+ for (; !aInnerIter->IsDone(); aInnerIter->Next()) {
+ nsINode* node = aInnerIter->GetCurrentNode();
+ // Detect the bug of content iterator, but shouldn't cause a crash in
+ // release builds.
+ MOZ_ASSERT(node);
+ nsIContent* innercontent =
+ node && node->IsContent() ? node->AsContent() : nullptr;
+ SelectFramesForContent(innercontent, aSelected);
+ }
+
+ return NS_OK;
}
/**
* The idea of this helper method is to select or deselect "top to bottom",
* traversing through the frames
*/
nsresult
Selection::selectFrames(nsPresContext* aPresContext, nsRange* aRange,
@@ -4522,61 +4529,87 @@ Selection::selectFrames(nsPresContext* a
nsIFrame* frame = node->IsContent() ? node->AsContent()->GetPrimaryFrame()
: aPresContext->FrameManager()->GetRootFrame();
if (frame) {
frame->InvalidateFrameSubtree();
}
return NS_OK;
}
- nsCOMPtr<nsIContentIterator> iter = NS_NewContentSubtreeIterator();
- iter->Init(aRange);
// Loop through the content iterator for each content node; for each text
// node, call SetSelected on it:
- nsCOMPtr<nsIContent> content = do_QueryInterface(aRange->GetStartParent());
- if (!content) {
+ nsINode* startNode = aRange->GetStartParent();
+ nsIContent* startContent =
+ startNode && startNode->IsContent() ? startNode->AsContent() : nullptr;
+ if (!startContent) {
// Don't warn, bug 1055722
return NS_ERROR_UNEXPECTED;
}
// We must call first one explicitly
- if (content->IsNodeOfType(nsINode::eTEXT)) {
- nsIFrame* frame = content->GetPrimaryFrame();
- // The frame could be an SVG text frame, in which case we'll ignore it.
- if (frame && frame->IsTextFrame()) {
- nsTextFrame* textFrame = static_cast<nsTextFrame*>(frame);
- uint32_t startOffset = aRange->StartOffset();
- uint32_t endOffset;
- if (aRange->GetEndParent() == content) {
- endOffset = aRange->EndOffset();
+ bool isFirstContentTextNode = startContent->IsNodeOfType(nsINode::eTEXT);
+ nsINode* endNode = aRange->GetEndParent();
+ if (isFirstContentTextNode) {
+ nsIFrame* frame = startContent->GetPrimaryFrame();
+ // The frame could be an SVG text frame, in which case we don't treat it
+ // as a text frame.
+ if (frame) {
+ if (frame->IsTextFrame()) {
+ nsTextFrame* textFrame = static_cast<nsTextFrame*>(frame);
+ uint32_t startOffset = aRange->StartOffset();
+ uint32_t endOffset;
+ if (endNode == startContent) {
+ endOffset = aRange->EndOffset();
+ } else {
+ endOffset = startContent->Length();
+ }
+ textFrame->SetSelectedRange(startOffset, endOffset, aSelect,
+ mSelectionType);
} else {
- endOffset = content->Length();
+ frame->InvalidateFrameSubtree();
}
- textFrame->SetSelectedRange(startOffset, endOffset, aSelect,
- mSelectionType);
+ }
+ }
+
+ // If the range is in a node and the node is a leaf node, we don't need to
+ // walk the subtree.
+ if (aRange->Collapsed() ||
+ (startNode == endNode && !startNode->HasChildren())) {
+ if (!isFirstContentTextNode) {
+ SelectFramesForContent(startContent, aSelect);
}
- }
-
- iter->First();
+ return NS_OK;
+ }
+
+ nsCOMPtr<nsIContentIterator> iter = NS_NewContentSubtreeIterator();
+ iter->Init(aRange);
+ if (isFirstContentTextNode && !iter->IsDone()) {
+ iter->Next(); // first content has already been handled.
+ }
nsCOMPtr<nsIContentIterator> inneriter = NS_NewContentIterator();
- for (iter->First(); !iter->IsDone(); iter->Next()) {
- content = do_QueryInterface(iter->GetCurrentNode());
+ for (; !iter->IsDone(); iter->Next()) {
+ nsINode* node = iter->GetCurrentNode();
+ // Detect the bug of content iterator, but shouldn't cause a crash in
+ // release builds.
+ MOZ_ASSERT(node);
+ nsIContent* content =
+ node && node->IsContent() ? node->AsContent() : nullptr;
SelectAllFramesForContent(inneriter, content, aSelect);
}
// We must now do the last one if it is not the same as the first
- if (aRange->GetEndParent() != aRange->GetStartParent()) {
- nsresult res;
- content = do_QueryInterface(aRange->GetEndParent(), &res);
- NS_ENSURE_SUCCESS(res, res);
- NS_ENSURE_TRUE(content, res);
-
- if (content->IsNodeOfType(nsINode::eTEXT)) {
- nsIFrame* frame = content->GetPrimaryFrame();
+ if (endNode != startNode) {
+ nsIContent* endContent =
+ endNode && endNode->IsContent() ? endNode->AsContent() : nullptr;
+ if (NS_WARN_IF(!endContent)) {
+ return NS_ERROR_UNEXPECTED;
+ }
+ if (endContent->IsNodeOfType(nsINode::eTEXT)) {
+ nsIFrame* frame = endContent->GetPrimaryFrame();
// The frame could be an SVG text frame, in which case we'll ignore it.
if (frame && frame->IsTextFrame()) {
nsTextFrame* textFrame = static_cast<nsTextFrame*>(frame);
textFrame->SetSelectedRange(0, aRange->EndOffset(), aSelect,
mSelectionType);
}
}
}