Bug 1921903 - Ensure we don't create hidden widgets in non-macOS platforms. r=rkraesig,geckoview-reviewers,win-reviewers,m_kato

Pretty sure that we can't hit that case right now, and try with MOZ_ASSERT
agrees with me at least.

Popups don't get there. For puppet widgets, mParentWidget is provided
explicitly.

This code is all super messy and can be cleaned-up significantly.
See following patch.

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

Author: emilio

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, nullptr, true);
+  rv = view->CreateWidgetForParent(parentWidget, true);
   NS_ENSURE_SUCCESS(rv, false);
 
   // Setup hierarchical relationship in view manager

layout/base/nsDocumentViewer.cpp

@@ -2216,8 +2216,7 @@ nsresult nsDocumentViewer::MakeWindow(const nsSize& aSize,
     return NS_OK;
   }
 
-  bool shouldAttach = ShouldAttachToTopLevel();
-
+  const bool shouldAttach = ShouldAttachToTopLevel();
   if (shouldAttach) {
     // If the old view is already attached to our parent, detach
     DetachFromTopLevelWidget();
@@ -2242,27 +2241,14 @@ nsresult nsDocumentViewer::MakeWindow(const nsSize& aSize,
   // because when they're displayed, they're painted into *another* document's
   // widget.
   if (!mDocument->IsResourceDoc() && (mParentWidget || !aContainerView)) {
-    // pass in a native widget to be the parent widget ONLY if the view
-    // hierarchy will stand alone. otherwise the view will find its own parent
-    // widget and "do the right thing" to establish a parent/child widget
-    // relationship
-    widget::InitData initData;
-    widget::InitData* initDataPtr;
-    if (!mParentWidget) {
-      initDataPtr = &initData;
-      initData.mWindowType = widget::WindowType::Invisible;
-    } else {
-      initDataPtr = nullptr;
-    }
-
     if (shouldAttach) {
       // Reuse the top level parent widget.
       rv = view->AttachToTopLevelWidget(mParentWidget);
       mAttachedToParent = true;
-    } else if (!aContainerView && mParentWidget) {
-      rv = view->CreateWidgetForParent(mParentWidget, initDataPtr, true, false);
+    } else if (!mParentWidget || aContainerView) {
+      rv = view->CreateWidget(true, false);
     } else {
-      rv = view->CreateWidget(initDataPtr, true, false);
+      rv = view->CreateWidgetForParent(mParentWidget, true, false);
     }
     if (NS_FAILED(rv)) return rv;
   }
@@ -3252,7 +3238,7 @@ bool nsDocumentViewer::ShouldAttachToTopLevel() {
     return true;
   }
 
-  // FIXME(emilio): Can we unify this between macOS and aother platforms?
+  // TODO(emilio, bug 1919165): Unify this between macOS and other platforms?
 #ifdef XP_MACOSX
   return false;
 #else

view/nsView.cpp

@@ -502,17 +502,12 @@ struct DefaultWidgetInitData : public widget::InitData {
   }
 };
 
-nsresult nsView::CreateWidget(widget::InitData* aWidgetInitData,
-                              bool aEnableDragDrop, bool aResetVisibility) {
+nsresult nsView::CreateWidget(bool aEnableDragDrop, bool aResetVisibility) {
   AssertNoWindow();
-  MOZ_ASSERT(
-      !aWidgetInitData || aWidgetInitData->mWindowType != WindowType::Popup,
-      "Use CreateWidgetForPopup");
 
-  DefaultWidgetInitData defaultInitData;
-  aWidgetInitData = aWidgetInitData ? aWidgetInitData : &defaultInitData;
-  LayoutDeviceIntRect trect = CalcWidgetBounds(
-      aWidgetInitData->mWindowType, aWidgetInitData->mTransparencyMode);
+  DefaultWidgetInitData initData;
+  LayoutDeviceIntRect trect =
+      CalcWidgetBounds(initData.mWindowType, initData.mTransparencyMode);
 
   nsIWidget* parentWidget =
       GetParent() ? GetParent()->GetNearestWidget(nullptr) : nullptr;
@@ -523,7 +518,7 @@ nsresult nsView::CreateWidget(widget::InitData* aWidgetInitData,
 
   // XXX: using aForceUseIWidgetParent=true to preserve previous
   // semantics.  It's not clear that it's actually needed.
-  mWindow = parentWidget->CreateChild(trect, aWidgetInitData, true);
+  mWindow = parentWidget->CreateChild(trect, &initData, true);
   if (!mWindow) {
     return NS_ERROR_FAILURE;
   }
@@ -534,22 +529,16 @@ nsresult nsView::CreateWidget(widget::InitData* aWidgetInitData,
 }
 
 nsresult nsView::CreateWidgetForParent(nsIWidget* aParentWidget,
-                                       widget::InitData* aWidgetInitData,
                                        bool aEnableDragDrop,
                                        bool aResetVisibility) {
   AssertNoWindow();
-  MOZ_ASSERT(
-      !aWidgetInitData || aWidgetInitData->mWindowType != WindowType::Popup,
-      "Use CreateWidgetForPopup");
   MOZ_ASSERT(aParentWidget, "Parent widget required");
 
-  DefaultWidgetInitData defaultInitData;
-  aWidgetInitData = aWidgetInitData ? aWidgetInitData : &defaultInitData;
-
-  LayoutDeviceIntRect trect = CalcWidgetBounds(
-      aWidgetInitData->mWindowType, aWidgetInitData->mTransparencyMode);
+  DefaultWidgetInitData initData;
+  LayoutDeviceIntRect trect =
+      CalcWidgetBounds(initData.mWindowType, initData.mTransparencyMode);
 
-  mWindow = aParentWidget->CreateChild(trect, aWidgetInitData);
+  mWindow = aParentWidget->CreateChild(trect, &initData);
   if (!mWindow) {
     return NS_ERROR_FAILURE;
   }
@@ -622,7 +611,7 @@ void nsView::SetNeedsWindowPropertiesSync() {
 
 // Attach to a top level widget and start receiving mirrored events.
 nsresult nsView::AttachToTopLevelWidget(nsIWidget* aWidget) {
-  MOZ_ASSERT(nullptr != aWidget, "null widget ptr");
+  MOZ_ASSERT(aWidget, "null widget ptr");
 
   /// XXXjimm This is a temporary workaround to an issue w/document
   // viewer (bug 513162).

view/nsView.h

@@ -280,12 +280,9 @@ class nsView final : public nsIWidgetListener {
    * CreateWidget*() will look around in the view hierarchy for an
    * appropriate parent widget for the view.
    *
-   * @param aWidgetInitData data used to initialize this view's widget before
-   *        its create is called.
    * @return error status
    */
-  nsresult CreateWidget(mozilla::widget::InitData* aWidgetInitData = nullptr,
-                        bool aEnableDragDrop = true,
+  nsresult CreateWidget(bool aEnableDragDrop = true,
                         bool aResetVisibility = true);
 
   /**
@@ -294,7 +291,6 @@ class nsView final : public nsIWidgetListener {
    * as for |CreateWidget()|.
    */
   nsresult CreateWidgetForParent(nsIWidget* aParentWidget,
-                                 mozilla::widget::InitData* = nullptr,
                                  bool aEnableDragDrop = true,
                                  bool aResetVisibility = true);
 

widget/android/nsWindow.cpp

@@ -2241,6 +2241,9 @@ nsresult nsWindow::Create(nsIWidget* aParent, nsNativeWidget aNativeParent,
   mBounds = rect;
   SetSizeConstraints(SizeConstraints());
 
+  MOZ_DIAGNOSTIC_ASSERT(!aInitData ||
+                        aInitData->mWindowType != WindowType::Invisible);
+
   BaseCreate(nullptr, aInitData);
 
   NS_ASSERTION(IsTopLevel() || parent,

widget/gtk/nsWindow.cpp

@@ -755,10 +755,6 @@ void nsWindow::SetParent(nsIWidget* aNewParent) {
 }
 
 bool nsWindow::WidgetTypeSupportsAcceleration() {
-  if (mWindowType == WindowType::Invisible) {
-    return false;
-  }
-
   if (IsSmallPopup()) {
     return false;
   }
@@ -2969,7 +2965,7 @@ void nsWindow::MoveToWorkspace(const nsAString& workspaceIDStr) {
 
 void nsWindow::SetUserTimeAndStartupTokenForActivatedWindow() {
   nsGTKToolkit* toolkit = nsGTKToolkit::GetToolkit();
-  if (!toolkit || MOZ_UNLIKELY(mWindowType == WindowType::Invisible)) {
+  if (!toolkit) {
     return;
   }
 
@@ -5876,12 +5872,14 @@ nsresult nsWindow::Create(nsIWidget* aParent, nsNativeWidget aNativeParent,
                           widget::InitData* aInitData) {
   LOG("nsWindow::Create\n");
 
+  MOZ_DIAGNOSTIC_ASSERT(!aInitData ||
+                        aInitData->mWindowType != WindowType::Invisible);
+
   // only set the base parent if we're going to be a dialog or a
   // toplevel
   nsIWidget* baseParent =
       aInitData && (aInitData->mWindowType == WindowType::Dialog ||
-                    aInitData->mWindowType == WindowType::TopLevel ||
-                    aInitData->mWindowType == WindowType::Invisible)
+                    aInitData->mWindowType == WindowType::TopLevel)
           ? nullptr
           : aParent;
 
@@ -5914,9 +5912,9 @@ nsresult nsWindow::Create(nsIWidget* aParent, nsNativeWidget aNativeParent,
   ConstrainSize(&mBounds.width, &mBounds.height);
   mLastSizeRequest = mBounds.Size();
 
-  bool popupNeedsAlphaVisual = mWindowType == WindowType::Popup &&
-                               (aInitData && aInitData->mTransparencyMode ==
-                                                 TransparencyMode::Transparent);
+  bool popupNeedsAlphaVisual =
+      mWindowType == WindowType::Popup && aInitData &&
+      aInitData->mTransparencyMode == TransparencyMode::Transparent;
 
   // Figure out our parent window - only used for WindowType::Child
   nsWindow* parentnsWindow = nullptr;
@@ -5946,8 +5944,7 @@ nsresult nsWindow::Create(nsIWidget* aParent, nsNativeWidget aNativeParent,
   MOZ_ASSERT_IF(mWindowType == WindowType::Popup, parentnsWindow);
 
   if (mWindowType != WindowType::Dialog && mWindowType != WindowType::Popup &&
-      mWindowType != WindowType::TopLevel &&
-      mWindowType != WindowType::Invisible) {
+      mWindowType != WindowType::TopLevel) {
     MOZ_ASSERT_UNREACHABLE("Unexpected eWindowType");
     return NS_ERROR_FAILURE;
   }

widget/windows/WinEventObserver.cpp

@@ -34,11 +34,6 @@
 extern "C" IMAGE_DOS_HEADER __ImageBase;
 #define CURRENT_MODULE() reinterpret_cast<HMODULE>(&__ImageBase)
 
-// N.B.: if and when we eliminate the existing `WindowType::Invisible` hidden
-// window, we must switch to use of `kClassNameHidden` for the class name. (See
-// commentary therebeside.)
-const wchar_t kClassNameHidden2[] = L"MozillaHiddenWindowClass2";
-
 namespace mozilla::widget {
 
 LazyLogModule gWinEventWindowLog("WinEventWindow");
@@ -66,7 +61,7 @@ void WinEventWindow::Ensure() {
   HMODULE const hSelf = CURRENT_MODULE();
   WNDCLASSW const wc = {.lpfnWndProc = WinEventWindow::WndProc,
                         .hInstance = hSelf,
-                        .lpszClassName = kClassNameHidden2};
+                        .lpszClassName = kClassNameHidden};
   ATOM const atom = ::RegisterClassW(&wc);
   if (!atom) {
     // This is known to be possible when the atom table no longer has free

widget/windows/nsWindow.cpp

@@ -873,12 +873,12 @@ nsresult nsWindow::Create(nsIWidget* aParent, nsNativeWidget aNativeParent,
   widget::InitData defaultInitData;
   if (!aInitData) aInitData = &defaultInitData;
 
-  nsIWidget* baseParent =
-      aInitData->mWindowType == WindowType::Dialog ||
-              aInitData->mWindowType == WindowType::TopLevel ||
-              aInitData->mWindowType == WindowType::Invisible
-          ? nullptr
-          : aParent;
+  MOZ_DIAGNOSTIC_ASSERT(aInitData->mWindowType != WindowType::Invisible);
+
+  nsIWidget* baseParent = aInitData->mWindowType == WindowType::Dialog ||
+                                  aInitData->mWindowType == WindowType::TopLevel
+                              ? nullptr
+                              : aParent;
 
   mIsTopWidgetWindow = (nullptr == baseParent);
   mBounds = aRect;
@@ -888,7 +888,7 @@ nsresult nsWindow::Create(nsIWidget* aParent, nsNativeWidget aNativeParent,
 
   BaseCreate(baseParent, aInitData);
 
-  HWND parent;
+  HWND parent = nullptr;
   if (aParent) {  // has a nsIWidget parent
     parent = aParent ? (HWND)aParent->GetNativeData(NS_NATIVE_WINDOW) : nullptr;
     mParent = aParent;
@@ -913,9 +913,6 @@ nsresult nsWindow::Create(nsIWidget* aParent, nsNativeWidget aNativeParent,
     if (!aParent) {
       parent = nullptr;
     }
-  } else if (mWindowType == WindowType::Invisible) {
-    // Make sure CreateWindowEx succeeds at creating a toplevel window
-    desiredStyles.style &= ~WS_CHILDWINDOW;
   } else {
     // See if the caller wants to explictly set clip children and clip siblings
     if (aInitData->mClipChildren) {
@@ -1093,8 +1090,7 @@ nsresult nsWindow::Create(nsIWidget* aParent, nsNativeWidget aNativeParent,
                    SWP_NOMOVE | SWP_NOSIZE | SWP_NOACTIVATE);
   }
 
-  if (mWindowType != WindowType::Invisible &&
-      MouseScrollHandler::Device::IsFakeScrollableWindowNeeded()) {
+  if (MouseScrollHandler::Device::IsFakeScrollableWindowNeeded()) {
     // Ugly Thinkpad Driver Hack (Bugs 507222 and 594977)
     //
     // We create two zero-sized windows as descendants of the top-level window,
@@ -1263,8 +1259,6 @@ static LPWSTR const gStockApplicationIcon = MAKEINTRESOURCEW(32512);
 static const wchar_t* ChooseWindowClass(WindowType aWindowType) {
   const wchar_t* className = [aWindowType] {
     switch (aWindowType) {
-      case WindowType::Invisible:
-        return kClassNameHidden;
       case WindowType::Dialog:
         return kClassNameDialog;
       case WindowType::Popup:
@@ -1348,7 +1342,6 @@ DWORD nsWindow::WindowStyle() {
       [[fallthrough]];
 
     case WindowType::TopLevel:
-    case WindowType::Invisible:
       style = WS_OVERLAPPED | WS_CLIPCHILDREN | WS_DLGFRAME | WS_BORDER |
               WS_THICKFRAME | kTitlebarItemsWindowStyles;
       break;
@@ -5858,24 +5851,23 @@ bool nsWindow::ProcessMessageInternal(UINT msg, WPARAM& wParam, LPARAM& lParam,
       break;
 
     case WM_GESTURENOTIFY: {
-      if (mWindowType != WindowType::Invisible) {
-        // A GestureNotify event is dispatched to decide which single-finger
-        // panning direction should be active (including none) and if pan
-        // feedback should be displayed. Java and plugin windows can make their
-        // own calls.
-
-        GESTURENOTIFYSTRUCT* gestureinfo = (GESTURENOTIFYSTRUCT*)lParam;
-        nsPointWin touchPoint;
-        touchPoint = gestureinfo->ptsLocation;
-        touchPoint.ScreenToClient(mWnd);
-        WidgetGestureNotifyEvent gestureNotifyEvent(true, eGestureNotify, this);
-        gestureNotifyEvent.mRefPoint =
-            LayoutDeviceIntPoint::FromUnknownPoint(touchPoint);
-        nsEventStatus status;
-        DispatchEvent(&gestureNotifyEvent, status);
-        mDisplayPanFeedback = gestureNotifyEvent.mDisplayPanFeedback;
-        if (!mTouchWindow)
-          mGesture.SetWinGestureSupport(mWnd, gestureNotifyEvent.mPanDirection);
+      // A GestureNotify event is dispatched to decide which single-finger
+      // panning direction should be active (including none) and if pan
+      // feedback should be displayed. Java and plugin windows can make their
+      // own calls.
+
+      GESTURENOTIFYSTRUCT* gestureinfo = (GESTURENOTIFYSTRUCT*)lParam;
+      nsPointWin touchPoint;
+      touchPoint = gestureinfo->ptsLocation;
+      touchPoint.ScreenToClient(mWnd);
+      WidgetGestureNotifyEvent gestureNotifyEvent(true, eGestureNotify, this);
+      gestureNotifyEvent.mRefPoint =
+          LayoutDeviceIntPoint::FromUnknownPoint(touchPoint);
+      nsEventStatus status;
+      DispatchEvent(&gestureNotifyEvent, status);
+      mDisplayPanFeedback = gestureNotifyEvent.mDisplayPanFeedback;
+      if (!mTouchWindow) {
+        mGesture.SetWinGestureSupport(mWnd, gestureNotifyEvent.mPanDirection);
       }
       result = false;  // should always bubble to DefWindowProc
     } break;
@@ -6555,11 +6547,6 @@ void nsWindow::OnWindowPosChanging(WINDOWPOS* info) {
     }
   }
 
-  // prevent rude external programs from making hidden window visible
-  if (mWindowType == WindowType::Invisible) {
-    info->flags &= ~SWP_SHOWWINDOW;
-  }
-
   // When waking from sleep or switching out of tablet mode, Windows 10
   // Version 1809 will reopen popup windows that should be hidden. Detect
   // this case and refuse to show the window.
@@ -7192,7 +7179,7 @@ a11y::LocalAccessible* nsWindow::GetAccessible() {
   if (a11y::PlatformDisabledState() == a11y::ePlatformIsDisabled)
     return nullptr;
 
-  if (mInDtor || mOnDestroyCalled || mWindowType == WindowType::Invisible) {
+  if (mInDtor || mOnDestroyCalled) {
     return nullptr;
   }