Bug 1733465 part 3: Make nsPresContext::GetRootWidget() return an already_AddRefed pointer instead of a raw pointer. r=tnikkel

Also, make the same change to nsPresContext::GetTextInputHandlingWidget and
TextComposition::GetWidget, which are essentially aliases/wrappers for this
function.

This patch shouldn't change behavior at all, aside from:
* optimizing away some redundant reference counting and widget-lookups
* delaying some nsIWidget::Release() calls, which will now happen after we're
  actually done using the object, instead of happening when the getter
  completes. (It's unlikely this impacts behavior, because there are other
  objects that are keeping the nsIWidget instance alive.)

Motivation / "wins" from this patch:
* nsPresContext::GetRootWidget already works with a refcounted pointer
  internally. Before this patch, it drops the reference before returning the
  pointer.  This is a bit suspect and would cause security issues, in the
  unlikely event that this were the last strong reference to the object.  It
  can just as easily/efficiently transfer the strong reference to the caller,
  and let the caller determine when to release it.
* Many of the callers were already storing the return value in nsCOMPtr, which
  meant that they were incurring an additional AddRef/Release when
  populating/destructing that smart pointer. Now they can just take ownership
  of the already_AddRefed return value and avoid redundnat refcount-churn.
* For the callers that weren't storing the return value in nsCOMPtr, some of
  them were calling this getter twice in a row (once to test for truthiness and
  once to use the known-truthy value).  This was wasteful, both from the
  repeated lookup-work (since the function isn't a trivial getter), and from
  repeated refcount-churn.  This patch collapses these repeat-calls to a single
  call, avoiding those inefficiencies.

Differential Revision: https://phabricator.services.mozilla.com/D127180

accessible/windows/msaa/MsaaAccessible.cpp

@@ -251,8 +251,7 @@ HWND MsaaAccessible::GetHWNDFor(Accessible* aAccessible) {
     nsIWidget* widget = frame->GetNearestWidget();
     if (widget && widget->IsVisible()) {
       if (nsViewManager* vm = document->PresShellPtr()->GetViewManager()) {
-        nsCOMPtr<nsIWidget> rootWidget;
-        vm->GetRootWidget(getter_AddRefs(rootWidget));
+        nsCOMPtr<nsIWidget> rootWidget = vm->GetRootWidget();
         // Make sure the accessible belongs to popup. If not then use
         // document HWND (which might be different from root widget in the
         // case of window emulation).

dom/events/IMEStateManager.cpp

@@ -767,8 +767,12 @@ void IMEStateManager::OnClickInEditor(nsPresContext* aPresContext,
 
   nsCOMPtr<nsIWidget> widget(sWidget);
 
-  MOZ_ASSERT(!sPresContext->GetTextInputHandlingWidget() ||
-             sPresContext->GetTextInputHandlingWidget() == widget);
+#ifdef DEBUG
+  {
+    nsCOMPtr<nsIWidget> tihWidget = sPresContext->GetTextInputHandlingWidget();
+    MOZ_ASSERT(!tihWidget || tihWidget == widget);
+  }
+#endif  // DEBUG
 
   if (!aMouseEvent->IsTrusted()) {
     MOZ_LOG(sISMLog, LogLevel::Debug,
@@ -1001,8 +1005,12 @@ void IMEStateManager::UpdateIMEState(const IMEState& aNewIMEState,
 
   OwningNonNull<nsIWidget> widget(*sWidget);
 
-  MOZ_ASSERT(!sPresContext->GetTextInputHandlingWidget() ||
-             sPresContext->GetTextInputHandlingWidget() == widget);
+#ifdef DEBUG
+  {
+    nsCOMPtr<nsIWidget> tihWidget = sPresContext->GetTextInputHandlingWidget();
+    MOZ_ASSERT(!tihWidget || tihWidget == widget);
+  }
+#endif  // DEBUG
 
   // TODO: Investigate if we could put off to initialize IMEContentObserver
   //       later because a lot of callers need to be marked as
@@ -1215,8 +1223,12 @@ void IMEStateManager::SetInputContextForChildProcess(
 
   nsCOMPtr<nsIWidget> widget(sWidget);
 
-  MOZ_ASSERT(!sPresContext->GetTextInputHandlingWidget() ||
-             sPresContext->GetTextInputHandlingWidget() == widget);
+#ifdef DEBUG
+  {
+    nsCOMPtr<nsIWidget> tihWidget = sPresContext->GetTextInputHandlingWidget();
+    MOZ_ASSERT(!tihWidget || tihWidget == widget);
+  }
+#endif  // DEBUG
   MOZ_ASSERT(aInputContext.mOrigin == InputContext::ORIGIN_CONTENT);
 
   sActiveChildInputContext = aInputContext;
@@ -1883,7 +1895,7 @@ nsresult IMEStateManager::NotifyIME(IMEMessage aMessage,
     return NS_ERROR_INVALID_ARG;
   }
 
-  nsIWidget* widget = aPresContext->GetTextInputHandlingWidget();
+  nsCOMPtr<nsIWidget> widget = aPresContext->GetTextInputHandlingWidget();
   if (NS_WARN_IF(!widget)) {
     MOZ_LOG(sISMLog, LogLevel::Error,
             ("  NotifyIME(), FAILED due to no widget for the "
@@ -2015,7 +2027,12 @@ void IMEStateManager::CreateIMEContentObserver(EditorBase& aEditorBase) {
     return;
   }
 
-  MOZ_ASSERT(sPresContext->GetTextInputHandlingWidget() == widget);
+#ifdef DEBUG
+  {
+    nsCOMPtr<nsIWidget> tihWidget = sPresContext->GetTextInputHandlingWidget();
+    MOZ_ASSERT(tihWidget == widget);
+  }
+#endif  // DEBUG
 
   MOZ_LOG(sISMLog, LogLevel::Debug,
           ("  CreateIMEContentObserver() is creating an "

dom/events/TextComposition.h

@@ -13,6 +13,7 @@
 #include "nsTArray.h"
 #include "nsThreadUtils.h"
 #include "nsPresContext.h"
+#include "mozilla/AlreadyAddRefed.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/EventForwards.h"
 #include "mozilla/RangeBoundary.h"
@@ -75,7 +76,7 @@ class TextComposition final {
   //     error due to inaccessible Release() method.
   TextRangeArray* GetRanges() const { return mRanges; }
   // Returns the widget which is proper to call NotifyIME().
-  nsIWidget* GetWidget() const {
+  already_AddRefed<nsIWidget> GetWidget() const {
     return mPresContext ? mPresContext->GetRootWidget() : nullptr;
   }
   // Returns the tab parent which has this composition in its remote process.

dom/events/TouchEvent.cpp

@@ -242,8 +242,11 @@ bool TouchEvent::PrefEnabled(nsIDocShell* aDocShell) {
         // APZ might be disabled on this particular widget, in which case
         // TouchEvent support will also be disabled. Try to detect that.
         RefPtr<nsPresContext> pc = aDocShell->GetPresContext();
-        if (pc && pc->GetRootWidget()) {
-          enabled &= pc->GetRootWidget()->AsyncPanZoomEnabled();
+        if (pc) {
+          nsCOMPtr<nsIWidget> widget = pc->GetRootWidget();
+          if (widget) {
+            enabled &= widget->AsyncPanZoomEnabled();
+          }
         }
       }
 #endif

layout/base/nsPresContext.cpp

@@ -1075,21 +1075,14 @@ nsIWidget* nsPresContext::GetNearestWidget(nsPoint* aOffset) {
   return rootView->GetNearestWidget(aOffset);
 }
 
-nsIWidget* nsPresContext::GetRootWidget() const {
+already_AddRefed<nsIWidget> nsPresContext::GetRootWidget() const {
   NS_ENSURE_TRUE(mPresShell, nullptr);
   nsViewManager* vm = mPresShell->GetViewManager();
   if (!vm) {
     return nullptr;
   }
 
-  // XXXdholbert REVIEW NOTE: It's kind of sketchy that we're returning a raw
-  // pointer to a refcounted object here, when we've got an owning reference
-  // which we release just as we return the raw pointer.  Plus, it's wasteful
-  // to be incurring an AddRef/Release operation before the object actually
-  // even gets used (and potentially AddRef'ed again) by the caller.  I'll be
-  // cleaning this up to address these issues in the next patch in this series.
-  nsCOMPtr<nsIWidget> widget = vm->GetRootWidget();
-  return widget.get();
+  return vm->GetRootWidget();
 }
 
 // We may want to replace this with something faster, maybe caching the root

layout/base/nsPresContext.h

@@ -232,13 +232,13 @@ class nsPresContext : public nsISupports, public mozilla::SupportsWeakPtr {
   /**
    * Returns the root widget for this.
    */
-  nsIWidget* GetRootWidget() const;
+  already_AddRefed<nsIWidget> GetRootWidget() const;
 
   /**
    * Returns the widget which may have native focus and handles text input
    * like keyboard input, IME, etc.
    */
-  nsIWidget* GetTextInputHandlingWidget() const {
+  already_AddRefed<nsIWidget> GetTextInputHandlingWidget() const {
     // Currently, root widget for each PresContext handles text input.
     return GetRootWidget();
   }
@@ -1100,6 +1100,7 @@ class nsPresContext : public nsISupports, public mozilla::SupportsWeakPtr {
   void UpdateCharSet(NotNull<const Encoding*> aCharSet);
 
   void DoForceReflowForFontInfoUpdateFromStyle();
+
  public:
   // Used by the PresShell to force a reflow when some aspect of font info
   // has been updated, potentially affecting font selection and layout.

layout/base/nsRefreshDriver.cpp

@@ -993,7 +993,7 @@ void nsRefreshDriver::CreateVsyncRefreshTimer() {
   if (!mOwnTimer) {
     // If available, we fetch the widget-specific vsync source.
     nsPresContext* pc = GetPresContext();
-    nsIWidget* widget = pc->GetRootWidget();
+    nsCOMPtr<nsIWidget> widget = pc->GetRootWidget();
     if (widget) {
       if (RefPtr<gfx::VsyncSource> localVsyncSource =
               widget->GetVsyncSource()) {
@@ -2514,7 +2514,7 @@ void nsRefreshDriver::Tick(VsyncId aId, TimeStamp aNowTime,
 
     // Forward our composition payloads to the layer manager.
     if (!mCompositionPayloads.IsEmpty()) {
-      nsIWidget* widget = mPresContext->GetRootWidget();
+      nsCOMPtr<nsIWidget> widget = mPresContext->GetRootWidget();
       WindowRenderer* renderer = widget ? widget->GetWindowRenderer() : nullptr;
       if (renderer && renderer->AsWebRender()) {
         renderer->AsWebRender()->RegisterPayloads(mCompositionPayloads);

layout/xul/nsXULPopupManager.cpp

@@ -966,7 +966,7 @@ void nsXULPopupManager::ShowTooltipAtScreen(nsIContent* aPopup,
   // coordinates are relative to the root widget
   nsPresContext* rootPresContext = pc->GetRootPresContext();
   if (rootPresContext) {
-    nsIWidget* rootWidget = rootPresContext->GetRootWidget();
+    nsCOMPtr<nsIWidget> rootWidget = rootPresContext->GetRootWidget();
     if (rootWidget) {
       mousePoint -= rootWidget->WidgetToScreenOffset();
     }

widget/gtk/nsNativeThemeGTK.cpp

@@ -68,7 +68,7 @@ static inline gint GetMonitorScaleFactor(nsPresContext* aPresContext) {
   // the real monitor scale cannot go under 1.
   double scale = StaticPrefs::layout_css_devPixelsPerPx();
   if (scale <= 0) {
-    if (nsIWidget* rootWidget = aPresContext->GetRootWidget()) {
+    if (nsCOMPtr<nsIWidget> rootWidget = aPresContext->GetRootWidget()) {
       // We need to use GetDefaultScale() despite it returns monitor scale
       // factor multiplied by font scale factor because it is the only scale
       // updated in nsPuppetWidget.

widget/nsNativeBasicTheme.cpp

@@ -360,7 +360,7 @@ static std::pair<sRGBColor, sRGBColor> SystemColorPair(
 auto nsNativeBasicTheme::GetDPIRatioForScrollbarPart(nsPresContext* aPc)
     -> DPIRatio {
   if (auto* rootPc = aPc->GetRootPresContext()) {
-    if (auto* widget = rootPc->GetRootWidget()) {
+    if (nsCOMPtr<nsIWidget> widget = rootPc->GetRootWidget()) {
       return widget->GetDefaultScale();
     }
   }

widget/windows/nsNativeThemeWin.cpp

@@ -1462,7 +1462,7 @@ static bool AssumeThemePartAndStateAreTransparent(int32_t aPart,
 static inline double GetThemeDpiScaleFactor(nsPresContext* aPresContext) {
   if (WinUtils::IsPerMonitorDPIAware() ||
       StaticPrefs::layout_css_devPixelsPerPx() > 0.0) {
-    nsIWidget* rootWidget = aPresContext->GetRootWidget();
+    nsCOMPtr<nsIWidget> rootWidget = aPresContext->GetRootWidget();
     if (rootWidget) {
       double systemScale = WinUtils::SystemScaleFactor();
       return rootWidget->GetDefaultScale().scale / systemScale;
@@ -2007,7 +2007,7 @@ bool nsNativeThemeWin::GetWidgetPadding(nsDeviceContext* aContext,
     // the border padding. This should be addressed in nsWindow,
     // but currently can't be, see UpdateNonClientMargins.
     if (aAppearance == StyleAppearance::MozWindowTitlebarMaximized) {
-      nsIWidget* rootWidget = nullptr;
+      nsCOMPtr<nsIWidget> rootWidget;
       if (WinUtils::HasSystemMetricsForDpi()) {
         rootWidget = aFrame->PresContext()->GetRootWidget();
       }