Bug 1922956 - Further clean-up widget creation code-paths. r=win-reviewers,mac-reviewers,geckoview-reviewers,mstange,rkraesig,m_kato

This is all super-hacky (see a lot of the XXXcjones comments).

Simplify it... We never create widgets with a bare native parent but no
nsIWidget parent. Pass nsIWidgets and deal with things correctly.

There were also things that were dealing with stuff that can't happen,
like top level popups, children of PuppetWidgets, or such.

Instead of overriding Create(), let's just teach nsBaseWidget about
non-native (headless/puppet) widgets.

Remove lots of old APIs for the native window stuff that are unused
and/or unimplemented.

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

Author: emilio

docshell/base/nsDocShell.cpp

@@ -4322,13 +4322,11 @@ bool nsDocShell::FillLoadStateFromCurrentEntry(
 //*****************************************************************************
 
 NS_IMETHODIMP
-nsDocShell::InitWindow(nativeWindow aParentNativeWindow,
-                       nsIWidget* aParentWidget, int32_t aX, int32_t aY,
+nsDocShell::InitWindow(nsIWidget* aParentWidget, int32_t aX, int32_t aY,
                        int32_t aWidth, int32_t aHeight) {
   SetParentWidget(aParentWidget);
   SetPositionAndSize(aX, aY, aWidth, aHeight, 0);
   NS_ENSURE_TRUE(Initialize(), NS_ERROR_FAILURE);
-
   return NS_OK;
 }
 
@@ -4610,24 +4608,6 @@ nsDocShell::SetParentWidget(nsIWidget* aParentWidget) {
   return NS_OK;
 }
 
-NS_IMETHODIMP
-nsDocShell::GetParentNativeWindow(nativeWindow* aParentNativeWindow) {
-  NS_ENSURE_ARG_POINTER(aParentNativeWindow);
-
-  if (mParentWidget) {
-    *aParentNativeWindow = mParentWidget->GetNativeData(NS_NATIVE_WIDGET);
-  } else {
-    *aParentNativeWindow = nullptr;
-  }
-
-  return NS_OK;
-}
-
-NS_IMETHODIMP
-nsDocShell::SetParentNativeWindow(nativeWindow aParentNativeWindow) {
-  return NS_ERROR_NOT_IMPLEMENTED;
-}
-
 NS_IMETHODIMP
 nsDocShell::GetNativeHandle(nsAString& aNativeHandle) {
   // the nativeHandle should be accessed from nsIAppWindow

docshell/base/nsDocShellTreeOwner.cpp

@@ -505,8 +505,7 @@ nsDocShellTreeOwner::GetHasPrimaryContent(bool* aResult) {
 //*****************************************************************************
 
 NS_IMETHODIMP
-nsDocShellTreeOwner::InitWindow(nativeWindow aParentNativeWindow,
-                                nsIWidget* aParentWidget, int32_t aX,
+nsDocShellTreeOwner::InitWindow(nsIWidget* aParentWidget, int32_t aX,
                                 int32_t aY, int32_t aCX, int32_t aCY) {
   return NS_ERROR_NULL_POINTER;
 }
@@ -623,20 +622,6 @@ nsDocShellTreeOwner::SetParentWidget(nsIWidget* aParentWidget) {
   return NS_ERROR_NULL_POINTER;
 }
 
-NS_IMETHODIMP
-nsDocShellTreeOwner::GetParentNativeWindow(nativeWindow* aParentNativeWindow) {
-  nsCOMPtr<nsIBaseWindow> ownerWin = GetOwnerWin();
-  if (ownerWin) {
-    return ownerWin->GetParentNativeWindow(aParentNativeWindow);
-  }
-  return NS_ERROR_NULL_POINTER;
-}
-
-NS_IMETHODIMP
-nsDocShellTreeOwner::SetParentNativeWindow(nativeWindow aParentNativeWindow) {
-  return NS_ERROR_NULL_POINTER;
-}
-
 NS_IMETHODIMP
 nsDocShellTreeOwner::GetNativeHandle(nsAString& aNativeHandle) {
   // the nativeHandle should be accessed from nsIAppWindow

dom/base/nsFrameLoader.cpp

@@ -1004,8 +1004,7 @@ bool nsFrameLoader::Show(nsSubDocumentFrame* aFrame) {
   }
 
   RefPtr<nsDocShell> baseWindow = GetDocShell();
-  baseWindow->InitWindow(nullptr, view->GetWidget(), 0, 0, size.width,
-                         size.height);
+  baseWindow->InitWindow(view->GetWidget(), 0, 0, size.width, size.height);
   baseWindow->SetVisibility(true);
   NS_ENSURE_TRUE(GetDocShell(), false);
 

dom/ipc/BrowserChild.cpp

@@ -391,8 +391,7 @@ nsresult BrowserChild::Init(mozIDOMWindowProxy* aParent,
     NS_ERROR("couldn't create fake widget");
     return NS_ERROR_FAILURE;
   }
-  mPuppetWidget->InfallibleCreate(nullptr,
-                                  nullptr,  // no parents
+  mPuppetWidget->InfallibleCreate(nullptr,  // No parent
                                   LayoutDeviceIntRect(0, 0, 0, 0),
                                   nullptr);  // HandleWidgetEvent
 

gfx/tests/gtest/MockWidget.h

@@ -47,13 +47,11 @@ class MockWidget : public nsBaseWidget {
     return nullptr;
   }
 
-  virtual nsresult Create(nsIWidget* aParent, nsNativeWidget aNativeParent,
-                          const LayoutDeviceIntRect& aRect,
+  virtual nsresult Create(nsIWidget* aParent, const LayoutDeviceIntRect& aRect,
                           InitData* aInitData = nullptr) override {
     return NS_OK;
   }
-  virtual nsresult Create(nsIWidget* aParent, nsNativeWidget aNativeParent,
-                          const DesktopIntRect& aRect,
+  virtual nsresult Create(nsIWidget* aParent, const DesktopIntRect& aRect,
                           InitData* aInitData = nullptr) override {
     return NS_OK;
   }

layout/base/PresShell.cpp

@@ -10590,7 +10590,7 @@ bool PresShell::VerifyIncrementalReflow() {
   NS_ENSURE_TRUE(view, false);
 
   // now create the widget for the view
-  rv = view->CreateWidgetForParent(parentWidget, true);
+  rv = view->CreateWidget(parentWidget, true);
   NS_ENSURE_SUCCESS(rv, false);
 
   // Setup hierarchical relationship in view manager

layout/base/nsDocumentViewer.cpp

@@ -2245,10 +2245,8 @@ nsresult nsDocumentViewer::MakeWindow(const nsSize& aSize,
       // Reuse the top level parent widget.
       rv = view->AttachToTopLevelWidget(mParentWidget);
       mAttachedToParent = true;
-    } else if (!mParentWidget || aContainerView) {
-      rv = view->CreateWidget(true, false);
     } else {
-      rv = view->CreateWidgetForParent(mParentWidget, true, false);
+      rv = view->CreateWidget(mParentWidget, true, false);
     }
     if (NS_FAILED(rv)) return rv;
   }
@@ -3234,7 +3232,7 @@ bool nsDocumentViewer::ShouldAttachToTopLevel() {
   }
 
   // We always attach when using puppet widgets
-  if (nsIWidget::UsePuppetWidgets()) {
+  if (nsIWidget::UsePuppetWidgets() || mParentWidget->IsPuppetWidget()) {
     return true;
   }
 

toolkit/components/browser/nsWebBrowser.cpp

@@ -59,7 +59,6 @@ nsWebBrowser::nsWebBrowser(int aItemType)
     : mContentType(aItemType),
       mShouldEnableHistory(true),
       mWillChangeProcess(false),
-      mParentNativeWindow(nullptr),
       mProgressListener(nullptr),
       mWidgetListenerDelegate(this),
       mBackgroundColor(0),
@@ -89,8 +88,7 @@ nsIWidget* nsWebBrowser::EnsureWidget() {
   LayoutDeviceIntRect bounds(0, 0, 0, 0);
 
   mInternalWidget->SetWidgetListener(&mWidgetListenerDelegate);
-  NS_ENSURE_SUCCESS(mInternalWidget->Create(nullptr, mParentNativeWindow,
-                                            bounds, &widgetInit),
+  NS_ENSURE_SUCCESS(mInternalWidget->Create(mParentWidget, bounds, &widgetInit),
                     nullptr);
 
   return mInternalWidget;
@@ -149,8 +147,7 @@ already_AddRefed<nsWebBrowser> nsWebBrowser::Create(
   // events from subframes. To solve that we install our own chrome event
   // handler that always gets called (even for subframes) for any bubbling
   // event.
-
-  nsresult rv = docShell->InitWindow(nullptr, docShellParentWidget, 0, 0, 0, 0);
+  nsresult rv = docShell->InitWindow(docShellParentWidget, 0, 0, 0, 0);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return nullptr;
   }
@@ -860,8 +857,7 @@ nsWebBrowser::Cancel(nsresult aReason) {
 //*****************************************************************************
 
 NS_IMETHODIMP
-nsWebBrowser::InitWindow(nativeWindow aParentNativeWindow,
-                         nsIWidget* aParentWidget, int32_t aX, int32_t aY,
+nsWebBrowser::InitWindow(nsIWidget* aParentWidget, int32_t aX, int32_t aY,
                          int32_t aCX, int32_t aCY) {
   // nsIBaseWindow::InitWindow and nsIBaseWindow::Create
   // implementations have been merged into nsWebBrowser::Create
@@ -1013,32 +1009,7 @@ nsWebBrowser::GetParentWidget(nsIWidget** aParentWidget) {
 NS_IMETHODIMP
 nsWebBrowser::SetParentWidget(nsIWidget* aParentWidget) {
   NS_ENSURE_STATE(!mDocShell);
-
   mParentWidget = aParentWidget;
-  if (mParentWidget) {
-    mParentNativeWindow = mParentWidget->GetNativeData(NS_NATIVE_WIDGET);
-  } else {
-    mParentNativeWindow = nullptr;
-  }
-
-  return NS_OK;
-}
-
-NS_IMETHODIMP
-nsWebBrowser::GetParentNativeWindow(nativeWindow* aParentNativeWindow) {
-  NS_ENSURE_ARG_POINTER(aParentNativeWindow);
-
-  *aParentNativeWindow = mParentNativeWindow;
-
-  return NS_OK;
-}
-
-NS_IMETHODIMP
-nsWebBrowser::SetParentNativeWindow(nativeWindow aParentNativeWindow) {
-  NS_ENSURE_STATE(!mDocShell);
-
-  mParentNativeWindow = aParentNativeWindow;
-
   return NS_OK;
 }
 

toolkit/components/browser/nsWebBrowser.h

@@ -141,7 +141,6 @@ class nsWebBrowser final : public nsIWebBrowser,
   const uint32_t mContentType;
   bool mShouldEnableHistory;
   bool mWillChangeProcess;
-  nativeWindow mParentNativeWindow;
   nsIWebProgressListener* mProgressListener;
 
   nsCOMPtr<nsIPrintSettings> mPrintSettings;

view/nsView.cpp

@@ -502,43 +502,23 @@ struct DefaultWidgetInitData : public widget::InitData {
   }
 };
 
-nsresult nsView::CreateWidget(bool aEnableDragDrop, bool aResetVisibility) {
+nsresult nsView::CreateWidget(nsIWidget* aParent, bool aEnableDragDrop,
+                              bool aResetVisibility) {
   AssertNoWindow();
 
   DefaultWidgetInitData initData;
   LayoutDeviceIntRect trect =
       CalcWidgetBounds(initData.mWindowType, initData.mTransparencyMode);
 
-  nsIWidget* parentWidget =
-      GetParent() ? GetParent()->GetNearestWidget(nullptr) : nullptr;
-  if (!parentWidget) {
+  if (!aParent && GetParent()) {
+    aParent = GetParent()->GetNearestWidget(nullptr);
+  }
+  if (!aParent) {
     NS_ERROR("nsView::CreateWidget without suitable parent widget??");
     return NS_ERROR_FAILURE;
   }
 
-  // XXX: using aForceUseIWidgetParent=true to preserve previous
-  // semantics.  It's not clear that it's actually needed.
-  mWindow = parentWidget->CreateChild(trect, &initData, true);
-  if (!mWindow) {
-    return NS_ERROR_FAILURE;
-  }
-
-  InitializeWindow(aEnableDragDrop, aResetVisibility);
-
-  return NS_OK;
-}
-
-nsresult nsView::CreateWidgetForParent(nsIWidget* aParentWidget,
-                                       bool aEnableDragDrop,
-                                       bool aResetVisibility) {
-  AssertNoWindow();
-  MOZ_ASSERT(aParentWidget, "Parent widget required");
-
-  DefaultWidgetInitData initData;
-  LayoutDeviceIntRect trect =
-      CalcWidgetBounds(initData.mWindowType, initData.mTransparencyMode);
-
-  mWindow = aParentWidget->CreateChild(trect, &initData);
+  mWindow = aParent->CreateChild(trect, initData);
   if (!mWindow) {
     return NS_ERROR_FAILURE;
   }
@@ -549,7 +529,7 @@ nsresult nsView::CreateWidgetForParent(nsIWidget* aParentWidget,
 }
 
 nsresult nsView::CreateWidgetForPopup(widget::InitData* aWidgetInitData,
-                                      nsIWidget* aParentWidget) {
+                                      nsIWidget* aParent) {
   AssertNoWindow();
   MOZ_ASSERT(aWidgetInitData, "Widget init data required");
   MOZ_ASSERT(aWidgetInitData->mWindowType == WindowType::Popup,
@@ -558,31 +538,20 @@ nsresult nsView::CreateWidgetForPopup(widget::InitData* aWidgetInitData,
   LayoutDeviceIntRect trect = CalcWidgetBounds(
       aWidgetInitData->mWindowType, aWidgetInitData->mTransparencyMode);
 
-  // XXX/cjones: having these two separate creation cases seems ... um
-  // ... unnecessary, but it's the way the old code did it.  Please
-  // unify them by first finding a suitable parent nsIWidget, then
-  // getting rid of aForceUseIWidgetParent.
-  if (aParentWidget) {
-    // XXX: using aForceUseIWidgetParent=true to preserve previous
-    // semantics.  It's not clear that it's actually needed.
-    mWindow = aParentWidget->CreateChild(trect, aWidgetInitData, true);
-  } else {
-    nsIWidget* nearestParent =
-        GetParent() ? GetParent()->GetNearestWidget(nullptr) : nullptr;
-    if (!nearestParent) {
-      // Without a parent, we can't make a popup.  This can happen
-      // when printing
-      return NS_ERROR_FAILURE;
-    }
-
-    mWindow = nearestParent->CreateChild(trect, aWidgetInitData);
+  if (!aParent && GetParent()) {
+    aParent = GetParent()->GetNearestWidget(nullptr);
+  }
+  if (!aParent) {
+    NS_ERROR("nsView::CreateWidgetForPopup without suitable parent widget??");
+    // Without a parent, we can't make a popup. This used to be able to happen
+    // when printing, apparently.
+    return NS_ERROR_FAILURE;
   }
+  mWindow = aParent->CreateChild(trect, *aWidgetInitData);
   if (!mWindow) {
     return NS_ERROR_FAILURE;
   }
-
   InitializeWindow(/* aEnableDragDrop = */ true, /* aResetVisibility = */ true);
-
   return NS_OK;
 }