Bug 132402

Summary: Move Blob.slice() implementation into BlobRegistryImpl
Product: WebKit Reporter: Alexey Proskuryakov <ap>
Component: WebCore Misc.Assignee: Alexey Proskuryakov <ap>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
proposed patch andersca: review+

Alexey Proskuryakov
Reported 2014-04-30 14:21:34 PDT
Blob is an API object living in WebProcess that shouldn't know the details of snapshotting or size computation. That should happen on NetworkProcess side.
Attachments
proposed patch (21.44 KB, patch)
2014-04-30 14:49 PDT, Alexey Proskuryakov
andersca: review+
Alexey Proskuryakov
Comment 1 2014-04-30 14:43:43 PDT
File size tracking is tricky. At the moment, we capture the size in BlobBuilder, and also in Blob.slice(). This is inconsistent - Files that were used in these APIs are snapshotted, but Files that come from drag&drop or file chooser are not. This makes particularly little sense for BlobBuilder, because it does not need the size.
Alexey Proskuryakov
Comment 2 2014-04-30 14:49:16 PDT
Created attachment 230525 [details] proposed patch
WebKit Commit Bot
Comment 3 2014-04-30 14:50:50 PDT
Attachment 230525 [details] did not pass style-queue: ERROR: Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.h:93: The parameter name "url" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.
Anders Carlsson
Comment 4 2014-04-30 14:52:58 PDT
Comment on attachment 230525 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=230525&action=review > Source/WebCore/fileapi/ThreadableBlobRegistry.cpp:94 > if (isMainThread()) > blobRegistry().registerBlobURL(url, std::move(blobData)); I think yo should put an early return here instead of an else block. > Source/WebCore/fileapi/ThreadableBlobRegistry.cpp:112 > if (isMainThread()) > blobRegistry().registerBlobURL(url, srcURL); Same thing here. > Source/WebCore/fileapi/ThreadableBlobRegistry.cpp:127 > + unsigned long long resultSize; > + if (isMainThread()) > + resultSize = blobRegistry().registerBlobURLForSlice(newURL, srcURL, start, end); Same thing here. > Source/WebCore/fileapi/ThreadableBlobRegistry.cpp:137 > + semaphore.wait(DBL_MAX); Use std::numeric_limits<double>::max() instead.
Alexey Proskuryakov
Comment 5 2014-04-30 15:34:31 PDT
We normally do an early return for the unusual case, but here, it's the unusual (secondary thread) case that is long.
Alexey Proskuryakov
Comment 6 2014-04-30 15:53:07 PDT
Note You need to log in before you can comment on or make changes to this bug.