Bug 1433669: Flush the document instead of the shell in ContentEventHandler. r=masayuki
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 29 Jan 2018 12:14:05 +0100
changeset 401225 4a354c4ded0630dede088c0374f406c6dc556f1c
parent 401224 44b8352bbee0bb2bdd4b8cbbf2b0d62afa2cb80d
child 401226 39716502b1f1454ed21b72cf577e5da2770dc08c
push id58766
push userecoal95@gmail.com
push dateMon, 29 Jan 2018 11:52:42 +0000
treeherderautoland@4a354c4ded06 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmasayuki
bugs1433669
milestone60.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 1433669: Flush the document instead of the shell in ContentEventHandler. r=masayuki This allows us to maintain the pre-existing but not-asserted-before invariant of not doing layout on documents in the BFCache. The simpler fix here is something like: if (nsIDocument* doc = mPresShell->GetDocument()) { doc->FlushPendingNotifications(); } But referencing the document looks cleaner on most callsites I think. I can just do the above if you prefer. MozReview-Commit-ID: L4pTRW3eMAf
dom/events/ContentEventHandler.cpp
dom/events/ContentEventHandler.h
--- a/dom/events/ContentEventHandler.cpp
+++ b/dom/events/ContentEventHandler.cpp
@@ -239,32 +239,27 @@ ContentEventHandler::RawRange::SelectNod
 // 2. When the end node is an element node which doesn't have children,
 //    it includes the end (i.e., includes its close tag except empty element).
 //    Although, currently, any close tags don't cause line break, this also
 //    includes its open tag.  For example, if end position is { <br>, 0 }, the
 //    line break caused by the <br> should be included into the flatten text.
 
 ContentEventHandler::ContentEventHandler(nsPresContext* aPresContext)
   : mPresContext(aPresContext)
-  , mPresShell(aPresContext->GetPresShell())
+  , mDocument(aPresContext->Document())
 {
 }
 
 nsresult
 ContentEventHandler::InitBasic()
 {
-  NS_ENSURE_TRUE(mPresShell, NS_ERROR_NOT_AVAILABLE);
-
+  NS_ENSURE_TRUE(mDocument, NS_ERROR_NOT_AVAILABLE);
   // If text frame which has overflowing selection underline is dirty,
   // we need to flush the pending reflow here.
-  mPresShell->FlushPendingNotifications(FlushType::Layout);
-
-  // Flushing notifications can cause mPresShell to be destroyed (bug 577963).
-  NS_ENSURE_TRUE(!mPresShell->IsDestroying(), NS_ERROR_FAILURE);
-
+  mDocument->FlushPendingNotifications(FlushType::Layout);
   return NS_OK;
 }
 
 nsresult
 ContentEventHandler::InitRootContent(Selection* aNormalSelection)
 {
   MOZ_ASSERT(aNormalSelection);
 
@@ -278,17 +273,17 @@ ContentEventHandler::InitRootContent(Sel
     // If there is no selection range, we should compute the selection root
     // from ancestor limiter or root content of the document.
     nsresult rv =
       aNormalSelection->GetAncestorLimiter(getter_AddRefs(mRootContent));
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return NS_ERROR_FAILURE;
     }
     if (!mRootContent) {
-      mRootContent = mPresShell->GetDocument()->GetRootElement();
+      mRootContent = mDocument->GetRootElement();
       if (NS_WARN_IF(!mRootContent)) {
         return NS_ERROR_NOT_AVAILABLE;
       }
     }
     return NS_OK;
   }
 
   RefPtr<nsRange> range(aNormalSelection->GetRangeAt(0));
@@ -304,24 +299,24 @@ ContentEventHandler::InitRootContent(Sel
   // selection root from them.
   nsINode* startNode = range->GetStartContainer();
   nsINode* endNode = range->GetEndContainer();
   if (NS_WARN_IF(!startNode) || NS_WARN_IF(!endNode)) {
     return NS_ERROR_FAILURE;
   }
 
   // See bug 537041 comment 5, the range could have removed node.
-  if (NS_WARN_IF(startNode->GetComposedDoc() != mPresShell->GetDocument())) {
+  if (NS_WARN_IF(startNode->GetComposedDoc() != mDocument)) {
     return NS_ERROR_FAILURE;
   }
 
   NS_ASSERTION(startNode->GetComposedDoc() == endNode->GetComposedDoc(),
                "firstNormalSelectionRange crosses the document boundary");
 
-  mRootContent = startNode->GetSelectionRootContent(mPresShell);
+  mRootContent = startNode->GetSelectionRootContent(mDocument->GetShell());
   if (NS_WARN_IF(!mRootContent)) {
     return NS_ERROR_FAILURE;
   }
 
   return NS_OK;
 }
 
 nsresult
@@ -333,18 +328,20 @@ ContentEventHandler::InitCommon(Selectio
 
   mSelection = nullptr;
   mRootContent = nullptr;
   mFirstSelectedRawRange.Clear();
 
   nsresult rv = InitBasic();
   NS_ENSURE_SUCCESS(rv, rv);
 
-  nsCOMPtr<nsISelectionController> selectionController =
-    mPresShell->GetSelectionControllerForFocusedContent();
+  nsCOMPtr<nsISelectionController> selectionController;
+  if (nsIPresShell* shell = mDocument->GetShell()) {
+    selectionController = shell->GetSelectionControllerForFocusedContent();
+  }
   if (NS_WARN_IF(!selectionController)) {
     return NS_ERROR_NOT_AVAILABLE;
   }
   nsCOMPtr<nsISelection> selection;
   rv = selectionController->GetSelection(ToRawSelectionType(aSelectionType),
                                          getter_AddRefs(selection));
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return NS_ERROR_UNEXPECTED;
@@ -483,21 +480,17 @@ ContentEventHandler::Init(WidgetSelectio
   aEvent->mSucceeded = false;
 
   return NS_OK;
 }
 
 nsIContent*
 ContentEventHandler::GetFocusedContent()
 {
-  nsIDocument* doc = mPresShell->GetDocument();
-  if (!doc) {
-    return nullptr;
-  }
-  nsCOMPtr<nsPIDOMWindowOuter> window = doc->GetWindow();
+  nsCOMPtr<nsPIDOMWindowOuter> window = mDocument->GetWindow();
   nsCOMPtr<nsPIDOMWindowOuter> focusedWindow;
   return nsFocusManager::GetFocusedDescendant(
                            window,
                            nsFocusManager::eIncludeAllDescendants,
                            getter_AddRefs(focusedWindow));
 }
 
 bool
@@ -1069,17 +1062,18 @@ ContentEventHandler::ExpandToClusterBoun
   if (!aContent->IsNodeOfType(nsINode::eTEXT) ||
       *aXPOffset == 0 || *aXPOffset == aContent->TextLength()) {
     return NS_OK;
   }
 
   NS_ASSERTION(*aXPOffset <= aContent->TextLength(),
                "offset is out of range.");
 
-  RefPtr<nsFrameSelection> fs = mPresShell->FrameSelection();
+  MOZ_DIAGNOSTIC_ASSERT(mDocument->GetShell());
+  RefPtr<nsFrameSelection> fs = mDocument->GetShell()->FrameSelection();
   int32_t offsetInFrame;
   CaretAssociationHint hint =
     aForward ? CARET_ASSOCIATE_BEFORE : CARET_ASSOCIATE_AFTER;
   nsIFrame* frame = fs->GetFrameForNodeOffset(aContent, int32_t(*aXPOffset),
                                               hint, &offsetInFrame);
   if (frame) {
     int32_t startOffset, endOffset;
     nsresult rv = frame->GetOffsets(startOffset, endOffset);
@@ -2666,21 +2660,18 @@ ContentEventHandler::OnQuerySelectionAsT
   }
 
   if (!aEvent->mReply.mHasSelection) {
     aEvent->mSucceeded = true;
     aEvent->mReply.mTransferable = nullptr;
     return NS_OK;
   }
 
-  nsCOMPtr<nsIDocument> doc = mPresShell->GetDocument();
-  NS_ENSURE_TRUE(doc, NS_ERROR_FAILURE);
-
   rv = nsCopySupport::GetTransferableForSelection(
-         mSelection, doc, getter_AddRefs(aEvent->mReply.mTransferable));
+         mSelection, mDocument, getter_AddRefs(aEvent->mReply.mTransferable));
   NS_ENSURE_SUCCESS(rv, rv);
 
   aEvent->mSucceeded = true;
   return NS_OK;
 }
 
 nsresult
 ContentEventHandler::OnQueryCharacterAtPoint(WidgetQueryContentEvent* aEvent)
@@ -2688,17 +2679,19 @@ ContentEventHandler::OnQueryCharacterAtP
   nsresult rv = Init(aEvent);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   aEvent->mReply.mOffset = aEvent->mReply.mTentativeCaretOffset =
     WidgetQueryContentEvent::NOT_FOUND;
 
-  nsIFrame* rootFrame = mPresShell->GetRootFrame();
+  nsIPresShell* shell = mDocument->GetShell();
+  NS_ENSURE_TRUE(shell, NS_ERROR_FAILURE);
+  nsIFrame* rootFrame = shell->GetRootFrame();
   NS_ENSURE_TRUE(rootFrame, NS_ERROR_FAILURE);
   nsIWidget* rootWidget = rootFrame->GetNearestWidget();
   NS_ENSURE_TRUE(rootWidget, NS_ERROR_FAILURE);
 
   // The root frame's widget might be different, e.g., the event was fired on
   // a popup but the rootFrame is the document root.
   if (rootWidget != aEvent->mWidget) {
     NS_PRECONDITION(aEvent->mWidget, "The event must have the widget");
@@ -2801,30 +2794,30 @@ ContentEventHandler::OnQueryDOMWidgetHit
     return rv;
   }
 
   aEvent->mSucceeded = false;
   aEvent->mReply.mWidgetIsHit = false;
 
   NS_ENSURE_TRUE(aEvent->mWidget, NS_ERROR_FAILURE);
 
-  nsIDocument* doc = mPresShell->GetDocument();
-  NS_ENSURE_TRUE(doc, NS_ERROR_FAILURE);
-  nsIFrame* docFrame = mPresShell->GetRootFrame();
+  nsIPresShell* shell = mDocument->GetShell();
+  NS_ENSURE_TRUE(shell, NS_ERROR_FAILURE);
+  nsIFrame* docFrame = shell->GetRootFrame();
   NS_ENSURE_TRUE(docFrame, NS_ERROR_FAILURE);
 
   LayoutDeviceIntPoint eventLoc =
     aEvent->mRefPoint + aEvent->mWidget->WidgetToScreenOffset();
   CSSIntRect docFrameRect = docFrame->GetScreenRect();
   CSSIntPoint eventLocCSS(
     mPresContext->DevPixelsToIntCSSPixels(eventLoc.x) - docFrameRect.x,
     mPresContext->DevPixelsToIntCSSPixels(eventLoc.y) - docFrameRect.y);
 
   Element* contentUnderMouse =
-    doc->ElementFromPointHelper(eventLocCSS.x, eventLocCSS.y, false, false);
+    mDocument->ElementFromPointHelper(eventLocCSS.x, eventLocCSS.y, false, false);
   if (contentUnderMouse) {
     nsIWidget* targetWidget = nullptr;
     nsIFrame* targetFrame = contentUnderMouse->GetPrimaryFrame();
     nsIObjectFrame* pluginFrame = do_QueryFrame(targetFrame);
     if (pluginFrame) {
       targetWidget = pluginFrame->GetWidget();
     } else if (targetFrame) {
       targetWidget = targetFrame->GetNearestWidget();
--- a/dom/events/ContentEventHandler.h
+++ b/dom/events/ContentEventHandler.h
@@ -136,17 +136,17 @@ public:
   // eQueryDOMWidgetHittest event handler
   nsresult OnQueryDOMWidgetHittest(WidgetQueryContentEvent* aEvent);
 
   // NS_SELECTION_* event
   nsresult OnSelectionEvent(WidgetSelectionEvent* aEvent);
 
 protected:
   nsPresContext* mPresContext;
-  nsCOMPtr<nsIPresShell> mPresShell;
+  nsCOMPtr<nsIDocument> mDocument;
   // mSelection is typically normal selection but if OnQuerySelectedText()
   // is called, i.e., handling eQuerySelectedText, it's the specified selection
   // by WidgetQueryContentEvent::mInput::mSelectionType.
   RefPtr<Selection> mSelection;
   // mFirstSelectedRawRange is initialized from the first range of mSelection,
   // if it exists.  Otherwise, it is reset by Clear().
   RawRange mFirstSelectedRawRange;
   nsCOMPtr<nsIContent> mRootContent;