Bug 43941 – Handle blob resource

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Jian Li]

Need to add the support to handle blob resource.

[Jian Li]

Created attachment 64971 [details]
Proposed Patch

[WebKit Review Bot]

Attachment 64971 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/platform/network/ResourceHandleClient.h:47:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
WebCore/platform/network/ResourceHandle.cpp:32:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/platform/network/BlobResourceHandle.cpp:209:  One line control clauses should not use braces.  [whitespace/braces] [4]
WebCore/platform/network/BlobResourceHandle.cpp:237:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebCore/platform/network/BlobResourceHandle.cpp:398:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebCore/platform/network/BlobResourceHandle.cpp:443:  An else if statement should be written as an if statement when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
WebCore/platform/network/BlobResourceHandle.cpp:446:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 7 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Jian Li]

Created attachment 64972 [details]
Proposed Patch

Fix style errors.

[WebKit Review Bot]

Attachment 64972 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/platform/network/ResourceHandleClient.h:47:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
WebCore/platform/network/ResourceHandle.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 2 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Eric Seidel (no email)]

Attachment 64971 [details] did not build on mac:
Build output: http://queues.webkit.org/results/3722453

[Eric Seidel (no email)]

Attachment 64972 [details] did not build on mac:
Build output: http://queues.webkit.org/results/3719462

[Jian Li]

Created attachment 64976 [details]
Proposed Patch

Fix mac warnings.

[WebKit Review Bot]

Attachment 64976 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/platform/network/ResourceHandleClient.h:47:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
WebCore/platform/network/ResourceHandle.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 2 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Darin Fisher (:fishd, Google)]

Comment on attachment 64976 [details]
Proposed Patch

WebCore/page/SecurityOrigin.cpp:128
 +      if (url.protocolIs("blob"))
I thought we were using the "blobdata" scheme.

WebCore/platform/network/BlobResourceHandle.cpp:53
 +  static const int ok = 200;
nit: might be nice to name these http status codes with a prefix like httpOK, httpPartialContent, etc.

WebCore/platform/network/BlobResourceHandle.cpp:74
 +  class BlobResourceSynchronousLoader : public ResourceHandleClient {
can this class be in an anonymous namespace?

WebCore/platform/network/BlobResourceHandle.cpp:108
 +      m_data.resize(static_cast<size_t>(response.expectedContentLength()));
do you need to worry if expected content length is outrageously large?  it is a 64-bit value, so this could be large.  is there a security concern?

WebCore/platform/network/BlobResourceHandle.cpp:216
 +  bool BlobResourceHandle::parseRangeHeader(const String& range)
should this code be in HTTPParsers.{h,cpp}?

those are just nits, so R=me

[Jian Li]

Committed as http://trac.webkit.org/changeset/65827.

[Jian Li]

(In reply to comment #9)
> (From update of attachment 64976 [details])
> WebCore/page/SecurityOrigin.cpp:128
>  +      if (url.protocolIs("blob"))
> I thought we were using the "blobdata" scheme.
The latest version of File API uses "blob" scheme.

> 
> WebCore/platform/network/BlobResourceHandle.cpp:53
>  +  static const int ok = 200;
> nit: might be nice to name these http status codes with a prefix like httpOK, httpPartialContent, etc.
Fixed.

> 
> WebCore/platform/network/BlobResourceHandle.cpp:74
>  +  class BlobResourceSynchronousLoader : public ResourceHandleClient {
> can this class be in an anonymous namespace?
Fixed.

> 
> WebCore/platform/network/BlobResourceHandle.cpp:108
>  +      m_data.resize(static_cast<size_t>(response.expectedContentLength()));
> do you need to worry if expected content length is outrageously large?  it is a 64-bit value, so this could be large.  is there a security concern?
Yes. I've already handled it with a check right before this.

> 
> WebCore/platform/network/BlobResourceHandle.cpp:216
>  +  bool BlobResourceHandle::parseRangeHeader(const String& range)
> should this code be in HTTPParsers.{h,cpp}?
Fixed.

> 
> those are just nits, so R=me