* Implement our own queuing of network jobs to allow special handling of synchronous...
authorhausmann <hausmann@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 25 Oct 2007 13:40:53 +0000 (13:40 +0000)
committerhausmann <hausmann@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 25 Oct 2007 13:40:53 +0000 (13:40 +0000)
* This should be thread-safe besides QWebNetworkJob::{ref,deref}

Signed-off-by: Simon Hausmann <hausmann@kde.org>
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@27039 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/platform/qt/Skipped
WebCore/ChangeLog
WebCore/platform/network/qt/ResourceHandleQt.cpp
WebKit/qt/Api/qwebnetworkinterface.cpp
WebKit/qt/Api/qwebnetworkinterface.h
WebKit/qt/Api/qwebnetworkinterface_p.h
WebKit/qt/ChangeLog

index a9bb464b6d48e627c7582e8f9908848befad5055..1734366f554690223a2bdb9c5cd71fa7a3c3073e 100644 (file)
@@ -1,3 +1,13 @@
+2007-10-25  Holger Freyther  <zecke@selfish.org>
+
+        Reviewed by Simon Hausmann <hausmann@kde.org>.
+
+        * Implement our own queuing of network jobs to allow special handling of synchronous jobs. This makes us pass the fast/dom/xmlhttprequest-html-response-encoding.html test without a crash. Sync jobs will get a special treatment over the normals ones and in theory more than one sync job is supported.
+        * This should be thread-safe besides QWebNetworkJob::{ref,deref}
+        
+
+        * platform/qt/Skipped:
+
 2007-10-25  Holger Freyther  <zecke@selfish.org>
 
         Reviewed by Simon Hausmann <hausmann@kde.org>.
index 817e594cb99bc92df20059ae62f37b16d003a4ae..bdb0d71487f4aa48efe358258d6f3dd0d8bda5e9 100644 (file)
@@ -86,7 +86,6 @@ fast/dom/Window/get-set-properties.html
 fast/dom/Window/window-properties.html
 
 # ------- failures for tests with results
-fast/dom/xmlhttprequest-html-response-encoding.html
 fast/dom/Window/window-early-properties.html
 fast/forms/drag-out-of-textarea.html
 fast/forms/select-double-onchange.html
index 793606f6516be7c04a977ef3b3ac93ad536dfe98..ee48923ca1203dcbeb96f7cf8369e63b17352f98 100644 (file)
@@ -1,3 +1,14 @@
+2007-10-25  Holger Freyther  <zecke@selfish.org>
+
+        Reviewed by Simon Hausmann <hausmann@kde.org>.
+
+        * Implement our own queuing of network jobs to allow special handling of synchronous jobs. This makes us pass the fast/dom/xmlhttprequest-html-response-encoding.html test without a crash. Sync jobs will get a special treatment over the normals ones and in theory more than one sync job is supported.
+        * This should be thread-safe besides QWebNetworkJob::{ref,deref}
+        
+
+        * platform/network/qt/ResourceHandleQt.cpp:
+        (WebCore::ResourceHandle::loadResourceSynchronously):
+
 2007-10-25  Alp Toker  <alp@atoker.com>
 
         Reviewed by Mark Rowe.
index 52beb855198eec81f1cff654b572eee1842ee815..a84b05ae2020071d42f2c5661a4af898db9e589b 100644 (file)
@@ -171,7 +171,7 @@ void ResourceHandle::loadResourceSynchronously(const ResourceRequest& request, R
         return;
     }
 
-    QWebNetworkManager::self()->add(&handle, QWebNetworkInterface::defaultInterface());
+    QWebNetworkManager::self()->add(&handle, QWebNetworkInterface::defaultInterface(), QWebNetworkManager::SynchronousJob);
     syncLoader.waitForCompletion();
     error = syncLoader.resourceError();
     data = syncLoader.data();
index 6dff52cf473fe69f2dd66f37d00c3509f231c31d..43bf1dbced198f6d51dc97bff370720077e5a722 100644 (file)
@@ -49,8 +49,8 @@
 #define DEBUG if (1) {} else qDebug
 #endif
 
-static QWebNetworkInterface *default_interface = 0;
-static QWebNetworkManager *manager = 0;
+static QWebNetworkInterface *s_default_interface = 0;
+static QWebNetworkManager *s_manager = 0;
 
 using namespace WebCore;
 
@@ -294,6 +294,7 @@ QWebNetworkJob::QWebNetworkJob()
 QWebNetworkJob::~QWebNetworkJob()
 {
     delete d;
+    d = 0;
 }
 
 /*!
@@ -405,7 +406,9 @@ QWebFrame *QWebNetworkJob::frame() const
 */
 QWebNetworkManager::QWebNetworkManager()
     : QObject(0)
+    , m_scheduledWork(false)
 {
+    connect(this, SIGNAL(scheduleWork()), SLOT(doWork()), Qt::QueuedConnection);
 }
 
 QWebNetworkManager *QWebNetworkManager::self()
@@ -413,13 +416,13 @@ QWebNetworkManager *QWebNetworkManager::self()
     // ensure everything's constructed and connected
     QWebNetworkInterface::defaultInterface();
 
-    return manager;
+    return s_manager;
 }
 
-bool QWebNetworkManager::add(ResourceHandle *handle, QWebNetworkInterface *interface)
+bool QWebNetworkManager::add(ResourceHandle *handle, QWebNetworkInterface *interface, JobMode jobMode)
 {
     if (!interface)
-        interface = default_interface;
+        interface = s_default_interface;
 
     ASSERT(interface);
 
@@ -440,6 +443,11 @@ bool QWebNetworkManager::add(ResourceHandle *handle, QWebNetworkInterface *inter
 
     DEBUG() << "QWebNetworkManager::add:" <<  job->d->request.httpHeader.toString();
 
+    if (jobMode == AsynchronousJob) {
+        Q_ASSERT(!m_synchronousJobs.contains(job));
+        m_synchronousJobs[job] = 1;
+    }
+
     interface->addJob(job);
 
     return true;
@@ -459,6 +467,8 @@ void QWebNetworkManager::cancel(ResourceHandle *handle)
 
 void QWebNetworkManager::started(QWebNetworkJob *job)
 {
+    Q_ASSERT(job->d);
+
     ResourceHandleClient* client = 0;
     if (job->d->resourceHandle) {
         client = job->d->resourceHandle->client();
@@ -564,6 +574,9 @@ void QWebNetworkManager::data(QWebNetworkJob *job, const QByteArray &data)
 
 void QWebNetworkManager::finished(QWebNetworkJob *job, int errorCode)
 {
+    if (m_synchronousJobs.contains(job))
+        m_synchronousJobs.remove(job);
+
     ResourceHandleClient* client = 0;
     if (job->d->resourceHandle) {
         client = job->d->resourceHandle->client();
@@ -641,11 +654,11 @@ void QWebNetworkInterfacePrivate::sendFileData(QWebNetworkJob* job, int statusCo
         response.setStatusLine(statusCode);
         response.setContentLength(data.length());
         job->setResponse(response);
-        emit q->started(job);
+        q->started(job);
         if (!data.isEmpty())
-            emit q->data(job, data);
+            q->data(job, data);
     }
-    emit q->finished(job, error);
+    q->finished(job, error);
 }
 
 void QWebNetworkInterfacePrivate::parseDataUrl(QWebNetworkJob* job)
@@ -695,10 +708,106 @@ void QWebNetworkInterfacePrivate::parseDataUrl(QWebNetworkJob* job)
     job->setResponse(response);
 
     int error = statusCode >= 400 ? 1 : 0;
-    emit q->started(job);
+    q->started(job);
     if (!data.isEmpty())
-        emit q->data(job, data);
-    emit q->finished(job, error);
+        q->data(job, data);
+    q->finished(job, error);
+}
+
+void QWebNetworkManager::queueStart(QWebNetworkJob* job)
+{
+    Q_ASSERT(job->d);
+
+    QMutexLocker locker(&m_queueMutex);
+    job->ref();
+    m_startedJobs.append(job);
+    doScheduleWork();
+}
+
+void QWebNetworkManager::queueData(QWebNetworkJob* job, const QByteArray& data)
+{
+    ASSERT(job->d);
+
+    QMutexLocker locker(&m_queueMutex);
+    job->ref();
+    m_receivedData.append(new JobData(job,data));
+    doScheduleWork();
+}
+
+void QWebNetworkManager::queueFinished(QWebNetworkJob* job, int errorCode)
+{
+    Q_ASSERT(job->d);
+    
+    QMutexLocker locker(&m_queueMutex);
+    job->ref();
+    m_finishedJobs.append(new JobFinished(job, errorCode));
+    doScheduleWork();
+}
+
+void QWebNetworkManager::doScheduleWork()
+{
+    if (!m_scheduledWork) {
+        m_scheduledWork = true;
+        emit scheduleWork();
+    }
+}
+
+
+/*
+ * WARNING: This is very fragile. doWork will dispatch jobs which then
+ * can create new jobs in the meantime, which can fill the queue while
+ * we are working on it.
+ */
+void QWebNetworkManager::doWork()
+{
+    m_queueMutex.lock();
+    m_scheduledWork = false;
+    bool hasSyncJobs = m_synchronousJobs.size();
+    const QHash<QWebNetworkJob*, int> syncJobs = m_synchronousJobs;
+    m_queueMutex.unlock();
+
+    foreach(QWebNetworkJob* job, m_startedJobs) {
+        if (hasSyncJobs && !syncJobs.contains(job))
+            continue;
+
+        m_queueMutex.lock();
+        m_startedJobs.removeAll(job);
+        m_queueMutex.unlock();
+
+        started(job);
+        job->deref();
+    }
+
+    foreach(JobData* jobData, m_receivedData) {
+        if (hasSyncJobs && !syncJobs.contains(jobData->job))
+            continue;
+
+        m_queueMutex.lock();
+        m_receivedData.removeAll(jobData);
+        m_queueMutex.unlock();
+        
+        data(jobData->job, jobData->data);
+        jobData->job->deref();
+        delete jobData;
+    }
+
+    foreach(JobFinished* jobFinished, m_finishedJobs) {
+        if (hasSyncJobs && !syncJobs.contains(jobFinished->job))
+            continue;
+
+        m_queueMutex.lock();
+        m_finishedJobs.removeAll(jobFinished);
+        m_queueMutex.unlock();
+
+        finished(jobFinished->job, jobFinished->errorCode);
+        jobFinished->job->deref();
+        delete jobFinished;
+    }
+
+    m_queueMutex.lock();
+    if (hasSyncJobs && m_synchronousJobs.size() == 0)
+        doScheduleWork();
+    m_queueMutex.unlock();
 }
 
 /*!
@@ -721,8 +830,8 @@ static bool gRoutineAdded = false;
 
 static void gCleanupInterface()
 {
-    delete default_interface;
-    default_interface = 0;
+    delete s_default_interface;
+    s_default_interface = 0;
 }
 
 /*!
@@ -731,11 +840,11 @@ static void gCleanupInterface()
 */
 void QWebNetworkInterface::setDefaultInterface(QWebNetworkInterface *defaultInterface)
 {
-    if (default_interface == defaultInterface)
+    if (s_default_interface == defaultInterface)
         return;
-    if (default_interface)
-        delete default_interface;
-    default_interface = defaultInterface;
+    if (s_default_interface)
+        delete s_default_interface;
+    s_default_interface = defaultInterface;
     if (!gRoutineAdded) {
         qAddPostRoutine(gCleanupInterface);
         gRoutineAdded = true;
@@ -749,10 +858,10 @@ void QWebNetworkInterface::setDefaultInterface(QWebNetworkInterface *defaultInte
 */
 QWebNetworkInterface *QWebNetworkInterface::defaultInterface()
 {
-    if (!default_interface) {
+    if (!s_default_interface) {
         setDefaultInterface(new QWebNetworkInterface);
     }
-    return default_interface;
+    return s_default_interface;
 }
 
 
@@ -765,15 +874,8 @@ QWebNetworkInterface::QWebNetworkInterface(QObject *parent)
     d = new QWebNetworkInterfacePrivate;
     d->q = this;
 
-    if (!manager)
-        manager = new QWebNetworkManager;
-
-    QObject::connect(this, SIGNAL(started(QWebNetworkJob*)),
-                     manager, SLOT(started(QWebNetworkJob*)), Qt::QueuedConnection);
-    QObject::connect(this, SIGNAL(data(QWebNetworkJob*, const QByteArray &)),
-                     manager, SLOT(data(QWebNetworkJob*, const QByteArray &)), Qt::QueuedConnection);
-    QObject::connect(this, SIGNAL(finished(QWebNetworkJob*, int)),
-                     manager, SLOT(finished(QWebNetworkJob*, int)), Qt::QueuedConnection);
+    if (!s_manager)
+        s_manager = new QWebNetworkManager;
 }
 
 /*!
@@ -862,6 +964,24 @@ void QWebNetworkInterface::cancelJob(QWebNetworkJob *job)
         QWebNetworkManager::self()->cancelHttpJob(job);
 }
 
+void QWebNetworkInterface::started(QWebNetworkJob* job)
+{
+    Q_ASSERT(s_manager);
+    s_manager->queueStart(job);
+}
+
+void QWebNetworkInterface::data(QWebNetworkJob* job, const QByteArray& data)
+{
+    Q_ASSERT(s_manager);
+    s_manager->queueData(job, data);
+}
+
+void QWebNetworkInterface::finished(QWebNetworkJob* job, int errorCode)
+{
+    Q_ASSERT(s_manager);
+    s_manager->queueFinished(job, errorCode);
+}
+
 /////////////////////////////////////////////////////////////////////////////
 WebCoreHttp::WebCoreHttp(QObject* parent, const HostInfo &hi)
     : QObject(parent), info(hi),
@@ -914,7 +1034,7 @@ void WebCoreHttp::scheduleNextRequest()
     while (!job && !m_pendingRequests.isEmpty()) {
         job = m_pendingRequests.takeFirst();
         if (job->cancelled()) {
-            emit job->networkInterface()->finished(job, 1);
+            job->networkInterface()->finished(job, 1);
             job = 0;
         }
     }
@@ -968,7 +1088,7 @@ void WebCoreHttp::onResponseHeaderReceived(const QHttpResponseHeader &resp)
 
     job->setResponse(resp);
 
-    emit job->networkInterface()->started(job);
+    job->networkInterface()->started(job);
 }
 
 void WebCoreHttp::onReadyRead()
@@ -986,7 +1106,7 @@ void WebCoreHttp::onReadyRead()
     QByteArray data;
     data.resize(http->bytesAvailable());
     http->read(data.data(), data.length());
-    emit req->networkInterface()->data(req, data);
+    req->networkInterface()->data(req, data);
 }
 
 void WebCoreHttp::onRequestFinished(int id, bool error)
@@ -1011,9 +1131,10 @@ void WebCoreHttp::onRequestFinished(int id, bool error)
         QByteArray data;
         data.resize(http->bytesAvailable());
         http->read(data.data(), data.length());
-        emit req->networkInterface()->data(req, data);
+        req->networkInterface()->data(req, data);
     }
-    emit req->networkInterface()->finished(req, error ? 1 : 0);
+
+    req->networkInterface()->finished(req, error ? 1 : 0);
     connection[c].current = 0;
     connection[c].id = -1;
     scheduleNextRequest();
@@ -1049,7 +1170,7 @@ void WebCoreHttp::cancel(QWebNetworkJob* request)
     m_inCancel = false;
 
     if (doEmit)
-        emit request->networkInterface()->finished(request, 1);
+        request->networkInterface()->finished(request, 1);
 
     if (m_pendingRequests.isEmpty()
         && !connection[0].current && !connection[1].current)
index 945193fca33f19b8a82fa3040bd792d3d9f06370..0d9cc4953f1ab7f5f10f4e34cebef2996e504f2d 100644 (file)
@@ -126,11 +126,13 @@ public:
 
     virtual void addJob(QWebNetworkJob *job);
     virtual void cancelJob(QWebNetworkJob *job);
-    
-signals:
+
+protected:
     void started(QWebNetworkJob*);
     void data(QWebNetworkJob*, const QByteArray &data);
     void finished(QWebNetworkJob*, int errorCode);
+    
+signals:
     /**
      * Signal is emitted when an SSL error occurs.
      */
index 9e794e9f8eebd522ca237bd4d9d949f1e74cf38b..b097efa4698b63f053310c0b94ca6c669f4a7add 100644 (file)
@@ -24,6 +24,7 @@
 
 #include "qwebnetworkinterface.h"
 #include <qthread.h>
+#include <qmutex.h>
 
 namespace WebCore {
     struct HostInfo;
@@ -69,34 +70,75 @@ public:
     QWebObjectPluginConnector *connector;
 };
 
-
 class QWebNetworkManager : public QObject
 {
     Q_OBJECT
 public:
+    enum JobMode {
+        AsynchronousJob,
+        SynchronousJob
+    };
+
     static QWebNetworkManager *self();
 
-    bool add(WebCore::ResourceHandle *resourceHandle, QWebNetworkInterface *interface);
+    bool add(WebCore::ResourceHandle *resourceHandle, QWebNetworkInterface *interface, JobMode = AsynchronousJob);
     void cancel(WebCore::ResourceHandle *resourceHandle);
 
     void addHttpJob(QWebNetworkJob *job);
     void cancelHttpJob(QWebNetworkJob *job);
 
-public slots:
+protected:
+    void queueStart(QWebNetworkJob*);
+    void queueData(QWebNetworkJob*, const QByteArray&);
+    void queueFinished(QWebNetworkJob*, int errorCode);
+
+private:
     void started(QWebNetworkJob *);
     void data(QWebNetworkJob *, const QByteArray &data);
     void finished(QWebNetworkJob *, int errorCode);
+    void doScheduleWork();
 
 signals:
     void fileRequest(QWebNetworkJob*);
+    void scheduleWork();
 
 private slots:
     void httpConnectionClosed(const WebCore::HostInfo &);
+    void doWork();
 
 private:
     friend class QWebNetworkInterface;
     QWebNetworkManager();
     QHash<WebCore::HostInfo, WebCore::WebCoreHttp *> m_hostMapping;
+
+    struct JobData {
+        JobData(QWebNetworkJob* _job, const QByteArray& _data)
+            : job(_job)
+            , data(_data)
+        {}
+
+        QWebNetworkJob* job;
+        QByteArray data;
+    };
+
+    struct JobFinished {
+        JobFinished(QWebNetworkJob* _job, int _errorCode)
+            : job(_job)
+            , errorCode(_errorCode)
+        {}
+
+        QWebNetworkJob* job;
+        int errorCode;
+    };
+
+
+
+    QMutex m_queueMutex;
+    bool m_scheduledWork;
+    QList<QWebNetworkJob*> m_startedJobs;
+    QList<JobData*> m_receivedData;
+    QList<JobFinished*> m_finishedJobs;
+    QHash<QWebNetworkJob*, int> m_synchronousJobs;
 };
 
 
index 2bb2729e9bd64b9541361e342eae1e389152c748..ab2ff5e15edfe62db8524cce2f9bd0cdb950e0c2 100644 (file)
@@ -1,3 +1,43 @@
+2007-10-25  Holger Freyther  <zecke@selfish.org>
+
+        Reviewed by Simon Hausmann <hausmann@kde.org>.
+
+        * Implement our own queuing of network jobs to allow special handling of synchronous jobs. This makes us pass the fast/dom/xmlhttprequest-html-response-encoding.html test without a crash. Sync jobs will get a special treatment over the normals ones and in theory more than one sync job is supported.
+        * This should be thread-safe besides QWebNetworkJob::{ref,deref}
+        
+
+        * Api/qwebnetworkinterface.cpp:
+        (QWebNetworkJob::~QWebNetworkJob):
+        (QWebNetworkManager::QWebNetworkManager):
+        (QWebNetworkManager::self):
+        (QWebNetworkManager::add):
+        (QWebNetworkManager::started):
+        (QWebNetworkManager::finished):
+        (QWebNetworkInterfacePrivate::sendFileData):
+        (QWebNetworkInterfacePrivate::parseDataUrl):
+        (QWebNetworkManager::queueStart):
+        (QWebNetworkManager::queueData):
+        (QWebNetworkManager::queueFinished):
+        (QWebNetworkManager::doScheduleWork):
+        (QWebNetworkManager::doWork):
+        (gCleanupInterface):
+        (QWebNetworkInterface::setDefaultInterface):
+        (QWebNetworkInterface::defaultInterface):
+        (QWebNetworkInterface::QWebNetworkInterface):
+        (QWebNetworkInterface::started):
+        (QWebNetworkInterface::data):
+        (QWebNetworkInterface::finished):
+        (WebCoreHttp::scheduleNextRequest):
+        (WebCoreHttp::onResponseHeaderReceived):
+        (WebCoreHttp::onReadyRead):
+        (WebCoreHttp::onRequestFinished):
+        (WebCoreHttp::cancel):
+        * Api/qwebnetworkinterface.h:
+        * Api/qwebnetworkinterface_p.h:
+        (QWebNetworkManager::):
+        (QWebNetworkManager::JobData::JobData):
+        (QWebNetworkManager::JobFinished::JobFinished):
+
 2007-10-25  Holger Freyther  <zecke@selfish.org>
 
         Reviewed by Simon Hausmann <hausmann@kde.org>.