[webkit-reviews] review granted: [Bug 43941] Handle blob resource : [Attachment 64976] Proposed Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Aug 20 14:43:41 PDT 2010


Darin Fisher (:fishd, Google) <fishd at chromium.org> has granted Jian Li
<jianli at chromium.org>'s request for review:
Bug 43941: Handle blob resource
https://bugs.webkit.org/show_bug.cgi?id=43941

Attachment 64976: Proposed Patch
https://bugs.webkit.org/attachment.cgi?id=64976&action=review

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
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


More information about the webkit-reviews mailing list