Bug 132402 - Move Blob.slice() implementation into BlobRegistryImpl
Summary: Move Blob.slice() implementation into BlobRegistryImpl
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexey Proskuryakov
Depends on:
Reported: 2014-04-30 14:21 PDT by Alexey Proskuryakov
Modified: 2014-04-30 15:53 PDT (History)
1 user (show)

See Also:

proposed patch (21.44 KB, patch)
2014-04-30 14:49 PDT, Alexey Proskuryakov
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 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.
Comment 1 Alexey Proskuryakov 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.
Comment 2 Alexey Proskuryakov 2014-04-30 14:49:16 PDT
Created attachment 230525 [details]
proposed patch
Comment 3 WebKit Commit Bot 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.
Comment 4 Anders Carlsson 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.
Comment 5 Alexey Proskuryakov 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.
Comment 6 Alexey Proskuryakov 2014-04-30 15:53:07 PDT
Committed <http://trac.webkit.org/r168054>.