[Webkit-unassigned] [Bug 43151] Add WebKit API for HTML5 FileSystem API

[bugzilla-daemon at webkit.org]

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


Darin Fisher (:fishd, Google) <fishd at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #63011|review+                     |review-
               Flag|                            |




--- Comment #6 from Darin Fisher (:fishd, Google) <fishd at chromium.org>  2010-07-29 21:39:09 PST ---
(From update of attachment 63011)
WebKit/chromium/public/WebAsyncFileSystem.h:50
 +          SuccessError = 0,
nit: in the WebKit API we use the following format for enums:

  enum Foo {
      FooBar,
      FooBaz,
  };

So, these should be re-worded like so:

  enum Error {
      ErrorNone,
      ErrorNoModificationAllowed,
      ErrorNotFound,
      ...
  };

One more note:  It seems a bit odd for these error codes, which are generic
file system error codes, to be defined here only for use with WebAsyncFileSystem.
We also have WebFileSystem, and it seems like these could be useful for it as
well.  In fact, it seems like we should try to unify WebFileSystem and WebAsyncFileSystem
so that they expose similar APIs, with the exception of WebFileSystem containing only
synchronous methods whereas WebAsyncFileSystem contains asynchronous methods.
how about just merging these asynchronous methods into WebFileSystem?

WebKit/chromium/public/WebAsyncFileSystem.h:71
 +          virtual ~Callbacks() { }
can this destructor be made protected?  i'm assuming that the
embedder who receives a Callbacks pointer is not expected to
delete it, so making it protected would enforce that.

WebKit/chromium/public/WebAsyncFileSystem.h:86
 +      virtual void getFile(const WebString& path, GetFileFlags flags, Callbacks*) { WEBKIT_ASSERT_NOT_REACHED(); }
it is not clear what getFile or getDirectory do.  what result is
returned to Callbacks?

WebKit/chromium/public/WebAsyncFileSystem.h:81
 +  
it would be very helpful to document how each of these methods is expected
to work.  what callbacks get called and with what information?

WebKit/chromium/public/WebAsyncFileSystem.h:80
 +      virtual void requestFileSystem(bool persistent, long long size, const WebSecurityOrigin&, WebFrame*, Callbacks*) { WEBKIT_ASSERT_NOT_REACHED(); }
this seems like a function that should be on WebFrameClient since it is
specific to a WebFrame.  policy functions generally go on WebFrameClient.
The WebSecurityOrigin can be retrieved from the WebFrame, so it does not
need to be an explicit parameter.  since requestFileSystem is an async
method, you could just have a method on WebFrame that is called to complete
the filesystem request.  you'd need to have some way of identifying the
filesystem request.  or, you could define a separate callback interface
for just this one thing (e.g., like WebFileChooserCompletion).

WebKit/chromium/public/WebAsyncFileSystem.h:64
 +          bool create;
how about just expressing this as a bit-mask?  then define enums for it?

WebKit/chromium/public/WebKitClient.h:268
 +      virtual WebAsyncFileSystem* asyncFileSystem() { return 0; }
the name of this method suggests that a WebAsyncFileSystem is not created
each time this method is called.  so, you should probably make WebAsyncFileSystem's
destructor protected so that it is clear that WebKit code should not be deleting
a WebAsyncFileSystem pointer.

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