Summary: | Remove unnecessary parameter from AsyncFileWriterClient::didTruncate | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eric U. <ericu> | ||||||
Component: | DOM | Assignee: | Eric U. <ericu> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Enhancement | CC: | commit-queue, kinuko, michaeln | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 44358 | ||||||||
Attachments: |
|
Description
Eric U.
2010-09-23 12:34:42 PDT
Created attachment 68582 [details]
Patch
Comment on attachment 68582 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=68582&action=review Just a few things to address. > WebCore/fileapi/FileWriter.cpp:@ > void FileWriter::truncate(long long posi Please init m_truncateLength to 0 in constructor. > WebCore/fileapi/FileWriter.cpp:174 > + ASSERT(m_truncateLength >= 0); Why are there both of these asserts? Should they also be in FileWriter::truncate when m_truncateLength is set? > WebCore/fileapi/FileWriter.cpp:178 > m_readyState = DONE; Set m_truncateLength be set back to 0 here. > WebKit/chromium/src/AsyncFileWriterChromium.h:-64 > - virtual void didTruncate(long long length); What code calls this api? (I don't see a change to it.) Created attachment 68605 [details]
Patch
(In reply to comment #2) > (From update of attachment 68582 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=68582&action=review > > Just a few things to address. > > > WebCore/fileapi/FileWriter.cpp:@ > > void FileWriter::truncate(long long posi > > Please init m_truncateLength to 0 in constructor. I used -1, as 0 is a legal value. > > WebCore/fileapi/FileWriter.cpp:174 > > + ASSERT(m_truncateLength >= 0); > > Why are there both of these asserts? Typo--fixed. However, the spec has changed, and now you can use truncate to extend the file as well, so I've rolled that in to this change. > Should they also be in FileWriter::truncate when m_truncateLength is set? Actually, there should be a soft error there [a thrown exception], so that we can assert here. Fixed. > > WebCore/fileapi/FileWriter.cpp:178 > > m_readyState = DONE; > > Set m_truncateLength be set back to 0 here. Done [-1]. > > WebKit/chromium/src/AsyncFileWriterChromium.h:-64 > > - virtual void didTruncate(long long length); > > What code calls this api? (I don't see a change to it.) I haven't written it yet ;'>. My Chromium implementation is starting at http://codereview.chromium.org/3440021/show, and the WebCore implementation will follow shortly thereafter. Comment on attachment 68605 [details] Patch Clearing flags on attachment: 68605 Committed r68242: <http://trac.webkit.org/changeset/68242> All reviewed patches have been landed. Closing bug. |