[Webkit-unassigned] [Bug 45440] [FileSystem] Add File and FileWriter accessor methods in FileEntry

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=45440





--- Comment #5 from Kinuko Yasuda <kinuko at chromium.org>  2010-09-09 17:02:55 PST ---
Thanks for your review,

I moved all the FileWriterCallback stuff in the ENABLE_FILE_WRITER guard.
They'll be included in the build target if FILE_WRITER is enabled regardless of FILE_SYSTEM flag, but it has ENABLE guard in the source code too so there should be no problem.  (Also FileWriterCallback itself does not rely on other FILE_SYSTEM files.)

Eric, yeah I was afraid you may have similar code in your env.  Feeling sorry about the duplicated effort.
In this patch I also added FileWriter.cpp and idl in CMakeLists.txt with ENABLE_FILE_WRITER guard -- was that ok?  Let me know if you had intentionally excluded them.

(In reply to comment #2)
> (From update of attachment 66999 [details])
> > diff --git a/WebCore/GNUmakefile.am b/WebCore/GNUmakefile.am
> > index 0f57413269abc4d3fce9c87aa1427903fd553860..d51bb2ccb621b657a17cc45f41b94b68036fbed4 100644
> > --- a/WebCore/GNUmakefile.am
> > +++ b/WebCore/GNUmakefile.am
> > @@ -3021,10 +3021,14 @@ webcore_built_sources += \
> 
> This needs an if ENABLE_FILE_WRITER guard.

> > +       DerivedSources/WebCore/JSFileWriterCallback.cpp \
> > +       DerivedSources/WebCore/JSFileWriterCallback.h \
> > +       WebCore/fileapi/FileWriterCallback.h \

Moved these three in the ENABLE_FILE_WRITER guard.

> > diff --git a/WebCore/WebCore.pro b/WebCore/WebCore.pro
> > index ea6e90145a76071ad62bfa0527dca7e89919870e..2310c3bc056abd2215b8f1ccca5d6d4a4eda31df 100644
> > --- a/WebCore/WebCore.pro
> > +++ b/WebCore/WebCore.pro
> > @@ -2621,6 +2621,7 @@ contains(DEFINES, ENABLE_FILE_SYSTEM=1) {
> >          fileapi/EntryArray.h \
> >          fileapi/EntryCallback.h \
> >          fileapi/ErrorCallback.h \
> > +        fileapi/FileCallback.h \
> 
> Should FileWriterCallback.h go in here too?
> Do we need a nested guard for ENABLE_FILE_WRITER=1?

Moved it in a separate (not nested) ENABLE_FILE_WRITER guard.

> > +#if ENABLE(FILE_SYSTEM)
> 
> I think you want to guard on both FILE_WRITER and FILE_SYSTEM here.

Fixed.

(In reply to comment #3)
> (From update of attachment 66999 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=66999&action=prettypatch
> 
> > WebCore/WebCore.vcproj/WebCore.vcproj:4826
> > +				<FileConfiguration
> The FileConfiguration parts should not be included because they're used to exclude the file. I think we still want the file to be included in the build target, though the feature is not enabled for Windows.

Removed the FileConfiguration parts.  (Maybe I should remove them for other FS stuff too?)

> > WebCore/fileapi/FileWriterCallback.idl:33
> > +        Conditional=FILE_SYSTEM&FILE_WRITER,
> This condition "FILE_SYSTEM&FILE_WRITER" seems not to be consistent with "ENABLE(FILE_SYSTEM)" used in FileWriterCallback.h.

Fixed.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.