Bug 44389

Summary: Switch the Blob implementation to using the blob data registration model
Product: WebKit Reporter: Jian Li <jianli>
Component: WebCore JavaScriptAssignee: Jian Li <jianli>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dimich, eric, fishd, kinuko, levin, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 51366    
Attachments:
Description Flags
Proposed Patch fishd: review+, jianli: commit-queue-

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.