diff options
author | Jocelyn Turcotte <jocelyn.turcotte@digia.com> | 2014-01-20 18:26:37 +0100 |
---|---|---|
committer | The Qt Project <gerrit-noreply@qt-project.org> | 2014-01-22 19:49:50 +0100 |
commit | 6fb392ee9e4c185861e9b0d7010b174364f86673 (patch) | |
tree | c2a3cfa98821106f2c45ce8358481873a9a28947 | |
parent | 8a8ae0abc76315d9c4b29379040f24643ad66ebd (diff) |
Clear callbacks with an empty value on page destruction
The current implementation offers no way to cancel async requests.
This means that normal applications could easily allow callbacks
to dereference a destroyed object unless they use a smart pointer
within the callback function object.
This patch will empty the pending callback list by calling each of
them with an empty value. This will at least allow applications to
cover the cases where the page is expected to have a shorter or equal
lifetime than objects referenced in the callback.
Change-Id: Ia9fc556b03f5d83f904a0ff4b05dc9e440ea488c
Reviewed-by: Pierre Rossi <pierre.rossi@gmail.com>
-rw-r--r-- | src/webenginewidgets/api/qwebenginepage.cpp | 18 | ||||
-rw-r--r-- | tests/auto/widgets/qwebengineframe/tst_qwebengineframe.cpp | 17 | ||||
-rw-r--r-- | tests/auto/widgets/util.h | 14 |
3 files changed, 43 insertions, 6 deletions
diff --git a/src/webenginewidgets/api/qwebenginepage.cpp b/src/webenginewidgets/api/qwebenginepage.cpp index 1a3cf5dde..d05a43d0d 100644 --- a/src/webenginewidgets/api/qwebenginepage.cpp +++ b/src/webenginewidgets/api/qwebenginepage.cpp @@ -58,6 +58,15 @@ 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<VariantCallback> callback, m_variantCallbacks) + (*callback)(QVariant()); + m_variantCallbacks.clear(); + Q_FOREACH (QExplicitlySharedDataPointer<StringCallback> callback, m_stringCallbacks) + (*callback)(QString()); + m_stringCallbacks.clear(); + delete history; } @@ -148,17 +157,20 @@ void QWebEnginePagePrivate::close() void QWebEnginePagePrivate::didRunJavaScript(const QVariant& result, quint64 requestId) { - (*m_variantCallbacks.take(requestId))(result); + if (QExplicitlySharedDataPointer<VariantCallback> callback = m_variantCallbacks.take(requestId)) + (*callback)(result); } void QWebEnginePagePrivate::didFetchDocumentMarkup(const QString& result, quint64 requestId) { - (*m_stringCallbacks.take(requestId))(result); + if (QExplicitlySharedDataPointer<StringCallback> callback = m_stringCallbacks.take(requestId)) + (*callback)(result); } void QWebEnginePagePrivate::didFetchDocumentInnerText(const QString& result, quint64 requestId) { - (*m_stringCallbacks.take(requestId))(result); + if (QExplicitlySharedDataPointer<StringCallback> callback = m_stringCallbacks.take(requestId)) + (*callback)(result); } void QWebEnginePagePrivate::updateAction(QWebEnginePage::WebAction action) const diff --git a/tests/auto/widgets/qwebengineframe/tst_qwebengineframe.cpp b/tests/auto/widgets/qwebengineframe/tst_qwebengineframe.cpp index dd78a4076..99ad21152 100644 --- a/tests/auto/widgets/qwebengineframe/tst_qwebengineframe.cpp +++ b/tests/auto/widgets/qwebengineframe/tst_qwebengineframe.cpp @@ -58,6 +58,7 @@ private Q_SLOTS: void javaScriptWindowObjectCleared_data(); void javaScriptWindowObjectCleared(); void javaScriptWindowObjectClearedOnEvaluate(); + void asyncAndDelete(); void earlyToHtml(); void setHtml(); void setHtmlWithImageResource(); @@ -400,6 +401,22 @@ void tst_QWebEngineFrame::javaScriptWindowObjectClearedOnEvaluate() #endif } +void tst_QWebEngineFrame::asyncAndDelete() +{ + QWebEnginePage *page = new QWebEnginePage; + CallbackSpy<QString> plainTextSpy; + CallbackSpy<QString> htmlSpy; + page->toPlainText(ref(plainTextSpy)); + page->toHtml(ref(htmlSpy)); + + delete page; + // Pending callbacks should be called with an empty value in the page's destructor. + QCOMPARE(plainTextSpy.waitForResult(), QString()); + QVERIFY(plainTextSpy.wasCalled()); + QCOMPARE(htmlSpy.waitForResult(), QString()); + QVERIFY(htmlSpy.wasCalled()); +} + void tst_QWebEngineFrame::earlyToHtml() { QString html("<html><head></head><body></body></html>"); diff --git a/tests/auto/widgets/util.h b/tests/auto/widgets/util.h index e5e991f89..a9a3d487c 100644 --- a/tests/auto/widgets/util.h +++ b/tests/auto/widgets/util.h @@ -93,24 +93,32 @@ public: // Tells tr1::ref the result of void operator()(const T &result) when decltype isn't available. typedef void result_type; - CallbackSpy() { + CallbackSpy() : called(false) { timeoutTimer.setSingleShot(true); QObject::connect(&timeoutTimer, SIGNAL(timeout()), &eventLoop, SLOT(quit())); } T waitForResult() { - timeoutTimer.start(10000); - eventLoop.exec(); + if (!called) { + timeoutTimer.start(10000); + eventLoop.exec(); + } return result; } + bool wasCalled() const { + return called; + } + void operator()(const T &result) { this->result = result; + called = true; eventLoop.quit(); } private: Q_DISABLE_COPY(CallbackSpy) + bool called; QTimer timeoutTimer; QEventLoop eventLoop; T result; |