[Webkit-unassigned] [Bug 43134] Add idl and mock implementation for HTML5 FileSystem API

[bugzilla-daemon at webkit.org]

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





--- Comment #5 from Kinuko Yasuda <kinuko at chromium.org>  2010-07-28 21:19:27 PST ---
Thanks for your careful review!  I fixed the issues you pointed out.

(In reply to comment #3)
> (From update of attachment 62864 [details])
> i think you might need to modify JSValueToNative in CodeGeneratorV8.pm to make the patch build in chromium. for [Callback] parameters, i think you would want to return "$value".

I have another proposing patch (issue 43130 - I've cc'ed you) that hopefully fixes this.  (I'll look for alternative solution if it's not the right solution.)

> WebCore/bindings/scripts/CodeGenerator.pm:363
>  +      $ret =~ s/^eXCLUSIVE/exclusiveFlag/ if $ret =~ /^eXCLUSIVE$/;
> i think isCreate() or isCreateFlag() would be a better name for a boolean getter. same for exclusiveFlag(). also, you should add a test for these cases to WebCore/bindings/scripts/test/TestObj.idl, and change the expected outputs in WebCore/bindings/scripts/test/{JS|V8|...}/. Please make sure WebKitTools/Scripts/run-bindings-tests passes.
> 
> WebCore/storage/DOMFileSystem.cpp:35
>  +  #include "DOMFileSystem.h"
> i believe the common pattern is:
> 
> #include "config.h"
> #include "DOMFileSystem.h"
> 
> #if ENABLE(FILE_SYSTEM)
> 
> WebCore/storage/DOMFileSystem.cpp:38
>  +  #include "EntryCallback.h"
> looks like this include is not yet needed.
> 
> WebCore/storage/DOMFileSystem.cpp:39
>  +  #include "ErrorCallback.h"
> ditto.
> 
> WebCore/storage/DOMFileSystem.h:44
>  +  class ErrorCallback;
> these 2 classes don't need to be forward-declared just yet.
> 
> WebCore/storage/DOMFileSystem.h:45
>  +  class ScriptExecutionContext;
> ditto.
> 
> WebCore/storage/Entry.cpp:34
>  +  #include "Entry.h"
> same comment about moving this outside #if ENABLE(FILE_SYSTEM)
> 
> WebCore/storage/Entry.cpp:50
>  +          m_name = fullPath.substring(index);
> should there be a "else m_name = fullPath;" here?
> 
> WebCore/storage/Entry.h:34
>  +  #include "DOMFileSystem.h"
> either remove this include, or the forward-declaration below.
> 
> WebCore/storage/ErrorCallback.h:43
>  +  class ErrorCallback : public ThreadSafeShared<ErrorCallback> {
> this callback is ThreadSafeShared, and all other callbacks are RefCounted. is this intended?

Good catch... it was a mistake.

> WebCore/storage/Flags.h:50
>  +      void setEXCLUSIVE(bool exclusive) { m_exclusive = exclusive; }
> you should modify WK_ucfirst in CodeGenerator.pm the same way you modified WK_lcfirst, and convert setCREATE() and setEXCLUSIVE() to setCreate() and setExclusive().
> 
> WebCore/storage/Metadata.h:53
>  +      double m_modificationTime;
> empty line before this field.

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