[Webkit-unassigned] [Bug 37218] Implement FileStreamProxy that calls FileStream methods on FileThread for FileAPI

[bugzilla-daemon at webkit.org]

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


Jian Li <jianli at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #53009|review?                     |review-
               Flag|                            |




--- Comment #17 from Jian Li <jianli at chromium.org>  2010-04-09 16:48:56 PST ---
(From update of attachment 53009)
Only some minor things to resolve.

> diff --git a/WebCore/html/FileStreamProxy.cpp b/WebCore/html/FileStreamProxy.cpp
> +static void derefProxyOnContext(ScriptExecutionContext*, FileStreamProxy* proxy)
> +{
> +    proxy->deref();
Can we add an ASSERT before this line, like:
       ASSERT(proxy->hasOneRef());
This way, we can ensure that the caller, i.e., FileReader/FileWriter, has
already
released the reference and now it should only has ref count 1 now.

> diff --git a/WebCore/html/FileStreamProxy.h b/WebCore/html/FileStreamProxy.h
> +// A proxy module that calls corresponding FileStream methods on the file thread.  Note: you must call stop() before destroying a FileStreamProxy instance, e.g. in the desctructor of an object that holds a reference to the proxy.
Probably we could say the following for Note part:
   // ... Note: you must call stop() first and then release the reference to
the FileStreamProxy instance.

> +    // Stop() stops any pending tasks, close the file and derefs itself; the caller should not recycle the instance after calling stop().
It is not needed to repeat stop() in the comment. Also we should use "closes"
to be consistent with "stops".
Probably it is better to say "destruct".
      // Stops the proxy and schedules it to be destructed. All the pending
tasks will be aborted and the file stream will be closed.
      // Note: the caller should not recycle the instance after calling stop().

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