Bug 1433883: Remove weak pres context pointer from ContentEventHandler. r?masayuki draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 29 Jan 2018 13:04:00 +0100
changeset 748261 400e3b87b90b64654ea93b8de0a0217ee1c6921b
parent 748254 03d052426eb3a099e68bdfff2e7d96bc2845614c
push id97103
push userbmo:emilio@crisal.io
push dateMon, 29 Jan 2018 13:06:52 +0000
reviewersmasayuki
bugs1433883
milestone60.0a1
Bug 1433883: Remove weak pres context pointer from ContentEventHandler. r?masayuki Holding a weak pres context pointer across stuff that does flushes is dangerous. Hopefully, we just poke at it when we find a frame (thus a pres context should be around, and it rather be the one that we started poking at). I think it'd be better to just not keep the member around, since frames can reach the pres context easily. MozReview-Commit-ID: HcepvzcSsaH
dom/events/ContentEventHandler.cpp
dom/events/ContentEventHandler.h
--- a/dom/events/ContentEventHandler.cpp
+++ b/dom/events/ContentEventHandler.cpp
@@ -238,18 +238,17 @@ ContentEventHandler::RawRange::SelectNod
 //    caused by <br> should be included into the flatten text.
 // 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)
-  , mDocument(aPresContext->Document())
+  : mDocument(aPresContext->Document())
 {
 }
 
 nsresult
 ContentEventHandler::InitBasic()
 {
   NS_ENSURE_TRUE(mDocument, NS_ERROR_NOT_AVAILABLE);
   // If text frame which has overflowing selection underline is dirty,
@@ -509,26 +508,28 @@ ContentEventHandler::QueryContentRect(ns
   nsIFrame* frame = aContent->GetPrimaryFrame();
   NS_ENSURE_TRUE(frame, NS_ERROR_FAILURE);
 
   // get rect for first frame
   nsRect resultRect(nsPoint(0, 0), frame->GetRect().Size());
   nsresult rv = ConvertToRootRelativeOffset(frame, resultRect);
   NS_ENSURE_SUCCESS(rv, rv);
 
+  nsPresContext* presContext = frame->PresContext();
+
   // account for any additional frames
-  while ((frame = frame->GetNextContinuation()) != nullptr) {
+  while ((frame = frame->GetNextContinuation())) {
     nsRect frameRect(nsPoint(0, 0), frame->GetRect().Size());
     rv = ConvertToRootRelativeOffset(frame, frameRect);
     NS_ENSURE_SUCCESS(rv, rv);
     resultRect.UnionRect(resultRect, frameRect);
   }
 
   aEvent->mReply.mRect = LayoutDeviceIntRect::FromUnknownRect(
-      resultRect.ToOutsidePixels(mPresContext->AppUnitsPerDevPixel()));
+      resultRect.ToOutsidePixels(presContext->AppUnitsPerDevPixel()));
   // Returning empty rect may cause native IME confused, let's make sure to
   // return non-empty rect.
   EnsureNonEmptyRect(aEvent->mReply.mRect);
   aEvent->mSucceeded = true;
 
   return NS_OK;
 }
 
@@ -1855,20 +1856,20 @@ ContentEventHandler::GetLineBreakerRectB
     if (kWritingMode.IsVertical()) {
       if (kWritingMode.IsLineInverted()) {
         // above of top-left corner of aFrame.
         result.mRect.x = 0;
       } else {
         // above of top-right corner of aFrame.
         result.mRect.x = aFrame->GetRect().XMost() - result.mRect.width;
       }
-      result.mRect.y = -mPresContext->AppUnitsPerDevPixel();
+      result.mRect.y = -aFrame->PresContext()->AppUnitsPerDevPixel();
     } else {
       // left of top-left corner of aFrame.
-      result.mRect.x = -mPresContext->AppUnitsPerDevPixel();
+      result.mRect.x = -aFrame->PresContext()->AppUnitsPerDevPixel();
       result.mRect.y = 0;
     }
   }
   return result;
 }
 
 ContentEventHandler::FrameRelativeRect
 ContentEventHandler::GuessLineBreakerRectAfter(nsIContent* aTextContent)
@@ -1903,35 +1904,36 @@ ContentEventHandler::GuessLineBreakerRec
   result.mBaseFrame = lastTextFrame;
   return result;
 }
 
 ContentEventHandler::FrameRelativeRect
 ContentEventHandler::GuessFirstCaretRectIn(nsIFrame* aFrame)
 {
   const WritingMode kWritingMode = aFrame->GetWritingMode();
+  nsPresContext* presContext = aFrame->PresContext();
 
   // Computes the font height, but if it's not available, we should use
   // default font size of Firefox.  The default font size in default settings
   // is 16px.
   RefPtr<nsFontMetrics> fontMetrics =
     nsLayoutUtils::GetInflatedFontMetricsForFrame(aFrame);
   const nscoord kMaxHeight =
     fontMetrics ? fontMetrics->MaxHeight() :
-                  16 * mPresContext->AppUnitsPerDevPixel();
+                  16 * presContext->AppUnitsPerDevPixel();
 
   nsRect caretRect;
   const nsRect kContentRect = aFrame->GetContentRect() - aFrame->GetPosition();
   caretRect.y = kContentRect.y;
   if (!kWritingMode.IsVertical()) {
     if (kWritingMode.IsBidiLTR()) {
       caretRect.x = kContentRect.x;
     } else {
       // Move 1px left for the space of caret itself.
-      const nscoord kOnePixel = mPresContext->AppUnitsPerDevPixel();
+      const nscoord kOnePixel = presContext->AppUnitsPerDevPixel();
       caretRect.x = kContentRect.XMost() - kOnePixel;
     }
     caretRect.height = kMaxHeight;
     // However, don't add kOnePixel here because it may cause 2px width at
     // aligning the edge to device pixels.
     caretRect.width = 1;
   } else {
     if (kWritingMode.IsVerticalLR()) {
@@ -2171,18 +2173,18 @@ ContentEventHandler::OnQueryTextRectArra
       // base frame.
       lastCharRect = charRect;
       lastFrame = baseFrame;
       rv = ConvertToRootRelativeOffset(baseFrame, charRect);
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
 
-      rect = LayoutDeviceIntRect::FromUnknownRect(
-               charRect.ToOutsidePixels(mPresContext->AppUnitsPerDevPixel()));
+      rect = LayoutDeviceIntRect::FromUnknownRect(charRect.ToOutsidePixels(
+        baseFrame->PresContext()->AppUnitsPerDevPixel()));
       // Returning empty rect may cause native IME confused, let's make sure to
       // return non-empty rect.
       EnsureNonEmptyRect(rect);
 
       aEvent->mReply.mRectArray.AppendElement(rect);
       offset++;
 
       // If it's not a line breaker or the line breaker length is same as
@@ -2323,17 +2325,19 @@ ContentEventHandler::OnQueryTextRect(Wid
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return NS_ERROR_UNEXPECTED;
     }
     nsRect rect;
     FrameAndNodeOffset lastFrame = GetLastFrameInRangeForTextRect(rawRange);
     // If there is at least one frame which can be used for computing a rect
     // for a character or a line breaker, we should use it for guessing the
     // caret rect at the end of the contents.
+    nsPresContext* presContext;
     if (lastFrame) {
+      presContext = lastFrame->PresContext();
       if (NS_WARN_IF(!lastFrame->GetContent())) {
         return NS_ERROR_FAILURE;
       }
       FrameRelativeRect relativeRect;
       // If there is a <br> frame at the end, it represents an empty line at
       // the end with moz-<br> or content <br> in a block level element.
       if (lastFrame->IsBrFrame()) {
         relativeRect = GetLineBreakerRectBefore(lastFrame);
@@ -2359,29 +2363,30 @@ ContentEventHandler::OnQueryTextRect(Wid
     }
     // Otherwise, if there are no contents in mRootContent, guess caret rect in
     // its frame (with its font height and content box).
     else {
       nsIFrame* rootContentFrame = mRootContent->GetPrimaryFrame();
       if (NS_WARN_IF(!rootContentFrame)) {
         return NS_ERROR_FAILURE;
       }
+      presContext = rootContentFrame->PresContext();
       FrameRelativeRect relativeRect = GuessFirstCaretRectIn(rootContentFrame);
       if (NS_WARN_IF(!relativeRect.IsValid())) {
         return NS_ERROR_FAILURE;
       }
       rect = relativeRect.RectRelativeTo(rootContentFrame);
       rv = ConvertToRootRelativeOffset(rootContentFrame, rect);
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
       aEvent->mReply.mWritingMode = rootContentFrame->GetWritingMode();
     }
     aEvent->mReply.mRect = LayoutDeviceIntRect::FromUnknownRect(
-      rect.ToOutsidePixels(mPresContext->AppUnitsPerDevPixel()));
+      rect.ToOutsidePixels(presContext->AppUnitsPerDevPixel()));
     EnsureNonEmptyRect(aEvent->mReply.mRect);
     aEvent->mSucceeded = true;
     return NS_OK;
   }
 
   nsRect rect, frameRect;
   nsPoint ptOffset;
 
@@ -2553,17 +2558,17 @@ ContentEventHandler::OnQueryTextRect(Wid
     if (firstFrame.mFrame == lastFrame.mFrame) {
       rect.IntersectRect(rect, frameRect);
     } else {
       rect.UnionRect(rect, frameRect);
     }
   }
 
   aEvent->mReply.mRect = LayoutDeviceIntRect::FromUnknownRect(
-      rect.ToOutsidePixels(mPresContext->AppUnitsPerDevPixel()));
+      rect.ToOutsidePixels(lastFrame->PresContext()->AppUnitsPerDevPixel()));
   // Returning empty rect may cause native IME confused, let's make sure to
   // return non-empty rect.
   EnsureNonEmptyRect(aEvent->mReply.mRect);
   aEvent->mReply.mWritingMode = lastFrame->GetWritingMode();
   aEvent->mSucceeded = true;
   return NS_OK;
 }
 
@@ -2803,18 +2808,18 @@ ContentEventHandler::OnQueryDOMWidgetHit
   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);
+    docFrame->PresContext()->DevPixelsToIntCSSPixels(eventLoc.x) - docFrameRect.x,
+    docFrame->PresContext()->DevPixelsToIntCSSPixels(eventLoc.y) - docFrameRect.y);
 
   Element* contentUnderMouse =
     mDocument->ElementFromPointHelper(eventLocCSS.x, eventLocCSS.y, false, false);
   if (contentUnderMouse) {
     nsIWidget* targetWidget = nullptr;
     nsIFrame* targetFrame = contentUnderMouse->GetPrimaryFrame();
     nsIObjectFrame* pluginFrame = do_QueryFrame(targetFrame);
     if (pluginFrame) {
--- a/dom/events/ContentEventHandler.h
+++ b/dom/events/ContentEventHandler.h
@@ -135,17 +135,16 @@ public:
   nsresult OnQueryCharacterAtPoint(WidgetQueryContentEvent* aEvent);
   // eQueryDOMWidgetHittest event handler
   nsresult OnQueryDOMWidgetHittest(WidgetQueryContentEvent* aEvent);
 
   // NS_SELECTION_* event
   nsresult OnSelectionEvent(WidgetSelectionEvent* aEvent);
 
 protected:
-  nsPresContext* mPresContext;
   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;