Summary: | Switch the Blob implementation to using the blob data registration model | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jian Li <jianli> | ||||
Component: | WebCore JavaScript | Assignee: | 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
Jian Li
2010-08-21 18:55:38 PDT
Created attachment 65043 [details]
Proposed Patch
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
(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. http://trac.webkit.org/changeset/66452 might have broken Qt Linux Release minimal, Qt Linux ARMv5 Release, and Qt Linux ARMv7 Release Committed as http://trac.webkit.org/changeset/66452. |