Bug 1936164 - Don't reparent widgets when moving views around. r=tnik…

…kel,layout-reviewers This wasn't done consistently (otherwise bug 1936194 wouldn't have worked). Now that we recreate the popups when needed, we can rely on that instead of reparenting the widget. Popups are the only type of widget we should ever reparent, as other widgets are created at the top level which can't be moved around. Differential Revision: https://phabricator.services.mozilla.com/D232202
mozilla · Dec 16, 2024 · 77dc05d · 77dc05d
1 parent 5fccb56
commit 77dc05d
Show file tree

Hide file tree

Showing 13 changed files with 21 additions and 116 deletions.
diff --git a/view/nsViewManager.cpp b/view/nsViewManager.cpp
@@ -616,53 +616,6 @@ void nsViewManager::DispatchEvent(WidgetGUIEvent* aEvent, nsView* aView,
   *aStatus = nsEventStatus_eIgnore;
 }
 
-// Recursively reparent widgets if necessary
-
-void nsViewManager::ReparentChildWidgets(nsView* aView, nsIWidget* aNewWidget) {
-  MOZ_ASSERT(aNewWidget, "null widget");
-
-  if (nsIWidget* widget = aView->GetWidget()) {
-    // Check to see if the parent widget is the
-    // same as the new parent. If not then reparent
-    // the widget, otherwise there is nothing more
-    // to do for the view and its descendants
-    if (widget->GetParent() != aNewWidget) {
-      widget->SetParent(aNewWidget);
-    }
-    return;
-  }
-
-  // Need to check each of the views children to see
-  // if they have a widget and reparent it.
-
-  for (nsView* kid = aView->GetFirstChild(); kid; kid = kid->GetNextSibling()) {
-    ReparentChildWidgets(kid, aNewWidget);
-  }
-}
-
-// Reparent a view and its descendant views widgets if necessary
-
-void nsViewManager::ReparentWidgets(nsView* aView, nsView* aParent) {
-  MOZ_ASSERT(aParent, "Must have a parent");
-  MOZ_ASSERT(aView, "Must have a view");
-
-  // Quickly determine whether the view has pre-existing children or a
-  // widget. In most cases the view will not have any pre-existing
-  // children when this is called.  Only in the case
-  // where a view has been reparented by removing it from
-  // a reinserting it into a new location in the view hierarchy do we
-  // have to consider reparenting the existing widgets for the view and
-  // it's descendants.
-  if (aView->HasWidget() || aView->GetFirstChild()) {
-    nsIWidget* parentWidget = aParent->GetNearestWidget(nullptr);
-    if (parentWidget) {
-      ReparentChildWidgets(aView, parentWidget);
-      return;
-    }
-    NS_WARNING("Can not find a widget for the parent view");
-  }
-}
-
 void nsViewManager::InsertChild(nsView* aParent, nsView* aChild,
                                 nsView* aSibling, bool aAfter) {
   MOZ_ASSERT(nullptr != aParent, "null ptr");
@@ -682,7 +635,6 @@ void nsViewManager::InsertChild(nsView* aParent, nsView* aChild,
         // insert at end of document order, i.e., before first view
         // this is the common case, by far
         aParent->InsertChild(aChild, nullptr);
-        ReparentWidgets(aChild, aParent);
       } else {
         // insert at beginning of document order, i.e., after last view
         nsView* kid = aParent->GetFirstChild();
@@ -693,7 +645,6 @@ void nsViewManager::InsertChild(nsView* aParent, nsView* aChild,
         }
         // prev is last view or null if there are no children
         aParent->InsertChild(aChild, prev);
-        ReparentWidgets(aChild, aParent);
       }
     } else {
       nsView* kid = aParent->GetFirstChild();
@@ -707,11 +658,9 @@ void nsViewManager::InsertChild(nsView* aParent, nsView* aChild,
       if (aAfter) {
         // insert after 'kid' in document order, i.e. before in view order
         aParent->InsertChild(aChild, prev);
-        ReparentWidgets(aChild, aParent);
       } else {
         // insert before 'kid' in document order, i.e. after in view order
         aParent->InsertChild(aChild, kid);
-        ReparentWidgets(aChild, aParent);
       }
     }
   }

diff --git a/view/nsViewManager.h b/view/nsViewManager.h
@@ -309,8 +309,6 @@ class nsViewManager final {
   static void CollectVMsForWillPaint(nsView* aView, nsViewManager* aParentVM,
                                      nsTArray<RefPtr<nsViewManager>>& aVMs);
 
-  void ReparentChildWidgets(nsView* aView, nsIWidget* aNewWidget);
-  void ReparentWidgets(nsView* aView, nsView* aParent);
   void InvalidateWidgetArea(nsView* aWidgetView,
                             const nsRegion& aDamagedRegion);
 

diff --git a/widget/android/nsWindow.cpp b/widget/android/nsWindow.cpp
@@ -2306,7 +2306,7 @@ void nsWindow::OnGeckoViewReady() {
   acc->OnReady();
 }
 
-void nsWindow::DidChangeParent(nsIWidget*) {
+void nsWindow::DidClearParent(nsIWidget*) {
   // if we are now in the toplevel window's hierarchy, schedule a redraw
   if (FindTopLevel() == nsWindow::TopWindow()) {
     RedrawAll();

diff --git a/widget/android/nsWindow.h b/widget/android/nsWindow.h
@@ -158,7 +158,7 @@ class nsWindow final : public nsBaseWidget {
                                 const LayoutDeviceIntRect& aRect,
                                 InitData* aInitData) override;
   void Destroy() override;
-  void DidChangeParent(nsIWidget* aNewParent) override;
+  void DidClearParent(nsIWidget*) override;
   float GetDPI() override;
   double GetDefaultScaleInternal() override;
   void Show(bool aState) override;

diff --git a/widget/cocoa/nsChildView.h b/widget/cocoa/nsChildView.h
@@ -469,7 +469,7 @@ class nsChildView final : public nsBaseWidget {
 
   nsCocoaWindow* GetAppWindowWidget() const;
 
-  void DidChangeParent(nsIWidget*) override;
+  void DidClearParent(nsIWidget*) override;
 
   mozilla::widget::TextInputHandler* GetTextInputHandler() {
     return mTextInputHandler;

diff --git a/widget/cocoa/nsChildView.mm b/widget/cocoa/nsChildView.mm
@@ -249,7 +249,7 @@ static inline void FlipCocoaScreenCoordinate(NSPoint& inPoint) {
   // mGeckoChild are used throughout the ChildView class to tell if it's safe
   // to use a ChildView object.
   [mView widgetDestroyed];  // Safe if mView is nil.
-  SetParent(nullptr);
+  ClearParent();
   TearDownView();  // Safe if called twice.
 }
 
@@ -524,23 +524,15 @@ static void ManipulateViewWithoutNeedingDisplay(NSView* aView,
 }
 
 // Change the parent of this widget
-void nsChildView::DidChangeParent(nsIWidget*) {
+void nsChildView::DidClearParent(nsIWidget*) {
   NS_OBJC_BEGIN_TRY_IGNORE_BLOCK;
 
   if (mOnDestroyCalled) {
     return;
   }
 
-  nsCOMPtr<nsIWidget> kungFuDeathGrip(this);
-
   // we hold a ref to mView, so this is safe
   [mView removeFromSuperview];
-  mParentView = mParent
-                    ? (NSView<mozView>*)mParent->GetNativeData(NS_NATIVE_WIDGET)
-                    : nullptr;
-  if (mParentView) {
-    [mParentView addSubview:mView];
-  }
 
   NS_OBJC_END_TRY_IGNORE_BLOCK;
 }

diff --git a/widget/gtk/nsWindow.cpp b/widget/gtk/nsWindow.cpp
@@ -715,29 +715,6 @@ bool nsWindow::WidgetTypeSupportsAcceleration() {
   return true;
 }
 
-void nsWindow::DidChangeParent(nsIWidget* aOldParent) {
-  LOG("nsWindow::DidChangeParent new parent %p -> %p\n", aOldParent, mParent);
-  if (!mParent) {
-    return;
-  }
-
-  auto* newParent = static_cast<nsWindow*>(mParent);
-  if (mIsDestroyed || newParent->IsDestroyed()) {
-    return;
-  }
-
-  if (!IsTopLevelWidget()) {
-    GdkWindow* window = GetToplevelGdkWindow();
-    GdkWindow* parentWindow = newParent->GetToplevelGdkWindow();
-    gdk_window_reparent(window, parentWindow, 0, 0);
-    SetHasMappedToplevel(newParent->mHasMappedToplevel);
-    return;
-  }
-
-  GtkWindow* newParentWidget = GTK_WINDOW(newParent->GetGtkWidget());
-  GtkWindowSetTransientFor(GTK_WINDOW(mShell), newParentWidget);
-}
-
 static void InitPenEvent(WidgetMouseEvent& aGeckoEvent, GdkEvent* aEvent) {
   // Find the source of the event
   GdkDevice* device = gdk_event_get_source_device(aEvent);

diff --git a/widget/gtk/nsWindow.h b/widget/gtk/nsWindow.h
@@ -338,7 +338,6 @@ class nsWindow final : public nsBaseWidget {
   void SetTransparencyMode(TransparencyMode aMode) override;
   TransparencyMode GetTransparencyMode() override;
   void SetInputRegion(const InputRegion&) override;
-  void DidChangeParent(nsIWidget* aOldParent) override;
 
   nsresult SynthesizeNativeMouseEvent(LayoutDeviceIntPoint aPoint,
                                       NativeMouseMessage aNativeMessage,

diff --git a/widget/nsBaseWidget.cpp b/widget/nsBaseWidget.cpp
@@ -426,22 +426,20 @@ void nsBaseWidget::BaseCreate(nsIWidget* aParent, widget::InitData* aInitData) {
   }
 }
 
-void nsIWidget::SetParent(nsIWidget* aNewParent) {
+void nsIWidget::ClearParent() {
+  if (!mParent) {
+    return;
+  }
   nsCOMPtr<nsIWidget> kungFuDeathGrip = this;
   nsCOMPtr<nsIWidget> oldParent = mParent;
-  if (mParent) {
-    mParent->RemoveFromChildList(this);
-  }
-  mParent = aNewParent;
-  if (mParent) {
-    mParent->AddToChildList(this);
-  }
-  DidChangeParent(oldParent);
+  oldParent->RemoveFromChildList(this);
+  mParent = nullptr;
+  DidClearParent(oldParent);
 }
 
 void nsIWidget::RemoveAllChildren() {
   while (nsCOMPtr<nsIWidget> kid = mLastChild) {
-    kid->SetParent(nullptr);
+    kid->ClearParent();
     MOZ_ASSERT(kid != mLastChild);
   }
 }

diff --git a/widget/nsIWidget.h b/widget/nsIWidget.h
@@ -513,14 +513,8 @@ class nsIWidget : public nsISupports {
    */
   bool Destroyed() const { return mOnDestroyCalled; }
 
-  /**
-   * Reparent a widget
-   *
-   * Change the widget's parent. Null parents are allowed.
-   *
-   * @param     aNewParent   new parent
-   */
-  void SetParent(nsIWidget* aNewParent);
+  /** Clear the widget's parent. */
+  void ClearParent();
 
   /**
    * Return the parent Widget of this Widget or nullptr if this is a
@@ -531,8 +525,8 @@ class nsIWidget : public nsISupports {
    */
   nsIWidget* GetParent() const { return mParent; }
 
-  /** Gets called when mParent changes after creation. */
-  virtual void DidChangeParent(nsIWidget* aOldParent) {}
+  /** Gets called when mParent is cleared. */
+  virtual void DidClearParent(nsIWidget* aOldParent) {}
 
   /**
    * Return the top level Widget of this Widget

diff --git a/widget/uikit/nsWindow.mm b/widget/uikit/nsWindow.mm
@@ -760,7 +760,7 @@ - (UIAccessibilityContainerType)accessibilityContainerType {
 void nsWindow::Destroy() {
   for (uint32_t i = 0; i < mChildren.Length(); ++i) {
     // why do we still have children?
-    mChildren[i]->SetParent(nullptr);
+    mChildren[i]->ClearParent();
   }
 
   if (mParent) mParent->mChildren.RemoveElement(this);

diff --git a/widget/windows/nsWindow.cpp b/widget/windows/nsWindow.cpp
@@ -1441,13 +1441,11 @@ void nsWindow::DissociateFromNativeWindow() {
   mPrevWndProc.reset();
 }
 
-void nsWindow::DidChangeParent(nsIWidget*) {
+void nsWindow::DidClearParent(nsIWidget*) {
   if (mWindowType == WindowType::Popup || !mWnd) {
     return;
   }
-  HWND newParent =
-      mParent ? (HWND)mParent->GetNativeData(NS_NATIVE_WINDOW) : nullptr;
-  ::SetParent(mWnd, newParent);
+  ::SetParent(mWnd, nullptr);
   RecreateDirectManipulationIfNeeded();
 }
 

diff --git a/widget/windows/nsWindow.h b/widget/windows/nsWindow.h
@@ -195,7 +195,7 @@ class nsWindow final : public nsBaseWidget {
   void Destroy() override;
   float GetDPI() override;
   double GetDefaultScaleInternal() override;
-  void DidChangeParent(nsIWidget* aOldParent) override;
+  void DidClearParent(nsIWidget* aOldParent) override;
   int32_t LogToPhys(double aValue);
   mozilla::DesktopToLayoutDeviceScale GetDesktopToDeviceScale() override {
     if (mozilla::widget::WinUtils::IsPerMonitorDPIAware()) {