Remove keeping browser context as shared pointer in web context adapter

In widgets we document that web contents can not out live browser
context ie. WebEngineProfile can not be deleted before WebEnginePage
which uses it.

In qml we can not be sure the order in which objects are garbage
collected. We used shared pointers to keep order of destruction.
Unfortunately shared pointers do not work well with corner cases,
and we added more and more code to deal with that (shutdown methods
+ qpointers wrapping qsharedpointers). In order to remove growing
complexity remove usage of shared pointers to keep strict deletion order.

Remove shared pointer from WebContentsAdapter and simply track the WebContents,
that is used by the given BrowserContext. Force deletion of webcontents first.

Change-Id: I05f886a0094d971b03f9a35e12c4b4672f0fe4ce
Reviewed-by: Jüri Valdmann <juri.valdmann@qt.io>

4 files changed, 36 insertions, 5 deletions

diff --git a/src/webengine/api/qquickwebengineprofile.cpp b/src/webengine/api/qquickwebengineprofile.cpp
index c89e4d522..1dead56ac 100644
--- a/src/webengine/api/qquickwebengineprofile.cpp
+++ b/src/webengine/api/qquickwebengineprofile.cpp
@@ -44,6 +44,7 @@
 #include "qquickwebengineprofile_p.h"
 #include "qquickwebenginescript_p.h"
 #include "qquickwebenginesettings_p.h"
+#include "qquickwebengineview_p_p.h"
 #include "qwebenginecookiestore.h"
 
 #include <QQmlEngine>
@@ -153,6 +154,10 @@ QQuickWebEngineProfilePrivate::QQuickWebEngineProfilePrivate(QSharedPointer<Brow
 
 QQuickWebEngineProfilePrivate::~QQuickWebEngineProfilePrivate()
 {
+    while (!m_webContentsAdapterClients.isEmpty()) {
+       m_webContentsAdapterClients.first()->destroy();
+    }
+
     Q_FOREACH (QQuickWebEngineDownloadItem *download, m_ongoingDownloads) {
         if (download)
             download->cancel();
@@ -164,6 +169,16 @@ QQuickWebEngineProfilePrivate::~QQuickWebEngineProfilePrivate()
         m_browserContext->shutdown();
 }
 
+void QQuickWebEngineProfilePrivate::addWebContentsAdapterClient(QQuickWebEngineViewPrivate *adapter)
+{
+    m_webContentsAdapterClients.append(adapter);
+}
+
+void QQuickWebEngineProfilePrivate::removeWebContentsAdapterClient(QQuickWebEngineViewPrivate*adapter)
+{
+    m_webContentsAdapterClients.removeAll(adapter);
+}
+
 QSharedPointer<QtWebEngineCore::BrowserContextAdapter> QQuickWebEngineProfilePrivate::browserContext() const
 {
     return m_browserContext ? m_browserContext->browserContextRef : nullptr;
diff --git a/src/webengine/api/qquickwebengineprofile_p.h b/src/webengine/api/qquickwebengineprofile_p.h
index 61968bf77..489a3efdf 100644
--- a/src/webengine/api/qquickwebengineprofile_p.h
+++ b/src/webengine/api/qquickwebengineprofile_p.h
@@ -65,12 +65,15 @@ QT_BEGIN_NAMESPACE
 
 class QQuickWebEngineDownloadItem;
 class QQuickWebEngineSettings;
+class QQuickWebEngineViewPrivate;
 
 class QQuickWebEngineProfilePrivate : public QtWebEngineCore::BrowserContextAdapterClient {
 public:
     Q_DECLARE_PUBLIC(QQuickWebEngineProfile)
     QQuickWebEngineProfilePrivate(QSharedPointer<QtWebEngineCore::BrowserContextAdapter> browserContext);
     ~QQuickWebEngineProfilePrivate();
+    void addWebContentsAdapterClient(QQuickWebEngineViewPrivate *adapter);
+    void removeWebContentsAdapterClient(QQuickWebEngineViewPrivate *adapter);
 
     QSharedPointer<QtWebEngineCore::BrowserContextAdapter> browserContext() const;
     QQuickWebEngineSettings *settings() const { return m_settings.data(); }
@@ -94,6 +97,7 @@ private:
     QPointer<QWebEngineBrowserContext> m_browserContext;
     QMap<quint32, QPointer<QQuickWebEngineDownloadItem> > m_ongoingDownloads;
     QList<QQuickWebEngineScript *> m_userScripts;
+    QVector<QQuickWebEngineViewPrivate *> m_webContentsAdapterClients;
 };
 
 QT_END_NAMESPACE
diff --git a/src/webengine/api/qquickwebengineview.cpp b/src/webengine/api/qquickwebengineview.cpp
index fde7b40dc..3d873c49f 100644
--- a/src/webengine/api/qquickwebengineview.cpp
+++ b/src/webengine/api/qquickwebengineview.cpp
@@ -125,6 +125,7 @@ QQuickWebEngineViewPrivate::QQuickWebEngineViewPrivate()
     , m_defaultZoomFactor(1.0)
     , m_ui2Enabled(false)
 {
+    m_profile->d_ptr->addWebContentsAdapterClient(this);
     QString platform = qApp->platformName().toLower();
     if (platform == QLatin1Literal("eglfs"))
         m_ui2Enabled = true;
@@ -152,6 +153,18 @@ QQuickWebEngineViewPrivate::QQuickWebEngineViewPrivate()
 
 QQuickWebEngineViewPrivate::~QQuickWebEngineViewPrivate()
 {
+    m_profile->d_ptr->removeWebContentsAdapterClient(this);
+    adapter->stopFinding();
+    if (faviconProvider)
+        faviconProvider->detach(q_ptr);
+}
+
+void QQuickWebEngineViewPrivate::destroy()
+{
+   // the profile for this web contens is about to be
+   // garbage collected, delete WebContent first and
+   // let the QQuickWebEngineView be collected later by gc.
+   delete q_ptr->d_ptr.take();
 }
 
 UIDelegatesManager *QQuickWebEngineViewPrivate::ui()
@@ -729,10 +742,6 @@ QQuickWebEngineView::QQuickWebEngineView(QQuickItem *parent)
 
 QQuickWebEngineView::~QQuickWebEngineView()
 {
-    Q_D(QQuickWebEngineView);
-    d->adapter->stopFinding();
-    if (d->faviconProvider)
-        d->faviconProvider->detach(this);
 }
 
 void QQuickWebEngineViewPrivate::ensureContentsAdapter()
@@ -898,7 +907,9 @@ void QQuickWebEngineViewPrivate::setProfile(QQuickWebEngineProfile *profile)
 
     if (profile == m_profile)
         return;
+    m_profile->d_ptr->removeWebContentsAdapterClient(this);
     m_profile = profile;
+    m_profile->d_ptr->addWebContentsAdapterClient(this);
     Q_EMIT q->profileChanged();
     m_settings->setParentSettings(profile->settings());
 
@@ -1362,7 +1373,7 @@ void QQuickWebEngineView::geometryChanged(const QRectF &newGeometry, const QRect
 void QQuickWebEngineView::itemChange(ItemChange change, const ItemChangeData &value)
 {
     Q_D(QQuickWebEngineView);
-    if (d->adapter->isInitialized() && (change == ItemSceneChange || change == ItemVisibleHasChanged)) {
+    if (d && d->adapter->isInitialized() && (change == ItemSceneChange || change == ItemVisibleHasChanged)) {
         if (window() && isVisible())
             d->adapter->wasShown();
         else
diff --git a/src/webengine/api/qquickwebengineview_p_p.h b/src/webengine/api/qquickwebengineview_p_p.h
index 73607aa59..6051ab3be 100644
--- a/src/webengine/api/qquickwebengineview_p_p.h
+++ b/src/webengine/api/qquickwebengineview_p_p.h
@@ -88,6 +88,7 @@ public:
     QQuickWebEngineView *q_ptr;
     QQuickWebEngineViewPrivate();
     ~QQuickWebEngineViewPrivate();
+    void destroy();
 
     QtWebEngineCore::UIDelegatesManager *ui();