WebKit Bugzilla
New
Browse
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
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-
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
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
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