Bug 1367690 Optimize Selection::selectFrames() r=mats
authorMasayuki Nakano <masayuki@d-toybox.com>
Sun, 28 May 2017 09:37:10 +0900
changeset 409145 429241f2e18f70aa47ddeba7fa70f1514ab16990
parent 409144 89357ce3b8124ce475b340c41269d7acaebd1acb
child 409146 331d128e1f58379ebd8722237893fbf774929c91
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmats
bugs1367690
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
@@ -4459,56 +4459,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,
@@ -4525,61 +4532,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);
       }
     }
   }