Bug 1553769 - Make nsIWidget::SetFocus infallible, and make it take an enum class. r=NeilDeakin

Only gtk returns failure ever, and nobody checks the result anyway.

Use an enum class so that it's clear from the caller what it means.

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

--HG--
extra : moz-landing-system : lando

dom/base/nsFocusManager.cpp

@@ -1093,7 +1093,7 @@ void nsFocusManager::EnsureCurrentWidgetFocused() {
   if (!widget) {
     return;
   }
-  widget->SetFocus(false);
+  widget->SetFocus(nsIWidget::Raise::No);
 }
 
 bool ActivateOrDeactivateChild(BrowserParent* aParent, void* aArg) {
@@ -1629,13 +1629,12 @@ bool nsFocusManager::Blur(nsPIDOMWindowOuter* aWindowToClear,
         } else {
           // note that the presshell's widget is being retrieved here, not the
           // one for the object frame.
-          nsViewManager* vm = presShell->GetViewManager();
-          if (vm) {
+          if (nsViewManager* vm = presShell->GetViewManager()) {
             nsCOMPtr<nsIWidget> widget;
             vm->GetRootWidget(getter_AddRefs(widget));
             if (widget) {
               // set focus to the top level window but don't raise it.
-              widget->SetFocus(false);
+              widget->SetFocus(nsIWidget::Raise::No);
             }
           }
         }
@@ -1821,11 +1820,10 @@ void nsFocusManager::Focus(nsPIDOMWindowOuter* aWindow, Element* aElement,
     if (objectFrame) objectFrameWidget = objectFrame->GetWidget();
   }
   if (aAdjustWidgets && !objectFrameWidget && !sTestMode) {
-    nsViewManager* vm = presShell->GetViewManager();
-    if (vm) {
+    if (nsViewManager* vm = presShell->GetViewManager()) {
       nsCOMPtr<nsIWidget> widget;
       vm->GetRootWidget(getter_AddRefs(widget));
-      if (widget) widget->SetFocus(false);
+      if (widget) widget->SetFocus(nsIWidget::Raise::No);
     }
   }
 
@@ -1878,8 +1876,9 @@ void nsFocusManager::Focus(nsPIDOMWindowOuter* aWindow, Element* aElement,
       // that we might no longer be in the same document, due to the events we
       // fired above when aIsNewDocument.
       if (presShell->GetDocument() == aElement->GetComposedDoc()) {
-        if (aAdjustWidgets && objectFrameWidget && !sTestMode)
-          objectFrameWidget->SetFocus(false);
+        if (aAdjustWidgets && objectFrameWidget && !sTestMode) {
+          objectFrameWidget->SetFocus(nsIWidget::Raise::No);
+        }
 
         // if the object being focused is a remote browser, activate remote
         // content
@@ -1911,11 +1910,12 @@ void nsFocusManager::Focus(nsPIDOMWindowOuter* aWindow, Element* aElement,
     // the root widget.
     if (aAdjustWidgets && objectFrameWidget && mFocusedWindow == aWindow &&
         mFocusedElement == nullptr && !sTestMode) {
-      nsViewManager* vm = presShell->GetViewManager();
-      if (vm) {
+      if (nsViewManager* vm = presShell->GetViewManager()) {
         nsCOMPtr<nsIWidget> widget;
         vm->GetRootWidget(getter_AddRefs(widget));
-        if (widget) widget->SetFocus(false);
+        if (widget) {
+          widget->SetFocus(nsIWidget::Raise::No);
+        }
       }
     }
 
@@ -2189,11 +2189,10 @@ void nsFocusManager::RaiseWindow(nsPIDOMWindowOuter* aWindow) {
     return;
   }
 
-  nsViewManager* vm = presShell->GetViewManager();
-  if (vm) {
+  if (nsViewManager* vm = presShell->GetViewManager()) {
     nsCOMPtr<nsIWidget> widget;
     vm->GetRootWidget(getter_AddRefs(widget));
-    if (widget) widget->SetFocus(true);
+    if (widget) widget->SetFocus(nsIWidget::Raise::Yes);
   }
 #else
   nsCOMPtr<nsIBaseWindow> treeOwnerAsWin =
@@ -2201,7 +2200,7 @@ void nsFocusManager::RaiseWindow(nsPIDOMWindowOuter* aWindow) {
   if (treeOwnerAsWin) {
     nsCOMPtr<nsIWidget> widget;
     treeOwnerAsWin->GetMainWidget(getter_AddRefs(widget));
-    if (widget) widget->SetFocus(true);
+    if (widget) widget->SetFocus(nsIWidget::Raise::Yes);
   }
 #endif
 }

dom/ipc/BrowserParent.cpp

@@ -2812,9 +2812,8 @@ mozilla::ipc::IPCResult BrowserParent::RecvSetNativeChildOfShareableWindow(
 }
 
 mozilla::ipc::IPCResult BrowserParent::RecvDispatchFocusToTopLevelWindow() {
-  nsCOMPtr<nsIWidget> widget = GetTopLevelWidget();
-  if (widget) {
-    widget->SetFocus(false);
+  if (nsCOMPtr<nsIWidget> widget = GetTopLevelWidget()) {
+    widget->SetFocus(nsIWidget::Raise::No);
   }
   return IPC_OK();
 }

dom/plugins/ipc/PluginWidgetParent.cpp

@@ -141,7 +141,7 @@ void PluginWidgetParent::ParentDestroy() {
 mozilla::ipc::IPCResult PluginWidgetParent::RecvSetFocus(const bool& aRaise) {
   ENSURE_CHANNEL;
   PWLOG("PluginWidgetParent::RecvSetFocus(%d)\n", aRaise);
-  mWidget->SetFocus(aRaise);
+  mWidget->SetFocus(aRaise ? nsIWidget::Raise::Yes : nsIWidget::Raise::No);
   return IPC_OK();
 }
 

gfx/tests/gtest/MockWidget.h

@@ -61,7 +61,7 @@ class MockWidget : public nsBaseWidget {
 
   virtual void Enable(bool aState) override {}
   virtual bool IsEnabled() const override { return true; }
-  virtual nsresult SetFocus(bool aRaise) override { return NS_OK; }
+  virtual void SetFocus(Raise) override {}
   virtual nsresult ConfigureChildren(
       const nsTArray<Configuration>& aConfigurations) override {
     return NS_OK;

widget/PluginWidgetProxy.cpp

@@ -161,11 +161,10 @@ void PluginWidgetProxy::SetNativeData(uint32_t aDataType, uintptr_t aVal) {
   }
 }
 
-nsresult PluginWidgetProxy::SetFocus(bool aRaise) {
+void PluginWidgetProxy::SetFocus(Raise aRaise) {
   ENSURE_CHANNEL;
-  PWLOG("PluginWidgetProxy::SetFocus(%d)\n", aRaise);
-  mActor->SendSetFocus(aRaise);
-  return NS_OK;
+  PWLOG("PluginWidgetProxy::SetFocus(%d)\n", aRaise == Raise::Yes);
+  mActor->SendSetFocus(aRaise == Raise::Yes);
 }
 
 }  // namespace widget

widget/PluginWidgetProxy.h

@@ -43,7 +43,7 @@ class PluginWidgetProxy final : public PuppetWidget {
          const LayoutDeviceIntRect& aRect,
          nsWidgetInitData* aInitData = nullptr) override;
   virtual void Destroy() override;
-  virtual nsresult SetFocus(bool aRaise = false) override;
+  virtual void SetFocus(Raise) override;
   virtual void SetParent(nsIWidget* aNewParent) override;
 
   virtual nsIWidget* GetParent(void) override;

widget/PuppetWidget.cpp

@@ -260,12 +260,10 @@ nsresult PuppetWidget::ConfigureChildren(
   return NS_OK;
 }
 
-nsresult PuppetWidget::SetFocus(bool aRaise) {
-  if (aRaise && mBrowserChild) {
+void PuppetWidget::SetFocus(Raise aRaise) {
+  if (aRaise == Raise::Yes && mBrowserChild) {
     mBrowserChild->SendRequestFocus(true);
   }
-
-  return NS_OK;
 }
 
 void PuppetWidget::Invalidate(const LayoutDeviceIntRect& aRect) {

widget/PuppetWidget.h

@@ -111,7 +111,7 @@ class PuppetWidget : public nsBaseWidget,
   virtual void Enable(bool aState) override { mEnabled = aState; }
   virtual bool IsEnabled() const override { return mEnabled; }
 
-  virtual nsresult SetFocus(bool aRaise = false) override;
+  virtual void SetFocus(Raise) override;
 
   virtual nsresult ConfigureChildren(
       const nsTArray<Configuration>& aConfigurations) override;

widget/android/nsWindow.cpp

@@ -1743,11 +1743,9 @@ nsWindow* nsWindow::FindTopLevel() {
   return this;
 }
 
-nsresult nsWindow::SetFocus(bool aRaise) {
-  nsWindow* top = FindTopLevel();
-  top->BringToFront();
-
-  return NS_OK;
+void nsWindow::SetFocus(Raise) {
+  // FIXME: Shouldn't this account for the argument?
+  FindTopLevel()->BringToFront();
 }
 
 void nsWindow::BringToFront() {

widget/android/nsWindow.h

@@ -249,7 +249,7 @@ class nsWindow final : public nsBaseWidget {
   virtual void Enable(bool aState) override;
   virtual bool IsEnabled() const override;
   virtual void Invalidate(const LayoutDeviceIntRect& aRect) override;
-  virtual nsresult SetFocus(bool aRaise = false) override;
+  virtual void SetFocus(Raise) override;
   virtual LayoutDeviceIntRect GetScreenBounds() override;
   virtual LayoutDeviceIntPoint WidgetToScreenOffset() override;
   virtual nsresult DispatchEvent(mozilla::WidgetGUIEvent* aEvent,

widget/cocoa/nsChildView.h

@@ -332,7 +332,7 @@ class nsChildView final : public nsBaseWidget {
 
   virtual void Enable(bool aState) override;
   virtual bool IsEnabled() const override;
-  virtual nsresult SetFocus(bool aRaise) override;
+  virtual void SetFocus(Raise) override;
   virtual LayoutDeviceIntRect GetBounds() override;
   virtual LayoutDeviceIntRect GetClientBounds() override;
   virtual LayoutDeviceIntRect GetScreenBounds() override;

widget/cocoa/nsChildView.mm

@@ -372,7 +372,7 @@ void nsChildView::ReleaseTitlebarCGContext() {
 
 nsresult nsChildView::Create(nsIWidget* aParent, nsNativeWidget aNativeParent,
                              const LayoutDeviceIntRect& aRect, nsWidgetInitData* aInitData) {
-  NS_OBJC_BEGIN_TRY_ABORT_BLOCK_NSRESULT;
+  NS_OBJC_BEGIN_TRY_ABORT_BLOCK;
 
   // Because the hidden window is created outside of an event loop,
   // we need to provide an autorelease pool to avoid leaking cocoa objects
@@ -723,14 +723,12 @@ void nsChildView::Enable(bool aState) {}
 
 bool nsChildView::IsEnabled() const { return true; }
 
-nsresult nsChildView::SetFocus(bool aRaise) {
+void nsChildView::SetFocus(Raise) {
   NS_OBJC_BEGIN_TRY_ABORT_BLOCK_NSRESULT;
 
   NSWindow* window = [mView window];
   if (window) [window makeFirstResponder:mView];
-  return NS_OK;
-
-  NS_OBJC_END_TRY_ABORT_BLOCK_NSRESULT;
+  NS_OBJC_END_TRY_ABORT_BLOCK;
 }
 
 // Override to set the cursor on the mac

widget/cocoa/nsCocoaWindow.h

@@ -230,7 +230,7 @@ class nsCocoaWindow final : public nsBaseWidget, public nsPIWidgetCocoa {
   virtual void SetFakeModal(bool aState) override;
   virtual bool IsRunningAppModal() override;
   virtual bool IsVisible() const override;
-  virtual nsresult SetFocus(bool aState = false) override;
+  virtual void SetFocus(Raise) override;
   virtual LayoutDeviceIntPoint WidgetToScreenOffset() override;
   virtual LayoutDeviceIntPoint GetClientOffset() override;
   virtual LayoutDeviceIntSize ClientToWindowSize(const LayoutDeviceIntSize& aClientSize) override;

widget/cocoa/nsCocoaWindow.mm

@@ -1838,12 +1838,14 @@ void nsCocoaWindow::SetMenuBar(nsMenuBarX* aMenuBar) {
     mMenuBar->Paint();
 }
 
-nsresult nsCocoaWindow::SetFocus(bool aState) {
-  if (!mWindow) return NS_OK;
+void nsCocoaWindow::SetFocus(Raise aRaise) {
+  if (!mWindow) return;
 
   if (mPopupContentView) {
-    mPopupContentView->SetFocus(aState);
-  } else if (aState && ([mWindow isVisible] || [mWindow isMiniaturized])) {
+    return mPopupContentView->SetFocus(aRaise);
+  }
+
+  if (aRaise == Raise::Yes && ([mWindow isVisible] || [mWindow isMiniaturized])) {
     if ([mWindow isMiniaturized]) {
       [mWindow deminiaturize:nil];
     }
@@ -1851,8 +1853,6 @@ nsresult nsCocoaWindow::SetFocus(bool aState) {
     [mWindow makeKeyAndOrderFront:nil];
     SendSetZLevelEvent();
   }
-
-  return NS_OK;
 }
 
 LayoutDeviceIntPoint nsCocoaWindow::WidgetToScreenOffset() {

widget/gtk/nsWindow.cpp

@@ -1466,20 +1466,20 @@ guint32 nsWindow::GetLastUserInputTime() {
   return timestamp;
 }
 
-nsresult nsWindow::SetFocus(bool aRaise) {
+void nsWindow::SetFocus(Raise aRaise) {
   // Make sure that our owning widget has focus.  If it doesn't try to
   // grab it.  Note that we don't set our focus flag in this case.
 
-  LOGFOCUS(("  SetFocus %d [%p]\n", aRaise, (void*)this));
+  LOGFOCUS(("  SetFocus %d [%p]\n", aRaise == Raise::Yes, (void*)this));
 
   GtkWidget* owningWidget = GetMozContainerWidget();
-  if (!owningWidget) return NS_ERROR_FAILURE;
+  if (!owningWidget) return;
 
   // Raise the window if someone passed in true and the prefs are
   // set properly.
   GtkWidget* toplevelWidget = gtk_widget_get_toplevel(owningWidget);
 
-  if (gRaiseWindows && aRaise && toplevelWidget &&
+  if (gRaiseWindows && aRaise == Raise::Yes && toplevelWidget &&
       !gtk_widget_has_focus(owningWidget) &&
       !gtk_widget_has_focus(toplevelWidget)) {
     GtkWidget* top_window = GetToplevelWidget();
@@ -1491,10 +1491,10 @@ nsresult nsWindow::SetFocus(bool aRaise) {
   }
 
   RefPtr<nsWindow> owningWindow = get_window_for_gtk_widget(owningWidget);
-  if (!owningWindow) return NS_ERROR_FAILURE;
+  if (!owningWindow) return;
 
-  if (aRaise) {
-    // aRaise == true means request toplevel activation.
+  if (aRaise == Raise::Yes) {
+    // means request toplevel activation.
 
     // This is asynchronous.
     // If and when the window manager accepts the request, then the focus
@@ -1513,12 +1513,11 @@ nsresult nsWindow::SetFocus(bool aRaise) {
 
       if (GTKToolkit) GTKToolkit->SetFocusTimestamp(0);
     }
-
-    return NS_OK;
+    return;
   }
 
-  // aRaise == false means that keyboard events should be dispatched
-  // from this widget.
+  // aRaise == No means that keyboard events should be dispatched from this
+  // widget.
 
   // Ensure owningWidget is the focused GtkWidget within its toplevel window.
   //
@@ -1537,7 +1536,7 @@ nsresult nsWindow::SetFocus(bool aRaise) {
   // If this is the widget that already has focus, return.
   if (gFocusWindow == this) {
     LOGFOCUS(("  already have focus [%p]\n", (void*)this));
-    return NS_OK;
+    return;
   }
 
   // Set this window to be the focused child window
@@ -1548,8 +1547,6 @@ nsresult nsWindow::SetFocus(bool aRaise) {
   }
 
   LOGFOCUS(("  widget now has focus in SetFocus() [%p]\n", (void*)this));
-
-  return NS_OK;
 }
 
 LayoutDeviceIntRect nsWindow::GetScreenBounds() {

widget/gtk/nsWindow.h

@@ -139,7 +139,7 @@ class nsWindow final : public nsBaseWidget {
   void SetZIndex(int32_t aZIndex) override;
   virtual void SetSizeMode(nsSizeMode aMode) override;
   virtual void Enable(bool aState) override;
-  virtual nsresult SetFocus(bool aRaise = false) override;
+  virtual void SetFocus(Raise) override;
   virtual LayoutDeviceIntRect GetScreenBounds() override;
   virtual LayoutDeviceIntRect GetClientBounds() override;
   virtual LayoutDeviceIntSize GetClientSize() override;

widget/headless/HeadlessWidget.cpp

@@ -200,18 +200,17 @@ void HeadlessWidget::Show(bool aState) {
 
 bool HeadlessWidget::IsVisible() const { return mVisible; }
 
-nsresult HeadlessWidget::SetFocus(bool aRaise) {
-  LOGFOCUS(("  SetFocus %d [%p]\n", aRaise, (void*)this));
+void HeadlessWidget::SetFocus(Raise aRaise) {
+  LOGFOCUS(("  SetFocus %d [%p]\n", aRaise == Raise::Yes, (void*)this));
 
-  // aRaise == true means we request activation of our toplevel window.
-  if (aRaise) {
+  // This means we request activation of our toplevel window.
+  if (aRaise == Raise::Yes) {
     HeadlessWidget* topLevel = (HeadlessWidget*)GetTopLevelWidget();
 
     // The toplevel only becomes active if it's currently visible; otherwise, it
     // will be activated anyway when it's shown.
     if (topLevel->IsVisible()) topLevel->RaiseWindow();
   }
-  return NS_OK;
 }
 
 void HeadlessWidget::Enable(bool aState) { mEnabled = aState; }

widget/headless/HeadlessWidget.h

@@ -89,7 +89,7 @@ class HeadlessWidget : public nsBaseWidget {
                                   nsIScreen* aTargetScreen = nullptr) override;
   virtual void Enable(bool aState) override;
   virtual bool IsEnabled() const override;
-  virtual nsresult SetFocus(bool aRaise) override;
+  virtual void SetFocus(Raise) override;
   virtual nsresult ConfigureChildren(
       const nsTArray<Configuration>& aConfigurations) override {
     MOZ_ASSERT_UNREACHABLE(

widget/nsIWidget.h

@@ -853,16 +853,18 @@ class nsIWidget : public nsISupports {
    */
   virtual bool IsEnabled() const = 0;
 
+  /*
+   * Whether we should request activation of this widget's toplevel window.
+   */
+  enum class Raise {
+    No,
+    Yes,
+  };
+
   /**
    * Request activation of this window or give focus to this widget.
-   *
-   * @param aRaise If true, this function requests activation of this
-   *               widget's toplevel window.
-   *               If false, the appropriate toplevel window (which in
-   *               the case of popups may not be this widget's toplevel
-   *               window) is already active.
    */
-  virtual nsresult SetFocus(bool aRaise = false) = 0;
+  virtual void SetFocus(Raise) = 0;
 
   /**
    * Get this widget's outside dimensions relative to its parent widget. For

widget/uikit/nsWindow.h

@@ -35,7 +35,7 @@ class nsWindow final : public nsBaseWidget {
   virtual void Enable(bool aState) override {}
   virtual bool IsEnabled() const override { return true; }
   virtual bool IsVisible() const override { return mVisible; }
-  virtual nsresult SetFocus(bool aState = false) override;
+  virtual void SetFocus(Raise) override;
   virtual LayoutDeviceIntPoint WidgetToScreenOffset() override;
 
   virtual void SetBackgroundColor(const nscolor& aColor) override;

widget/uikit/nsWindow.mm

@@ -591,10 +591,9 @@ void nsWindow::Invalidate(const LayoutDeviceIntRect& aRect) {
   [mNativeView setNeedsDisplayInRect:DevPixelsToUIKitPoints(mBounds, BackingScaleFactor())];
 }
 
-nsresult nsWindow::SetFocus(bool aRaise) {
+void nsWindow::SetFocus(Raise) {
   [[mNativeView window] makeKeyWindow];
   [mNativeView becomeFirstResponder];
-  return NS_OK;
 }
 
 void nsWindow::WillPaintWindow() {

widget/windows/nsWindow.cpp

@@ -2165,25 +2165,24 @@ bool nsWindow::IsEnabled() const {
  *
  **************************************************************/
 
-nsresult nsWindow::SetFocus(bool aRaise) {
+void nsWindow::SetFocus(Raise aRaise) {
   if (mWnd) {
 #ifdef WINSTATE_DEBUG_OUTPUT
     if (mWnd == WinUtils::GetTopLevelHWND(mWnd)) {
       MOZ_LOG(gWindowsLog, LogLevel::Info,
-              ("*** SetFocus: [  top] raise=%d\n", aRaise));
+              ("*** SetFocus: [  top] raise=%d\n", aRaise == Raise::Yes));
     } else {
       MOZ_LOG(gWindowsLog, LogLevel::Info,
-              ("*** SetFocus: [child] raise=%d\n", aRaise));
+              ("*** SetFocus: [child] raise=%d\n", aRaise == Raise::Yes));
     }
 #endif
     // Uniconify, if necessary
     HWND toplevelWnd = WinUtils::GetTopLevelHWND(mWnd);
-    if (aRaise && ::IsIconic(toplevelWnd)) {
+    if (aRaise == Raise::Yes && ::IsIconic(toplevelWnd)) {
       ::ShowWindow(toplevelWnd, SW_RESTORE);
     }
     ::SetFocus(mWnd);
   }
-  return NS_OK;
 }
 
 /**************************************************************

widget/windows/nsWindow.h

@@ -144,7 +144,7 @@ class nsWindow final : public nsWindowBase {
   virtual void SuppressAnimation(bool aSuppress) override;
   virtual void Enable(bool aState) override;
   virtual bool IsEnabled() const override;
-  virtual nsresult SetFocus(bool aRaise) override;
+  virtual void SetFocus(Raise) override;
   virtual LayoutDeviceIntRect GetBounds() override;
   virtual LayoutDeviceIntRect GetScreenBounds() override;
   virtual MOZ_MUST_USE nsresult