Bug 1433883: Remove weak pres context pointer from ContentEventHandler. r=masayuki
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 29 Jan 2018 13:04:00 +0100
changeset 401230 0255a42c0eb9ad6a8f7ff3d726f1519bc4912fa8
parent 401229 2c4940ee43bb0fc31e196c6c9a3251e65e19065c
child 401231 e87115ab4b57cd8ee8374e431e7ff22c1b99322d
push id58771
push userecoal95@gmail.com
push dateMon, 29 Jan 2018 13:09:35 +0000
treeherderautoland@0255a42c0eb9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmasayuki
bugs1433883
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 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;