WebKit Bugzilla
New
Browse
Search+
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
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-
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
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
Committed as
http://trac.webkit.org/changeset/66452
.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug