RESOLVED FIXED 44389
Switch the Blob implementation to using the blob data registration model
https://bugs.webkit.org/show_bug.cgi?id=44389
Summary Switch the Blob implementation to using the blob data registration model
Jian Li
Reported Sunday, August 22, 2010 2:55:38 AM UTC
We need to update the Blob implementation in order to switch to the blob data registration model.
Attachments
Proposed Patch (117.15 KB, patch)
2010-08-21 19:29 PDT, Jian Li
fishd: review+
jianli: commit-queue-
Jian Li
Comment 1 Sunday, August 22, 2010 3:29:34 AM UTC
Created attachment 65043 [details] Proposed Patch
Darin Fisher (:fishd, Google)
Comment 2 Sunday, August 22, 2010 5:17:22 AM UTC
Comment on attachment 65043 [details] Proposed Patch 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
David Levin
Comment 3 Sunday, August 22, 2010 7:39:35 AM UTC
(In reply to comment #2) > (From update of attachment 65043 [details]) > WebCore/platform/network/FormData.cpp:222 > + name = name.replace("-", ""); // For safty, remove '-' from the filename snce some servers may not like it. > snce -> since safty -> safety as well.
WebKit Review Bot
Comment 4 Tuesday, August 31, 2010 5:53:50 AM UTC
http://trac.webkit.org/changeset/66452 might have broken Qt Linux Release minimal, Qt Linux ARMv5 Release, and Qt Linux ARMv7 Release
Jian Li
Comment 5 Tuesday, August 31, 2010 7:30:52 AM UTC
Note You need to log in before you can comment on or make changes to this bug.