From 06864c75f4d009bf8fd4e9f88215bb88341ed873 Mon Sep 17 00:00:00 2001
From: Allan Sandfeld Jensen <allan.jensen@theqtcompany.com>
Date: Thu, 30 Jun 2016 17:41:31 +0200
Subject: [PATCH] Switch WebContentsAdapter to using shared pointers
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

QExplicitSharedDataPointer is meant for value objects, not for shared
objects. Instead switch to using QSharedPointer.

Change-Id: Ib3791bbcfde627a67508f2819e141d8c538a4a50
Reviewed-by: Michael Brüning <michael.bruning@qt.io>
Reviewed-by: Michal Klocek <michal.klocek@theqtcompany.com>
---
 src/core/web_contents_adapter.cpp               |  6 +++---
 src/core/web_contents_adapter.h                 |  6 +++---
 src/core/web_contents_adapter_client.h          |  2 +-
 src/core/web_contents_delegate_qt.cpp           | 17 +++++------------
 src/core/web_contents_delegate_qt.h             |  2 +-
 src/core/web_engine_settings.h                  |  1 -
 .../api/qquickwebenginenewviewrequest_p.h       |  2 +-
 src/webengine/api/qquickwebengineview.cpp       | 12 ++++++------
 src/webengine/api/qquickwebengineview_p_p.h     |  4 ++--
 src/webenginewidgets/api/qwebenginepage.cpp     | 12 ++++++------
 src/webenginewidgets/api/qwebenginepage_p.h     |  4 ++--
 .../api/qwebenginescriptcollection.cpp          | 12 ++++++------
 .../api/qwebenginescriptcollection_p.h          |  8 ++++----
 13 files changed, 40 insertions(+), 48 deletions(-)

diff --git a/src/core/web_contents_adapter.cpp b/src/core/web_contents_adapter.cpp
index 709efe9b3..f447b5480 100644
--- a/src/core/web_contents_adapter.cpp
+++ b/src/core/web_contents_adapter.cpp
@@ -321,14 +321,14 @@ WebContentsAdapterPrivate::~WebContentsAdapterPrivate()
     webContents.reset();
 }
 
-QExplicitlySharedDataPointer<WebContentsAdapter> WebContentsAdapter::createFromSerializedNavigationHistory(QDataStream &input, WebContentsAdapterClient *adapterClient)
+QSharedPointer<WebContentsAdapter> WebContentsAdapter::createFromSerializedNavigationHistory(QDataStream &input, WebContentsAdapterClient *adapterClient)
 {
     int currentIndex;
     ScopedVector<content::NavigationEntry> entries;
     deserializeNavigationHistory(input, &currentIndex, &entries, adapterClient->browserContextAdapter()->browserContext());
 
     if (currentIndex == -1)
-        return QExplicitlySharedDataPointer<WebContentsAdapter>();
+        return QSharedPointer<WebContentsAdapter>();
 
     // Unlike WebCore, Chromium only supports Restoring to a new WebContents instance.
     content::WebContents* newWebContents = createBlankWebContents(adapterClient, adapterClient->browserContextAdapter()->browserContext());
@@ -346,7 +346,7 @@ QExplicitlySharedDataPointer<WebContentsAdapter> WebContentsAdapter::createFromS
             content::ChildProcessSecurityPolicy::GetInstance()->GrantReadFile(id, *file);
     }
 
-    return QExplicitlySharedDataPointer<WebContentsAdapter>(new WebContentsAdapter(newWebContents));
+    return QSharedPointer<WebContentsAdapter>::create(newWebContents);
 }
 
 WebContentsAdapter::WebContentsAdapter(content::WebContents *webContents)
diff --git a/src/core/web_contents_adapter.h b/src/core/web_contents_adapter.h
index ce033bdb4..0d9218d38 100644
--- a/src/core/web_contents_adapter.h
+++ b/src/core/web_contents_adapter.h
@@ -41,7 +41,7 @@
 #include "web_contents_adapter_client.h"
 
 #include <QScopedPointer>
-#include <QSharedData>
+#include <QSharedPointer>
 #include <QString>
 #include <QUrl>
 
@@ -61,9 +61,9 @@ class BrowserContextQt;
 class MessagePassingInterface;
 class WebContentsAdapterPrivate;
 
-class QWEBENGINE_EXPORT WebContentsAdapter : public QSharedData {
+class QWEBENGINE_EXPORT WebContentsAdapter : public QEnableSharedFromThis<WebContentsAdapter> {
 public:
-    static QExplicitlySharedDataPointer<WebContentsAdapter> createFromSerializedNavigationHistory(QDataStream &input, WebContentsAdapterClient *adapterClient);
+    static QSharedPointer<WebContentsAdapter> createFromSerializedNavigationHistory(QDataStream &input, WebContentsAdapterClient *adapterClient);
     // Takes ownership of the WebContents.
     WebContentsAdapter(content::WebContents *webContents = 0);
     ~WebContentsAdapter();
diff --git a/src/core/web_contents_adapter_client.h b/src/core/web_contents_adapter_client.h
index f0927c9e5..449f382cf 100644
--- a/src/core/web_contents_adapter_client.h
+++ b/src/core/web_contents_adapter_client.h
@@ -206,7 +206,7 @@ public:
     virtual void loadFinished(bool success, const QUrl &url, bool isErrorPage = false, int errorCode = 0, const QString &errorDescription = QString()) = 0;
     virtual void focusContainer() = 0;
     virtual void unhandledKeyEvent(QKeyEvent *event) = 0;
-    virtual void adoptNewWindow(WebContentsAdapter *newWebContents, WindowOpenDisposition disposition, bool userGesture, const QRect & initialGeometry) = 0;
+    virtual void adoptNewWindow(QSharedPointer<WebContentsAdapter> newWebContents, WindowOpenDisposition disposition, bool userGesture, const QRect & initialGeometry) = 0;
     virtual bool isBeingAdopted() = 0;
     virtual void close() = 0;
     virtual void windowCloseRejected() = 0;
diff --git a/src/core/web_contents_delegate_qt.cpp b/src/core/web_contents_delegate_qt.cpp
index bf12537d1..97f0e515d 100644
--- a/src/core/web_contents_delegate_qt.cpp
+++ b/src/core/web_contents_delegate_qt.cpp
@@ -93,7 +93,7 @@ content::WebContents *WebContentsDelegateQt::OpenURLFromTab(content::WebContents
 {
     content::WebContents *target = source;
     if (params.disposition != CURRENT_TAB) {
-        WebContentsAdapter *targetAdapter = createWindow(0, params.disposition, gfx::Rect(), params.user_gesture);
+        QSharedPointer<WebContentsAdapter> targetAdapter = createWindow(0, params.disposition, gfx::Rect(), params.user_gesture);
         if (targetAdapter)
             target = targetAdapter->webContents();
     }
@@ -139,7 +139,7 @@ bool WebContentsDelegateQt::ShouldPreserveAbortedURLs(content::WebContents *sour
 void WebContentsDelegateQt::AddNewContents(content::WebContents* source, content::WebContents* new_contents, WindowOpenDisposition disposition, const gfx::Rect& initial_pos, bool user_gesture, bool* was_blocked)
 {
     Q_UNUSED(source)
-    WebContentsAdapter *newAdapter = createWindow(new_contents, disposition, initial_pos, user_gesture);
+    QWeakPointer<WebContentsAdapter> newAdapter = createWindow(new_contents, disposition, initial_pos, user_gesture);
     if (was_blocked)
         *was_blocked = !newAdapter;
 }
@@ -372,20 +372,13 @@ void WebContentsDelegateQt::overrideWebPreferences(content::WebContents *, conte
     m_viewClient->webEngineSettings()->overrideWebPreferences(webPreferences);
 }
 
-WebContentsAdapter *WebContentsDelegateQt::createWindow(content::WebContents *new_contents, WindowOpenDisposition disposition, const gfx::Rect& initial_pos, bool user_gesture)
+QWeakPointer<WebContentsAdapter> WebContentsDelegateQt::createWindow(content::WebContents *new_contents, WindowOpenDisposition disposition, const gfx::Rect& initial_pos, bool user_gesture)
 {
-    WebContentsAdapter *newAdapter = new WebContentsAdapter(new_contents);
-    // Do the first ref-count manually to be able to know if the application is handling adoptNewWindow through the public API.
-    newAdapter->ref.ref();
+    QSharedPointer<WebContentsAdapter> newAdapter = QSharedPointer<WebContentsAdapter>::create(new_contents);
 
     m_viewClient->adoptNewWindow(newAdapter, static_cast<WebContentsAdapterClient::WindowOpenDisposition>(disposition), user_gesture, toQt(initial_pos));
 
-    if (!newAdapter->ref.deref()) {
-        // adoptNewWindow didn't increase the ref-count, newAdapter and its new_contents (if non-null) need to be discarded.
-        delete newAdapter;
-        newAdapter = 0;
-    }
-
+    // If the client didn't reference the adapter, it will be deleted now, and the weak pointer zeroed.
     return newAdapter;
 }
 
diff --git a/src/core/web_contents_delegate_qt.h b/src/core/web_contents_delegate_qt.h
index 7ead8dc7c..9aace06dd 100644
--- a/src/core/web_contents_delegate_qt.h
+++ b/src/core/web_contents_delegate_qt.h
@@ -117,7 +117,7 @@ public:
     void launchExternalURL(const QUrl &url, ui::PageTransition page_transition, bool is_main_frame);
 
 private:
-    WebContentsAdapter *createWindow(content::WebContents *new_contents, WindowOpenDisposition disposition, const gfx::Rect& initial_pos, bool user_gesture);
+    QWeakPointer<WebContentsAdapter> createWindow(content::WebContents *new_contents, WindowOpenDisposition disposition, const gfx::Rect& initial_pos, bool user_gesture);
 
     WebContentsAdapterClient *m_viewClient;
     QString m_lastSearchedString;
diff --git a/src/core/web_engine_settings.h b/src/core/web_engine_settings.h
index 3d3d734d0..3036a31a6 100644
--- a/src/core/web_engine_settings.h
+++ b/src/core/web_engine_settings.h
@@ -39,7 +39,6 @@
 
 #include "qtwebenginecoreglobal.h"
 
-#include <QExplicitlySharedDataPointer>
 #include <QScopedPointer>
 #include <QHash>
 #include <QUrl>
diff --git a/src/webengine/api/qquickwebenginenewviewrequest_p.h b/src/webengine/api/qquickwebenginenewviewrequest_p.h
index b408812ba..c08ef0aba 100644
--- a/src/webengine/api/qquickwebenginenewviewrequest_p.h
+++ b/src/webengine/api/qquickwebenginenewviewrequest_p.h
@@ -72,7 +72,7 @@ private:
     QQuickWebEngineNewViewRequest();
     QQuickWebEngineView::NewViewDestination m_destination;
     bool m_isUserInitiated;
-    QExplicitlySharedDataPointer<QtWebEngineCore::WebContentsAdapter> m_adapter;
+    QSharedPointer<QtWebEngineCore::WebContentsAdapter> m_adapter;
     QUrl m_requestedUrl;
     friend class QQuickWebEngineView;
     friend class QQuickWebEngineViewPrivate;
diff --git a/src/webengine/api/qquickwebengineview.cpp b/src/webengine/api/qquickwebengineview.cpp
index ddca1b670..95aaa39e6 100644
--- a/src/webengine/api/qquickwebengineview.cpp
+++ b/src/webengine/api/qquickwebengineview.cpp
@@ -527,7 +527,7 @@ void QQuickWebEngineViewPrivate::unhandledKeyEvent(QKeyEvent *event)
         q->window()->sendEvent(q->parentItem(), event);
 }
 
-void QQuickWebEngineViewPrivate::adoptNewWindow(WebContentsAdapter *newWebContents, WindowOpenDisposition disposition, bool userGesture, const QRect &)
+void QQuickWebEngineViewPrivate::adoptNewWindow(QSharedPointer<WebContentsAdapter> newWebContents, WindowOpenDisposition disposition, bool userGesture, const QRect &)
 {
     Q_Q(QQuickWebEngineView);
     QQuickWebEngineNewViewRequest request;
@@ -728,7 +728,7 @@ QAccessible::State QQuickWebEngineViewAccessible::state() const
 class WebContentsAdapterOwner : public QObject
 {
 public:
-    typedef QExplicitlySharedDataPointer<QtWebEngineCore::WebContentsAdapter> AdapterPtr;
+    typedef QSharedPointer<QtWebEngineCore::WebContentsAdapter> AdapterPtr;
     WebContentsAdapterOwner(const AdapterPtr &ptr)
         : adapter(ptr)
     {}
@@ -755,9 +755,9 @@ void QQuickWebEngineViewPrivate::adoptWebContents(WebContentsAdapter *webContent
 
     // This throws away the WebContentsAdapter that has been used until now.
     // All its states, particularly the loading URL, are replaced by the adopted WebContentsAdapter.
-    WebContentsAdapterOwner *adapterOwner = new WebContentsAdapterOwner(adapter);
+    WebContentsAdapterOwner *adapterOwner = new WebContentsAdapterOwner(adapter->sharedFromThis());
     adapterOwner->deleteLater();
-    adapter = webContents;
+    adapter = webContents->sharedFromThis();
     adapter->initialize(this);
 
     // associate the webChannel with the new adapter
@@ -810,7 +810,7 @@ void QQuickWebEngineViewPrivate::ensureContentsAdapter()
 {
     Q_Q(QQuickWebEngineView);
     if (!adapter) {
-        adapter = new WebContentsAdapter();
+        adapter = QSharedPointer<WebContentsAdapter>::create();
         adapter->initialize(this);
         if (m_backgroundColor != Qt::white)
             adapter->backgroundColorChanged();
@@ -970,7 +970,7 @@ void QQuickWebEngineViewPrivate::setProfile(QQuickWebEngineProfile *profile)
     if (adapter && adapter->browserContext() != browserContextAdapter()->browserContext()) {
         // When the profile changes we need to create a new WebContentAdapter and reload the active URL.
         QUrl activeUrl = adapter->activeUrl();
-        adapter = 0;
+        adapter.reset();
         ensureContentsAdapter();
 
         if (!explicitUrl.isValid() && activeUrl.isValid())
diff --git a/src/webengine/api/qquickwebengineview_p_p.h b/src/webengine/api/qquickwebengineview_p_p.h
index aab86bc3a..0fd187ab3 100644
--- a/src/webengine/api/qquickwebengineview_p_p.h
+++ b/src/webengine/api/qquickwebengineview_p_p.h
@@ -142,7 +142,7 @@ public:
     virtual void loadFinished(bool success, const QUrl &url, bool isErrorPage = false, int errorCode = 0, const QString &errorDescription = QString()) Q_DECL_OVERRIDE;
     virtual void focusContainer() Q_DECL_OVERRIDE;
     virtual void unhandledKeyEvent(QKeyEvent *event) Q_DECL_OVERRIDE;
-    virtual void adoptNewWindow(QtWebEngineCore::WebContentsAdapter *newWebContents, WindowOpenDisposition disposition, bool userGesture, const QRect &) Q_DECL_OVERRIDE;
+    virtual void adoptNewWindow(QSharedPointer<QtWebEngineCore::WebContentsAdapter> newWebContents, WindowOpenDisposition disposition, bool userGesture, const QRect &) Q_DECL_OVERRIDE;
     virtual bool isBeingAdopted() Q_DECL_OVERRIDE;
     virtual void close() Q_DECL_OVERRIDE;
     virtual void windowCloseRejected() Q_DECL_OVERRIDE;
@@ -190,7 +190,7 @@ public:
     static QQuickWebEngineScript *userScripts_at(QQmlListProperty<QQuickWebEngineScript> *p, int idx);
     static void userScripts_clear(QQmlListProperty<QQuickWebEngineScript> *p);
 
-    QExplicitlySharedDataPointer<QtWebEngineCore::WebContentsAdapter> adapter;
+    QSharedPointer<QtWebEngineCore::WebContentsAdapter> adapter;
     QScopedPointer<QQuickWebEngineViewExperimental> e;
     QScopedPointer<QQuickWebEngineViewport> v;
     QScopedPointer<QQuickWebEngineHistory> m_history;
diff --git a/src/webenginewidgets/api/qwebenginepage.cpp b/src/webenginewidgets/api/qwebenginepage.cpp
index b35bb54ec..9685d19f1 100644
--- a/src/webenginewidgets/api/qwebenginepage.cpp
+++ b/src/webenginewidgets/api/qwebenginepage.cpp
@@ -120,13 +120,13 @@ QWebEnginePage::WebAction editorActionForKeyEvent(QKeyEvent* event)
 }
 
 QWebEnginePagePrivate::QWebEnginePagePrivate(QWebEngineProfile *_profile)
-    : adapter(new WebContentsAdapter)
+    : adapter(QSharedPointer<WebContentsAdapter>::create())
     , history(new QWebEngineHistory(new QWebEngineHistoryPrivate(this)))
     , profile(_profile ? _profile : QWebEngineProfile::defaultProfile())
     , settings(new QWebEngineSettings(profile->settings()))
     , view(0)
     , isLoading(false)
-    , scriptCollection(new QWebEngineScriptCollectionPrivate(browserContextAdapter()->userScriptController(), adapter.data()))
+    , scriptCollection(new QWebEngineScriptCollectionPrivate(browserContextAdapter()->userScriptController(), adapter))
     , m_isBeingAdopted(false)
     , m_backgroundColor(Qt::white)
     , fullscreenMode(false)
@@ -266,7 +266,7 @@ void QWebEnginePagePrivate::unhandledKeyEvent(QKeyEvent *event)
         QGuiApplication::sendEvent(view->parentWidget(), event);
 }
 
-void QWebEnginePagePrivate::adoptNewWindow(WebContentsAdapter *newWebContents, WindowOpenDisposition disposition, bool userGesture, const QRect &initialGeometry)
+void QWebEnginePagePrivate::adoptNewWindow(QSharedPointer<WebContentsAdapter> newWebContents, WindowOpenDisposition disposition, bool userGesture, const QRect &initialGeometry)
 {
     Q_Q(QWebEnginePage);
     Q_UNUSED(userGesture);
@@ -451,13 +451,13 @@ void QWebEnginePagePrivate::_q_webActionTriggered(bool checked)
 
 void QWebEnginePagePrivate::recreateFromSerializedHistory(QDataStream &input)
 {
-    QExplicitlySharedDataPointer<WebContentsAdapter> newWebContents = WebContentsAdapter::createFromSerializedNavigationHistory(input, this);
+    QSharedPointer<WebContentsAdapter> newWebContents = WebContentsAdapter::createFromSerializedNavigationHistory(input, this);
     if (newWebContents) {
-        adapter = newWebContents.data();
+        adapter = std::move(newWebContents);
         adapter->initialize(this);
         if (webChannel)
             adapter->setWebChannel(webChannel);
-        scriptCollection.d->rebindToContents(adapter.data());
+        scriptCollection.d->rebindToContents(adapter);
     }
 }
 
diff --git a/src/webenginewidgets/api/qwebenginepage_p.h b/src/webenginewidgets/api/qwebenginepage_p.h
index d0023d7bb..41ba84dd0 100644
--- a/src/webenginewidgets/api/qwebenginepage_p.h
+++ b/src/webenginewidgets/api/qwebenginepage_p.h
@@ -95,7 +95,7 @@ public:
     virtual void loadFinished(bool success, const QUrl &url, bool isErrorPage = false, int errorCode = 0, const QString &errorDescription = QString()) Q_DECL_OVERRIDE;
     virtual void focusContainer() Q_DECL_OVERRIDE;
     virtual void unhandledKeyEvent(QKeyEvent *event) Q_DECL_OVERRIDE;
-    virtual void adoptNewWindow(QtWebEngineCore::WebContentsAdapter *newWebContents, WindowOpenDisposition disposition, bool userGesture, const QRect &initialGeometry) Q_DECL_OVERRIDE;
+    virtual void adoptNewWindow(QSharedPointer<QtWebEngineCore::WebContentsAdapter> newWebContents, WindowOpenDisposition disposition, bool userGesture, const QRect &initialGeometry) Q_DECL_OVERRIDE;
     virtual bool isBeingAdopted() Q_DECL_OVERRIDE;
     virtual void close() Q_DECL_OVERRIDE;
     virtual void windowCloseRejected() Q_DECL_OVERRIDE;
@@ -143,7 +143,7 @@ public:
 
     void setFullScreenMode(bool);
 
-    QExplicitlySharedDataPointer<QtWebEngineCore::WebContentsAdapter> adapter;
+    QSharedPointer<QtWebEngineCore::WebContentsAdapter> adapter;
     QWebEngineHistory *history;
     QWebEngineProfile *profile;
     QWebEngineSettings *settings;
diff --git a/src/webenginewidgets/api/qwebenginescriptcollection.cpp b/src/webenginewidgets/api/qwebenginescriptcollection.cpp
index 8d581b60a..1ba16db9d 100644
--- a/src/webenginewidgets/api/qwebenginescriptcollection.cpp
+++ b/src/webenginewidgets/api/qwebenginescriptcollection.cpp
@@ -164,7 +164,7 @@ QList<QWebEngineScript> QWebEngineScriptCollection::toList() const
 }
 
 
-QWebEngineScriptCollectionPrivate::QWebEngineScriptCollectionPrivate(QtWebEngineCore::UserScriptControllerHost *controller, QtWebEngineCore::WebContentsAdapter *webContents)
+QWebEngineScriptCollectionPrivate::QWebEngineScriptCollectionPrivate(QtWebEngineCore::UserScriptControllerHost *controller, QSharedPointer<QtWebEngineCore::WebContentsAdapter> webContents)
     : m_scriptController(controller)
     , m_contents(webContents)
 {
@@ -221,14 +221,14 @@ void QWebEngineScriptCollectionPrivate::reserve(int capacity)
     m_scriptController->reserve(m_contents.data(), capacity);
 }
 
-void QWebEngineScriptCollectionPrivate::rebindToContents(QtWebEngineCore::WebContentsAdapter *page)
+void QWebEngineScriptCollectionPrivate::rebindToContents(QSharedPointer<QtWebEngineCore::WebContentsAdapter> contents)
 {
     Q_ASSERT(m_contents);
-    Q_ASSERT(page);
-    Q_ASSERT(m_contents != page);
+    Q_ASSERT(contents);
+    Q_ASSERT(m_contents != contents);
 
     Q_FOREACH (const UserScript &script, m_scriptController->registeredScripts(m_contents.data())) {
-        m_scriptController->addUserScript(script, page);
+        m_scriptController->addUserScript(script, contents.data());
     }
-    m_contents = page;
+    m_contents = contents;
 }
diff --git a/src/webenginewidgets/api/qwebenginescriptcollection_p.h b/src/webenginewidgets/api/qwebenginescriptcollection_p.h
index 931f6c0e8..911764868 100644
--- a/src/webenginewidgets/api/qwebenginescriptcollection_p.h
+++ b/src/webenginewidgets/api/qwebenginescriptcollection_p.h
@@ -53,8 +53,8 @@
 #include "qwebenginescript.h"
 #include "web_contents_adapter.h"
 
-#include <QtCore/QExplicitlySharedDataPointer>
 #include <QtCore/QSet>
+#include <QtCore/QSharedPointer>
 
 namespace QtWebEngineCore {
 class UserScriptControllerHost;
@@ -63,14 +63,14 @@ class UserScriptControllerHost;
 QT_BEGIN_NAMESPACE
 class QWebEngineScriptCollectionPrivate {
 public:
-    QWebEngineScriptCollectionPrivate(QtWebEngineCore::UserScriptControllerHost *, QtWebEngineCore::WebContentsAdapter * = 0);
+    QWebEngineScriptCollectionPrivate(QtWebEngineCore::UserScriptControllerHost *, QSharedPointer<QtWebEngineCore::WebContentsAdapter> = QSharedPointer<QtWebEngineCore::WebContentsAdapter>());
 
     int count() const;
     bool contains(const QWebEngineScript &) const;
     QList<QWebEngineScript> toList(const QString &scriptName = QString()) const;
     QWebEngineScript find(const QString & name) const;
 
-    void rebindToContents(QtWebEngineCore::WebContentsAdapter *contents);
+    void rebindToContents(QSharedPointer<QtWebEngineCore::WebContentsAdapter> contents);
 
     void insert(const QWebEngineScript &);
     bool remove(const QWebEngineScript &);
@@ -79,7 +79,7 @@ public:
 
 private:
     QtWebEngineCore::UserScriptControllerHost *m_scriptController;
-    QExplicitlySharedDataPointer<QtWebEngineCore::WebContentsAdapter> m_contents;
+    QSharedPointer<QtWebEngineCore::WebContentsAdapter> m_contents;
 };
 
 QT_END_NAMESPACE
-- 
GitLab