[tor-commits] [tor-browser] 34/37: Bug 1745595 - record window size changes from app-initiated resizes asynchronously after the first size-allocate. r=stransky, a=RyanVM

gitolite role git at cupani.torproject.org
Wed Jun 22 18:27:43 UTC 2022


This is an automated email from the git hooks/post-receive script.

richard pushed a commit to branch tor-browser-91.11.0esr-11.5-1
in repository tor-browser.

commit 5b767700fb7287ed4fce8449269218a8a7a0cb0c
Author: Karl Tomlinson <karlt+ at karlt.net>
AuthorDate: Sat May 28 19:56:11 2022 +1200

    Bug 1745595 - record window size changes from app-initiated resizes asynchronously after the first size-allocate. r=stransky, a=RyanVM
    
    This provides a consistent order of bounds changes corresponding to resize
    event notifications.  Previously the size reported in bounds could bounce as
    requested sizes were saved, then overwritten in OnSizeAllocate() with
    previously queued sizes, and then usually eventually rewritten with the final
    size in OnSizeAllocate() again.
    
    Differential Revision: https://phabricator.services.mozilla.com/D147728
---
 widget/gtk/nsWindow.cpp | 112 +++++++++++++++++++++---------------------------
 widget/gtk/nsWindow.h   |  16 ++++---
 2 files changed, 58 insertions(+), 70 deletions(-)

diff --git a/widget/gtk/nsWindow.cpp b/widget/gtk/nsWindow.cpp
index ae0064ddda0af..8ac97d414031c 100644
--- a/widget/gtk/nsWindow.cpp
+++ b/widget/gtk/nsWindow.cpp
@@ -460,7 +460,7 @@ nsWindow::nsWindow()
       mHasAlphaVisual(false),
       mLastMotionPressure(0),
       mLastSizeMode(nsSizeMode_Normal),
-      mBoundsAreValid(true),
+      mHasReceivedSizeAllocate(false),
       mPopupTrackInHierarchy(false),
       mPopupTrackInHierarchyConfigured(false),
       mHiddenPopupPositioned(false),
@@ -477,7 +477,7 @@ nsWindow::nsWindow()
       mPreferredPopupRect(),
       mPreferredPopupRectFlushed(false),
       mWaitingForMoveToRectCallback(false),
-      mNewSizeAfterMoveToRect(LayoutDeviceIntRect(0, 0, 0, 0))
+      mResizedAfterMoveToRect(false)
 #ifdef ACCESSIBILITY
       ,
       mRootAccessible(nullptr)
@@ -618,7 +618,8 @@ void nsWindow::OnDestroy(void) {
 }
 
 bool nsWindow::AreBoundsSane() {
-  return mBounds.width > 0 && mBounds.height > 0;
+  // Check requested size, as mBounds might not have been updated.
+  return !mLastSizeRequest.IsEmpty();
 }
 
 static GtkWidget* EnsureInvisibleContainer() {
@@ -1159,13 +1160,9 @@ void nsWindow::ResizeInt(const Maybe<LayoutDeviceIntPoint>& aMove,
   LOG(("  ConstrainSize: w:%d h;%d\n", aSize.width, aSize.height));
 
   // For top-level windows, aSize should possibly be
-  // interpreted as frame bounds, but NativeResize treats these as window
+  // interpreted as frame bounds, but NativeMoveResize treats these as window
   // bounds (Bug 581866).
-  mBounds.SizeTo(aSize);
-
-  // We set correct mBounds in advance here. This can be invalided by state
-  // event.
-  mBoundsAreValid = true;
+  mLastSizeRequest = aSize;
 
   // Recalculate aspect ratio when resized from DOM
   if (mAspectRatio != 0.0) {
@@ -1184,8 +1181,27 @@ void nsWindow::ResizeInt(const Maybe<LayoutDeviceIntPoint>& aMove,
 
   NotifyRollupGeometryChange();
 
-  // send a resize notification if this is a toplevel
-  if (mIsTopLevel || mListenForResizes) {
+  // We optimistically assume size changes immediately in two cases:
+  // 1. Override-redirect window: Size is controlled by only us.
+  // 2. Managed window that has not not yet received a size-allocate event:
+  //    Resize() Callers expect initial sizes to be applied synchronously.
+  //    If the size request is not honored, then we'll correct in
+  //    OnSizeAllocate().
+  //
+  // When a managed window has already received a size-allocate, we cannot
+  // assume we'll always get a notification if our request does not get
+  // honored: "If the configure request has not changed, we don't ever resend
+  // it, because it could mean fighting the user or window manager."
+  // https://gitlab.gnome.org/GNOME/gtk/-/blob/3.24.31/gtk/gtkwindow.c#L9782
+  // So we don't update mBounds until OnSizeAllocate() when we know the
+  // request is granted.
+  if ((mIsTopLevel || mListenForResizes) &&
+      (!mHasReceivedSizeAllocate ||
+       gtk_window_get_window_type(GTK_WINDOW(mShell)) == GTK_WINDOW_POPUP)) {
+    mBounds.SizeTo(aSize);
+    if (mCompositorWidgetDelegate) {
+      mCompositorWidgetDelegate->NotifyClientSizeChanged(aSize);
+    }
     DispatchResized();
   }
 }
@@ -1932,20 +1948,20 @@ void nsWindow::NativeMoveResizeWaylandPopupCallback(
 
   // We ignore the callback position data because the another resize has been
   // called before the callback have been triggered.
-  if (mNewSizeAfterMoveToRect.height > 0 || mNewSizeAfterMoveToRect.width > 0) {
+  if (mResizedAfterMoveToRect) {
     LOG_POPUP(
         ("  Another resize called during waiting for callback, calling "
          "Resize(%d, %d)\n",
-         mNewSizeAfterMoveToRect.width, mNewSizeAfterMoveToRect.height));
+         mLastSizeRequest.width, mLastSizeRequest.height));
     // Set the preferred size to zero to avoid wrong size of popup because the
     // mPreferredPopupRect is used in nsMenuPopupFrame to set dimensions
     mPreferredPopupRect = nsRect(0, 0, 0, 0);
 
     // We need to schedule another resize because the window has been resized
     // again before callback was called.
-    Resize(mNewSizeAfterMoveToRect.width, mNewSizeAfterMoveToRect.height, true);
+    ResizeInt(Nothing(), mLastSizeRequest, true);
     DispatchResized();
-    mNewSizeAfterMoveToRect.width = mNewSizeAfterMoveToRect.height = 0;
+    mResizedAfterMoveToRect = false;
     return;
   }
 
@@ -1972,8 +1988,11 @@ void nsWindow::NativeMoveResizeWaylandPopupCallback(
 
   bool needsPositionUpdate =
       (newBounds.x != mBounds.x || newBounds.y != mBounds.y);
-  bool needsSizeUpdate =
-      (newWidth != mBounds.width || newHeight != mBounds.height);
+  // Beware that gtk_window_resize() requests sizes asynchronously and so
+  // newBounds might not have the size from the most recent
+  // gtk_window_resize().
+  bool needsSizeUpdate = newWidth != mLastSizeRequest.width ||
+                         newHeight != mLastSizeRequest.height;
   // Update view
   if (needsSizeUpdate) {
     LOG_POPUP(("  needSizeUpdate\n"));
@@ -2050,8 +2069,8 @@ void nsWindow::NativeMoveResizeWaylandPopup(GdkPoint* aPosition,
   // Compositor may be confused by windows with width/height = 0
   // and positioning such windows leads to Bug 1555866.
   if (!AreBoundsSane()) {
-    LOG_POPUP(("  Bounds are not sane (width: %d height: %d)\n", mBounds.width,
-               mBounds.height));
+    LOG_POPUP(("  Bounds are not sane (width: %d height: %d)\n",
+               mLastSizeRequest.width, mLastSizeRequest.height));
     return;
   }
 
@@ -2375,10 +2394,6 @@ void nsWindow::SetSizeMode(nsSizeMode aMode) {
       break;
   }
 
-  // Request mBounds update from configure event as we may not get
-  // OnSizeAllocate for size state changes (Bug 1489463).
-  mBoundsAreValid = false;
-
   mSizeState = mSizeMode;
 }
 
@@ -3619,27 +3634,6 @@ gboolean nsWindow::OnConfigureEvent(GtkWidget* aWidget,
   // complete.  wtf?
   NotifyWindowMoved(mBounds.x, mBounds.y);
 
-  // A GTK app would usually update its client area size in response to
-  // a "size-allocate" signal.
-  // However, we need to set mBounds in advance at Resize()
-  // as JS code expects immediate window size change.
-  // If Gecko requests a resize from GTK, but subsequently,
-  // before a corresponding "size-allocate" signal is emitted, the window is
-  // resized to its former size via other means, such as maximizing,
-  // then there is no "size-allocate" signal from which to update
-  // the value of mBounds. Similarly, if Gecko's resize request is refused
-  // by the window manager, then there will be no "size-allocate" signal.
-  // In the refused request case, the window manager is required to dispatch
-  // a ConfigureNotify event. mBounds can then be updated here.
-  // This seems to also be sufficient to update mBounds when Gecko resizes
-  // the window from maximized size and then immediately maximizes again.
-  if (!mBoundsAreValid) {
-    GtkAllocation allocation = {-1, -1, 0, 0};
-    gtk_window_get_size(GTK_WINDOW(mShell), &allocation.width,
-                        &allocation.height);
-    OnSizeAllocate(&allocation);
-  }
-
   return FALSE;
 }
 
@@ -3671,12 +3665,12 @@ void nsWindow::OnSizeAllocate(GtkAllocation* aAllocation) {
     }
   }
 
-  mBoundsAreValid = true;
+  mHasReceivedSizeAllocate = true;
 
   LayoutDeviceIntSize size = GdkRectToDevicePixels(*aAllocation).Size();
   if (mBounds.Size() == size) {
     LOG(("  Already the same size"));
-    // We were already resized at nsWindow::OnConfigureEvent() so skip it.
+    // mBounds was set at Create() or Resize().
     return;
   }
 
@@ -5064,6 +5058,7 @@ nsresult nsWindow::Create(nsIWidget* aParent, nsNativeWidget aNativeParent,
   mPreferredPopupRectFlushed = false;
 
   ConstrainSize(&mBounds.width, &mBounds.height);
+  mLastSizeRequest = mBounds.Size();
 
   GtkWidget* eventWidget = nullptr;
   bool popupNeedsAlphaVisual = (mWindowType == eWindowType_popup &&
@@ -5714,7 +5709,7 @@ void nsWindow::NativeResize() {
     return;
   }
 
-  GdkRectangle size = DevicePixelsToGdkSizeRoundUp(mBounds.Size());
+  GdkRectangle size = DevicePixelsToGdkSizeRoundUp(mLastSizeRequest);
 
   LOG(("nsWindow::NativeResize [%p] %d %d\n", (void*)this, size.width,
        size.height));
@@ -5725,7 +5720,7 @@ void nsWindow::NativeResize() {
     gtk_window_resize(GTK_WINDOW(mShell), size.width, size.height);
     if (mWaitingForMoveToRectCallback) {
       LOG_POPUP(("  waiting for move to rect, schedulling "));
-      mNewSizeAfterMoveToRect = mBounds;
+      mResizedAfterMoveToRect = true;
     }
   } else if (mContainer) {
     GtkWidget* widget = GTK_WIDGET(mContainer);
@@ -5740,12 +5735,6 @@ void nsWindow::NativeResize() {
     gdk_window_resize(mGdkWindow, size.width, size.height);
   }
 
-  // Notify the GtkCompositorWidget of a ClientSizeChange
-  // This is different than OnSizeAllocate to catch initial sizing
-  if (mCompositorWidgetDelegate) {
-    mCompositorWidgetDelegate->NotifyClientSizeChanged(GetClientSize());
-  }
-
   // Does it need to be shown because bounds were previously insane?
   if (mNeedsShow && mIsShown) {
     NativeShow(true);
@@ -5769,7 +5758,7 @@ void nsWindow::NativeMoveResize() {
     return;
   }
 
-  GdkRectangle size = DevicePixelsToGdkSizeRoundUp(mBounds.Size());
+  GdkRectangle size = DevicePixelsToGdkSizeRoundUp(mLastSizeRequest);
   GdkPoint topLeft = DevicePixelsToGdkPointRoundDown(mBounds.TopLeft());
 
   LOG(("nsWindow::NativeMoveResize [%p] %d,%d -> %d x %d\n", (void*)this,
@@ -5804,12 +5793,6 @@ void nsWindow::NativeMoveResize() {
     }
   }
 
-  // Notify the GtkCompositorWidget of a ClientSizeChange
-  // This is different than OnSizeAllocate to catch initial sizing
-  if (mCompositorWidgetDelegate) {
-    mCompositorWidgetDelegate->NotifyClientSizeChanged(GetClientSize());
-  }
-
   // Does it need to be shown because bounds were previously insane?
   if (mNeedsShow && mIsShown) {
     NativeShow(true);
@@ -8337,6 +8320,7 @@ void nsWindow::SetDrawsInTitlebar(bool aState) {
     gtk_widget_realize(GTK_WIDGET(mShell));
     gtk_widget_reparent(GTK_WIDGET(mContainer), GTK_WIDGET(mShell));
     mNeedsShow = true;
+    mLastSizeRequest = mBounds.Size();
     NativeResize();
 
     // Label mShell toplevel window so property_notify_event_cb callback
@@ -9235,9 +9219,9 @@ void nsWindow::LockAspectRatio(bool aShouldLock) {
     AddCSDDecorationSize(&decWidth, &decHeight);
 
     float width =
-        (float)DevicePixelsToGdkCoordRoundDown(mBounds.width) + (float)decWidth;
-    float height = (float)DevicePixelsToGdkCoordRoundDown(mBounds.height) +
-                   (float)decHeight;
+        DevicePixelsToGdkCoordRoundDown(mLastSizeRequest.width) + decWidth;
+    float height =
+        DevicePixelsToGdkCoordRoundDown(mLastSizeRequest.height) + decHeight;
 
     mAspectRatio = width / height;
     LOG(("nsWindow::LockAspectRatio() [%p] width %f height %f aspect %f\n",
diff --git a/widget/gtk/nsWindow.h b/widget/gtk/nsWindow.h
index 664f29fa7e962..c14f6f40ac4c5 100644
--- a/widget/gtk/nsWindow.h
+++ b/widget/gtk/nsWindow.h
@@ -523,6 +523,12 @@ class nsWindow final : public nsBaseWidget {
   nsSizeMode mSizeState;
   float mAspectRatio;
   float mAspectRatioSaved;
+  // The size requested, which might not be reflected in mBounds.  Used in
+  // WaylandPopupSetDirectPosition() to remember intended size for popup
+  // positioning, in LockAspect() to remember the intended aspect ratio, and
+  // to remember a size requested while waiting for moved-to-rect when
+  // OnSizeAllocate() might change mBounds.Size().
+  LayoutDeviceIntSize mLastSizeRequest;
   nsIntPoint mClientOffset;
 
   // This field omits duplicate scroll events caused by GNOME bug 726878.
@@ -584,10 +590,8 @@ class nsWindow final : public nsBaseWidget {
   // Remember the last sizemode so that we can restore it when
   // leaving fullscreen
   nsSizeMode mLastSizeMode;
-  // We can't detect size state changes correctly so set this flag
-  // to force update mBounds after a size state change from a configure
-  // event.
-  bool mBoundsAreValid;
+  // We can expect at least one size-allocate event after early resizes.
+  bool mHasReceivedSizeAllocate;
 
   static bool DragInProgress(void);
 
@@ -760,10 +764,10 @@ class nsWindow final : public nsBaseWidget {
    * and we're waiting for move-to-rect callback.
    *
    * If another resize request comes between move-to-rect call and
-   * move-to-rect callback we store it to mNewSizeAfterMoveToRect.
+   * move-to-rect callback we set mResizedAfterMoveToRect.
    */
   bool mWaitingForMoveToRectCallback;
-  LayoutDeviceIntRect mNewSizeAfterMoveToRect;
+  bool mResizedAfterMoveToRect;
 
   /**
    * |mIMContext| takes all IME related stuff.

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the tor-commits mailing list