Merge runJavaScript functions in QWEPage, QQWEView

These two are identical except for the use of QJSValue to
wrap a JS callback. And we would like to have a function on
WebContentsAdapterClient for QWEFrame to call. So we wrap
the JS callback in a std::function<> callback and merge the
APIs.

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

7 files changed, 81 insertions, 81 deletions

diff --git a/src/core/api/qwebenginepage.cpp b/src/core/api/qwebenginepage.cpp
index c2a737d72..146b1804e 100644
--- a/src/core/api/qwebenginepage.cpp
+++ b/src/core/api/qwebenginepage.cpp
@@ -489,10 +489,16 @@ void QWebEnginePagePrivate::windowCloseRejected()
     // Do nothing for now.
 }
 
-void QWebEnginePagePrivate::didRunJavaScript(quint64 requestId, const QVariant& result)
+void QWebEnginePagePrivate::runJavaScript(const QString &script, quint32 worldId,
+                                          const std::function<void(const QVariant &)> &callback)
 {
-    if (auto callback = m_variantCallbacks.take(requestId))
-        callback(result);
+    ensureInitialized();
+    if (adapter->lifecycleState() == WebContentsAdapter::LifecycleState::Discarded) {
+        qWarning("runJavaScript: disabled in Discarded state");
+        if (callback)
+            callback(QVariant());
+    } else
+        adapter->runJavaScript(script, worldId, callback);
 }
 
 void QWebEnginePagePrivate::didFetchDocumentMarkup(quint64 requestId, const QString& result)
@@ -971,11 +977,9 @@ QWebEnginePage::~QWebEnginePage()
         setDevToolsPage(nullptr);
         emit _q_aboutToDelete();
 
-        for (auto varFun : std::as_const(d_ptr->m_variantCallbacks))
-            varFun(QVariant());
+        d_ptr->adapter->clearJavaScriptCallbacks();
         for (auto strFun : std::as_const(d_ptr->m_stringCallbacks))
             strFun(QString());
-        d_ptr->m_variantCallbacks.clear();
         d_ptr->m_stringCallbacks.clear();
     }
 }
@@ -2029,40 +2033,13 @@ void QWebEnginePage::setZoomFactor(qreal factor)
 
 void QWebEnginePage::runJavaScript(const QString& scriptSource, const std::function<void(const QVariant &)> &resultCallback)
 {
-    Q_D(QWebEnginePage);
-    d->ensureInitialized();
-    if (d->adapter->lifecycleState() == WebContentsAdapter::LifecycleState::Discarded) {
-        qWarning("runJavaScript: disabled in Discarded state");
-        if (resultCallback)
-            resultCallback(QVariant());
-        return;
-    }
-    quint64 requestId = d->adapter->runJavaScriptCallbackResult(scriptSource, QWebEngineScript::MainWorld);
-    if (requestId)
-        d->m_variantCallbacks.insert(requestId, resultCallback);
-    else if (resultCallback)
-        resultCallback(QVariant());
+    runJavaScript(scriptSource, QWebEngineScript::MainWorld, resultCallback);
 }
 
 void QWebEnginePage::runJavaScript(const QString& scriptSource, quint32 worldId, const std::function<void(const QVariant &)> &resultCallback)
 {
     Q_D(QWebEnginePage);
-    d->ensureInitialized();
-    if (d->adapter->lifecycleState() == WebContentsAdapter::LifecycleState::Discarded) {
-        qWarning("runJavaScript: disabled in Discarded state");
-        if (resultCallback)
-            resultCallback(QVariant());
-        return;
-    }
-    if (resultCallback) {
-        quint64 requestId = d->adapter->runJavaScriptCallbackResult(scriptSource, worldId);
-        if (requestId)
-            d->m_variantCallbacks.insert(requestId, resultCallback);
-        else
-            resultCallback(QVariant());
-    } else {
-        d->adapter->runJavaScript(scriptSource, worldId);
-    }
+    d->runJavaScript(scriptSource, worldId, resultCallback);
 }
 
 /*!
diff --git a/src/core/api/qwebenginepage_p.h b/src/core/api/qwebenginepage_p.h
index 31ace7e9d..42aba54d4 100644
--- a/src/core/api/qwebenginepage_p.h
+++ b/src/core/api/qwebenginepage_p.h
@@ -130,7 +130,8 @@ public:
     void javascriptDialog(QSharedPointer<QtWebEngineCore::JavaScriptDialogController>) override;
     void runFileChooser(QSharedPointer<QtWebEngineCore::FilePickerController>) override;
     void showColorDialog(QSharedPointer<QtWebEngineCore::ColorChooserController>) override;
-    void didRunJavaScript(quint64 requestId, const QVariant &result) override;
+    void runJavaScript(const QString &script, quint32 worldId,
+                       const std::function<void(const QVariant &)> &callback) override;
     void didFetchDocumentMarkup(quint64 requestId, const QString &result) override;
     void didFetchDocumentInnerText(quint64 requestId, const QString &result) override;
     void didPrintPage(quint64 requestId, QSharedPointer<QByteArray> result) override;
@@ -214,7 +215,6 @@ public:
     QPrinter *currentPrinter = nullptr;
 #endif
 
-    mutable QMap<quint64, std::function<void(const QVariant &)>> m_variantCallbacks;
     mutable QMap<quint64, std::function<void(const QString &)>> m_stringCallbacks;
     QMap<quint64, std::function<void(const QByteArray &)>> m_pdfResultCallbacks;
     mutable QAction *actions[QWebEnginePage::WebActionCount];
diff --git a/src/core/web_contents_adapter.cpp b/src/core/web_contents_adapter.cpp
index 6decf8780..1140bf11a 100644
--- a/src/core/web_contents_adapter.cpp
+++ b/src/core/web_contents_adapter.cpp
@@ -174,10 +174,9 @@ static QVariant fromJSValue(const base::Value *result)
     return ret;
 }
 
-static void callbackOnEvaluateJS(WebContentsAdapterClient *adapterClient, quint64 requestId, base::Value result)
+static void callbackOnEvaluateJS(WebContentsAdapter *adapter, quint64 requestId, base::Value result)
 {
-    if (requestId)
-        adapterClient->didRunJavaScript(requestId, fromJSValue(&result));
+    adapter->didRunJavaScript(requestId, result);
 }
 
 #if QT_CONFIG(webengine_printing_and_pdf)
@@ -1039,36 +1038,50 @@ QAccessibleInterface *WebContentsAdapter::browserAccessible()
 }
 #endif // QT_CONFIG(accessibility)
 
-void WebContentsAdapter::runJavaScript(const QString &javaScript, quint32 worldId)
+void WebContentsAdapter::runJavaScript(const QString &javaScript, quint32 worldId,
+                                       const std::function<void(const QVariant &)> &callback)
 {
-    CHECK_INITIALIZED();
+    auto exit = [&] {
+        if (callback)
+            callback(QVariant());
+    };
+
+    if (!isInitialized())
+        return exit();
     content::RenderFrameHost *rfh =  m_webContents->GetPrimaryMainFrame();
     Q_ASSERT(rfh);
     if (!static_cast<content::RenderFrameHostImpl*>(rfh)->GetAssociatedLocalFrame()) {
         qWarning() << "Local frame is gone, not running script";
-        return;
+        return exit();
+    }
+
+    content::RenderFrameHost::JavaScriptResultCallback wrappedCallback = base::NullCallback();
+    if (callback) {
+        wrappedCallback = base::BindOnce(&callbackOnEvaluateJS, this, m_nextRequestId);
+        m_javaScriptCallbacks.insert(m_nextRequestId, callback);
+        ++m_nextRequestId;
     }
     if (worldId == 0)
-        rfh->ExecuteJavaScript(toString16(javaScript), base::NullCallback());
+        rfh->ExecuteJavaScript(toString16(javaScript), std::move(wrappedCallback));
     else
-        rfh->ExecuteJavaScriptInIsolatedWorld(toString16(javaScript), base::NullCallback(), worldId);
+        rfh->ExecuteJavaScriptInIsolatedWorld(toString16(javaScript), std::move(wrappedCallback),
+                                              worldId);
 }
 
-quint64 WebContentsAdapter::runJavaScriptCallbackResult(const QString &javaScript, quint32 worldId)
+void WebContentsAdapter::didRunJavaScript(quint64 requestId, const base::Value &result)
 {
-    CHECK_INITIALIZED(0);
-    content::RenderFrameHost *rfh =  m_webContents->GetPrimaryMainFrame();
-    Q_ASSERT(rfh);
-    if (!static_cast<content::RenderFrameHostImpl*>(rfh)->GetAssociatedLocalFrame()) {
-        qWarning() << "Local frame is gone, not running script";
-        return 0;
-    }
-    content::RenderFrameHost::JavaScriptResultCallback callback = base::BindOnce(&callbackOnEvaluateJS, m_adapterClient, m_nextRequestId);
-    if (worldId == 0)
-        rfh->ExecuteJavaScript(toString16(javaScript), std::move(callback));
-    else
-        rfh->ExecuteJavaScriptInIsolatedWorld(toString16(javaScript), std::move(callback), worldId);
-    return m_nextRequestId++;
+    Q_ASSERT(requestId);
+    auto callback = m_javaScriptCallbacks.take(requestId);
+    Q_ASSERT(callback);
+    callback(fromJSValue(&result));
+}
+
+// Called when QWebEnginePage is deleted
+void WebContentsAdapter::clearJavaScriptCallbacks()
+{
+    for (auto varFun : std::as_const(m_javaScriptCallbacks))
+        varFun(QVariant());
+    m_javaScriptCallbacks.clear();
 }
 
 quint64 WebContentsAdapter::fetchDocumentMarkup()
diff --git a/src/core/web_contents_adapter.h b/src/core/web_contents_adapter.h
index a5cad8664..0c46e7d81 100644
--- a/src/core/web_contents_adapter.h
+++ b/src/core/web_contents_adapter.h
@@ -16,8 +16,10 @@
 #define WEB_CONTENTS_ADAPTER_H
 
 #include <QtCore/QSharedPointer>
+#include <QtCore/QMap>
 #include <QtCore/QString>
 #include <QtCore/QUrl>
+#include <QtCore/QVariant>
 #include <QtCore/QPointer>
 #include <QtGui/qtgui-config.h>
 #include <QtWebEngineCore/private/qtwebenginecoreglobal_p.h>
@@ -27,6 +29,7 @@
 
 #include "web_contents_adapter_client.h"
 
+#include <functional>
 #include <memory>
 #include <optional>
 
@@ -36,6 +39,10 @@ struct WebPreferences;
 }
 }
 
+namespace base {
+class Value;
+}
+
 namespace content {
 class WebContents;
 class SiteInstance;
@@ -129,8 +136,10 @@ public:
     void serializeNavigationHistory(QDataStream &output);
     void setZoomFactor(qreal);
     qreal currentZoomFactor() const;
-    void runJavaScript(const QString &javaScript, quint32 worldId);
-    quint64 runJavaScriptCallbackResult(const QString &javaScript, quint32 worldId);
+    void runJavaScript(const QString &javaScript, quint32 worldId,
+                       const std::function<void(const QVariant &)> &callback);
+    void didRunJavaScript(quint64 requestId, const base::Value &result);
+    void clearJavaScriptCallbacks();
     quint64 fetchDocumentMarkup();
     quint64 fetchDocumentInnerText();
     void updateWebPreferences(const blink::web_pref::WebPreferences &webPreferences);
@@ -255,6 +264,7 @@ private:
     WebContentsAdapterClient *m_adapterClient;
     quint64 m_nextRequestId;
     QMap<QUrl, bool> m_pendingMouseLockPermissions;
+    QMap<quint64, std::function<void(const QVariant &)>> m_javaScriptCallbacks;
     std::unique_ptr<content::DropData> m_currentDropData;
     uint m_currentDropAction;
     bool m_updateDragActionCalled;
diff --git a/src/core/web_contents_adapter_client.h b/src/core/web_contents_adapter_client.h
index a1ad301ed..bb350bba2 100644
--- a/src/core/web_contents_adapter_client.h
+++ b/src/core/web_contents_adapter_client.h
@@ -175,7 +175,8 @@ public:
     virtual void javascriptDialog(QSharedPointer<JavaScriptDialogController>) = 0;
     virtual void runFileChooser(QSharedPointer<FilePickerController>) = 0;
     virtual void showColorDialog(QSharedPointer<ColorChooserController>) = 0;
-    virtual void didRunJavaScript(quint64 requestId, const QVariant& result) = 0;
+    virtual void runJavaScript(const QString &script, quint32 worldId,
+                               const std::function<void(const QVariant &)> &callback) = 0;
     virtual void didFetchDocumentMarkup(quint64 requestId, const QString& result) = 0;
     virtual void didFetchDocumentInnerText(quint64 requestId, const QString& result) = 0;
     virtual void didPrintPage(quint64 requestId, QSharedPointer<QByteArray>) = 0;
diff --git a/src/webenginequick/api/qquickwebengineview.cpp b/src/webenginequick/api/qquickwebengineview.cpp
index 11ed33b5e..85c5e1a73 100644
--- a/src/webenginequick/api/qquickwebengineview.cpp
+++ b/src/webenginequick/api/qquickwebengineview.cpp
@@ -1296,19 +1296,18 @@ bool QQuickWebEngineView::activeFocusOnPress() const
     return d->m_activeFocusOnPress;
 }
 
-void QQuickWebEngineViewPrivate::didRunJavaScript(quint64 requestId, const QVariant &result)
+void QQuickWebEngineViewPrivate::runJavaScript(
+        const QString &script, quint32 worldId,
+        const std::function<void(const QVariant &)> &callback)
 {
-    Q_Q(QQuickWebEngineView);
-    QJSValue callback = m_callbacks.take(requestId);
-    QJSValueList args;
-    args.append(qmlEngine(q)->toScriptValue(result));
-    callback.call(args);
+    ensureContentsAdapter();
+    adapter->runJavaScript(script, worldId, callback);
 }
 
 void QQuickWebEngineViewPrivate::didPrintPage(quint64 requestId, QSharedPointer<QByteArray> result)
 {
     Q_Q(QQuickWebEngineView);
-    QJSValue callback = m_callbacks.take(requestId);
+    QJSValue callback = m_printCallbacks.take(requestId);
     QJSValueList args;
     args.append(qmlEngine(q)->toScriptValue(*(result.data())));
     callback.call(args);
@@ -1487,16 +1486,15 @@ void QQuickWebEngineView::runJavaScript(const QString &script, const QJSValue &c
 void QQuickWebEngineView::runJavaScript(const QString &script, quint32 worldId, const QJSValue &callback)
 {
     Q_D(QQuickWebEngineView);
-    d->ensureContentsAdapter();
+    std::function<void(const QVariant &)> wrappedCallback;
     if (!callback.isUndefined()) {
-        quint64 requestId = d_ptr->adapter->runJavaScriptCallbackResult(script, worldId);
-        if (requestId) {
-            d->m_callbacks.insert(requestId, callback);
-        } else {
-            callback.call();
-        }
-    } else
-        d->adapter->runJavaScript(script, worldId);
+        wrappedCallback = [this, callback](const QVariant &result) {
+            QJSValueList args;
+            args.append(qmlEngine(this)->toScriptValue(result));
+            callback.call(args);
+        };
+    }
+    d->runJavaScript(script, worldId, wrappedCallback);
 }
 
 qreal QQuickWebEngineView::zoomFactor() const
@@ -1593,7 +1591,7 @@ void QQuickWebEngineView::printToPdf(const QJSValue &callback, PrintedPageSizeId
 
     d->ensureContentsAdapter();
     quint64 requestId = d->adapter->printToPDFCallbackResult(pageLayout, ranges);
-    d->m_callbacks.insert(requestId, callback);
+    d->m_printCallbacks.insert(requestId, callback);
 #else
     Q_UNUSED(pageSizeId);
     Q_UNUSED(orientation);
diff --git a/src/webenginequick/api/qquickwebengineview_p_p.h b/src/webenginequick/api/qquickwebengineview_p_p.h
index cf4da7c40..eed8342eb 100644
--- a/src/webenginequick/api/qquickwebengineview_p_p.h
+++ b/src/webenginequick/api/qquickwebengineview_p_p.h
@@ -92,7 +92,8 @@ public:
     void runFileChooser(QSharedPointer<QtWebEngineCore::FilePickerController>) override;
     void desktopMediaRequested(QtWebEngineCore::DesktopMediaController *) override;
     void showColorDialog(QSharedPointer<QtWebEngineCore::ColorChooserController>) override;
-    void didRunJavaScript(quint64, const QVariant&) override;
+    void runJavaScript(const QString &script, quint32 worldId,
+                       const std::function<void(const QVariant &)> &callback) override;
     void didFetchDocumentMarkup(quint64, const QString&) override { }
     void didFetchDocumentInnerText(quint64, const QString&) override { }
     void didPrintPage(quint64 requestId, QSharedPointer<QByteArray>) override;
@@ -157,7 +158,7 @@ public:
     bool m_fullscreenMode;
     bool isLoading;
     bool m_activeFocusOnPress;
-    QMap<quint64, QJSValue> m_callbacks;
+    QMap<quint64, QJSValue> m_printCallbacks;
     QQmlWebChannel *m_webChannel;
     QPointer<QQuickWebEngineView> inspectedView;
     QPointer<QQuickWebEngineView> devToolsView;