[Webkit-unassigned] [Bug 45808] Add Worker support for FileSystem API

[bugzilla-daemon at webkit.org]

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





--- Comment #8 from Kinuko Yasuda <kinuko at chromium.org>  2010-09-15 15:28:30 PST ---
(In reply to comment #3)
> +    ~LocalFileSystem() { }
> If the intention is for the destructor to never be called, then it shouldn't be defined.

Removed.

> +    // FIXME: See if access is allowed.
> This is a scary FIXME to have in code!

Added access and availability checks.

> +COMPILE_ASSERT(int(WorkerContext::TEMPORARY) == int(AsyncFileSystem::Temporary), enum_mismatch);
> Coding style says that we should use static_cast, not C-style casts.

Fixed.

> +        // They are placed here and in all capital letters to enforce compile-time enum checking.
> How do capital letters help compile time enum checking?

If there're constant attributes in idl the code generator automatically generates enum checking code with the enum values (unless we specify DontCheckEnums).

> I haven't reviewed the substance of the patch, largely because of naming issues that were making reading too hard. Specifically, "requestFileSystem()" has taken me aback, as a file system is not something I'd expect to be requested.

The names LocalFileSystem and requestFileSystem are taken from its spec:
http://dev.w3.org/2009/dap/file-system/file-dir-sys.html#using-localfilesystem

In terms of class/file naming there're too many *FileSystem classes and I admit it might be hard to read -- DOMFileSystem corresponds to JavaScript's FileSystem interface in the FileSystem API and AsyncFileSystem is a platform-dependent lower layer implementation for the filesystems.

If there're any feedbacks I could do to improve the design / readability I'm glad to change the code.
Thanks,

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