[webkit-reviews] review granted: [Bug 44389] Switch the Blob implementation to using the blob data registration model : [Attachment 65043] Proposed Patch

[bugzilla-daemon at webkit.org]

Darin Fisher (:fishd, Google) <fishd at chromium.org> has granted Jian Li
<jianli at chromium.org>'s request for review:
Bug 44389: Switch the Blob implementation to using the blob data registration
model
https://bugs.webkit.org/show_bug.cgi?id=44389

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

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
WebCore/html/Blob.cpp:93
 +	    static_cast<const File*>(this)->captureSnapshot(size,
modificationTime);
nit: add a FIXME about eliminating this synchronous file operation perhaps?

WebCore/html/Blob.h:66
 +	virtual unsigned long long size() const { return static_cast<long
long>(m_size); }
nit: looks like this static_cast shouldn't be necessary since m_size is type
"long long"

WebCore/html/BlobBuilder.cpp:88
 +	    file->captureSnapshot(snapshotSize, snapshotModificationTime);
same comment about snapshotting asynchronously one day.

WebCore/html/File.cpp:74
 +	// come up with an exception to throw if file size is not represetable.

represetable -> representable

WebCore/html/File.cpp:86
 +	if (!getFileSize(m_path, snapshotSize) ||
!getFileModificationTime(m_path, modificationTime)) {
it would be nice to combine these into a single file system call.  that way in
chromium, it will be only a single synchronous IPC.  this can be deferred to
another patch of course.

WebCore/html/File.h:69
 +	void captureSnapshot(long long& snapshotSize, double&
snapshotModificationTime) const;
please put a warning comment about the fact that this does a synchronous file
operation.
we want people to think twice before calling this function.

WebCore/html/FormDataList.h:56
 +	class Item {
nit: nicer to put inner class definitions at the top of the public section?

WebCore/platform/network/FormData.cpp:143
 +		 break;
indentation

WebCore/platform/network/FormData.cpp:222
 +			name = name.replace("-", ""); // For safty, remove '-'
from the filename snce some servers may not like it.
snce -> since

those are just nits, so R=me