Bug 46390

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:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

44358

Attachments:

Description	Flags
Patch	none
Patch	none

Eric U.

Reported 2010-09-23 12:34:42 PDT

The FileWriter knows the length it sent to truncate, and we're never going to get the wrong answer back. We might as well cache what we sent rather than have an extra parameter through 10 layers of calls. It's going to get cached at some level anyway; it might as well be right in the FileWriter.

Attachments
Patch (5.27 KB, patch) 2010-09-23 13:39 PDT, Eric U.	no flags	Details Formatted Diff Diff
Patch (5.86 KB, patch) 2010-09-23 15:20 PDT, Eric U.	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Eric U.

Comment 1 2010-09-23 13:39:29 PDT

Created attachment 68582 [details] Patch

David Levin

Comment 2 2010-09-23 14:39:31 PDT

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.)

Eric U.

Comment 3 2010-09-23 15:20:58 PDT

Created attachment 68605 [details] Patch

Eric U.

Comment 4 2010-09-23 15:24:52 PDT

(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.

WebKit Commit Bot

Comment 5 2010-09-24 01:15:28 PDT

Comment on attachment 68605 [details] Patch Clearing flags on attachment: 68605 Committed r68242: <http://trac.webkit.org/changeset/68242>

WebKit Commit Bot

Comment 6 2010-09-24 01:15:33 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.