Bug 44971 - Fix ThreadableBlobRegistry methods not to rely on WorkerContext
Summary: Fix ThreadableBlobRegistry methods not to rely on WorkerContext
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Jian Li
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-31 12:02 PDT by Jian Li
Modified: 2010-08-31 13:58 PDT (History)
4 users (show)

See Also:


Attachments
Proposed Patch (5.48 KB, patch)
2010-08-31 12:10 PDT, Jian Li
fishd: review+
jianli: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jian Li 2010-08-31 12:02:05 PDT
Fix ThreadableBlobRegistry methods not to rely on WorkerContext. This is because WorkerContext could be destructed when we're performing cleanup in ScriptExecutionContext destructor.
Comment 1 Jian Li 2010-08-31 12:10:59 PDT
Created attachment 66082 [details]
Proposed Patch
Comment 2 David Levin 2010-08-31 13:46:30 PDT
Mid-air collision -- :)

Here's my comments. Please feel free to address and submit. (In other words, r=me also.)

> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> +2010-08-31  Jian Li  <jianli@chromium.org>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Fix ThreadableBlobRegistry methods not to rely on WorkerContext.
> +        https://bugs.webkit.org/show_bug.cgi?id=44971
> +
> +        This could happen when we're performing some blob related cleanup

"This"
What could happen?

> +        in ScriptExecutionContext destructor when WorkerContext dies. The fix
> +        is to use isMainThread and callOnMainThread.
> +

> diff --git a/WebCore/fileapi/ThreadableBlobRegistry.cpp b/WebCore/fileapi/ThreadableBlobRegistry.cpp

> +static void registerBlobURLTask(void* context)
>  {
> -    blobRegistry().registerBlobURL(url, blobData);
> +    OwnPtr<BlobRegistryContext> blobRegistryContext = adoptPtr(static_cast<BlobRegistryContext*>(context));
> +    blobRegistry().registerBlobURL(blobRegistryContext->url, blobRegistryContext->blobData.release());
>  }
>  
> -void ThreadableBlobRegistry::registerBlobURL(ScriptExecutionContext* scriptExecutionContext, const KURL& url, PassOwnPtr<BlobData> blobData)
> +void ThreadableBlobRegistry::registerBlobURL(ScriptExecutionContext*, const KURL& url, PassOwnPtr<BlobData> blobData)
>  {
> -    if (scriptExecutionContext->isWorkerContext())
> -        postTaskToMainThread(scriptExecutionContext, createCallbackTask(&registerBlobURLTask, url, blobData));
> +    if (isMainThread())
> +        blobRegistry().registerBlobURL(url, blobData);
>      else
> -        registerBlobURLTask(scriptExecutionContext, url, blobData);
> +        callOnMainThread(&registerBlobURLTask, new BlobRegistryContext(url, blobData));

Please use a create method and a leakPtr (see "[webkit-dev] Naked new considered harmful").
Comment 3 Jian Li 2010-08-31 13:58:54 PDT
Committed as http://trac.webkit.org/changeset/66528.