Bug 44971

Summary: Fix ThreadableBlobRegistry methods not to rely on WorkerContext
Product: WebKit Reporter: Jian Li <jianli>
Component: WebCore JavaScriptAssignee: Jian Li <jianli>
Status: RESOLVED FIXED    
Severity: Normal CC: dimich, fishd, kinuko, levin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Proposed Patch fishd: review+, jianli: commit-queue-

Jian Li
Reported 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.
Attachments
Proposed Patch (5.48 KB, patch)
2010-08-31 12:10 PDT, Jian Li
fishd: review+
jianli: commit-queue-
Jian Li
Comment 1 2010-08-31 12:10:59 PDT
Created attachment 66082 [details] Proposed Patch
David Levin
Comment 2 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").
Jian Li
Comment 3 2010-08-31 13:58:54 PDT
Note You need to log in before you can comment on or make changes to this bug.