Close popups when parent window is moved

When an HTML select box was clicked inside of a QWebEngineView and
the parent QWebEngineView window was moved using the mouse
(via window decoration toolbar for example) the popup window
would stay around instead of being closed.

This happened because of the usage of the Qt:Tool window flag for the
popup window, which implies a tool that floats near its parent window.

The fix is threefold:
1) Use Qt::Popup instead, similarly to how QMenu does it.
   Whenever the parent window is moved, the popup will now get a
   CloseEvent.
2) Handle the CloseEvent by telling Chromium to close and destroy
   the popup.
3) On Windows the OS might send mouse move events to the popup RWHVQD
   instance after its parent QWebEngineView, RWHVQD,
   QWebEnginePagePrivate (client adapter) is destroyed. We need to
   guard the mouse forwarding code not to access the client adapter
   if it has already been destroyed.

The second point is done by telling Chromium that its popup lost focus
which it interprets as a sign to hide it, and automatically destroy
it. This will destroy the underlying RWHVQtDelegateWidget and
RWHVQt instances.

Task-number: QTBUG-59891
Change-Id: I47f94a93c495a6caa5de92a6022eaca154994eda
Reviewed-by: Michael Brüning <michael.bruning@qt.io>

5 files changed, 30 insertions, 10 deletions

diff --git a/src/core/render_widget_host_view_qt.cpp b/src/core/render_widget_host_view_qt.cpp
index 7cb23bb58..3641398f6 100644
--- a/src/core/render_widget_host_view_qt.cpp
+++ b/src/core/render_widget_host_view_qt.cpp
@@ -1072,7 +1072,11 @@ bool RenderWidgetHostViewQt::forwardEvent(QEvent *event)
     case QEvent::MouseMove:
         // Skip second MouseMove event when a window is being adopted, so that Chromium
         // can properly handle further move events.
-        if (m_adapterClient->isBeingAdopted())
+        // Also make sure the adapter client exists to prevent a null pointer dereference,
+        // because it's possible for a QWebEnginePagePrivate (adapter) instance to be destroyed,
+        // and then the OS (observed on Windows) might still send mouse move events to a still
+        // existing popup RWHVQDW instance.
+        if (m_adapterClient && m_adapterClient->isBeingAdopted())
             return false;
         handleMouseEvent(static_cast<QMouseEvent*>(event));
         break;
@@ -1171,6 +1175,15 @@ QVariant RenderWidgetHostViewQt::inputMethodQuery(Qt::InputMethodQuery query)
     }
 }
 
+void RenderWidgetHostViewQt::closePopup()
+{
+    // We notify the popup to be closed by telling it that it lost focus. WebKit does the rest
+    // (hiding the widget and automatic memory cleanup via
+    // RenderWidget::CloseWidgetSoon() -> RenderWidgetHostImpl::ShutdownAndDestroyWidget(true).
+    m_host->SetActive(false);
+    m_host->Blur();
+}
+
 void RenderWidgetHostViewQt::ProcessAckedTouchEvent(const content::TouchEventWithLatencyInfo &touch, content::InputEventAckState ack_result) {
     Q_UNUSED(touch);
     const bool eventConsumed = ack_result == content::INPUT_EVENT_ACK_STATE_CONSUMED;
diff --git a/src/core/render_widget_host_view_qt.h b/src/core/render_widget_host_view_qt.h
index 0c62b7279..cc62fa110 100644
--- a/src/core/render_widget_host_view_qt.h
+++ b/src/core/render_widget_host_view_qt.h
@@ -174,6 +174,7 @@ public:
     void windowChanged() override;
     bool forwardEvent(QEvent *) override;
     QVariant inputMethodQuery(Qt::InputMethodQuery query) override;
+    void closePopup() override;
 
     // Overridden from content::TextInputManager::Observer
     void OnUpdateTextInputStateCalled(content::TextInputManager *text_input_manager, RenderWidgetHostViewBase *updated_view, bool did_update_state) override;
diff --git a/src/core/render_widget_host_view_qt_delegate.h b/src/core/render_widget_host_view_qt_delegate.h
index bcd0f49f7..3ac3ccc1e 100644
--- a/src/core/render_widget_host_view_qt_delegate.h
+++ b/src/core/render_widget_host_view_qt_delegate.h
@@ -78,6 +78,7 @@ public:
     virtual void windowChanged() = 0;
     virtual bool forwardEvent(QEvent *) = 0;
     virtual QVariant inputMethodQuery(Qt::InputMethodQuery query) = 0;
+    virtual void closePopup() = 0;
 };
 
 class QWEBENGINE_EXPORT RenderWidgetHostViewQtDelegate {
diff --git a/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.cpp b/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.cpp
index 98482ae78..c32e89f0c 100644
--- a/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.cpp
+++ b/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.cpp
@@ -218,20 +218,24 @@ void RenderWidgetHostViewQtDelegateWidget::initAsPopup(const QRect& screenRect)
     // to be destroyed.
     setAttribute(Qt::WA_ShowWithoutActivating);
     setFocusPolicy(Qt::NoFocus);
-
-#ifdef Q_OS_MACOS
-    // macOS doesn't like Qt::ToolTip when QWebEngineView is inside a modal dialog, specifically by
-    // not forwarding click events to the popup. So we use Qt::Tool which behaves the same way, but
-    // works on macOS too.
-    setWindowFlags(Qt::Tool | Qt::FramelessWindowHint | Qt::WindowDoesNotAcceptFocus);
-#else
-    setWindowFlags(Qt::ToolTip | Qt::FramelessWindowHint | Qt::WindowDoesNotAcceptFocus);
-#endif
+    setWindowFlags(Qt::Popup | Qt::FramelessWindowHint | Qt::WindowDoesNotAcceptFocus);
 
     setGeometry(screenRect);
     show();
 }
 
+void RenderWidgetHostViewQtDelegateWidget::closeEvent(QCloseEvent *event)
+{
+    Q_UNUSED(event);
+
+    // If a close event was received from the window manager (e.g. when moving the parent window,
+    // clicking outside the popup area)
+    // make sure to notify the Chromium WebUI popup and its underlying
+    // RenderWidgetHostViewQtDelegate instance to be closed.
+    if (m_isPopup)
+        m_client->closePopup();
+}
+
 QRectF RenderWidgetHostViewQtDelegateWidget::screenRect() const
 {
     return QRectF(x(), y(), width(), height());
diff --git a/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.h b/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.h
index 79958132c..ab24b1e41 100644
--- a/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.h
+++ b/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.h
@@ -83,6 +83,7 @@ protected:
     void resizeEvent(QResizeEvent *resizeEvent) override;
     void showEvent(QShowEvent *) override;
     void hideEvent(QHideEvent *) override;
+    void closeEvent(QCloseEvent *event) override;
 
     QVariant inputMethodQuery(Qt::InputMethodQuery query) const override;