[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 #52761|review?                     |review-
               Flag|                            |




--- Comment #4 from Jian Li <jianli at chromium.org>  2010-04-07 11:57:03 PST ---
(From update of attachment 52761)
> --- /dev/null
> +++ b/WebCore/html/FileStreamProxy.cpp
> +#include "FileThread.h"
> +#include "FileThreadTask.h"
This include should be placed above so that it will be in the alphabetical
order.

> FileStreamProxy::~FileStreamProxy()
> {
>     if (fileThread())
>         fileThread()->unscheduleTasks(m_stream.get());
Is it possible that m_context is destructed before FileStreamProxy instance?

> +void FileStreamProxy::read(char* buffer, int length)
> +{
> +    fileThread()->postTask(createFileThreadTask(m_stream.get(), &FileStream::read, buffer, length));
> +}
> +
> +void FileStreamProxy::write(Blob* blob, long long position, size_t length)
We should use either size_t or int for length in read() and write(), for
consistency.

> +FileThread* FileStreamProxy::fileThread()
> +{
> +    ASSERT(m_context->fileThread()); // FIXME
What is this FIXME about? 
> +    return m_context->fileThread();
> +}

> +void FileStreamProxy::abort()
> +{
> +    if (fileThread())
Do we really need to check for fileThread()?
> +        fileThread()->unscheduleTasks(m_stream.get());
> +}

> +static void notifyWriteOnContext(ScriptExecutionContext* context, FileStreamProxy* stream, int nwritten)
Better to name it as bytesWritten.

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