Bug 44362

Summary: Implement the DOM FileWriter class
Product: WebKit Reporter: Eric U. <ericu>
Component: DOMAssignee: Eric U. <ericu>
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    
Description Flags
Implement html/FileWriter
fishd: review+, fishd: commit-queue-
Fixed whitespace, pulled extra entry out of vcproj none

Description Eric U. 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.
Comment 1 Eric U. 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.
Comment 2 Kinuko Yasuda 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.

 +      , m_length(length)
Don't we want to initialize m_readyState as well?

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

 +      DCHECK_GT(bytes, 0);
I don't think we have DCHECK_XXX stuff in WebKit.  (ASSERT(...) is used everywhere in WebKit)

 +          return;
In what case does this happen?

 +      FileError* error() const { return m_error; };

 +      RefPtr<FileError*> m_error;
'*' is not necessary.

 +      RefPtr<AsyncFileWriter*> m_writer;

 +      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.
Comment 3 Kinuko Yasuda 2010-08-21 00:26:02 PDT
 +  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?
Comment 4 Eric U. 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.
Comment 5 Eric U. 2010-08-26 16:33:45 PDT
Created attachment 65644 [details]
Comment 6 Darin Fisher (:fishd, Google) 2010-08-27 14:42:35 PDT
Comment on attachment 65644 [details]

nit: looks like there are some extra new lines here that should be nuked.

Comment 7 Eric U. 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.
Comment 8 WebKit Commit Bot 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>
Comment 9 WebKit Commit Bot 2010-08-28 01:49:35 PDT
All reviewed patches have been landed.  Closing bug.