From 8596813a10b66b539746bb890bd4304e5f65b348 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Tue, 5 May 2015 16:34:14 +0200 Subject: Improve thread safety and behavior of custom URL requests Adds thread protection between the UI and IO threads access to the QIODevice, and changes it to a QPointer, so we can tell if the user deletes it. Change-Id: I3e7b3f682d4c5a145d75cd5822affcfc9012cff7 Reviewed-by: Andras Becsi --- src/core/url_request_custom_job.cpp | 65 +++++++++++++++++++++++--- src/core/url_request_custom_job.h | 14 +++++- src/webenginewidgets/api/qwebengineprofile.cpp | 4 ++ 3 files changed, 75 insertions(+), 8 deletions(-) (limited to 'src') diff --git a/src/core/url_request_custom_job.cpp b/src/core/url_request_custom_job.cpp index 105f90746..0b8aaf9ba 100644 --- a/src/core/url_request_custom_job.cpp +++ b/src/core/url_request_custom_job.cpp @@ -57,25 +57,41 @@ URLRequestCustomJob::URLRequestCustomJob(URLRequest *request, NetworkDelegate *n : URLRequestJob(request, networkDelegate) , m_device(0) , m_schemeHandler(schemeHandler) + , m_error(0) + , m_started(false) , m_weakFactory(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(); } void URLRequestCustomJob::Start() { + DCHECK_CURRENTLY_ON(content::BrowserThread::IO); content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE, base::Bind(&URLRequestCustomJob::startAsync, m_weakFactory.GetWeakPtr())); } 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(); URLRequestJob::Kill(); @@ -83,6 +99,7 @@ void URLRequestCustomJob::Kill() bool URLRequestCustomJob::GetMimeType(std::string *mimeType) const { + DCHECK_CURRENTLY_ON(content::BrowserThread::IO); if (m_mimeType.size() > 0) { *mimeType = m_mimeType; return true; @@ -92,6 +109,7 @@ 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; return true; @@ -101,25 +119,35 @@ bool URLRequestCustomJob::GetCharset(std::string* charset) void URLRequestCustomJob::setReplyMimeType(const std::string &mimeType) { + DCHECK_CURRENTLY_ON(content::BrowserThread::UI); m_mimeType = mimeType; } void URLRequestCustomJob::setReplyCharset(const std::string &charset) { + DCHECK_CURRENTLY_ON(content::BrowserThread::UI); m_charset = charset; } void URLRequestCustomJob::setReplyDevice(QIODevice *device) { + DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + QMutexLocker lock(&m_mutex); m_device = device; if (m_device && !m_device->isReadable()) m_device->open(QIODevice::ReadOnly); - content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE, base::Bind(&URLRequestCustomJob::notifyStarted, m_weakFactory.GetWeakPtr())); + + if (m_device && m_device->isReadable()) + content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE, base::Bind(&URLRequestCustomJob::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); @@ -132,16 +160,41 @@ bool URLRequestCustomJob::ReadRawData(IOBuffer *buf, int bufSize, int *bytesRead void URLRequestCustomJob::notifyStarted() { - if (m_device && m_device->isReadable()) - NotifyHeadersComplete(); + DCHECK_CURRENTLY_ON(content::BrowserThread::IO); + Q_ASSERT(!m_started); + m_started = true; + NotifyHeadersComplete(); +} + +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_weakFactory.GetWeakPtr())); +} + +void URLRequestCustomJob::notifyFailure() +{ + DCHECK_CURRENTLY_ON(content::BrowserThread::IO); + QMutexLocker lock(&m_mutex); + if (m_device) + m_device->close(); + if (m_started) + NotifyDone(URLRequestStatus(URLRequestStatus::FAILED, m_error)); else - NotifyStartError(URLRequestStatus(URLRequestStatus::FAILED, ERR_INVALID_URL)); + NotifyStartError(URLRequestStatus(URLRequestStatus::FAILED, m_error)); } void URLRequestCustomJob::startAsync() { - m_delegate.reset(new URLRequestCustomJobDelegate(this)); - m_schemeHandler->handleJob(m_delegate.get()); + DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + Q_ASSERT(!m_started); + Q_ASSERT(!m_delegate); + QMutexLocker lock(&m_mutex); + m_delegate = new URLRequestCustomJobDelegate(this); + lock.unlock(); + m_schemeHandler->handleJob(m_delegate); } } // namespace diff --git a/src/core/url_request_custom_job.h b/src/core/url_request_custom_job.h index 448bfe6af..ca20c719d 100644 --- a/src/core/url_request_custom_job.h +++ b/src/core/url_request_custom_job.h @@ -41,6 +41,8 @@ #include "net/url_request/url_request_job.h" #include +#include +#include QT_FORWARD_DECLARE_CLASS(QIODevice) @@ -63,19 +65,27 @@ public: void setReplyCharset(const std::string &); void setReplyDevice(QIODevice *); + void fail(int); + protected: virtual ~URLRequestCustomJob(); void startAsync(); void notifyStarted(); + void notifyFailure(); private: - QIODevice *m_device; - scoped_ptr m_delegate; + QMutex m_mutex; + QPointer m_device; + QPointer m_delegate; CustomUrlSchemeHandler *m_schemeHandler; std::string m_mimeType; std::string m_charset; + int m_error; + bool m_started; base::WeakPtrFactory m_weakFactory; + friend class URLRequestCustomJobDelegate; + DISALLOW_COPY_AND_ASSIGN(URLRequestCustomJob); }; diff --git a/src/webenginewidgets/api/qwebengineprofile.cpp b/src/webenginewidgets/api/qwebengineprofile.cpp index e63519d2c..adccfca2a 100644 --- a/src/webenginewidgets/api/qwebengineprofile.cpp +++ b/src/webenginewidgets/api/qwebengineprofile.cpp @@ -494,6 +494,10 @@ void QWebEngineProfilePrivate::installUrlSchemeHandler(QWebEngineUrlSchemeHandle return; } + if (m_urlSchemeHandlers.contains(scheme)) { + qWarning() << "URL scheme handler already installed for the scheme: " << scheme; + return; + } m_urlSchemeHandlers.insert(scheme, handler); browserContext()->customUrlSchemeHandlers().append(handler->d_func()); browserContext()->updateCustomUrlSchemeHandlers(); -- cgit v1.2.3