Bug 1433669: Flush the document instead of the shell in ContentEventHandler. r?masayuki draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 29 Jan 2018 12:14:05 +0100
changeset 748254 03d052426eb3a099e68bdfff2e7d96bc2845614c
parent 748253 a622549ed3235891ec88193d9d249d784010b22b
child 748259 00ccd3d22dcbf4147d709634d3eff5999f90039b
child 748261 400e3b87b90b64654ea93b8de0a0217ee1c6921b
push id97096
push userbmo:emilio@crisal.io
push dateMon, 29 Jan 2018 11:17:44 +0000
reviewersmasayuki
bugs1433669
milestone60.0a1
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;