Bug 44362

Summary: Implement the DOM FileWriter class
Product: WebKit Reporter: Eric U. <ericu>
Component: DOMAssignee: Eric U. <ericu>
Status: RESOLVED FIXED    
Severity: Enhancement CC: commit-queue, fishd, jianli, kinuko, levin, michaeln, mike
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 44358    
Attachments:
Description Flags
Implement html/FileWriter
none
Patch
fishd: review+, fishd: commit-queue-
Fixed whitespace, pulled extra entry out of vcproj none

Eric U.
Reported 2010-08-20 14:45:32 PDT
This bug is for fleshing out html/FileWriter; it will depend on an abstract class which will be implemented in a separate bug.
Attachments
Implement html/FileWriter (20.81 KB, patch)
2010-08-20 15:17 PDT, Eric U. 		no flags
DetailsFormatted DiffDiff
Patch (26.94 KB, patch)
2010-08-26 16:33 PDT, Eric U. 		fishd: review+
fishd: commit-queue-
DetailsFormatted DiffDiff
Fixed whitespace, pulled extra entry out of vcproj (26.72 KB, patch)
2010-08-27 14:54 PDT, Eric U. 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Eric U.
Comment 1 2010-08-20 15:17:59 PDT
Created attachment 65000 [details] Implement html/FileWriter This implements FileWriter, but delegates the hard work to the forthcoming implementation of AsyncFileWriter, which is abstract.
Kinuko Yasuda
Comment 2 2010-08-21 00:01:50 PDT
I'm not a reviewer and I haven't looked into it carefully yet, but here're some first comments. WebCore/html/FileWriter.cpp:45 + , m_length(length) Don't we want to initialize m_readyState as well? WebCore/html/FileWriter.cpp:76 + ec = INVALID_STATE_ERR; I think we need to save the error value to m_error to return the last error for FileWriter.error attribute. WebCore/html/FileWriter.cpp:134 + DCHECK_GT(bytes, 0); I don't think we have DCHECK_XXX stuff in WebKit. (ASSERT(...) is used everywhere in WebKit) WebCore/html/FileWriter.cpp:170 + return; In what case does this happen? WebCore/html/FileWriter.h:69 + FileError* error() const { return m_error; }; m_error.get()? WebCore/html/FileWriter.h:109 + RefPtr<FileError*> m_error; '*' is not necessary. WebCore/html/FileWriter.h:111 + RefPtr<AsyncFileWriter*> m_writer; ditto. WebCore/html/FileWriter.cpp:58 + return m_readyState == WRITING || ActiveDOMObject::hasPendingActivity(); If we want to keep an instance while its hasPendingActivity() returns true, we need to add the class name to WebCore/bindings/scripts/CodeGeneratorV8.pm::IsActiveDomType() for v8. Otherwise it gets GC'ed even it is defined as a subclass of ActiveDOMObject.
Kinuko Yasuda
Comment 3 2010-08-21 00:26:02 PDT
WebCore/html/FileWriter.cpp:39 + FileWriter::FileWriter(ScriptExecutionContext* context, AsyncFileWriter* writer, const String& path, long long length) Does this assume the creator/caller of FileWriter holds the AsyncFileWriter's reference while the writer is writing?
Eric U.
Comment 4 2010-08-23 13:58:45 PDT
Comment on attachment 65000 [details] Implement html/FileWriter This patch needs a lot of work; I'm retracting it and will repost soon.
Eric U.
Comment 5 2010-08-26 16:33:45 PDT
Created attachment 65644 [details] Patch
Darin Fisher (:fishd, Google)
Comment 6 2010-08-27 14:42:35 PDT
Comment on attachment 65644 [details] Patch WebCore/html/FileWriterClient.h:54 + nit: looks like there are some extra new lines here that should be nuked. R=me
Eric U.
Comment 7 2010-08-27 14:54:26 PDT
Created attachment 65771 [details] Fixed whitespace, pulled extra entry out of vcproj I removed those two blank lines, and I also removed an entry for FileWriterCallback from the vcproj file--that snuck in from the next changelist. Everything else is the same.
WebKit Commit Bot
Comment 8 2010-08-28 01:49:28 PDT
Comment on attachment 65771 [details] Fixed whitespace, pulled extra entry out of vcproj Clearing flags on attachment: 65771 Committed r66303: <http://trac.webkit.org/changeset/66303>
WebKit Commit Bot
Comment 9 2010-08-28 01:49:35 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.