Bug 1653576 - Generify EventIsInsideWindow to allow passing a point. r=tnikkel
authorKartikaya Gupta <kgupta@mozilla.com>
Tue, 11 Aug 2020 22:15:51 +0000
changeset 544332 15b0c4b2dbe858c102a77e36e821eb4d6d2130c7
parent 544331 1d001eccdda116eb3aa4aa6f61e80a5402adada0
child 544333 510e2a73c290aaef08044b101fe65f2089c57cf7
push id123972
push userkgupta@mozilla.com
push dateTue, 11 Aug 2020 22:54:41 +0000
treeherderautoland@510e2a73c290 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs1653576
milestone81.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 1653576 - Generify EventIsInsideWindow to allow passing a point. r=tnikkel This allows us to reuse this function and the GetPopupsToRollup function when an event point is different than what comes out of GetMessagePos. For instance, the WM_POINTERDOWN code can now reuse this functions properly. This fixes a latent bug where it was calling GetPopupsToRollup which was using an possibly-incorrect point internally. Differential Revision: https://phabricator.services.mozilla.com/D86754
widget/windows/nsWindow.cpp
widget/windows/nsWindow.h
--- a/widget/windows/nsWindow.cpp
+++ b/widget/windows/nsWindow.cpp
@@ -7963,40 +7963,46 @@ void nsWindow::ClearCachedResources() {
   ::EnumChildWindows(mWnd, nsWindow::ClearResourcesCallback, 0);
 }
 
 static bool IsDifferentThreadWindow(HWND aWnd) {
   return ::GetCurrentThreadId() != ::GetWindowThreadProcessId(aWnd, nullptr);
 }
 
 // static
-bool nsWindow::EventIsInsideWindow(nsWindow* aWindow) {
+bool nsWindow::EventIsInsideWindow(nsWindow* aWindow,
+                                   Maybe<POINT> aEventPoint) {
   RECT r;
   ::GetWindowRect(aWindow->mWnd, &r);
-  DWORD pos = ::GetMessagePos();
   POINT mp;
-  mp.x = GET_X_LPARAM(pos);
-  mp.y = GET_Y_LPARAM(pos);
+  if (aEventPoint) {
+    mp = *aEventPoint;
+  } else {
+    DWORD pos = ::GetMessagePos();
+    mp.x = GET_X_LPARAM(pos);
+    mp.y = GET_Y_LPARAM(pos);
+  }
 
   // was the event inside this window?
   return static_cast<bool>(::PtInRect(&r, mp));
 }
 
 // static
 bool nsWindow::GetPopupsToRollup(nsIRollupListener* aRollupListener,
-                                 uint32_t* aPopupsToRollup) {
+                                 uint32_t* aPopupsToRollup,
+                                 Maybe<POINT> aEventPoint) {
   // If we're dealing with menus, we probably have submenus and we don't want
   // to rollup some of them if the click is in a parent menu of the current
   // submenu.
   *aPopupsToRollup = UINT32_MAX;
   AutoTArray<nsIWidget*, 5> widgetChain;
   uint32_t sameTypeCount = aRollupListener->GetSubmenuWidgetChain(&widgetChain);
   for (uint32_t i = 0; i < widgetChain.Length(); ++i) {
     nsIWidget* widget = widgetChain[i];
-    if (EventIsInsideWindow(static_cast<nsWindow*>(widget))) {
+    if (EventIsInsideWindow(static_cast<nsWindow*>(widget), aEventPoint)) {
       // Don't roll up if the mouse event occurred within a menu of the
       // same type. If the mouse event occurred in a menu higher than that,
       // roll up, but pass the number of popups to Rollup so that only those
       // of the same type close up.
       if (i < sameTypeCount) {
         return false;
       }
 
@@ -8089,29 +8095,23 @@ bool nsWindow::DealWithPopups(HWND aWnd,
         break;
       }
       return false;
     case WM_POINTERDOWN: {
       WinPointerEvents pointerEvents;
       if (!pointerEvents.ShouldRollupOnPointerEvent(nativeMessage, aWParam)) {
         return false;
       }
-      if (!GetPopupsToRollup(rollupListener, &popupsToRollup)) {
-        return false;
-      }
-      // Can't use EventIsInsideWindow to check whether the event is inside
-      // the popup window. It's because EventIsInsideWindow gets message
-      // coordinates by GetMessagePos, which returns physical screen
-      // coordinates at WM_POINTERDOWN.
       POINT pt;
       pt.x = GET_X_LPARAM(aLParam);
       pt.y = GET_Y_LPARAM(aLParam);
-      RECT r;
-      ::GetWindowRect(popupWindow->mWnd, &r);
-      if (::PtInRect(&r, pt) != 0) {
+      if (!GetPopupsToRollup(rollupListener, &popupsToRollup, Some(pt))) {
+        return false;
+      }
+      if (EventIsInsideWindow(popupWindow, Some(pt))) {
         // Don't roll up if the event is inside the popup window.
         return false;
       }
     } break;
     case WM_MOUSEWHEEL:
     case WM_MOUSEHWHEEL:
       // We need to check if the popup thinks that it should cause closing
       // itself when mouse wheel events are fired outside the rollup widget.
--- a/widget/windows/nsWindow.h
+++ b/widget/windows/nsWindow.h
@@ -446,17 +446,18 @@ class nsWindow final : public nsWindowBa
   virtual bool ProcessMessage(UINT msg, WPARAM& wParam, LPARAM& lParam,
                               LRESULT* aRetValue);
   bool ExternalHandlerProcessMessage(UINT aMessage, WPARAM& aWParam,
                                      LPARAM& aLParam, MSGResult& aResult);
   bool ProcessMessageForPlugin(MSG aMsg, MSGResult& aResult);
   LRESULT ProcessCharMessage(const MSG& aMsg, bool* aEventDispatched);
   LRESULT ProcessKeyUpMessage(const MSG& aMsg, bool* aEventDispatched);
   LRESULT ProcessKeyDownMessage(const MSG& aMsg, bool* aEventDispatched);
-  static bool EventIsInsideWindow(nsWindow* aWindow);
+  static bool EventIsInsideWindow(
+      nsWindow* aWindow, Maybe<POINT> aEventPoint = mozilla::Nothing());
   // Convert nsEventStatus value to a windows boolean
   static bool ConvertStatus(nsEventStatus aStatus);
   static void PostSleepWakeNotification(const bool aIsSleepMode);
   int32_t ClientMarginHitTestPoint(int32_t mx, int32_t my);
   TimeStamp GetMessageTimeStamp(LONG aEventTime) const;
   static void UpdateFirstEventTime(DWORD aEventTime);
   void FinishLiveResizing(ResizeState aNewState);
   nsIntPoint GetTouchCoordinates(WPARAM wParam, LPARAM lParam);
@@ -505,17 +506,18 @@ class nsWindow final : public nsWindowBa
 
   /**
    * Popup hooks
    */
   static void ScheduleHookTimer(HWND aWnd, UINT aMsgId);
   static void RegisterSpecialDropdownHooks();
   static void UnregisterSpecialDropdownHooks();
   static bool GetPopupsToRollup(nsIRollupListener* aRollupListener,
-                                uint32_t* aPopupsToRollup);
+                                uint32_t* aPopupsToRollup,
+                                Maybe<POINT> aEventPoint = mozilla::Nothing());
   static bool NeedsToHandleNCActivateDelayed(HWND aWnd);
   static bool DealWithPopups(HWND inWnd, UINT inMsg, WPARAM inWParam,
                              LPARAM inLParam, LRESULT* outResult);
 
   /**
    * Window transparency helpers
    */
 #ifdef MOZ_XUL