Bug 1834042 - Make nsWindow::ConstrainPosition account for decoration…

…s. r=stransky

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

widget/PuppetWidget.cpp

@@ -72,9 +72,6 @@ static bool MightNeedIMEFocus(const widget::InitData* aInitData) {
 #endif
 }
 
-// Arbitrary, fungible.
-const size_t PuppetWidget::kMaxDimension = 4000;
-
 NS_IMPL_ISUPPORTS_INHERITED(PuppetWidget, nsBaseWidget,
                             TextEventDispatcherListener)
 

widget/PuppetWidget.h

@@ -57,8 +57,6 @@ class PuppetWidget : public nsBaseWidget,
   typedef nsBaseWidget Base;
 
   // The width and height of the "widget" are clamped to this.
-  static const size_t kMaxDimension;
-
  public:
   explicit PuppetWidget(BrowserChild* aBrowserChild);
 
@@ -90,12 +88,6 @@ class PuppetWidget : public nsBaseWidget,
 
   virtual bool IsVisible() const override { return mVisible; }
 
-  virtual void ConstrainPosition(bool /*ignored aAllowSlop*/, int32_t* aX,
-                                 int32_t* aY) override {
-    *aX = kMaxDimension;
-    *aY = kMaxDimension;
-  }
-
   // Widget position is controlled by the parent process via BrowserChild.
   virtual void Move(double aX, double aY) override {}
 

widget/android/nsWindow.cpp

@@ -2310,14 +2310,13 @@ void nsWindow::Show(bool aState) {
 
 bool nsWindow::IsVisible() const { return mIsVisible; }
 
-void nsWindow::ConstrainPosition(bool aAllowSlop, int32_t* aX, int32_t* aY) {
-  ALOG("nsWindow[%p]::ConstrainPosition %d [%d %d]", (void*)this, aAllowSlop,
-       *aX, *aY);
+void nsWindow::ConstrainPosition(DesktopIntPoint& aPoint) {
+  ALOG("nsWindow[%p]::ConstrainPosition [%d %d]", (void*)this, aPoint.x.value,
+       aPoint.y.value);
 
-  // constrain toplevel windows; children we don't care about
+  // Constrain toplevel windows; children we don't care about
   if (IsTopLevel()) {
-    *aX = 0;
-    *aY = 0;
+    aPoint = DesktopIntPoint();
   }
 }
 

widget/android/nsWindow.h

@@ -147,8 +147,7 @@ class nsWindow final : public nsBaseWidget {
   virtual double GetDefaultScaleInternal() override;
   virtual void Show(bool aState) override;
   virtual bool IsVisible() const override;
-  virtual void ConstrainPosition(bool aAllowSlop, int32_t* aX,
-                                 int32_t* aY) override;
+  virtual void ConstrainPosition(DesktopIntPoint&) override;
   virtual void Move(double aX, double aY) override;
   virtual void Resize(double aWidth, double aHeight, bool aRepaint) override;
   virtual void Resize(double aX, double aY, double aWidth, double aHeight,

widget/cocoa/nsCocoaWindow.h

@@ -253,7 +253,7 @@ class nsCocoaWindow final : public nsBaseWidget, public nsPIWidgetCocoa {
 
   virtual void* GetNativeData(uint32_t aDataType) override;
 
-  virtual void ConstrainPosition(bool aAllowSlop, int32_t* aX, int32_t* aY) override;
+  virtual void ConstrainPosition(DesktopIntPoint&) override;
   virtual void SetSizeConstraints(const SizeConstraints& aConstraints) override;
   virtual void Move(double aX, double aY) override;
   virtual nsSizeMode SizeMode() override { return mSizeMode; }

widget/cocoa/nsCocoaWindow.mm

@@ -1141,9 +1141,7 @@ static unsigned int WindowMaskForBorderStyle(BorderStyle aBorderStyle) {
 
 bool nsCocoaWindow::IsEnabled() const { return true; }
 
-#define kWindowPositionSlop 20
-
-void nsCocoaWindow::ConstrainPosition(bool aAllowSlop, int32_t* aX, int32_t* aY) {
+void nsCocoaWindow::ConstrainPosition(DesktopIntPoint& aPoint) {
   NS_OBJC_BEGIN_TRY_IGNORE_BLOCK;
 
   if (!mWindow || ![mWindow screen]) {
@@ -1163,38 +1161,24 @@ static unsigned int WindowMaskForBorderStyle(BorderStyle aBorderStyle) {
   nsCOMPtr<nsIScreenManager> screenMgr = do_GetService("@mozilla.org/gfx/screenmanager;1");
   if (screenMgr) {
     nsCOMPtr<nsIScreen> screen;
-    screenMgr->ScreenForRect(*aX, *aY, width, height, getter_AddRefs(screen));
+    screenMgr->ScreenForRect(aPoint.x, aPoint.y, width, height, getter_AddRefs(screen));
 
     if (screen) {
       screen->GetRectDisplayPix(&(screenBounds.x), &(screenBounds.y), &(screenBounds.width),
                                 &(screenBounds.height));
     }
   }
 
-  if (aAllowSlop) {
-    if (*aX < screenBounds.x - width + kWindowPositionSlop) {
-      *aX = screenBounds.x - width + kWindowPositionSlop;
-    } else if (*aX >= screenBounds.x + screenBounds.width - kWindowPositionSlop) {
-      *aX = screenBounds.x + screenBounds.width - kWindowPositionSlop;
-    }
-
-    if (*aY < screenBounds.y - height + kWindowPositionSlop) {
-      *aY = screenBounds.y - height + kWindowPositionSlop;
-    } else if (*aY >= screenBounds.y + screenBounds.height - kWindowPositionSlop) {
-      *aY = screenBounds.y + screenBounds.height - kWindowPositionSlop;
-    }
-  } else {
-    if (*aX < screenBounds.x) {
-      *aX = screenBounds.x;
-    } else if (*aX >= screenBounds.x + screenBounds.width - width) {
-      *aX = screenBounds.x + screenBounds.width - width;
-    }
+  if (aPoint.x < screenBounds.x) {
+    aPoint.x = screenBounds.x;
+  } else if (aPoint.x >= screenBounds.x + screenBounds.width - width) {
+    aPoint.x = screenBounds.x + screenBounds.width - width;
+  }
 
-    if (*aY < screenBounds.y) {
-      *aY = screenBounds.y;
-    } else if (*aY >= screenBounds.y + screenBounds.height - height) {
-      *aY = screenBounds.y + screenBounds.height - height;
-    }
+  if (aPoint.y < screenBounds.y) {
+    aPoint.y = screenBounds.y;
+  } else if (aPoint.y >= screenBounds.y + screenBounds.height - height) {
+    aPoint.y = screenBounds.y + screenBounds.height - height;
   }
 
   NS_OBJC_END_TRY_IGNORE_BLOCK;

widget/gtk/nsWindow.cpp

@@ -339,7 +339,6 @@ static uint32_t gLastTouchID = 0;
 static GUniquePtr<GdkEventCrossing> sStoredLeaveNotifyEvent;
 
 #define NS_WINDOW_TITLE_MAX_LENGTH 4095
-#define kWindowPositionSlop 20
 
 // cursor cache
 static GdkCursor* gCursorCache[eCursorCount];
@@ -820,7 +819,7 @@ void nsWindow::RegisterTouchWindow() {
   mTouches.Clear();
 }
 
-void nsWindow::ConstrainPosition(bool aAllowSlop, int32_t* aX, int32_t* aY) {
+void nsWindow::ConstrainPosition(DesktopIntPoint& aPoint) {
   if (!mShell || GdkIsWaylandDisplay()) {
     return;
   }
@@ -833,52 +832,38 @@ void nsWindow::ConstrainPosition(bool aAllowSlop, int32_t* aX, int32_t* aY) {
 
   /* get our playing field. use the current screen, or failing that
     for any reason, use device caps for the default screen. */
-  nsCOMPtr<nsIScreen> screen;
   nsCOMPtr<nsIScreenManager> screenmgr =
       do_GetService("@mozilla.org/gfx/screenmanager;1");
-  if (screenmgr) {
-    screenmgr->ScreenForRect(*aX, *aY, logWidth, logHeight,
-                             getter_AddRefs(screen));
+  if (!screenmgr) {
+    return;
   }
-
+  nsCOMPtr<nsIScreen> screen;
+  screenmgr->ScreenForRect(aPoint.x, aPoint.y, logWidth, logHeight,
+                           getter_AddRefs(screen));
   // We don't have any screen so leave the coordinates as is
-  if (!screen) return;
-
-  nsIntRect screenRect;
-  if (mSizeMode != nsSizeMode_Fullscreen) {
-    // For normalized windows, use the desktop work area.
-    screen->GetAvailRectDisplayPix(&screenRect.x, &screenRect.y,
-                                   &screenRect.width, &screenRect.height);
-  } else {
-    // For full screen windows, use the desktop.
-    screen->GetRectDisplayPix(&screenRect.x, &screenRect.y, &screenRect.width,
-                              &screenRect.height);
+  if (!screen) {
+    return;
   }
 
-  if (aAllowSlop) {
-    if (*aX < screenRect.x - logWidth + kWindowPositionSlop) {
-      *aX = screenRect.x - logWidth + kWindowPositionSlop;
-    } else if (*aX >= screenRect.XMost() - kWindowPositionSlop) {
-      *aX = screenRect.XMost() - kWindowPositionSlop;
-    }
-
-    if (*aY < screenRect.y - logHeight + kWindowPositionSlop) {
-      *aY = screenRect.y - logHeight + kWindowPositionSlop;
-    } else if (*aY >= screenRect.YMost() - kWindowPositionSlop) {
-      *aY = screenRect.YMost() - kWindowPositionSlop;
-    }
-  } else {
-    if (*aX < screenRect.x) {
-      *aX = screenRect.x;
-    } else if (*aX >= screenRect.XMost() - logWidth) {
-      *aX = screenRect.XMost() - logWidth;
-    }
+  // For normalized windows, use the desktop work area.
+  // For full screen windows, use the desktop.
+  DesktopIntRect screenRect = mSizeMode == nsSizeMode_Fullscreen
+                                  ? screen->GetRectDisplayPix()
+                                  : screen->GetAvailRectDisplayPix();
+  // Expand for the decoration size if needed.
+  if (DrawsToCSDTitlebar()) {
+    screenRect.Inflate(mClientOffset.x, mClientOffset.y);
+  }
+  if (aPoint.x < screenRect.x) {
+    aPoint.x = screenRect.x;
+  } else if (aPoint.x >= screenRect.XMost() - logWidth) {
+    aPoint.x = screenRect.XMost() - logWidth;
+  }
 
-    if (*aY < screenRect.y) {
-      *aY = screenRect.y;
-    } else if (*aY >= screenRect.YMost() - logHeight) {
-      *aY = screenRect.YMost() - logHeight;
-    }
+  if (aPoint.y < screenRect.y) {
+    aPoint.y = screenRect.y;
+  } else if (aPoint.y >= screenRect.YMost() - logHeight) {
+    aPoint.y = screenRect.YMost() - logHeight;
   }
 }
 

widget/gtk/nsWindow.h

@@ -156,7 +156,7 @@ class nsWindow final : public nsBaseWidget {
   void SetModal(bool aModal) override;
   bool IsVisible() const override;
   bool IsMapped() const override;
-  void ConstrainPosition(bool aAllowSlop, int32_t* aX, int32_t* aY) override;
+  void ConstrainPosition(DesktopIntPoint&) override;
   void SetSizeConstraints(const SizeConstraints& aConstraints) override;
   void LockAspectRatio(bool aShouldLock) override;
   void Move(double aX, double aY) override;

widget/nsBaseWidget.h

@@ -252,7 +252,7 @@ class nsBaseWidget : public nsIWidget, public nsSupportsWeakReference {
   mozilla::DesktopToLayoutDeviceScale GetDesktopToDeviceScaleByScreen()
       override;
 
-  void ConstrainPosition(bool aAllowSlop, int32_t* aX, int32_t* aY) override {}
+  void ConstrainPosition(DesktopIntPoint&) override {}
   void MoveClient(const DesktopPoint& aOffset) override;
   void ResizeClient(const DesktopSize& aSize, bool aRepaint) override;
   void ResizeClient(const DesktopRect& aRect, bool aRepaint) override;

widget/nsIWidget.h

@@ -370,6 +370,7 @@ class nsIWidget : public nsISupports {
   typedef mozilla::CSSToScreenScale CSSToScreenScale;
   typedef mozilla::DesktopIntRect DesktopIntRect;
   typedef mozilla::DesktopPoint DesktopPoint;
+  typedef mozilla::DesktopIntPoint DesktopIntPoint;
   typedef mozilla::DesktopRect DesktopRect;
   typedef mozilla::DesktopSize DesktopSize;
   typedef mozilla::CSSPoint CSSPoint;
@@ -697,18 +698,8 @@ class nsIWidget : public nsISupports {
   /**
    * Perform platform-dependent sanity check on a potential window position.
    * This is guaranteed to work only for top-level windows.
-   *
-   * @param aAllowSlop: if true, allow the window to slop offscreen;
-   *                    the window should be partially visible. if false,
-   *                    force the entire window onscreen (or at least
-   *                    the upper-left corner, if it's too large).
-   * @param aX in: an x position expressed in screen coordinates.
-   *           out: the x position constrained to fit on the screen(s).
-   * @param aY in: an y position expressed in screen coordinates.
-   *           out: the y position constrained to fit on the screen(s).
-   *
-   **/
-  virtual void ConstrainPosition(bool aAllowSlop, int32_t* aX, int32_t* aY) = 0;
+   */
+  virtual void ConstrainPosition(DesktopIntPoint&) = 0;
 
   /**
    * NOTE:

widget/windows/nsWindow.cpp

@@ -2375,7 +2375,7 @@ void nsWindow::SuppressAnimation(bool aSuppress) {
 // Constrain a potential move to fit onscreen
 // Position (aX, aY) is specified in Windows screen (logical) pixels,
 // except when using per-monitor DPI, in which case it's device pixels.
-void nsWindow::ConstrainPosition(bool aAllowSlop, int32_t* aX, int32_t* aY) {
+void nsWindow::ConstrainPosition(DesktopIntPoint& aPoint) {
   if (!mIsTopWidgetWindow)  // only a problem for top-level windows
     return;
 
@@ -2400,7 +2400,7 @@ void nsWindow::ConstrainPosition(bool aAllowSlop, int32_t* aX, int32_t* aY) {
   nsCOMPtr<nsIScreen> screen;
   int32_t left, top, width, height;
 
-  screenmgr->ScreenForRect(*aX, *aY, logWidth, logHeight,
+  screenmgr->ScreenForRect(aPoint.x, aPoint.y, logWidth, logHeight,
                            getter_AddRefs(screen));
   if (mFrameState->GetSizeMode() != nsSizeMode_Fullscreen) {
     // For normalized windows, use the desktop work area.
@@ -2420,28 +2420,15 @@ void nsWindow::ConstrainPosition(bool aAllowSlop, int32_t* aX, int32_t* aY) {
   screenRect.top = top;
   screenRect.bottom = top + height;
 
-  if (aAllowSlop) {
-    if (*aX < screenRect.left - logWidth + kWindowPositionSlop)
-      *aX = screenRect.left - logWidth + kWindowPositionSlop;
-    else if (*aX >= screenRect.right - kWindowPositionSlop)
-      *aX = screenRect.right - kWindowPositionSlop;
+  if (aPoint.x < screenRect.left)
+    aPoint.x = screenRect.left;
+  else if (aPoint.x >= screenRect.right - logWidth)
+    aPoint.x = screenRect.right - logWidth;
 
-    if (*aY < screenRect.top - logHeight + kWindowPositionSlop)
-      *aY = screenRect.top - logHeight + kWindowPositionSlop;
-    else if (*aY >= screenRect.bottom - kWindowPositionSlop)
-      *aY = screenRect.bottom - kWindowPositionSlop;
-
-  } else {
-    if (*aX < screenRect.left)
-      *aX = screenRect.left;
-    else if (*aX >= screenRect.right - logWidth)
-      *aX = screenRect.right - logWidth;
-
-    if (*aY < screenRect.top)
-      *aY = screenRect.top;
-    else if (*aY >= screenRect.bottom - logHeight)
-      *aY = screenRect.bottom - logHeight;
-  }
+  if (aPoint.y < screenRect.top)
+    aPoint.y = screenRect.top;
+  else if (aPoint.y >= screenRect.bottom - logHeight)
+    aPoint.y = screenRect.bottom - logHeight;
 }
 
 /**************************************************************

widget/windows/nsWindow.h

@@ -185,7 +185,7 @@ class nsWindow final : public nsBaseWidget {
 
   void Show(bool aState) override;
   bool IsVisible() const override;
-  void ConstrainPosition(bool aAllowSlop, int32_t* aX, int32_t* aY) override;
+  void ConstrainPosition(DesktopIntPoint&) override;
   void SetSizeConstraints(const SizeConstraints& aConstraints) override;
   void LockAspectRatio(bool aShouldLock) override;
   const SizeConstraints GetSizeConstraints() override;

xpfe/appshell/AppWindow.cpp

@@ -867,13 +867,15 @@ NS_IMETHODIMP AppWindow::Center(nsIAppWindow* aRelative, bool aScreen,
   const LayoutDeviceIntSize ourDevSize = GetSize();
   const DesktopIntSize ourSize =
       RoundedToInt(ourDevSize / DevicePixelsPerDesktopPixel());
-  rect.x += (rect.width - ourSize.width) / 2;
-  rect.y += (rect.height - ourSize.height) / (aAlert ? 3 : 2);
+  auto newPos =
+      rect.TopLeft() +
+      DesktopIntPoint((rect.width - ourSize.width) / 2,
+                      (rect.height - ourSize.height) / (aAlert ? 3 : 2));
   if (windowCoordinates) {
-    mWindow->ConstrainPosition(false, &rect.x, &rect.y);
+    mWindow->ConstrainPosition(newPos);
   }
 
-  SetPositionDesktopPix(rect.x, rect.y);
+  SetPositionDesktopPix(newPos.x, newPos.y);
 
   // If moving the window caused it to change size, re-do the centering.
   if (GetSize() != ourDevSize) {
@@ -1305,7 +1307,7 @@ bool AppWindow::LoadPositionFromXUL(int32_t aSpecWidth, int32_t aSpecHeight) {
                       cssSize.height);
     }
   }
-  mWindow->ConstrainPosition(false, &specPoint.x.value, &specPoint.y.value);
+  mWindow->ConstrainPosition(specPoint);
   if (specPoint != curPoint) {
     SetPositionDesktopPix(specPoint.x, specPoint.y);
   }
@@ -1977,8 +1979,8 @@ nsresult AppWindow::SetPersistentValue(const nsAtom* aAttr,
 void AppWindow::MaybeSavePersistentPositionAndSize(
     PersistentAttributes aAttributes, Element& aRootElement,
     const nsAString& aPersistString, bool aShouldPersist) {
-  if ((aAttributes& PersistentAttributes{PersistentAttribute::Position,
-                                         PersistentAttribute::Size})
+  if ((aAttributes & PersistentAttributes{PersistentAttribute::Position,
+                                          PersistentAttribute::Size})
           .isEmpty()) {
     return;
   }