Bug 1367690 Optimize Selection::selectFrames() r?mats draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Sun, 28 May 2017 09:37:10 +0900
changeset 585674 9a385a47a1a536e7d2b1b5f26e5efe51675c5d4a
parent 584316 e983fbfdd7c9334719e569348ba5cd1a4e656734
child 630778 d042cebb30830c3205b0a087438f0643c3060cfd
push id61172
push usermasayuki@d-toybox.com
push dateSun, 28 May 2017 00:43:55 +0000
reviewersmats
bugs1367690
milestone55.0a1
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
layout/generic/Selection.h
layout/generic/nsSelection.cpp
--- 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);
       }
     }
   }