Bug 1811487 - Clean-up popup hide / rollup APIs. r=cmartin,stransky

I'm about to extend them for bug 1811486, where I want to force in some
cases the rolled up popups to hide synchronously. These APIs use a ton
of boolean arguments that make them error prone, so refactor them a bit
to use strongly typed enums and flags.

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

Author: emilio

browser/components/customizableui/test/browser_1484275_PanelMultiView_toggle_with_other_popup.js

@@ -43,8 +43,8 @@ add_task(async function test_PanelMultiView_toggle_with_other_popup() {
           eventTypeToWait: "mouseup",
         });
 
-      if (AppConstants.platform == "win") {
-        // On Windows, the operation will close both popups.
+      // On Windows and macOS, the operation will close both popups.
+      if (AppConstants.platform == "win" || AppConstants.platform == "macosx") {
         await gCUITestUtils.hidePanelMultiView(PanelUI.panel, clickFn);
         await new Promise(resolve => executeSoon(resolve));
 

browser/components/customizableui/test/browser_PanelMultiView_keyboard.js

@@ -290,14 +290,11 @@ add_task(async function testTabOpenMenulist() {
   await shown;
   ok(gMainMenulist.open, "menulist open");
   let menuHidden = BrowserTestUtils.waitForEvent(popup, "popuphidden");
-  let panelHidden = BrowserTestUtils.waitForEvent(gPanel, "popuphidden");
   EventUtils.synthesizeKey("KEY_Tab");
   await menuHidden;
   ok(!gMainMenulist.open, "menulist closed after Tab");
-  // Tab in an open menulist closes the menulist, but also dismisses the panel
-  // above it (bug 1566673). So, we just wait for the panel to hide rather than
-  // using hidePopup().
-  await panelHidden;
+  is(gPanel.state, "open", "Panel should be open");
+  await hidePopup();
 });
 
 if (AppConstants.platform == "macosx") {
@@ -327,14 +324,11 @@ if (AppConstants.platform == "macosx") {
     );
 
     let menuHidden = BrowserTestUtils.waitForEvent(popup, "popuphidden");
-    let panelHidden = BrowserTestUtils.waitForEvent(gPanel, "popuphidden");
     EventUtils.synthesizeKey("KEY_Tab");
     await menuHidden;
     ok(!gMainMenulist.open, "menulist closed after Tab");
-    // Tab in an open menulist closes the menulist, but also dismisses the panel
-    // above it (bug 1566673). So, we just wait for the panel to hide rather than
-    // using hidePopup().
-    await panelHidden;
+    is(gPanel.state, "open", "Panel should be open");
+    await hidePopup();
   });
 }
 

dom/xul/XULButtonElement.cpp

@@ -95,9 +95,9 @@ void XULButtonElement::HandleEnterKeyPress(WidgetEvent& aEvent) {
 #ifdef XP_WIN
     if (XULPopupElement* popup = GetContainingPopupElement()) {
       if (nsXULPopupManager* pm = nsXULPopupManager::GetInstance()) {
-        pm->HidePopup(popup, /* aHideChain = */ true,
-                      /* aDeselectMenu = */ true, /* aAsynchronous = */ true,
-                      /* aIsCancel = */ false);
+        pm->HidePopup(
+            popup, {HidePopupOption::HideChain, HidePopupOption::DeselectMenu,
+                    HidePopupOption::Async});
       }
     }
 #endif
@@ -211,7 +211,11 @@ void XULButtonElement::CloseMenuPopup(bool aDeselectMenu) {
     return;
   }
   if (auto* popup = GetMenuPopupContent()) {
-    pm->HidePopup(popup, false, aDeselectMenu, true, false);
+    HidePopupOptions options{HidePopupOption::Async};
+    if (aDeselectMenu) {
+      options += HidePopupOption::DeselectMenu;
+    }
+    pm->HidePopup(popup, options);
   }
 }
 

dom/xul/XULPopupElement.cpp

@@ -106,9 +106,14 @@ void XULPopupElement::OpenPopupAtScreenRect(const nsAString& aPosition,
 
 void XULPopupElement::HidePopup(bool aCancel) {
   nsXULPopupManager* pm = nsXULPopupManager::GetInstance();
-  if (pm) {
-    pm->HidePopup(this, false, true, false, aCancel);
+  if (!pm) {
+    return;
+  }
+  HidePopupOptions options{HidePopupOption::DeselectMenu};
+  if (aCancel) {
+    options += HidePopupOption::IsRollup;
   }
+  pm->HidePopup(this, options);
 }
 
 static Modifiers ConvertModifiers(const ActivateMenuItemOptions& aModifiers) {

dom/xul/nsXULPopupListener.cpp

@@ -170,7 +170,9 @@ void nsXULPopupListener::ClosePopup() {
     // popup is hidden. Use asynchronous hiding just to be safe so we don't
     // fire events during destruction.
     nsXULPopupManager* pm = nsXULPopupManager::GetInstance();
-    if (pm) pm->HidePopup(mPopupContent, false, true, true, false);
+    if (pm)
+      pm->HidePopup(mPopupContent,
+                    {HidePopupOption::DeselectMenu, HidePopupOption::Async});
     mPopupContent = nullptr;  // release the popup
   }
 }  // ClosePopup

layout/xul/nsMenuBarListener.cpp

@@ -207,7 +207,7 @@ nsresult nsMenuBarListener::KeyUp(Event* aKeyEvent) {
       // handle key events when menubar is active and IME should be
       // disabled.
       if (nsXULPopupManager* pm = nsXULPopupManager::GetInstance()) {
-        pm->Rollup(0, false, nullptr, nullptr);
+        pm->Rollup({});
       }
       // If menubar active state is changed or the menubar is destroyed
       // during closing the popups, we should do nothing anymore.

layout/xul/nsMenuPopupFrame.cpp

@@ -62,6 +62,7 @@
 #include <algorithm>
 
 #include "X11UndefineNone.h"
+#include "nsXULPopupManager.h"
 
 using namespace mozilla;
 using mozilla::dom::Document;
@@ -2460,7 +2461,8 @@ void nsMenuPopupFrame::CheckForAnchorChange(nsRect& aRect) {
     if (pm) {
       // As the caller will be iterating over the open popups, hide
       // asyncronously.
-      pm->HidePopup(mContent, false, true, true, false);
+      pm->HidePopup(mContent,
+                    {HidePopupOption::DeselectMenu, HidePopupOption::Async});
     }
 
     return;