Intermittent crash in fast/files/read-blob-async.html on the GTK+ debug
authorjianli@chromium.org <jianli@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 7 Feb 2011 23:28:35 +0000 (23:28 +0000)
committerjianli@chromium.org <jianli@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 7 Feb 2011 23:28:35 +0000 (23:28 +0000)
bots
https://bugs.webkit.org/show_bug.cgi?id=53104

Reviewed by David Levin.

Source/WebCore:

Covered by the existing tests.

* fileapi/FileStreamProxy.cpp:
(WebCore::FileStreamProxy::startOnFileThread):
* platform/network/BlobRegistryImpl.cpp:
(WebCore::BlobRegistryImpl::createResourceHandle):
* platform/network/BlobResourceHandle.cpp:
(WebCore::BlobResourceHandle::BlobResourceHandle):
(WebCore::BlobResourceHandle::cancel):
(WebCore::delayedStartBlobResourceHandle):
(WebCore::BlobResourceHandle::start): Keep BlobResourceHandle alive
till the delay function is called.
(WebCore::BlobResourceHandle::doStart):
(WebCore::doNotifyFinish):
(WebCore::BlobResourceHandle::notifyFinish): Notify the client via the
standalone function to prevent the handle from being disposed immediately
by the client which will make the calls in the stack that're still bound
to the handle suffer.
* platform/network/BlobResourceHandle.h:
* platform/network/ResourceHandle.h: Change cancel() to virtual so that
BlobResourceHandle::cancel will be called when we abort a FileReader.

LayoutTests:

* platform/gtk/Skipped: Enable the flakey test that has been fixed.

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

LayoutTests/ChangeLog
LayoutTests/platform/gtk/Skipped
Source/WebCore/ChangeLog
Source/WebCore/fileapi/FileStreamProxy.cpp
Source/WebCore/platform/network/BlobRegistryImpl.cpp
Source/WebCore/platform/network/BlobResourceHandle.cpp
Source/WebCore/platform/network/BlobResourceHandle.h
Source/WebCore/platform/network/ResourceHandle.h

index 0274a84..e935a54 100644 (file)
@@ -1,3 +1,13 @@
+2011-02-07  Jian Li  <jianli@chromium.org>
+
+        Reviewed by David Levin.
+
+        Intermittent crash in fast/files/read-blob-async.html on the GTK+ debug
+        bots
+        https://bugs.webkit.org/show_bug.cgi?id=53104
+
+        * platform/gtk/Skipped: Enable the flakey test that has been fixed.
+
 2011-02-07  Adam Barth  <abarth@webkit.org>
 
         Remove wrong platform-specific expectation.
index f55c5f3..5e626ea 100644 (file)
@@ -4679,9 +4679,6 @@ editing/selection/doubleclick-whitespace.html
 # https://bugs.webkit.org/show_bug.cgi?id=53605
 editing/text-iterator/basic-iteration.html
 
-# https://bugs.webkit.org/show_bug.cgi?id=53673
-fast/files/workers/worker-read-blob-async.html
-
 # https://bugs.webkit.org/show_bug.cgi?id=53680
 fast/frames/sandboxed-iframe-scripting.html
 
index 6c26741..3546f1c 100644 (file)
@@ -1,3 +1,33 @@
+2011-02-07  Jian Li  <jianli@chromium.org>
+
+        Reviewed by David Levin.
+
+        Intermittent crash in fast/files/read-blob-async.html on the GTK+ debug
+        bots
+        https://bugs.webkit.org/show_bug.cgi?id=53104
+
+        Covered by the existing tests.
+
+        * fileapi/FileStreamProxy.cpp:
+        (WebCore::FileStreamProxy::startOnFileThread):
+        * platform/network/BlobRegistryImpl.cpp:
+        (WebCore::BlobRegistryImpl::createResourceHandle):
+        * platform/network/BlobResourceHandle.cpp:
+        (WebCore::BlobResourceHandle::BlobResourceHandle):
+        (WebCore::BlobResourceHandle::cancel):
+        (WebCore::delayedStartBlobResourceHandle):
+        (WebCore::BlobResourceHandle::start): Keep BlobResourceHandle alive
+        till the delay function is called.
+        (WebCore::BlobResourceHandle::doStart):
+        (WebCore::doNotifyFinish):
+        (WebCore::BlobResourceHandle::notifyFinish): Notify the client via the
+        standalone function to prevent the handle from being disposed immediately
+        by the client which will make the calls in the stack that're still bound
+        to the handle suffer.
+        * platform/network/BlobResourceHandle.h:
+        * platform/network/ResourceHandle.h: Change cancel() to virtual so that
+        BlobResourceHandle::cancel will be called when we abort a FileReader.
+
 2011-02-07  Sheriff Bot  <webkit.review.bot@gmail.com>
 
         Unreviewed, rolling out r77845.
index 5daf983..92b07cd 100644 (file)
@@ -83,6 +83,8 @@ static void didStart(ScriptExecutionContext*, FileStreamProxy* proxy)
 
 void FileStreamProxy::startOnFileThread()
 {
+    if (!client())
+        return;
     m_stream->start();
     m_context->postTask(createCallbackTask(&didStart, this));
 }
index 2c4e8fa..83517f1 100644 (file)
@@ -68,7 +68,9 @@ PassRefPtr<ResourceHandle> BlobRegistryImpl::createResourceHandle(const Resource
     if (!shouldLoadResource(request))
         return 0;
 
-    return BlobResourceHandle::create(m_blobs.get(request.url().string()), request, client);
+    RefPtr<BlobResourceHandle> handle = BlobResourceHandle::create(m_blobs.get(request.url().string()), request, client);
+    handle->start();
+    return handle.release();
 }
 
 bool BlobRegistryImpl::loadResourceSynchronously(const ResourceRequest& request, ResourceError& error, ResourceResponse& response, Vector<char>& data)
index 753052a..24c9088 100644 (file)
@@ -138,11 +138,6 @@ void BlobResourceHandle::loadResourceSynchronously(PassRefPtr<BlobStorageData> b
     handle->start();
 }
 
-static void delayedStart(void* context)
-{
-    static_cast<BlobResourceHandle*>(context)->start();
-}
-
 BlobResourceHandle::BlobResourceHandle(PassRefPtr<BlobStorageData> blobData, const ResourceRequest& request, ResourceHandleClient* client, bool async)
     : ResourceHandle(request, client, false, false)
     , m_blobData(blobData)
@@ -158,11 +153,9 @@ BlobResourceHandle::BlobResourceHandle(PassRefPtr<BlobStorageData> blobData, con
     , m_readItemCount(0)
     , m_fileOpened(false)
 {    
-    if (m_async) {
-        // We need to take a ref.
+    if (m_async)
         m_asyncStream = client->createAsyncFileStream(this);
-        callOnMainThread(delayedStart, this);
-    } else
+    else
         m_stream = FileStream::create();
 }
 
@@ -187,10 +180,32 @@ void BlobResourceHandle::cancel()
     }
 
     m_aborted = true;
+
+    ResourceHandle::cancel();
+}
+
+void delayedStartBlobResourceHandle(void* context)
+{
+    RefPtr<BlobResourceHandle> handle = adoptRef(static_cast<BlobResourceHandle*>(context));
+    handle->doStart();
 }
 
 void BlobResourceHandle::start()
 {
+    if (m_async) {
+        // Keep BlobResourceHandle alive until delayedStartBlobResourceHandle runs.
+        ref();
+
+        // Finish this async call quickly and return.
+        callOnMainThread(delayedStartBlobResourceHandle, this);
+        return;
+    }
+
+    doStart();
+}
+
+void BlobResourceHandle::doStart()
+{
     // Do not continue if the request is aborted or an error occurs.
     if (m_aborted || m_errorCode)
         return;
@@ -578,10 +593,23 @@ void BlobResourceHandle::notifyFail(int errorCode)
         client()->didFail(this, ResourceError(String(), errorCode, firstRequest().url(), String()));
 }
 
+static void doNotifyFinish(void* context)
+{
+    BlobResourceHandle* handle = static_cast<BlobResourceHandle*>(context);
+    if (handle->client())
+        handle->client()->didFinishLoading(handle, 0);
+}
+
 void BlobResourceHandle::notifyFinish()
 {
-    if (client())
-        client()->didFinishLoading(this, 0);
+    if (m_async) {
+        // Schedule to notify the client from a standalone function because the client might dispose the handle immediately from the callback function
+        // while we still have BlobResourceHandle calls in the stack.
+        callOnMainThread(doNotifyFinish, this);
+        return;
+    }
+
+    doNotifyFinish(this);
 }
 
 } // namespace WebCore
index 63e8578..1e9e94a 100644 (file)
@@ -69,9 +69,12 @@ public:
     int readSync(char*, int);
 
 private:
+    friend void delayedStartBlobResourceHandle(void*);
+
     BlobResourceHandle(PassRefPtr<BlobStorageData>, const ResourceRequest&, ResourceHandleClient*, bool async);
     virtual ~BlobResourceHandle();
 
+    void doStart();
     void getSizeForNext();
     void seek();
     void consumeData(const char* data, int bytesRead);
index b0912d3..8ad66e4 100644 (file)
@@ -173,7 +173,7 @@ public:
 
     bool hasAuthenticationChallenge() const;
     void clearAuthentication();
-    void cancel();
+    virtual void cancel();
 
     // The client may be 0, in which case no callbacks will be made.
     ResourceHandleClient* client() const;