From d159f1178c164cc8bb3b22b94e5b030952b2b4f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 16 Dec 2024 23:21:49 +0000 Subject: [PATCH] Bug 1936164 - Don't reparent widgets when moving views around. r=tnikkel,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 --- view/nsViewManager.cpp | 51 ------------------------------------- view/nsViewManager.h | 2 -- widget/android/nsWindow.cpp | 2 +- widget/android/nsWindow.h | 2 +- widget/cocoa/nsChildView.h | 2 +- widget/cocoa/nsChildView.mm | 12 ++------- widget/gtk/nsWindow.cpp | 23 ----------------- widget/gtk/nsWindow.h | 1 - widget/nsBaseWidget.cpp | 18 ++++++------- widget/nsIWidget.h | 14 +++------- widget/uikit/nsWindow.mm | 2 +- widget/windows/nsWindow.cpp | 6 ++--- widget/windows/nsWindow.h | 2 +- 13 files changed, 21 insertions(+), 116 deletions(-) diff --git a/view/nsViewManager.cpp b/view/nsViewManager.cpp index e09ceb4fd264..5d87a837b2d4 100644 --- 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 index e2e2fc037d5a..4655da85d1b8 100644 --- a/view/nsViewManager.h +++ b/view/nsViewManager.h @@ -309,8 +309,6 @@ class nsViewManager final { static void CollectVMsForWillPaint(nsView* aView, nsViewManager* aParentVM, nsTArray>& 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 index 9bfbdf14c982..c377529e589f 100644 --- 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 index 0a8b7c8dba4e..64e40b1b5b4f 100644 --- 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 index 528f0170fc0b..51d42b9118cb 100644 --- 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 index 3904661878ba..1b9758036645 100644 --- a/widget/cocoa/nsChildView.mm +++ b/widget/cocoa/nsChildView.mm @@ -249,7 +249,7 @@ nsChildView::~nsChildView() { // 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 @@ void nsChildView::Show(bool aState) { } // Change the parent of this widget -void nsChildView::DidChangeParent(nsIWidget*) { +void nsChildView::DidClearParent(nsIWidget*) { NS_OBJC_BEGIN_TRY_IGNORE_BLOCK; if (mOnDestroyCalled) { return; } - nsCOMPtr kungFuDeathGrip(this); - // we hold a ref to mView, so this is safe [mView removeFromSuperview]; - mParentView = mParent - ? (NSView*)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 index c023491e780c..fbe2526b1118 100644 --- 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(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 index cc16b4692423..a83fee5e9a4f 100644 --- 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 index 62aed4d0ef43..a7de84625a77 100644 --- 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 kungFuDeathGrip = this; nsCOMPtr 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 kid = mLastChild) { - kid->SetParent(nullptr); + kid->ClearParent(); MOZ_ASSERT(kid != mLastChild); } } diff --git a/widget/nsIWidget.h b/widget/nsIWidget.h index 7c929f262a95..daebadcb136a 100644 --- 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 index 6a3ee4fd309c..9e0055c893e9 100644 --- a/widget/uikit/nsWindow.mm +++ b/widget/uikit/nsWindow.mm @@ -760,7 +760,7 @@ nsresult nsWindow::Create(nsIWidget* aParent, const LayoutDeviceIntRect& aRect, 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 index 2458409879e6..2f6d842c5325 100644 --- 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 index 4231b535de4e..88ea1cc685e9 100644 --- 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()) {