Reviewed by Zack
authorlars <lars@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 17 Jan 2007 13:18:33 +0000 (13:18 +0000)
committerlars <lars@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 17 Jan 2007 13:18:33 +0000 (13:18 +0000)
        Don't call receivedResponse twice for file: URLs.
        Never call back into the ResourceHandleClient once
        cancel() has been called on the ResourceHandle.

        Remove the inheritance from Shared<XxxClient> in the
        client classes.

        Answer asynchronously to most of the Policy checking methods
        in FrameLoaderClientQt to avoid some crashes in the loader.

        Small fixes in DumpRenderTree, so we don't by
        accident dump twice for the same test.

        Exclude one more test as it currently causes DumpRenderTree to
        hang forever.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@18906 268f45cc-cd09-0410-ab3c-d52691b4dbfc

17 files changed:
WebCore/ChangeLog
WebCore/WebCore.pro
WebCore/platform/network/qt/ResourceHandleManagerQt.cpp
WebCore/platform/network/qt/ResourceHandleManagerQt.h
WebKitQt/ChangeLog
WebKitQt/WebCoreSupport/ChromeClientQt.cpp
WebKitQt/WebCoreSupport/ChromeClientQt.h
WebKitQt/WebCoreSupport/ContextMenuClientQt.cpp
WebKitQt/WebCoreSupport/ContextMenuClientQt.h
WebKitQt/WebCoreSupport/EditorClientQt.cpp
WebKitQt/WebCoreSupport/EditorClientQt.h
WebKitQt/WebCoreSupport/FrameLoaderClientQt.cpp
WebKitQt/WebCoreSupport/FrameLoaderClientQt.h
WebKitTools/ChangeLog
WebKitTools/DumpRenderTree/DumpRenderTree.qtproj/DumpRenderTree.cpp
WebKitTools/DumpRenderTree/DumpRenderTree.qtproj/jsobjects.cpp
WebKitTools/DumpRenderTree/DumpRenderTree.qtproj/tests-skipped.txt

index e9a89b1d51107406f3e21f143f7a43e575103f71..d005900d3ad764ceb77547cb1c66c8339718f17e 100644 (file)
@@ -1,3 +1,22 @@
+2007-01-17  Lars Knoll <lars@trolltech.com>
+
+        Reviewed by Zack
+
+        Don't call receivedResponse twice for file: URLs.
+        Never call back into the ResourceHandleClient once
+        cancel() has been called on the ResourceHandle.
+
+        * WebCore.pro:
+        * platform/network/qt/ResourceHandleManagerQt.cpp:
+        (WebCore::ResourceHandleManager::cancel):
+        (WebCore::ResourceHandleManager::receivedResponse):
+        (WebCore::ResourceHandleManager::receivedData):
+        (WebCore::ResourceHandleManager::receivedFinished):
+        (WebCore::FileLoader::request):
+        (WebCore::FileLoader::parseDataUrl):
+        (WebCore::WebCoreHttp::scheduleNextRequest):
+        * platform/network/qt/ResourceHandleManagerQt.h:
+
 2007-01-17  Eric Seidel  <eric@webkit.org>
 
         No review, build fix only.
index 1b7a5440ac1b0454abad18c37e4826c83f11d7b6..fa05a6b611eca407b231bedac065b9ad5a68f62b 100644 (file)
@@ -104,7 +104,8 @@ MANUALMOC += \
     $$PWD/platform/qt/ScrollViewCanvasQt.h \
     $$PWD/platform/network/qt/ResourceHandleManagerQt.h \
     $$PWD/../WebKitQt/Api/qwebpage.h \
-    $$PWD/../WebKitQt/Api/qwebframe.h
+    $$PWD/../WebKitQt/Api/qwebframe.h \
+    $$PWD/../WebKitQt/WebCoreSupport/FrameLoaderClientQt.h
 
 LUT_FILES += \
     bindings/js/kjs_window.cpp \
index 2ee0317199834d66b03a8a1c3ce0720aa57d3670..036b92a7efb52c320b2c84e73b561d2534f7ddd4 100644 (file)
@@ -120,6 +120,7 @@ RequestQt::RequestQt(ResourceHandle* res, FrameQtClient *c)
 //     DEBUG() << request.toString();
 }
 
+
 void RequestQt::setURL(const KURL &u)
 {
     url = u;
@@ -179,7 +180,7 @@ void ResourceHandleManager::cancel(ResourceHandle* resource)
 
     DEBUG() << "ResourceHandleManager::cancel" << resource->url().path();
     
-    RequestQt* request = pendingRequests[resource];
+    RequestQt* request = pendingRequests.take(resource);
     if (!request)
         return;
     request->cancelled = true;
@@ -187,17 +188,14 @@ void ResourceHandleManager::cancel(ResourceHandle* resource)
     String protocol = request->hostInfo.protocol;
     if (protocol == "http") 
         emit networkCancel(request);
-
-    return;
 }
 
 
 void ResourceHandleManager::receivedResponse(RequestQt* request)
 {
-    RequestQt *req = pendingRequests.value(request->resource);
-    if (!req || request->cancelled)
+    if (request->cancelled)
         return;
-    Q_ASSERT(req == request);
+    Q_ASSERT(pendingRequests.value(request->resource) == request);
     DEBUG() << "ResourceHandleManager::receivedResponse:";
     DEBUG() << request->response.toString();
 
@@ -253,10 +251,9 @@ void ResourceHandleManager::receivedResponse(RequestQt* request)
 
 void ResourceHandleManager::receivedData(RequestQt* request, const QByteArray& data)
 {
-    RequestQt *req = pendingRequests.value(request->resource);
-    if (!req || request->cancelled || request->redirected)
+    if (request->cancelled)
         return;
-    Q_ASSERT(req == request);
+    Q_ASSERT(pendingRequests.value(request->resource) == request);
 
     ResourceHandleClient* client = request->resource->client();
     if (!client)
@@ -268,11 +265,12 @@ void ResourceHandleManager::receivedData(RequestQt* request, const QByteArray& d
 
 void ResourceHandleManager::receivedFinished(RequestQt* request, int errorCode)
 {
-    DEBUG() << "receivedFinished" << errorCode << request->url.path();
-    RequestQt *req = pendingRequests.value(request->resource);
-    if (!req)
+    if (request->cancelled) {
+        delete request;
         return;
-    Q_ASSERT(req == request);
+    }
+    DEBUG() << "receivedFinished" << errorCode << request->url.path();
+    Q_ASSERT(pendingRequests.value(request->resource) == request);
 
     pendingRequests.remove(request->resource);
 
@@ -286,12 +284,13 @@ void ResourceHandleManager::receivedFinished(RequestQt* request, int errorCode)
     if (!client)
         return;
 
-    if (errorCode || request->cancelled) {
+    if (errorCode) {
         //FIXME: error setting error was removed from ResourceHandle
         client->didFail(request->resource, ResourceError());
     } else {
         client->didFinishLoading(request->resource);
     }
+    DEBUG() << "receivedFinished done" << request->url.path();
     delete request;
 }
 
@@ -357,13 +356,11 @@ void FileLoader::request(RequestQt* request)
     if (!request->hostInfo.isLocalFile()) {
         statusCode = 404;
     } else if (request->postData.isEmpty()) {
-        QFile f(QString(request->request.path()));
-        DEBUG() << "opening" << QString(request->request.path());
+        QFile f(QString(request->qurl.path()));
+        DEBUG() << "opening" << QString(request->qurl.path());
 
         if (f.open(QIODevice::ReadOnly)) {
-            request->response.setStatusLine(200);
-            emit receivedResponse(request);
-        
+            request->response.setStatusLine(200);        
             data = f.readAll();
         } else {
             statusCode = 404;
@@ -388,7 +385,7 @@ void FileLoader::sendData(RequestQt* request, int statusCode, const QByteArray &
 
 void FileLoader::parseDataUrl(RequestQt* request)
 {
-    QByteArray data = request->qurl.toLatin1();
+    QByteArray data = request->qurl.toString().toLatin1();
     //qDebug() << "handling data url:" << data; 
 
     ASSERT(data.startsWith("data:"));
@@ -474,10 +471,20 @@ void WebCoreHttp::scheduleNextRequest()
         if (!connection[c].current)
             break;
     }
-    if (c >= 2 || m_pendingRequests.isEmpty())
+    if (c >= 2)
         return;
 
-    RequestQt *req = m_pendingRequests.takeFirst();
+    RequestQt *req = 0;
+    while (!req && !m_pendingRequests.isEmpty()) {
+        req = m_pendingRequests.takeFirst();
+        if (req->cancelled) {
+            emit m_loader->receivedFinished(req, 1);
+            req = 0;
+        }
+    }
+    if (!req)
+        return;
+    
     QHttp *http = connection[c].http;
     if (!req->postData.isEmpty())
         http->request(req->request, req->postData);
index ef7fb325c638cc94c6080be2944101bbfc82467c..5ef703361ebefcd5884e3a0e61c274577b5eda8e 100644 (file)
@@ -36,6 +36,7 @@
 #include <QThread>
 #include <QString>
 #include <QEvent>
+#include <QUrl>
 
 namespace WebCore {
 
@@ -55,13 +56,13 @@ struct HostInfo {
 class RequestQt
 {
 public:
-    RequestQt(ResourceHandle*, FrameQtClient *);
+    RequestQt(ResourceHandle*, FrameQtClient*);
     void setURL(const KURL &url);
     // not thread safe, don't use in other threads
     KURL url;
 
-    QString qurl;
-    FrameQtClient *client;
+    QUrl qurl;
+    FrameQtClientclient;
     ResourceHandle* resource;
 
     // to be used by other threads
index c3417a6d854ec775a88602377433e0c134b181df..5e3e9b7695e069efc0a9df0140e31d7d0cae697b 100644 (file)
@@ -1,3 +1,30 @@
+2007-01-17  Lars Knoll <lars@trolltech.com>
+
+        Reviewed by Zack
+
+        Remove the inheritance from Shared<XxxClient> in the
+        client classes.
+
+        Answer asynchronously to most of the Policy checking methods
+        in FrameLoaderClientQt to avoid some crashes in the loader.
+
+        * WebCoreSupport/ChromeClientQt.cpp:
+        * WebCoreSupport/ChromeClientQt.h:
+        * WebCoreSupport/ContextMenuClientQt.cpp:
+        * WebCoreSupport/ContextMenuClientQt.h:
+        * WebCoreSupport/EditorClientQt.cpp:
+        * WebCoreSupport/EditorClientQt.h:
+        * WebCoreSupport/FrameLoaderClientQt.cpp:
+        (WebCore::FrameLoaderClientQt::FrameLoaderClientQt):
+        (WebCore::FrameLoaderClientQt::callPolicyFunction):
+        (WebCore::FrameLoaderClientQt::slotCallPolicyFunction):
+        (WebCore::FrameLoaderClientQt::cancelPolicyCheck):
+        (WebCore::FrameLoaderClientQt::dispatchWillSubmitForm):
+        (WebCore::FrameLoaderClientQt::dispatchDecidePolicyForMIMEType):
+        (WebCore::FrameLoaderClientQt::dispatchDecidePolicyForNewWindowAction):
+        (WebCore::FrameLoaderClientQt::dispatchDecidePolicyForNavigationAction):
+        * WebCoreSupport/FrameLoaderClientQt.h:
+
 2007-01-17  Lars Knoll <lars@trolltech.com>
 
         Reviewed by Zack
index 850a867348259a1df2c7609ecf6c4b266eb9eced..b8301f455f8c121207b8c23ddfc905151a33f305 100644 (file)
@@ -52,16 +52,6 @@ ChromeClientQt::~ChromeClientQt()
     
 }
 
-void ChromeClientQt::ref()
-{
-    Shared<ChromeClientQt>::ref();
-}
-
-void ChromeClientQt::deref()
-{
-    Shared<ChromeClientQt>::deref();
-}
-
 void ChromeClientQt::setWindowRect(const FloatRect& rect)
 {
     if (!m_webPage)
index 7c8e6daa1c5e0aa15ca7d44458e7801970ba8e16..b758cbb7dd5c5a9005945cafe89300776b76f324 100644 (file)
@@ -40,16 +40,13 @@ namespace WebCore {
     class Page;
     struct FrameLoadRequest;
 
-    class ChromeClientQt : public ChromeClient,
-                           public Shared<ChromeClientQt> {
+    class ChromeClientQt : public ChromeClient
+    {
     public:
         ChromeClientQt(QWebPage* webPage);
         virtual ~ChromeClientQt();
         virtual void chromeDestroyed();
 
-        virtual void ref();
-        virtual void deref();
-
         virtual void setWindowRect(const FloatRect&);
         virtual FloatRect windowRect();
 
index f1eab8ae1f320f09d7b03040f983212ad52165fa..12b7644f89bf08c8c3f490f3a86f63312a48db3b 100644 (file)
 
 namespace WebCore {
     
-void ContextMenuClientQt::ref()
-{
-    Shared<ContextMenuClientQt>::ref();
-}
-
-void ContextMenuClientQt::deref()
-{
-    Shared<ContextMenuClientQt>::deref();
-}
-
 void ContextMenuClientQt::contextMenuDestroyed()
 {
     notImplemented();
index e47e2f782ebb759c32c31f2a1114c929ce39d090..248663afc1fffb28ed92c8246aa4f652485e6729 100644 (file)
 namespace WebCore {
     class ContextMenu;
 
-    class ContextMenuClientQt : public ContextMenuClient,
-                                public Shared<ContextMenuClientQt> {
+    class ContextMenuClientQt : public ContextMenuClient
+    {
     public:
-        virtual void ref();
-        virtual void deref();
-
         virtual void contextMenuDestroyed();
         
         virtual PlatformMenuDescription getCustomMenuFromDefaultItems(ContextMenu*);
index ba26d62e7f03191e9b3395c36e9aa23a82904ed6..33a767b6019877a4aead636740439d8cf8f86294 100644 (file)
 
 namespace WebCore {
 
-void EditorClientQt::ref()
-{
-    Shared<EditorClientQt>::ref();
-}
-
-void EditorClientQt::deref()
-{
-    Shared<EditorClientQt>::deref();
-}
-
 bool EditorClientQt::shouldDeleteRange(Range*)
 {
     notImplemented();
index ad0f9349ba26678a506e9544df596d580f01ee81..186b4e93974b89d0618f63d9db8dd6719be6f6a2 100644 (file)
 
 namespace WebCore {
 
-class EditorClientQt : public EditorClient, public Shared<EditorClientQt> {
+class EditorClientQt : public EditorClient {
 public:
-    virtual void ref();
-    virtual void deref();
     virtual void pageDestroyed();
 
     virtual bool shouldDeleteRange(Range*);
index ac8b0cb7d2079b1fb59c93ee01f530ae4c098b18..75fc92abcfe63498feeabbb6efc0aa3dbd94e508 100644 (file)
@@ -40,7 +40,11 @@ namespace WebCore
 
 FrameLoaderClientQt::FrameLoaderClientQt()
     : m_frame(0)
+    , m_webFrame(0)
+    , m_firstData(false)
+    , m_policyFunction(0)
 {
+    connect(this, SIGNAL(sigCallPolicyFunction(int)), this, SLOT(slotCallPolicyFunction(int)), Qt::QueuedConnection);
 }
 
 
@@ -60,14 +64,22 @@ void FrameLoaderClientQt::detachFrameLoader()
     m_frame = 0;
 }
 
-void FrameLoaderClientQt::ref()
+void FrameLoaderClientQt::callPolicyFunction(FramePolicyFunction function, PolicyAction action)
 {
-    Shared<FrameLoaderClientQt>::ref();
+    qDebug() << "FrameLoaderClientQt::callPolicyFunction";
+    ASSERT(!m_policyFunction);
+
+    m_policyFunction = function;
+    emit sigCallPolicyFunction(action);
 }
 
-void FrameLoaderClientQt::deref()
+void FrameLoaderClientQt::slotCallPolicyFunction(int action)
 {
-    Shared<FrameLoaderClientQt>::deref();
+    qDebug() << "FrameLoaderClientQt::slotCallPolicyFunction";
+    if (!m_frame || !m_policyFunction)
+        return;
+    (m_frame->loader()->*m_policyFunction)(WebCore::PolicyAction(action));
+    m_policyFunction = 0;
 }
 
 bool FrameLoaderClientQt::hasWebView() const
@@ -324,17 +336,16 @@ void FrameLoaderClientQt::dispatchShow()
 
 void FrameLoaderClientQt::cancelPolicyCheck()
 {
-    // don't need to do anything here as long as we don't start doing asyncronous policy checks
+    m_policyFunction = 0;
 }
 
 
 void FrameLoaderClientQt::dispatchWillSubmitForm(FramePolicyFunction function,
                                                  PassRefPtr<FormState>)
 {
+    notImplemented();
     // FIXME: This is surely too simple
-    if (!m_frame)
-        return;
-    (m_frame->loader()->*function)(PolicyUse);
+    callPolicyFunction(function, PolicyUse);
 }
 
 
@@ -702,26 +713,21 @@ WebCore::Frame* FrameLoaderClientQt::dispatchCreatePage()
 
 void FrameLoaderClientQt::dispatchDecidePolicyForMIMEType(FramePolicyFunction function, const WebCore::String&, const WebCore::ResourceRequest&)
 {
-    if (!m_frame)
-        return;
-    // FIXME: This is maybe too simple
-    (m_frame->loader()->*function)(PolicyUse);
+    // we need to call directly here
+    m_policyFunction = function;
+    slotCallPolicyFunction(PolicyUse);
 }
 
 void FrameLoaderClientQt::dispatchDecidePolicyForNewWindowAction(FramePolicyFunction function, const WebCore::NavigationAction&, const WebCore::ResourceRequest&, const WebCore::String&)
 {
-    if (!m_frame)
-        return;
-    // FIXME: This is maybe too simple
-    (m_frame->loader()->*function)(PolicyIgnore);
+    notImplemented();
+    callPolicyFunction(function, PolicyIgnore);
 }
 
 void FrameLoaderClientQt::dispatchDecidePolicyForNavigationAction(FramePolicyFunction function, const WebCore::NavigationAction&, const WebCore::ResourceRequest&)
 {
-    if (!m_frame)
-        return;
-    // FIXME: This is maybe too simple
-    (m_frame->loader()->*function)(PolicyUse);
+    notImplemented();
+    callPolicyFunction(function, PolicyUse);
 }
 
 void FrameLoaderClientQt::dispatchUnableToImplementPolicy(const WebCore::ResourceError&)
@@ -742,4 +748,4 @@ bool FrameLoaderClientQt::willUseArchive(WebCore::ResourceLoader*, const WebCore
 
 }
 
-
+#include "FrameLoaderClientQt.moc"
index afbc79772eae5b6d19484bf4f05284fe549e5530..e14f6414946c663531761a32c1a4f9dc2a2fd852 100644 (file)
@@ -28,6 +28,8 @@
 #ifndef FrameLoaderClientQt_H
 #define FrameLoaderClientQt_H
 
+#include <qobject.h>
+
 #include "FrameLoaderClient.h"
 #include "KURL.h"
 #include "FrameQt.h"
@@ -48,16 +50,20 @@ namespace WebCore {
 
     struct LoadErrorResetToken;
 
-    class FrameLoaderClientQt : public FrameLoaderClient, public Shared<FrameLoaderClientQt> {
+    class FrameLoaderClientQt : public QObject, public FrameLoaderClient {
+        Q_OBJECT
+
+        void callPolicyFunction(FramePolicyFunction function, PolicyAction action);
+    private slots:
+        void slotCallPolicyFunction(int);
+    signals:
+        void sigCallPolicyFunction(int);
     public:
         FrameLoaderClientQt();
         ~FrameLoaderClientQt();
         void setFrame(QWebFrame *webFrame, FrameQt *frame);
         virtual void detachFrameLoader();
 
-        virtual void ref();
-        virtual void deref();
-
         virtual bool hasWebView() const; // mainly for assertions
         virtual bool hasFrameView() const; // ditto
 
@@ -207,6 +213,7 @@ namespace WebCore {
         QWebFrame *m_webFrame;
         ResourceResponse m_response;
         bool m_firstData;
+        FramePolicyFunction m_policyFunction;
     };
 
 }
index 51dba30bc3b1d835c87f9e733f4dfd47add92df4..535531640f1e163b34c60b6310cc111c00185508 100644 (file)
@@ -1,3 +1,20 @@
+2007-01-17  Lars Knoll <lars@trolltech.com>
+
+        Reviewed by Zack
+
+        Small fixes in DumpRenderTree, so we don't by
+        accident dump twice for the same test.
+
+        Exclude one more test as it currently causes DumpRenderTree to
+        hang forever.
+
+        * DumpRenderTree/DumpRenderTree.qtproj/DumpRenderTree.cpp:
+        (WebCore::DumpRenderTree::readStdin):
+        (WebCore::DumpRenderTree::dump):
+        * DumpRenderTree/DumpRenderTree.qtproj/jsobjects.cpp:
+        (LayoutTestController::notifyDone):
+        * DumpRenderTree/DumpRenderTree.qtproj/tests-skipped.txt:
+
 2007-01-17  Lars Knoll <lars@trolltech.com>
 
         Reviewed by Zack
index f41cc6cd024a11b148c796aa77259441cc9c29e4..4754a7fe812ca2aac8f519815f093fb1d09c07ee 100644 (file)
@@ -111,12 +111,13 @@ void DumpRenderTree::open(const QUrl& url)
 void DumpRenderTree::readStdin(int /* socket */)
 {
     if (m_loading)
-        qDebug() << "=========================== still loading";
+        fprintf(stderr, "=========================== still loading\n");
 
     // Read incoming data from stdin...
     QByteArray line = m_stdin->readLine();
     if (line.endsWith('\n'))
         line.truncate(line.size()-1);
+    //fprintf(stderr, "\n    opening %s\n", line.constData());
     if (!line.isEmpty())
         open(QUrl(QString(line)));
 }
@@ -163,7 +164,7 @@ void DumpRenderTree::initJSObjects()
 
 void DumpRenderTree::dump()
 {
-    //qDebug() << ">>>>>>>>>>>>>>>>>>>>>> Dumping" << m_frame->loader()->URL().url();
+    //fprintf(stderr, "    Dumping\n");
     if (!m_notifier) {
         // Dump markup in single file mode...
         QString markup = frame->markup();
index ad7dd01ed6f07cc56a5078cd24eef6f9ca04afbc..dc1dd8ef1c1f60b47d8da7ac0c410bfc295ce1b9 100644 (file)
@@ -52,6 +52,8 @@ void LayoutTestController::waitUntilDone()
 void LayoutTestController::notifyDone()
 {
     //qDebug() << ">>>>notifyDone";
+    if (!timeoutTimer)
+        return;
     killTimer(timeoutTimer);
     timeoutTimer = 0;
     emit done();
index ac805ba1cd35013e15ae781a1bec88dfd4b17c7a..0798b2203c9629afd27a6a02e53c6145a09f9635 100644 (file)
@@ -1,5 +1,6 @@
 # These tests hang forever
 editing/selection/extend-by-word-002.html
+tables/mozilla/core/col_widths_fix_autoFixPer.html
 
 # tests that currently crash
 fast/borders/border-radius-huge-assert.html