Fix access to deleted memory on QWebEnginePage destruction

Suppose QWebEnginePage is destroyed while there's still a combobox popup
open. We would crash with the following stack trace:

1   QtWebEngineCore::RenderWidgetHostViewQt::dpiScale
2   QtWebEngineCore::RenderWidgetHostViewQt::GetViewBounds
3   content::RenderWidgetHostImpl::SendScreenRects
4   content::RenderWidgetHostImpl::OnRenderViewReady
...
16  base::MessageLoop::DoWork
17  WebEngineContext::destroy
18  `anonymous namespace'::destroyContext
19  qt_call_post_routines
20  QApplication::~QApplication

RenderWidgetHostViewQt still holds a pointer to WebContentsAdapterClient.
To fix this, expose the QObject owning the adapter client, and
hide RenderWidgetHostViewQt when it is destroyed so it won't try to render.

Change-Id: Ide5543197b35038a3e1c7491ceda3f5ad10f6f07
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

7 files changed, 21 insertions, 0 deletions

diff --git a/src/core/render_widget_host_view_qt.cpp b/src/core/render_widget_host_view_qt.cpp
index 220761ef3..24b148ca5 100644
--- a/src/core/render_widget_host_view_qt.cpp
+++ b/src/core/render_widget_host_view_qt.cpp
@@ -273,6 +273,7 @@ RenderWidgetHostViewQt::RenderWidgetHostViewQt(content::RenderWidgetHost* widget
 
 RenderWidgetHostViewQt::~RenderWidgetHostViewQt()
 {
+    QObject::disconnect(m_adapterClientDestroyedConnection);
 #ifndef QT_NO_ACCESSIBILITY
     QAccessible::removeActivationObserver(this);
 #endif // QT_NO_ACCESSIBILITY
@@ -288,6 +289,10 @@ void RenderWidgetHostViewQt::setAdapterClient(WebContentsAdapterClient *adapterC
     Q_ASSERT(!m_adapterClient);
 
     m_adapterClient = adapterClient;
+    QObject::disconnect(m_adapterClientDestroyedConnection);
+    m_adapterClientDestroyedConnection = QObject::connect(adapterClient->holdingQObject(),
+                                                          &QObject::destroyed, [this] {
+                                                            m_adapterClient = nullptr; });
     if (m_initPending)
         InitAsChild(0);
 }
diff --git a/src/core/render_widget_host_view_qt.h b/src/core/render_widget_host_view_qt.h
index 2a56f61a4..2e6563a67 100644
--- a/src/core/render_widget_host_view_qt.h
+++ b/src/core/render_widget_host_view_qt.h
@@ -226,6 +226,7 @@ private:
     bool m_didFirstVisuallyNonEmptyLayout;
     uint32 m_pendingOutputSurfaceId;
 
+    QMetaObject::Connection m_adapterClientDestroyedConnection;
     WebContentsAdapterClient *m_adapterClient;
     MultipleMouseClickHelper m_clickHelper;
 
diff --git a/src/core/web_contents_adapter_client.h b/src/core/web_contents_adapter_client.h
index e6d25a8fb..f0927c9e5 100644
--- a/src/core/web_contents_adapter_client.h
+++ b/src/core/web_contents_adapter_client.h
@@ -240,6 +240,7 @@ public:
     virtual void requestGeometryChange(const QRect &geometry) = 0;
     virtual void allowCertificateError(const QSharedPointer<CertificateErrorController> &errorController) = 0;
     virtual bool isEnabled() const = 0;
+    virtual const QObject *holdingQObject() const = 0;
 
     virtual QSharedPointer<BrowserContextAdapter> browserContextAdapter() = 0;
 
diff --git a/src/webengine/api/qquickwebengineview.cpp b/src/webengine/api/qquickwebengineview.cpp
index f0458b869..357f95bc4 100644
--- a/src/webengine/api/qquickwebengineview.cpp
+++ b/src/webengine/api/qquickwebengineview.cpp
@@ -628,6 +628,12 @@ WebEngineSettings *QQuickWebEngineViewPrivate::webEngineSettings() const
     return m_settings->d_ptr.data();
 }
 
+const QObject *QQuickWebEngineViewPrivate::holdingQObject() const
+{
+    Q_Q(const QQuickWebEngineView);
+    return q;
+}
+
 void QQuickWebEngineViewPrivate::setDevicePixelRatio(qreal devicePixelRatio)
 {
     Q_Q(QQuickWebEngineView);
diff --git a/src/webengine/api/qquickwebengineview_p_p.h b/src/webengine/api/qquickwebengineview_p_p.h
index 9ac86aa5c..2a6c2c879 100644
--- a/src/webengine/api/qquickwebengineview_p_p.h
+++ b/src/webengine/api/qquickwebengineview_p_p.h
@@ -172,6 +172,7 @@ public:
                                      int exitCode) Q_DECL_OVERRIDE;
     virtual void requestGeometryChange(const QRect &geometry) Q_DECL_OVERRIDE { Q_UNUSED(geometry); }
     virtual bool isEnabled() const Q_DECL_OVERRIDE;
+    const QObject *holdingQObject() const Q_DECL_OVERRIDE;
 
     virtual QSharedPointer<QtWebEngineCore::BrowserContextAdapter> browserContextAdapter() Q_DECL_OVERRIDE;
 
diff --git a/src/webenginewidgets/api/qwebenginepage.cpp b/src/webenginewidgets/api/qwebenginepage.cpp
index 2fd026682..0279c9343 100644
--- a/src/webenginewidgets/api/qwebenginepage.cpp
+++ b/src/webenginewidgets/api/qwebenginepage.cpp
@@ -440,6 +440,12 @@ QSharedPointer<BrowserContextAdapter> QWebEnginePagePrivate::browserContextAdapt
     return profile->d_ptr->browserContext();
 }
 
+const QObject *QWebEnginePagePrivate::holdingQObject() const
+{
+    Q_Q(const QWebEnginePage);
+    return q;
+}
+
 QWebEnginePage::QWebEnginePage(QObject* parent)
     : QObject(parent)
     , d_ptr(new QWebEnginePagePrivate())
diff --git a/src/webenginewidgets/api/qwebenginepage_p.h b/src/webenginewidgets/api/qwebenginepage_p.h
index 8e5be51d0..7b16ed667 100644
--- a/src/webenginewidgets/api/qwebenginepage_p.h
+++ b/src/webenginewidgets/api/qwebenginepage_p.h
@@ -125,6 +125,7 @@ public:
                                      int exitCode) Q_DECL_OVERRIDE;
     virtual void requestGeometryChange(const QRect &geometry) Q_DECL_OVERRIDE;
     virtual bool isEnabled() const Q_DECL_OVERRIDE;
+    const QObject *holdingQObject() const Q_DECL_OVERRIDE;
 
     virtual QSharedPointer<QtWebEngineCore::BrowserContextAdapter> browserContextAdapter() Q_DECL_OVERRIDE;