RESOLVED FIXED44389
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 2010-08-21 18:55:38 PDT
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 2010-08-21 19:29:34 PDT
Created attachment 65043 [details] Proposed Patch
Darin Fisher (:fishd, Google)
Comment 2 2010-08-21 21:17:22 PDT
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 2010-08-21 23:39:35 PDT
(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 2010-08-30 21:53:50 PDT
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 2010-08-30 23:30:52 PDT
Note You need to log in before you can comment on or make changes to this bug.