Bug 1924240 - Devirtualize parent handling. r=mac-reviewers,win-reviewers,geckoview-reviewers,bradwerth,m_kato,rkraesig

Move mParent to nsIWidget.h along with kids and so on. Instead of
ReparentNativeWidget add a more general DidChangeParent method.

Make mParent weak to prevent cycles. These are broken on Destroy() but
it feels kinda silly, and we were inconsistent before I started this
effort so pretty sure it can be done.

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

Author: emilio

view/nsViewManager.cpp

@@ -662,21 +662,13 @@ void nsViewManager::DispatchEvent(WidgetGUIEvent* aEvent, nsView* aView,
 void nsViewManager::ReparentChildWidgets(nsView* aView, nsIWidget* aNewWidget) {
   MOZ_ASSERT(aNewWidget, "null widget");
 
-  if (aView->HasWidget()) {
+  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
-    nsIWidget* widget = aView->GetWidget();
-    nsIWidget* parentWidget = widget->GetParent();
-    if (parentWidget) {
-      // Child widget
-      if (parentWidget != aNewWidget) {
-        widget->SetParent(aNewWidget);
-      }
-    } else {
-      // Toplevel widget (popup, dialog, etc)
-      widget->ReparentNativeWidget(aNewWidget);
+    if (widget->GetParent() != aNewWidget) {
+      widget->SetParent(aNewWidget);
     }
     return;
   }

widget/PuppetWidget.cpp

@@ -96,26 +96,15 @@ PuppetWidget::~PuppetWidget() { Destroy(); }
 void PuppetWidget::InfallibleCreate(nsIWidget* aParent,
                                     const LayoutDeviceIntRect& aRect,
                                     widget::InitData* aInitData) {
-  // FIXME(emilio): Why aParent = nullptr? Can we even get here with non-null
-  // aParent?
-  BaseCreate(/* aParent = */ nullptr, aInitData);
+  BaseCreate(aParent, aInitData);
 
   mBounds = aRect;
   mEnabled = true;
   mVisible = true;
 
-  mDrawTarget = gfxPlatform::GetPlatform()->CreateOffscreenContentDrawTarget(
-      IntSize(1, 1), SurfaceFormat::B8G8R8A8);
-
   mNeedIMEStateInit = MightNeedIMEFocus(aInitData);
 
-  PuppetWidget* parent = static_cast<PuppetWidget*>(aParent);
-  if (parent) {
-    parent->SetChild(this);
-    mWindowRenderer = parent->GetWindowRenderer();
-  } else {
-    Resize(mBounds.X(), mBounds.Y(), mBounds.Width(), mBounds.Height(), false);
-  }
+  Resize(mBounds.X(), mBounds.Y(), mBounds.Width(), mBounds.Height(), false);
   mMemoryPressureObserver = MemoryPressureObserver::Create(this);
 }
 
@@ -148,7 +137,6 @@ void PuppetWidget::Destroy() {
     mMemoryPressureObserver->Unregister();
     mMemoryPressureObserver = nullptr;
   }
-  mChild = nullptr;
   if (mWindowRenderer) {
     mWindowRenderer->Destroy();
   }
@@ -163,10 +151,6 @@ void PuppetWidget::Show(bool aState) {
   bool wasVisible = mVisible;
   mVisible = aState;
 
-  if (mChild) {
-    mChild->mVisible = aState;
-  }
-
   if (!wasVisible && mVisible) {
     // The previously attached widget listener is handy if
     // we're transitioning from page to page without dropping
@@ -187,11 +171,6 @@ void PuppetWidget::Resize(double aWidth, double aHeight, bool aRepaint) {
   mBounds.SizeTo(
       LayoutDeviceIntSize(NSToIntRound(aWidth), NSToIntRound(aHeight)));
 
-  if (mChild) {
-    mChild->Resize(aWidth, aHeight, aRepaint);
-    return;
-  }
-
   // XXX: roc says that |aRepaint| dictates whether or not to
   // invalidate the expanded area
   if (oldBounds.Size() < mBounds.Size() && aRepaint) {
@@ -225,11 +204,6 @@ void PuppetWidget::Invalidate(const LayoutDeviceIntRect& aRect) {
   debug_DumpInvalidate(stderr, this, &aRect, "PuppetWidget", 0);
 #endif
 
-  if (mChild) {
-    mChild->Invalidate(aRect);
-    return;
-  }
-
   if (mBrowserChild && !aRect.IsEmpty() && !mWidgetPaintTask.IsPending()) {
     mWidgetPaintTask = new WidgetPaintTask(this);
     nsCOMPtr<nsIRunnable> event(mWidgetPaintTask.get());
@@ -262,9 +236,6 @@ nsresult PuppetWidget::DispatchEvent(WidgetGUIEvent* aEvent,
   debug_DumpEvent(stdout, aEvent->mWidget, aEvent, "PuppetWidget", 0);
 #endif
 
-  MOZ_ASSERT(!mChild || mChild->mWindowType == WindowType::Popup,
-             "Unexpected event dispatch!");
-
   MOZ_ASSERT(!aEvent->AsKeyboardEvent() ||
                  aEvent->mFlags.mIsSynthesizedForTests ||
                  aEvent->AsKeyboardEvent()->AreAllEditCommandsInitialized(),
@@ -937,14 +908,6 @@ void PuppetWidget::SetCursor(const Cursor& aCursor) {
   mUpdateCursor = false;
 }
 
-void PuppetWidget::SetChild(PuppetWidget* aChild) {
-  MOZ_ASSERT(this != aChild, "can't parent a widget to itself");
-  MOZ_ASSERT(!aChild->mChild,
-             "fake widget 'hierarchy' only expected to have one level");
-
-  mChild = aChild;
-}
-
 NS_IMETHODIMP
 PuppetWidget::WidgetPaintTask::Run() {
   if (mWidget) {

widget/PuppetWidget.h

@@ -303,8 +303,6 @@ class PuppetWidget final : public nsBaseWidget,
  private:
   void Paint();
 
-  void SetChild(PuppetWidget* aChild);
-
   nsresult RequestIMEToCommitComposition(bool aCancel);
   nsresult NotifyIMEOfFocusChange(const IMENotification& aIMENotification);
   nsresult NotifyIMEOfSelectionChange(const IMENotification& aIMENotification);
@@ -347,14 +345,8 @@ class PuppetWidget final : public nsBaseWidget,
   // it's possible for BrowserChild to outlive the PuppetWidget, we clear
   // this weak reference in Destroy()
   BrowserChild* mBrowserChild;
-  // The "widget" to which we delegate events if we don't have an
-  // event handler.
-  RefPtr<PuppetWidget> mChild;
   nsRevocableEventPtr<WidgetPaintTask> mWidgetPaintTask;
   RefPtr<layers::MemoryPressureObserver> mMemoryPressureObserver;
-  // XXX/cjones: keeping this around until we teach LayerManager to do
-  // retained-content-only transactions
-  RefPtr<DrawTarget> mDrawTarget;
   // IME
   IMENotificationRequests mIMENotificationRequestsOfParent;
   InputContext mInputContext;

widget/android/nsWindow.cpp

@@ -2169,7 +2169,7 @@ void nsWindow::LogWindow(nsWindow* win, int index, int indent) {
   char spaces[] = "                    ";
   spaces[indent < 20 ? indent : 20] = 0;
   ALOG("%s [% 2d] 0x%p [parent 0x%p] [% 3d,% 3dx% 3d,% 3d] vis %d type %d",
-       spaces, index, win, win->mParent.get(), win->mBounds.x, win->mBounds.y,
+       spaces, index, win, win->mParent, win->mBounds.x, win->mBounds.y,
        win->mBounds.width, win->mBounds.height, win->mIsVisible,
        int(win->mWindowType));
   int i = 0;
@@ -2226,7 +2226,6 @@ nsresult nsWindow::Create(nsIWidget* aParent, const LayoutDeviceIntRect& aRect,
                         aInitData->mWindowType != WindowType::Invisible);
 
   BaseCreate(aParent, aInitData);
-  mParent = static_cast<nsWindow*>(aParent);
   MOZ_ASSERT_IF(!IsTopLevel(), aParent);
 
   if (IsTopLevel()) {
@@ -2251,23 +2250,17 @@ void nsWindow::Destroy() {
   // Stuff below may release the last ref to this
   nsCOMPtr<nsIWidget> kungFuDeathGrip(this);
 
-  while (RefPtr<nsWindow> kid = static_cast<nsWindow*>(mLastChild)) {
-    // why do we still have children?
-    ALOG("### Warning: Destroying window %p and reparenting child %p to null!",
-         this, kid.get());
-    RemoveChild(kid);
-    kid->mParent = nullptr;
-  }
+  RemoveAllChildren();
 
   // Ensure the compositor has been shutdown before this nsWindow is potentially
   // deleted
   nsBaseWidget::DestroyCompositor();
 
   nsBaseWidget::Destroy();
 
-  if (IsTopLevel()) gTopLevelWindows.RemoveElement(this);
-
-  SetParent(nullptr);
+  if (IsTopLevel()) {
+    gTopLevelWindows.RemoveElement(this);
+  }
 
   nsBaseWidget::OnDestroy();
 
@@ -2311,27 +2304,13 @@ void nsWindow::OnGeckoViewReady() {
   acc->OnReady();
 }
 
-void nsWindow::SetParent(nsIWidget* aNewParent) {
-  if (mParent == aNewParent) {
-    return;
-  }
-
-  if (mParent) {
-    mParent->RemoveChild(this);
-  }
-
-  mParent = static_cast<nsWindow*>(aNewParent);
-
-  if (mParent) {
-    mParent->AddChild(this);
-  }
-
+void nsWindow::DidChangeParent(nsIWidget*) {
   // if we are now in the toplevel window's hierarchy, schedule a redraw
-  if (FindTopLevel() == nsWindow::TopWindow()) RedrawAll();
+  if (FindTopLevel() == nsWindow::TopWindow()) {
+    RedrawAll();
+  }
 }
 
-nsIWidget* nsWindow::GetParent() { return mParent; }
-
 RefPtr<MozPromise<bool, bool, false>> nsWindow::OnLoadRequest(
     nsIURI* aUri, int32_t aWindowType, int32_t aFlags,
     nsIPrincipal* aTriggeringPrincipal, bool aHasUserGesture,
@@ -2523,9 +2502,10 @@ void nsWindow::Invalidate(const LayoutDeviceIntRect& aRect) {}
 nsWindow* nsWindow::FindTopLevel() {
   nsWindow* toplevel = this;
   while (toplevel) {
-    if (toplevel->IsTopLevel()) return toplevel;
-
-    toplevel = toplevel->mParent;
+    if (toplevel->IsTopLevel()) {
+      return toplevel;
+    }
+    toplevel = static_cast<nsWindow*>(toplevel->mParent);
   }
 
   ALOG(
@@ -2586,10 +2566,8 @@ LayoutDeviceIntRect nsWindow::GetScreenBounds() {
 LayoutDeviceIntPoint nsWindow::WidgetToScreenOffset() {
   LayoutDeviceIntPoint p(0, 0);
 
-  for (nsWindow* w = this; !!w; w = w->mParent) {
-    p.x += w->mBounds.x;
-    p.y += w->mBounds.y;
-
+  for (nsWindow* w = this; !!w; w = static_cast<nsWindow*>(w->mParent)) {
+    p += w->mBounds.TopLeft();
     if (w->IsTopLevel()) {
       break;
     }
@@ -3080,8 +3058,7 @@ nsresult nsWindow::SynthesizeNativeTouchPoint(uint32_t aPointerId,
 
   const auto& npzc = npzcSup->GetJavaNPZC();
   const auto& bounds = FindTopLevel()->mBounds;
-  aPoint.x -= bounds.x;
-  aPoint.y -= bounds.y;
+  aPoint -= bounds.TopLeft();
 
   DispatchToUiThread(
       "nsWindow::SynthesizeNativeTouchPoint",
@@ -3106,8 +3083,7 @@ nsresult nsWindow::SynthesizeNativeMouseEvent(
 
   const auto& npzc = npzcSup->GetJavaNPZC();
   const auto& bounds = FindTopLevel()->mBounds;
-  aPoint.x -= bounds.x;
-  aPoint.y -= bounds.y;
+  aPoint -= bounds.TopLeft();
 
   int32_t nativeMessage;
   switch (aNativeMessage) {

widget/android/nsWindow.h

@@ -71,7 +71,6 @@ class nsWindow final : public nsBaseWidget {
       bool aIsTopLevel);
 
  private:
-  RefPtr<nsWindow> mParent;
   nsCOMPtr<nsIUserIdleServiceInternal> mIdleService;
   mozilla::ScreenIntCoord mDynamicToolbarMaxHeight{0};
   mozilla::ScreenIntMargin mSafeAreaInsets;
@@ -159,8 +158,7 @@ class nsWindow final : public nsBaseWidget {
                                 const LayoutDeviceIntRect& aRect,
                                 InitData* aInitData) override;
   void Destroy() override;
-  void SetParent(nsIWidget* aNewParent) override;
-  nsIWidget* GetParent(void) override;
+  void DidChangeParent(nsIWidget* aNewParent) override;
   float GetDPI() override;
   double GetDefaultScaleInternal() override;
   void Show(bool aState) override;

widget/cocoa/nsChildView.h

@@ -304,8 +304,6 @@ class nsChildView final : public nsBaseWidget {
   void Show(bool aState) override;
   bool IsVisible() const override;
 
-  void SetParent(nsIWidget* aNewParent) override;
-  nsIWidget* GetParent() override;
   float GetDPI() override;
 
   void Move(double aX, double aY) override;
@@ -463,8 +461,6 @@ class nsChildView final : public nsBaseWidget {
                         const bool aIsVertical,
                         const LayoutDeviceIntPoint& aPoint) override;
 
-  void ResetParent();
-
   static bool DoHasPendingInputEvent();
   static uint32_t GetCurrentInputEventCount();
   static void UpdateCurrentInputEventCount();
@@ -473,7 +469,7 @@ class nsChildView final : public nsBaseWidget {
 
   nsCocoaWindow* GetAppWindowWidget() const;
 
-  void ReparentNativeWidget(nsIWidget* aNewParent) override;
+  void DidChangeParent(nsIWidget*) override;
 
   mozilla::widget::TextInputHandler* GetTextInputHandler() {
     return mTextInputHandler;
@@ -562,7 +558,6 @@ class nsChildView final : public nsBaseWidget {
   InputContext mInputContext;
 
   NSView* mParentView;
-  nsCOMPtr<nsIWidget> mParentWidget;
 
 #ifdef ACCESSIBILITY
   // weak ref to this childview's associated mozAccessible for speed reasons