From 1f17c9b3307b2ec409b3db22e8bd45fc623ae0cd Mon Sep 17 00:00:00 2001 From: Jocelyn Turcotte Date: Mon, 10 Feb 2014 12:14:11 +0100 Subject: Refactor the way callbacks are stored With the upcoming addition of a new type of callback result, this patch allows storing multiple callback types in the same QHash instead or requiring a different hash table just to please the type system. This does so by managing the ref-counted callback pointers directly instead of relying on a templated QExplicitlySharedDataPointer that requires a different type for each different callback pointer type. The ref-counting, construction and destruction is managed through a run-time type enum. Change-Id: I90ab2e1efc0c9703fc5b6ef57b38204ac8eea828 Reviewed-by: Simon Hausmann --- src/webenginewidgets/api/qwebenginepage.cpp | 100 +++++++++++++++++++++++----- src/webenginewidgets/api/qwebenginepage_p.h | 42 ++++++++++-- 2 files changed, 120 insertions(+), 22 deletions(-) diff --git a/src/webenginewidgets/api/qwebenginepage.cpp b/src/webenginewidgets/api/qwebenginepage.cpp index b082f219d..9636598e8 100644 --- a/src/webenginewidgets/api/qwebenginepage.cpp +++ b/src/webenginewidgets/api/qwebenginepage.cpp @@ -45,6 +45,82 @@ QT_BEGIN_NAMESPACE +CallbackDirectory::~CallbackDirectory() +{ + // "Cancel" pending callbacks by calling them with an invalid value. + // This guarantees that each callback is called exactly once. + Q_FOREACH (const CallbackSharedDataPointer &sharedPtr, m_callbackMap) { + switch (sharedPtr.type) { + case CallbackSharedDataPointer::Variant: + (*sharedPtr.variantCallback)(QVariant()); + break; + case CallbackSharedDataPointer::String: + (*sharedPtr.stringCallback)(QString()); + break; + default: + Q_UNREACHABLE(); + } + } +} + +void CallbackDirectory::registerCallback(quint64 requestId, const QExplicitlySharedDataPointer &callback) +{ + m_callbackMap.insert(requestId, CallbackSharedDataPointer(callback.data())); +} + +void CallbackDirectory::registerCallback(quint64 requestId, const QExplicitlySharedDataPointer &callback) +{ + m_callbackMap.insert(requestId, CallbackSharedDataPointer(callback.data())); +} + +void CallbackDirectory::invoke(quint64 requestId, const QVariant &result) +{ + CallbackSharedDataPointer sharedPtr = m_callbackMap.take(requestId); + if (sharedPtr) { + Q_ASSERT(sharedPtr.type == CallbackSharedDataPointer::Variant); + (*sharedPtr.variantCallback)(result); + } +} + +void CallbackDirectory::invoke(quint64 requestId, const QString &result) +{ + CallbackSharedDataPointer sharedPtr = m_callbackMap.take(requestId); + if (sharedPtr) { + Q_ASSERT(sharedPtr.type == CallbackSharedDataPointer::String); + (*sharedPtr.stringCallback)(result); + } +} + +void CallbackDirectory::CallbackSharedDataPointer::doRef() +{ + switch (type) { + case None: + break; + case Variant: + variantCallback->ref.ref(); + break; + case String: + stringCallback->ref.ref(); + break; + } +} + +void CallbackDirectory::CallbackSharedDataPointer::doDeref() +{ + switch (type) { + case None: + break; + case Variant: + if (!variantCallback->ref.deref()) + delete variantCallback; + break; + case String: + if (!stringCallback->ref.deref()) + delete stringCallback; + break; + } +} + QWebEnginePagePrivate::QWebEnginePagePrivate() : QObjectPrivate(QObjectPrivateVersion) , adapter(new WebContentsAdapter(SoftwareRenderingMode)) @@ -57,15 +133,6 @@ QWebEnginePagePrivate::QWebEnginePagePrivate() QWebEnginePagePrivate::~QWebEnginePagePrivate() { - // "Cancel" pending callbacks by calling them with an invalid value. - // This guarantees that each callback is called exactly once. - Q_FOREACH (QExplicitlySharedDataPointer callback, m_variantCallbacks) - (*callback)(QVariant()); - m_variantCallbacks.clear(); - Q_FOREACH (QExplicitlySharedDataPointer callback, m_stringCallbacks) - (*callback)(QString()); - m_stringCallbacks.clear(); - delete history; } @@ -165,20 +232,17 @@ void QWebEnginePagePrivate::close() void QWebEnginePagePrivate::didRunJavaScript(quint64 requestId, const QVariant& result) { - if (QExplicitlySharedDataPointer callback = m_variantCallbacks.take(requestId)) - (*callback)(result); + m_callbacks.invoke(requestId, result); } void QWebEnginePagePrivate::didFetchDocumentMarkup(quint64 requestId, const QString& result) { - if (QExplicitlySharedDataPointer callback = m_stringCallbacks.take(requestId)) - (*callback)(result); + m_callbacks.invoke(requestId, result); } void QWebEnginePagePrivate::didFetchDocumentInnerText(quint64 requestId, const QString& result) { - if (QExplicitlySharedDataPointer callback = m_stringCallbacks.take(requestId)) - (*callback)(result); + m_callbacks.invoke(requestId, result); } void QWebEnginePagePrivate::updateAction(QWebEnginePage::WebAction action) const @@ -512,14 +576,14 @@ void QWebEnginePage::toHtml(const QWebEngineCallback &resultCal { Q_D(const QWebEnginePage); quint64 requestId = d->adapter->fetchDocumentMarkup(); - d->m_stringCallbacks.insert(requestId, resultCallback.d); + d->m_callbacks.registerCallback(requestId, resultCallback.d); } void QWebEnginePage::toPlainText(const QWebEngineCallback &resultCallback) const { Q_D(const QWebEnginePage); quint64 requestId = d->adapter->fetchDocumentInnerText(); - d->m_stringCallbacks.insert(requestId, resultCallback.d); + d->m_callbacks.registerCallback(requestId, resultCallback.d); } void QWebEnginePage::setHtml(const QString &html, const QUrl &baseUrl) @@ -611,7 +675,7 @@ void QWebEnginePage::runJavaScript(const QString& scriptSource, const QWebEngine { Q_D(QWebEnginePage); quint64 requestId = d->adapter->runJavaScriptCallbackResult(scriptSource, xPath); - d->m_variantCallbacks.insert(requestId, resultCallback.d); + d->m_callbacks.registerCallback(requestId, resultCallback.d); } QWebEnginePage *QWebEnginePage::createWindow(WebWindowType type) diff --git a/src/webenginewidgets/api/qwebenginepage_p.h b/src/webenginewidgets/api/qwebenginepage_p.h index 86605c78d..9a4dc57fd 100644 --- a/src/webenginewidgets/api/qwebenginepage_p.h +++ b/src/webenginewidgets/api/qwebenginepage_p.h @@ -58,6 +58,43 @@ class QWebEngineHistory; class QWebEnginePage; class QWebEngineView; +class CallbackDirectory { +public: + typedef QtWebEnginePrivate::QWebEngineCallbackPrivateBase VariantCallback; + typedef QtWebEnginePrivate::QWebEngineCallbackPrivateBase StringCallback; + + ~CallbackDirectory(); + void registerCallback(quint64 requestId, const QExplicitlySharedDataPointer &callback); + void registerCallback(quint64 requestId, const QExplicitlySharedDataPointer &callback); + void invoke(quint64 requestId, const QVariant &result); + void invoke(quint64 requestId, const QString &result); + +private: + struct CallbackSharedDataPointer { + enum { + None, + Variant, + String + } type; + union { + VariantCallback *variantCallback; + StringCallback *stringCallback; + }; + CallbackSharedDataPointer() : type(None) { } + CallbackSharedDataPointer(VariantCallback *callback) : type(Variant), variantCallback(callback) { callback->ref.ref(); } + CallbackSharedDataPointer(StringCallback *callback) : type(String), stringCallback(callback) { callback->ref.ref(); } + CallbackSharedDataPointer(const CallbackSharedDataPointer &other) : type(other.type), variantCallback(other.variantCallback) { doRef(); } + ~CallbackSharedDataPointer() { doDeref(); } + operator bool () const { return type != None; } + + private: + void doRef(); + void doDeref(); + }; + + QHash m_callbackMap; +}; + class QWebEnginePagePrivate : public QObjectPrivate, public WebContentsAdapterClient { public: @@ -104,10 +141,7 @@ public: WebEngineContextMenuData m_menuData; QPointer m_rwhvDelegate; - typedef QtWebEnginePrivate::QWebEngineCallbackPrivateBase VariantCallback; - typedef QtWebEnginePrivate::QWebEngineCallbackPrivateBase StringCallback; - mutable QHash > m_variantCallbacks; - mutable QHash > m_stringCallbacks; + mutable CallbackDirectory m_callbacks; }; QT_END_NAMESPACE -- cgit v1.2.3