[Webkit-unassigned] [Bug 44732] Add DOMFileSystem implementation to support Entry manipulation operations

[bugzilla-daemon at webkit.org]

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





--- Comment #7 from Darin Fisher (:fishd, Google) <fishd at chromium.org>  2010-08-29 14:40:40 PST ---
(From update of attachment 65845)
> WebCore/platform/AsyncFileSystemCallbacks.h:46
> +    virtual void didSucceed(bool fireImmediate = true) = 0;
i guess the fireImmediate parameter exists because these callbacks can sometimes
be run synchronously from the AsyncFileSystem methods?  maybe that should not be
the case?  it'd be nice if this layer promised not to callback synchronously.

> :184
> +        m_asyncFileSystem->createDirectory(absolutePath, flags->isExclusive(), new EntryCallbacks(successCallback, errorCallback, scriptExecutionContext(), this, absolutePath, true));
since you need to allocate EntryCallbacks with exactly the same arguments in both cases here, how about allocating them once above the if statement?

  EntryCallbacks* callbacks = new EntryCallbacks(...);
  if (flags && flags->isCreate())
      m_asyncFileSystem->createDirectory(..., callbacks);
  else
      m_asyncFileSystem->directoryExists(..., callbacks);

That should improve readability a bit.  It'd also be nice to do this anywhere else that it is applicable.

> :83
> +    class DispatchCallbackTask : public ScriptExecutionContext::Task {
some of this machinery for callbacks seems like it is not specific to DOMFileSystem.
is it the case that we don't have code like this elsewhere in webcore?  maybe it is
a FIXME to break this out into a generic place?

i haven't had a chance to fully review this patch.  this is just some early feedback.

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