From eeb18149fa6d796dd5202f080b828099a9187046 Mon Sep 17 00:00:00 2001 From: Joerg Bornemann Date: Tue, 26 Jan 2016 17:35:06 +0100 Subject: Fix occasional "WeakPtrs must be checked on the same sequenced thread." Revert 7c7ee9a9, and fix the issue by passing a raw this pointer to startAsync. Bind will take care of calling AddRef/Release for us. Otherwise the WeakPtr would get assigned to the UI thread and must be invalidated in the same. Change-Id: Iee741dde521cf085a086e397a8154fa1384d58d1 Task-number: QTBUG-49670 Reviewed-by: Allan Sandfeld Jensen --- src/core/url_request_custom_job.cpp | 17 ++++++++--------- src/core/url_request_custom_job.h | 3 +-- 2 files changed, 9 insertions(+), 11 deletions(-) (limited to 'src') diff --git a/src/core/url_request_custom_job.cpp b/src/core/url_request_custom_job.cpp index 907f71c2e..84c898c3a 100644 --- a/src/core/url_request_custom_job.cpp +++ b/src/core/url_request_custom_job.cpp @@ -60,8 +60,7 @@ URLRequestCustomJob::URLRequestCustomJob(URLRequest *request, NetworkDelegate *n , m_schemeHandler(schemeHandler) , m_error(0) , m_started(false) - , m_weakFactoryIO(this) - , m_weakFactoryUI(this) + , m_weakFactory(this) { } @@ -79,7 +78,8 @@ URLRequestCustomJob::~URLRequestCustomJob() void URLRequestCustomJob::Start() { DCHECK_CURRENTLY_ON(content::BrowserThread::IO); - content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE, base::Bind(&URLRequestCustomJob::startAsync, m_weakFactoryIO.GetWeakPtr())); + content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE, + base::Bind(&URLRequestCustomJob::startAsync, this)); } void URLRequestCustomJob::Kill() @@ -94,8 +94,7 @@ void URLRequestCustomJob::Kill() if (m_device && m_device->isOpen()) m_device->close(); m_device = 0; - m_weakFactoryIO.InvalidateWeakPtrs(); - m_weakFactoryUI.InvalidateWeakPtrs(); + m_weakFactory.InvalidateWeakPtrs(); URLRequestJob::Kill(); } @@ -153,7 +152,7 @@ void URLRequestCustomJob::setReplyDevice(QIODevice *device) m_device->open(QIODevice::ReadOnly); if (m_device && m_device->isReadable()) - content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE, base::Bind(&URLRequestCustomJob::notifyStarted, m_weakFactoryUI.GetWeakPtr())); + content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE, base::Bind(&URLRequestCustomJob::notifyStarted, m_weakFactory.GetWeakPtr())); else fail(ERR_INVALID_URL); } @@ -181,7 +180,7 @@ void URLRequestCustomJob::redirect(const GURL &url) QMutexLocker lock(&m_mutex); m_redirect = url; - content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE, base::Bind(&URLRequestCustomJob::notifyStarted, m_weakFactoryUI.GetWeakPtr())); + content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE, base::Bind(&URLRequestCustomJob::notifyStarted, m_weakFactory.GetWeakPtr())); } void URLRequestCustomJob::abort() @@ -191,7 +190,7 @@ void URLRequestCustomJob::abort() if (m_device && m_device->isOpen()) m_device->close(); m_device = 0; - content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE, base::Bind(&URLRequestCustomJob::notifyCanceled, m_weakFactoryUI.GetWeakPtr())); + content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE, base::Bind(&URLRequestCustomJob::notifyCanceled, m_weakFactory.GetWeakPtr())); } void URLRequestCustomJob::notifyCanceled() @@ -216,7 +215,7 @@ void URLRequestCustomJob::fail(int error) DCHECK_CURRENTLY_ON(content::BrowserThread::UI); QMutexLocker lock(&m_mutex); m_error = error; - content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE, base::Bind(&URLRequestCustomJob::notifyFailure, m_weakFactoryUI.GetWeakPtr())); + content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE, base::Bind(&URLRequestCustomJob::notifyFailure, m_weakFactory.GetWeakPtr())); } void URLRequestCustomJob::notifyFailure() diff --git a/src/core/url_request_custom_job.h b/src/core/url_request_custom_job.h index be5cae43c..a994c467a 100644 --- a/src/core/url_request_custom_job.h +++ b/src/core/url_request_custom_job.h @@ -87,8 +87,7 @@ private: int m_error; GURL m_redirect; bool m_started; - base::WeakPtrFactory m_weakFactoryIO; - base::WeakPtrFactory m_weakFactoryUI; + base::WeakPtrFactory m_weakFactory; friend class URLRequestCustomJobDelegate; -- cgit v1.2.3 From fd7f2367515bd243a171ae1234b3443d6f24955d Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Wed, 27 Jan 2016 11:21:53 +0100 Subject: Fix multi-thread protection in custom url handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The classes were not properly protected against race conditions. To solve this there is now a class shared between the two thread that is not deleted until the classes on both threads have been deleted. Change-Id: I499bd98805ae7a195aca42f30610eb6c2b0fd0f7 Reviewed-by: Michael Brüning Reviewed-by: Joerg Bornemann --- src/core/api/qwebengineurlrequestjob.h | 4 +- src/core/url_request_custom_job.cpp | 231 +++++++++++++++++++-------- src/core/url_request_custom_job.h | 40 +++-- src/core/url_request_custom_job_delegate.cpp | 19 +-- src/core/url_request_custom_job_delegate.h | 8 +- 5 files changed, 206 insertions(+), 96 deletions(-) (limited to 'src') diff --git a/src/core/api/qwebengineurlrequestjob.h b/src/core/api/qwebengineurlrequestjob.h index 922299fd9..84741b791 100644 --- a/src/core/api/qwebengineurlrequestjob.h +++ b/src/core/api/qwebengineurlrequestjob.h @@ -44,8 +44,8 @@ #include namespace QtWebEngineCore { -class URLRequestCustomJob; class URLRequestCustomJobDelegate; +class URLRequestCustomJobShared; } // namespace QT_BEGIN_NAMESPACE @@ -76,7 +76,7 @@ public: private: QWebEngineUrlRequestJob(QtWebEngineCore::URLRequestCustomJobDelegate *); - friend class QtWebEngineCore::URLRequestCustomJob; + friend class QtWebEngineCore::URLRequestCustomJobShared; QtWebEngineCore::URLRequestCustomJobDelegate* d_ptr; }; diff --git a/src/core/url_request_custom_job.cpp b/src/core/url_request_custom_job.cpp index 84c898c3a..07087c247 100644 --- a/src/core/url_request_custom_job.cpp +++ b/src/core/url_request_custom_job.cpp @@ -56,45 +56,33 @@ namespace QtWebEngineCore { URLRequestCustomJob::URLRequestCustomJob(URLRequest *request, NetworkDelegate *networkDelegate, QWebEngineUrlSchemeHandler *schemeHandler) : URLRequestJob(request, networkDelegate) - , m_device(0) , m_schemeHandler(schemeHandler) - , m_error(0) - , m_started(false) - , m_weakFactory(this) + , m_shared(new URLRequestCustomJobShared(this)) { } URLRequestCustomJob::~URLRequestCustomJob() { - QMutexLocker lock(&m_mutex); - if (m_delegate) { - m_delegate->m_job = 0; - m_delegate->deleteLater(); - } - if (m_device && m_device->isOpen()) - m_device->close(); + if (m_shared) + m_shared->killJob(); +} + +static void startAsync(URLRequestCustomJobShared *shared) +{ + shared->startAsync(); } void URLRequestCustomJob::Start() { DCHECK_CURRENTLY_ON(content::BrowserThread::IO); - content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE, - base::Bind(&URLRequestCustomJob::startAsync, this)); + content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE, base::Bind(&startAsync, m_shared)); } void URLRequestCustomJob::Kill() { - DCHECK_CURRENTLY_ON(content::BrowserThread::IO); - QMutexLocker lock(&m_mutex); - if (m_delegate) { - m_delegate->m_job = 0; - m_delegate->deleteLater(); - } - m_delegate = 0; - if (m_device && m_device->isOpen()) - m_device->close(); - m_device = 0; - m_weakFactory.InvalidateWeakPtrs(); + if (m_shared) + m_shared->killJob(); + m_shared = 0; URLRequestJob::Kill(); } @@ -102,8 +90,11 @@ void URLRequestCustomJob::Kill() bool URLRequestCustomJob::GetMimeType(std::string *mimeType) const { DCHECK_CURRENTLY_ON(content::BrowserThread::IO); - if (m_mimeType.size() > 0) { - *mimeType = m_mimeType; + if (!m_shared) + return false; + QMutexLocker lock(&m_shared->m_mutex); + if (m_shared->m_mimeType.size() > 0) { + *mimeType = m_shared->m_mimeType; return true; } return false; @@ -112,8 +103,11 @@ bool URLRequestCustomJob::GetMimeType(std::string *mimeType) const bool URLRequestCustomJob::GetCharset(std::string* charset) { DCHECK_CURRENTLY_ON(content::BrowserThread::IO); - if (m_charset.size() > 0) { - *charset = m_charset; + if (!m_shared) + return false; + QMutexLocker lock(&m_shared->m_mutex); + if (m_shared->m_charset.size() > 0) { + *charset = m_shared->m_charset; return true; } return false; @@ -122,128 +116,223 @@ bool URLRequestCustomJob::GetCharset(std::string* charset) bool URLRequestCustomJob::IsRedirectResponse(GURL* location, int* http_status_code) { DCHECK_CURRENTLY_ON(content::BrowserThread::IO); - QMutexLocker lock(&m_mutex); - if (m_redirect.is_valid()) { - *location = m_redirect; + if (!m_shared) + return false; + QMutexLocker lock(&m_shared->m_mutex); + if (m_shared->m_redirect.is_valid()) { + *location = m_shared->m_redirect; *http_status_code = 303; return true; } return false; } -void URLRequestCustomJob::setReplyMimeType(const std::string &mimeType) +bool URLRequestCustomJob::ReadRawData(IOBuffer *buf, int bufSize, int *bytesRead) +{ + DCHECK_CURRENTLY_ON(content::BrowserThread::IO); + Q_ASSERT(bytesRead); + Q_ASSERT(m_shared); + QMutexLocker lock(&m_shared->m_mutex); + qint64 rv = m_shared->m_device ? m_shared->m_device->read(buf->data(), bufSize) : -1; + if (rv >= 0) { + *bytesRead = static_cast(rv); + return true; + } else { + NotifyDone(URLRequestStatus(URLRequestStatus::FAILED, ERR_FAILED)); + } + return false; +} + +URLRequestCustomJobShared::URLRequestCustomJobShared(URLRequestCustomJob *job) + : m_mutex(QMutex::Recursive) + , m_job(job) + , m_delegate(0) + , m_error(0) + , m_started(false) + , m_asyncInitialized(false) + , m_weakFactory(this) +{ +} + +URLRequestCustomJobShared::~URLRequestCustomJobShared() +{ + Q_ASSERT(!m_job); + Q_ASSERT(!m_delegate); +} + +void URLRequestCustomJobShared::killJob() +{ + DCHECK_CURRENTLY_ON(content::BrowserThread::IO); + QMutexLocker lock(&m_mutex); + m_job = 0; + bool doDelete = false; + if (m_delegate) { + m_delegate->deleteLater(); + } else { + // Do not delete yet if startAsync has not yet run. + doDelete = m_asyncInitialized; + } + if (m_device && m_device->isOpen()) + m_device->close(); + m_device = 0; + m_weakFactory.InvalidateWeakPtrs(); + lock.unlock(); + if (doDelete) + delete this; +} + +void URLRequestCustomJobShared::unsetJobDelegate() +{ + DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + QMutexLocker lock(&m_mutex); + m_delegate = 0; + bool doDelete = false; + if (m_job) + abort(); + else + doDelete = true; + lock.unlock(); + if (doDelete) + delete this; +} + +void URLRequestCustomJobShared::setReplyMimeType(const std::string &mimeType) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + QMutexLocker lock(&m_mutex); m_mimeType = mimeType; } -void URLRequestCustomJob::setReplyCharset(const std::string &charset) +void URLRequestCustomJobShared::setReplyCharset(const std::string &charset) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + QMutexLocker lock(&m_mutex); m_charset = charset; } -void URLRequestCustomJob::setReplyDevice(QIODevice *device) +void URLRequestCustomJobShared::setReplyDevice(QIODevice *device) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); QMutexLocker lock(&m_mutex); + if (!m_job) + return; m_device = device; if (m_device && !m_device->isReadable()) m_device->open(QIODevice::ReadOnly); if (m_device && m_device->isReadable()) - content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE, base::Bind(&URLRequestCustomJob::notifyStarted, m_weakFactory.GetWeakPtr())); + content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE, base::Bind(&URLRequestCustomJobShared::notifyStarted, m_weakFactory.GetWeakPtr())); else fail(ERR_INVALID_URL); } -bool URLRequestCustomJob::ReadRawData(IOBuffer *buf, int bufSize, int *bytesRead) -{ - DCHECK_CURRENTLY_ON(content::BrowserThread::IO); - Q_ASSERT(bytesRead); - QMutexLocker lock(&m_mutex); - qint64 rv = m_device ? m_device->read(buf->data(), bufSize) : -1; - if (rv >= 0) { - *bytesRead = static_cast(rv); - return true; - } else { - NotifyDone(URLRequestStatus(URLRequestStatus::FAILED, ERR_FAILED)); - } - return false; -} - -void URLRequestCustomJob::redirect(const GURL &url) +void URLRequestCustomJobShared::redirect(const GURL &url) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); - if (m_device || m_error) - return; QMutexLocker lock(&m_mutex); + if (m_device || m_error) + return; + if (!m_job) + return; m_redirect = url; - content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE, base::Bind(&URLRequestCustomJob::notifyStarted, m_weakFactory.GetWeakPtr())); + content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE, base::Bind(&URLRequestCustomJobShared::notifyStarted, m_weakFactory.GetWeakPtr())); } -void URLRequestCustomJob::abort() +void URLRequestCustomJobShared::abort() { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); QMutexLocker lock(&m_mutex); if (m_device && m_device->isOpen()) m_device->close(); m_device = 0; - content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE, base::Bind(&URLRequestCustomJob::notifyCanceled, m_weakFactory.GetWeakPtr())); + if (!m_job) + return; + content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE, base::Bind(&URLRequestCustomJobShared::notifyCanceled, m_weakFactory.GetWeakPtr())); } -void URLRequestCustomJob::notifyCanceled() +void URLRequestCustomJobShared::notifyCanceled() { DCHECK_CURRENTLY_ON(content::BrowserThread::IO); + QMutexLocker lock(&m_mutex); + if (!m_job) + return; if (m_started) - NotifyDone(URLRequestStatus(URLRequestStatus::CANCELED, ERR_ABORTED)); + m_job->NotifyDone(URLRequestStatus(URLRequestStatus::CANCELED, ERR_ABORTED)); else - NotifyStartError(URLRequestStatus(URLRequestStatus::CANCELED, ERR_ABORTED)); + m_job->NotifyStartError(URLRequestStatus(URLRequestStatus::CANCELED, ERR_ABORTED)); } -void URLRequestCustomJob::notifyStarted() +void URLRequestCustomJobShared::notifyStarted() { DCHECK_CURRENTLY_ON(content::BrowserThread::IO); + QMutexLocker lock(&m_mutex); + if (!m_job) + return; Q_ASSERT(!m_started); m_started = true; - NotifyHeadersComplete(); + m_job->NotifyHeadersComplete(); } -void URLRequestCustomJob::fail(int error) +void URLRequestCustomJobShared::fail(int error) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); QMutexLocker lock(&m_mutex); m_error = error; - content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE, base::Bind(&URLRequestCustomJob::notifyFailure, m_weakFactory.GetWeakPtr())); + if (!m_job) + return; + content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE, base::Bind(&URLRequestCustomJobShared::notifyFailure, m_weakFactory.GetWeakPtr())); } -void URLRequestCustomJob::notifyFailure() +void URLRequestCustomJobShared::notifyFailure() { DCHECK_CURRENTLY_ON(content::BrowserThread::IO); - m_mutex.lock(); + QMutexLocker lock(&m_mutex); + if (!m_job) + return; if (m_device) m_device->close(); const URLRequestStatus status(URLRequestStatus::FAILED, m_error); const bool started = m_started; - m_mutex.unlock(); if (started) - NotifyDone(status); + m_job->NotifyDone(status); else - NotifyStartError(status); + m_job->NotifyStartError(status); } -void URLRequestCustomJob::startAsync() +GURL URLRequestCustomJobShared::requestUrl() +{ + QMutexLocker lock(&m_mutex); + if (!m_job) + return GURL(); + return m_job->request()->url(); +} + +std::string URLRequestCustomJobShared::requestMethod() +{ + QMutexLocker lock(&m_mutex); + if (!m_job) + return std::string(); + return m_job->request()->method(); +} + +void URLRequestCustomJobShared::startAsync() { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); Q_ASSERT(!m_started); Q_ASSERT(!m_delegate); QMutexLocker lock(&m_mutex); + if (!m_job) { + lock.unlock(); + delete this; + return; + } m_delegate = new URLRequestCustomJobDelegate(this); - lock.unlock(); + m_asyncInitialized = true; QWebEngineUrlRequestJob *requestJob = new QWebEngineUrlRequestJob(m_delegate); - m_schemeHandler->requestStarted(requestJob); + if (m_job) + m_job->m_schemeHandler->requestStarted(requestJob); } } // namespace diff --git a/src/core/url_request_custom_job.h b/src/core/url_request_custom_job.h index a994c467a..eedb41814 100644 --- a/src/core/url_request_custom_job.h +++ b/src/core/url_request_custom_job.h @@ -50,6 +50,7 @@ QT_FORWARD_DECLARE_CLASS(QWebEngineUrlSchemeHandler) namespace QtWebEngineCore { class URLRequestCustomJobDelegate; +class URLRequestCustomJobShared; // A request job that handles reading custom URL schemes class URLRequestCustomJob : public net::URLRequestJob { @@ -62,6 +63,25 @@ public: virtual bool GetCharset(std::string *charset) Q_DECL_OVERRIDE; virtual bool IsRedirectResponse(GURL* location, int* http_status_code) Q_DECL_OVERRIDE; +protected: + virtual ~URLRequestCustomJob(); + +private: + QWebEngineUrlSchemeHandler *m_schemeHandler; + URLRequestCustomJobShared *m_shared; + + friend class URLRequestCustomJobShared; + + DISALLOW_COPY_AND_ASSIGN(URLRequestCustomJob); +}; + +// A shared state between URLRequestCustomJob living on the IO thread +// and URLRequestCustomJobDelegate living on the UI thread. +class URLRequestCustomJobShared { +public: + URLRequestCustomJobShared(URLRequestCustomJob *job); + ~URLRequestCustomJobShared(); + void setReplyMimeType(const std::string &); void setReplyCharset(const std::string &); void setReplyDevice(QIODevice *); @@ -70,28 +90,28 @@ public: void fail(int); void abort(); -protected: - virtual ~URLRequestCustomJob(); + void killJob(); + void unsetJobDelegate(); + void startAsync(); void notifyStarted(); void notifyFailure(); void notifyCanceled(); -private: + GURL requestUrl(); + std::string requestMethod(); + QMutex m_mutex; QPointer m_device; - QPointer m_delegate; - QWebEngineUrlSchemeHandler *m_schemeHandler; + URLRequestCustomJob *m_job; + URLRequestCustomJobDelegate *m_delegate; std::string m_mimeType; std::string m_charset; int m_error; GURL m_redirect; bool m_started; - base::WeakPtrFactory m_weakFactory; - - friend class URLRequestCustomJobDelegate; - - DISALLOW_COPY_AND_ASSIGN(URLRequestCustomJob); + bool m_asyncInitialized; + base::WeakPtrFactory m_weakFactory; }; } // namespace QtWebEngineCore diff --git a/src/core/url_request_custom_job_delegate.cpp b/src/core/url_request_custom_job_delegate.cpp index 0650242c8..1a3e08e52 100644 --- a/src/core/url_request_custom_job_delegate.cpp +++ b/src/core/url_request_custom_job_delegate.cpp @@ -44,39 +44,40 @@ namespace QtWebEngineCore { -URLRequestCustomJobDelegate::URLRequestCustomJobDelegate(URLRequestCustomJob *job) - : m_job(job) +URLRequestCustomJobDelegate::URLRequestCustomJobDelegate(URLRequestCustomJobShared *shared) + : m_shared(shared) { } URLRequestCustomJobDelegate::~URLRequestCustomJobDelegate() { + m_shared->unsetJobDelegate(); } QUrl URLRequestCustomJobDelegate::url() const { - return toQt(m_job->request()->url()); + return toQt(m_shared->requestUrl()); } QByteArray URLRequestCustomJobDelegate::method() const { - return QByteArray::fromStdString(m_job->request()->method()); + return QByteArray::fromStdString(m_shared->requestMethod()); } void URLRequestCustomJobDelegate::setReply(const QByteArray &contentType, QIODevice *device) { - m_job->setReplyMimeType(contentType.toStdString()); - m_job->setReplyDevice(device); + m_shared->setReplyMimeType(contentType.toStdString()); + m_shared->setReplyDevice(device); } void URLRequestCustomJobDelegate::abort() { - m_job->abort(); + m_shared->abort(); } void URLRequestCustomJobDelegate::redirect(const QUrl &url) { - m_job->redirect(toGurl(url)); + m_shared->redirect(toGurl(url)); } void URLRequestCustomJobDelegate::fail(Error error) @@ -102,7 +103,7 @@ void URLRequestCustomJobDelegate::fail(Error error) break; } if (net_error) - m_job->fail(net_error); + m_shared->fail(net_error); } } // namespace diff --git a/src/core/url_request_custom_job_delegate.h b/src/core/url_request_custom_job_delegate.h index 5b6137820..e066e85cc 100644 --- a/src/core/url_request_custom_job_delegate.h +++ b/src/core/url_request_custom_job_delegate.h @@ -46,7 +46,7 @@ QT_FORWARD_DECLARE_CLASS(QIODevice) namespace QtWebEngineCore { -class URLRequestCustomJob; +class URLRequestCustomJobShared; class QWEBENGINE_EXPORT URLRequestCustomJobDelegate : public QObject { Q_OBJECT @@ -72,10 +72,10 @@ public: void fail(Error); private: - URLRequestCustomJobDelegate(URLRequestCustomJob *job); + URLRequestCustomJobDelegate(URLRequestCustomJobShared *shared); - friend class URLRequestCustomJob; - URLRequestCustomJob *m_job; + friend class URLRequestCustomJobShared; + URLRequestCustomJobShared *m_shared; }; } // namespace -- cgit v1.2.3 From ae4b0583e4d62b6fe660b316e8a9c117c8b85881 Mon Sep 17 00:00:00 2001 From: Leena Miettinen Date: Fri, 22 Jan 2016 16:45:09 +0100 Subject: Doc: rendering to OpenGL Surface Task-number: QTBUG-50037 Change-Id: I9c7a09824172a09d8b454bf8ff5d358c00ce6b88 Reviewed-by: Joerg Bornemann --- src/webengine/doc/src/webengineview.qdoc | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'src') diff --git a/src/webengine/doc/src/webengineview.qdoc b/src/webengine/doc/src/webengineview.qdoc index a8cd1eb56..1d71a9cd9 100644 --- a/src/webengine/doc/src/webengineview.qdoc +++ b/src/webengine/doc/src/webengineview.qdoc @@ -29,6 +29,22 @@ \inqmlmodule QtWebEngine \since QtWebEngine 1.0 \brief A WebEngineView renders web content within a QML application. + + The WebEngineView type enables QML applications to render regions of dynamic web content. It + may share the screen with other QML types, such as a TabView, or fill the screen, as specified + within the QML application. + + \section1 Rendering to OpenGL Surface + + When using a QQuickRenderControl to render a Qt Quick user interface to an OpenGL surface, the + WebEngineView type is not rendered correctly. The web engine view attempts to use a global + OpenGL context created by \l QtWebEngine::initialize(), but there is no public API for accessing + that context in order to share it with the \c QQuickRenderControl context. + + To have the web engine view rendered correctly, it is possible to manually create a new + offscreen context that is shared with the \c QQuickRenderControl and to call the non-public + function \c qt_gl_set_global_share_context(), rather than calling \c initialize(). + If \c initialize() is called after setting a global context, it will do nothing. */ /*! -- cgit v1.2.3 From 64da0f73b2f8a2f936113fd556c9477cea441ded Mon Sep 17 00:00:00 2001 From: Joerg Bornemann Date: Fri, 15 Jan 2016 16:47:06 +0100 Subject: Fix crash on exit for view-owned QWebEngineUrlSchemeHandler objects For view-owned URL scheme handlers the destructor would remove the handler and then trigger URLRequestContextGetterQt::generateStorage. This would access the browser context from the IO thread while it already has been destroyed on the browser thread. Increment the ref count for the browser context before every call of generateStorage, and decrement it when generateStorage is finished. Task-number: QTBUG-50160 Change-Id: Id8b1505891ec56e93bf9d47f33bb8bc3304eb55a Reviewed-by: Kai Koehne --- src/core/url_request_context_getter_qt.cpp | 5 +++-- src/core/url_request_context_getter_qt.h | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/core/url_request_context_getter_qt.cpp b/src/core/url_request_context_getter_qt.cpp index 618354d1c..20efec295 100644 --- a/src/core/url_request_context_getter_qt.cpp +++ b/src/core/url_request_context_getter_qt.cpp @@ -127,8 +127,9 @@ void URLRequestContextGetterQt::updateStorageSettings() content::BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO), content::BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE) )); - if (m_storage) + if (m_storage) { content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE, base::Bind(&URLRequestContextGetterQt::generateStorage, this)); + } } } @@ -283,7 +284,7 @@ void URLRequestContextGetterQt::generateUserAgent() Q_ASSERT(m_urlRequestContext); Q_ASSERT(m_storage); - m_storage->set_http_user_agent_settings(new HttpUserAgentSettingsQt(m_browserContext)); + m_storage->set_http_user_agent_settings(new HttpUserAgentSettingsQt(m_browserContext.constData())); } void URLRequestContextGetterQt::updateHttpCache() diff --git a/src/core/url_request_context_getter_qt.h b/src/core/url_request_context_getter_qt.h index 8740c65a2..555d56784 100644 --- a/src/core/url_request_context_getter_qt.h +++ b/src/core/url_request_context_getter_qt.h @@ -94,7 +94,7 @@ private: bool m_ignoreCertificateErrors; QAtomicInt m_updateCookieStore; QAtomicInt m_updateHttpCache; - BrowserContextAdapter *m_browserContext; + QExplicitlySharedDataPointer m_browserContext; content::ProtocolHandlerMap m_protocolHandlers; QAtomicPointer m_proxyConfigService; -- cgit v1.2.3 From 52f039fe09d0c0cfd9e988b8b15293a9b5d7583d Mon Sep 17 00:00:00 2001 From: Joerg Bornemann Date: Thu, 28 Jan 2016 11:10:51 +0100 Subject: Fix crash on open link in new window MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The default implementation of QWebEngineView::createWindow returns a null pointer. Add missing null pointer check in adoptNewWindow. Change-Id: Ia6138f372ff169b9d32764b15550939adc247a1c Task-number: QTBUG-50718 Reviewed-by: Michael Brüning --- src/webenginewidgets/api/qwebenginepage.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/webenginewidgets/api/qwebenginepage.cpp b/src/webenginewidgets/api/qwebenginepage.cpp index aacf6c455..511ddfb0f 100644 --- a/src/webenginewidgets/api/qwebenginepage.cpp +++ b/src/webenginewidgets/api/qwebenginepage.cpp @@ -227,6 +227,8 @@ void QWebEnginePagePrivate::adoptNewWindow(WebContentsAdapter *newWebContents, W Q_UNUSED(userGesture); QWebEnginePage *newPage = q->createWindow(toWindowType(disposition)); + if (!newPage) + return; // Mark the new page as being in the process of being adopted, so that a second mouse move event // sent by newWebContents->initialize() gets filtered in RenderWidgetHostViewQt::forwardEvent. @@ -240,7 +242,7 @@ void QWebEnginePagePrivate::adoptNewWindow(WebContentsAdapter *newWebContents, W newPage->d_func()->m_isBeingAdopted = true; // Overwrite the new page's WebContents with ours. - if (newPage && newPage->d_func() != this) { + if (newPage->d_func() != this) { newPage->d_func()->adapter = newWebContents; newWebContents->initialize(newPage->d_func()); if (!initialGeometry.isEmpty()) -- cgit v1.2.3 From 23c652c3c1680f42adfd5c6500280b103ce1d095 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Fri, 29 Jan 2016 13:10:59 +0100 Subject: Disable printing in Chromium MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Disabling printing saves us compiling those files and files off a megabyte on the debug binary. To keep all the configure options in config, the common options are moved to a shared common.pri. Change-Id: Ieffdf9eb7dca58cfdafadd85bd24ea9c2be55ece Reviewed-by: Michael Brüning --- src/core/config/common.pri | 8 ++++++++ src/core/config/embedded_qnx.pri | 2 ++ src/core/config/linux.pri | 2 ++ src/core/config/mac_osx.pri | 2 ++ src/core/config/windows.pri | 2 ++ 5 files changed, 16 insertions(+) create mode 100644 src/core/config/common.pri (limited to 'src') diff --git a/src/core/config/common.pri b/src/core/config/common.pri new file mode 100644 index 000000000..793e26a61 --- /dev/null +++ b/src/core/config/common.pri @@ -0,0 +1,8 @@ +# Shared configuration for all our supported platforms + +# Trigger Qt-specific build conditions. +GYP_CONFIG += use_qt=1 +# We do not want to ship more external binary blobs, so let v8 embed its startup data. +GYP_CONFIG += v8_use_external_startup_data=0 +# Disable printing since we don't support it yet +GYP_CONFIG += enable_basic_printing=0 enable_print_preview=0 diff --git a/src/core/config/embedded_qnx.pri b/src/core/config/embedded_qnx.pri index 7fd35c976..34470d2d8 100644 --- a/src/core/config/embedded_qnx.pri +++ b/src/core/config/embedded_qnx.pri @@ -1,5 +1,7 @@ GYP_ARGS += "-D qt_os=\"embedded_qnx\" -I config/embedded_qnx.gypi" +include(common.pri) + GYP_CONFIG += \ disable_nacl=1 \ enable_plugins=0 \ diff --git a/src/core/config/linux.pri b/src/core/config/linux.pri index 9868d6848..1035e950e 100644 --- a/src/core/config/linux.pri +++ b/src/core/config/linux.pri @@ -1,3 +1,5 @@ +include(common.pri) + # linux_use_bundled_gold currently relies on a hardcoded relative path from chromium/src/out/(Release|Debug) # Disable it along with the -Wl,--threads flag just in case gold isn't installed on the system. GYP_CONFIG += \ diff --git a/src/core/config/mac_osx.pri b/src/core/config/mac_osx.pri index 93c77623c..940b47982 100644 --- a/src/core/config/mac_osx.pri +++ b/src/core/config/mac_osx.pri @@ -1,3 +1,5 @@ +include(common.pri) + QMAKE_CLANG_DIR = "/usr" QMAKE_CLANG_PATH = $$eval(QMAKE_MAC_SDK.macx-clang.$${QMAKE_MAC_SDK}.QMAKE_CXX) !isEmpty(QMAKE_CLANG_PATH) { diff --git a/src/core/config/windows.pri b/src/core/config/windows.pri index 1e875f308..760ed5b6f 100644 --- a/src/core/config/windows.pri +++ b/src/core/config/windows.pri @@ -1,5 +1,7 @@ GYP_ARGS += "-D qt_os=\"win32\" -I config/windows.gypi" +include(common.pri) + GYP_CONFIG += \ disable_nacl=1 \ remoting=0 \ -- cgit v1.2.3 From ef1f608f76f02f11b4bdcc8d52fa41cdf4cdf5c3 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Fri, 29 Jan 2016 15:57:18 +0100 Subject: Preserve webchannel and userscripts when restoring history Most page-state is set in webcontentsadapter::initialize except user scripts and webchannel. This patch ensures those are initialized too when changing to a new adapter during history restore. Change-Id: I4dca23ddab50480b1a72252a038834ce1802ad77 Task-number: QTBUG-50751 Reviewed-by: Kai Koehne Reviewed-by: David Rosca --- src/webenginewidgets/api/qwebenginepage.cpp | 14 ++++++++++++-- src/webenginewidgets/api/qwebenginepage_p.h | 1 + src/webenginewidgets/api/qwebenginescriptcollection.cpp | 12 ++++++++++++ src/webenginewidgets/api/qwebenginescriptcollection_p.h | 2 ++ 4 files changed, 27 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/webenginewidgets/api/qwebenginepage.cpp b/src/webenginewidgets/api/qwebenginepage.cpp index 511ddfb0f..b1bf33067 100644 --- a/src/webenginewidgets/api/qwebenginepage.cpp +++ b/src/webenginewidgets/api/qwebenginepage.cpp @@ -107,6 +107,7 @@ QWebEnginePagePrivate::QWebEnginePagePrivate(QWebEngineProfile *_profile) , m_isBeingAdopted(false) , m_backgroundColor(Qt::white) , fullscreenMode(false) + , webChannel(nullptr) { memset(actions, 0, sizeof(actions)); } @@ -407,8 +408,14 @@ void QWebEnginePagePrivate::recreateFromSerializedHistory(QDataStream &input) { QExplicitlySharedDataPointer newWebContents = WebContentsAdapter::createFromSerializedNavigationHistory(input, this); if (newWebContents) { + // Keep the old adapter referenced so the user-scripts are not + // unregistered immediately. + QExplicitlySharedDataPointer oldWebContents = adapter; adapter = newWebContents.data(); adapter->initialize(this); + if (webChannel) + adapter->setWebChannel(webChannel); + scriptCollection.d->rebindToContents(adapter.data()); } } @@ -519,7 +526,7 @@ QWebEngineSettings *QWebEnginePage::settings() const QWebChannel *QWebEnginePage::webChannel() const { Q_D(const QWebEnginePage); - return d->adapter->webChannel(); + return d->webChannel; } /*! @@ -536,7 +543,10 @@ QWebChannel *QWebEnginePage::webChannel() const void QWebEnginePage::setWebChannel(QWebChannel *channel) { Q_D(QWebEnginePage); - d->adapter->setWebChannel(channel); + if (d->webChannel != channel) { + d->webChannel = channel; + d->adapter->setWebChannel(channel); + } } /*! diff --git a/src/webenginewidgets/api/qwebenginepage_p.h b/src/webenginewidgets/api/qwebenginepage_p.h index 9cc4553d1..18110d923 100644 --- a/src/webenginewidgets/api/qwebenginepage_p.h +++ b/src/webenginewidgets/api/qwebenginepage_p.h @@ -151,6 +151,7 @@ public: bool m_isBeingAdopted; QColor m_backgroundColor; bool fullscreenMode; + QWebChannel *webChannel; mutable QtWebEngineCore::CallbackDirectory m_callbacks; mutable QAction *actions[QWebEnginePage::WebActionCount]; diff --git a/src/webenginewidgets/api/qwebenginescriptcollection.cpp b/src/webenginewidgets/api/qwebenginescriptcollection.cpp index 9967cde85..117c35b5a 100644 --- a/src/webenginewidgets/api/qwebenginescriptcollection.cpp +++ b/src/webenginewidgets/api/qwebenginescriptcollection.cpp @@ -220,3 +220,15 @@ void QWebEngineScriptCollectionPrivate::reserve(int capacity) { m_scriptController->reserve(m_contents, capacity); } + +void QWebEngineScriptCollectionPrivate::rebindToContents(QtWebEngineCore::WebContentsAdapter *page) +{ + Q_ASSERT(m_contents); + Q_ASSERT(page); + Q_ASSERT(m_contents != page); + + Q_FOREACH (const UserScript &script, m_scriptController->registeredScripts(m_contents)) { + m_scriptController->addUserScript(script, page); + } + m_contents = page; +} diff --git a/src/webenginewidgets/api/qwebenginescriptcollection_p.h b/src/webenginewidgets/api/qwebenginescriptcollection_p.h index cc6e88445..b5ae60a2c 100644 --- a/src/webenginewidgets/api/qwebenginescriptcollection_p.h +++ b/src/webenginewidgets/api/qwebenginescriptcollection_p.h @@ -69,6 +69,8 @@ public: QList toList(const QString &scriptName = QString()) const; QWebEngineScript find(const QString & name) const; + void rebindToContents(QtWebEngineCore::WebContentsAdapter *contents); + void insert(const QWebEngineScript &); bool remove(const QWebEngineScript &); void clear(); -- cgit v1.2.3 From ed5b65c70397f3ea9fa524e7f0b98a48a70ddc88 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Fri, 29 Jan 2016 14:46:13 +0100 Subject: Disable WebSpeech Also allows us to remove code handling libFLAC and libspeex since we no longer depend on those. Change-Id: Ifedc19b3c958215d298edd11f9126ea5b9cc09fa Reviewed-by: Joerg Bornemann --- src/3rdparty | 2 +- src/core/config/common.pri | 2 ++ src/core/config/linux.pri | 2 -- 3 files changed, 3 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/3rdparty b/src/3rdparty index b93352c98..3f655a31b 160000 --- a/src/3rdparty +++ b/src/3rdparty @@ -1 +1 @@ -Subproject commit b93352c98510fef629d11e8dda91e14aac855f62 +Subproject commit 3f655a31b4979b0862e5aec1c8f47b597464749f diff --git a/src/core/config/common.pri b/src/core/config/common.pri index 793e26a61..c5921a573 100644 --- a/src/core/config/common.pri +++ b/src/core/config/common.pri @@ -6,3 +6,5 @@ GYP_CONFIG += use_qt=1 GYP_CONFIG += v8_use_external_startup_data=0 # Disable printing since we don't support it yet GYP_CONFIG += enable_basic_printing=0 enable_print_preview=0 +# WebSpeech requires Google API keys and adds dependencies on speex and flac. +GYP_CONFIG += enable_web_speech=0 diff --git a/src/core/config/linux.pri b/src/core/config/linux.pri index 1035e950e..00e25ec58 100644 --- a/src/core/config/linux.pri +++ b/src/core/config/linux.pri @@ -36,11 +36,9 @@ use?(system_libevent): GYP_CONFIG += use_system_libevent=1 use?(system_libwebp): GYP_CONFIG += use_system_libwebp=1 use?(system_libsrtp): GYP_CONFIG += use_system_libsrtp=1 use?(system_libxslt): GYP_CONFIG += use_system_libxml=1 -use?(system_flac): GYP_CONFIG += use_system_flac=1 use?(system_jsoncpp): GYP_CONFIG += use_system_jsoncpp=1 use?(system_opus): GYP_CONFIG += use_system_opus=1 use?(system_snappy): GYP_CONFIG += use_system_snappy=1 -use?(system_speex): GYP_CONFIG += use_system_speex=1 use?(system_vpx): GYP_CONFIG += use_system_libvpx=1 use?(system_icu): GYP_CONFIG += use_system_icu=1 use?(system_ffmpeg): GYP_CONFIG += use_system_ffmpeg=1 -- cgit v1.2.3