[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 #29 from Kinuko Yasuda <kinuko at chromium.org>  2010-09-21 20:00:12 PST ---
Thanks for the iteration.

(In reply to comment #25)
> (From update of attachment 68211 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=68211&action=review
> 
> > WebCore/platform/AsyncFileSystem.cpp:37
> > +#include "NotImplemented.h"
> Out of order. (I wonder why the style bot didn't catch this. Maybe the #if ENABLE messed it up.)

Fixed.

> > WebKit/chromium/src/WorkerFileSystemCallbacksBridge.cpp:65
> > +        MutexLocker locker(m_mutex);
> This doesn't work because didFailOnMainThread may have just been called and posted a task to the Worker thread.
> Here's a solution.  Do the following:

Fixed (not exactly as mentioned, but tried to follow the way).

Turned WorkerFileSystemCallbacksBridge into ThreadSafeShared and changed its post methods to use mutex protected m_worker.
No wrapper class for worker's callback, and is touched only on the worker thread.

> > WebKit/chromium/src/WorkerFileSystemCallbacksBridge.cpp:142
> > +    PassRefPtr<ThreadableCallbacksBridgeWrapper> m_bridgeWrapper;
> PassRefPtr should only be used for function parameters. This should be RefPtr.

Gush, fixed.

> > WebKit/chromium/src/WorkerFileSystemCallbacksBridge.h:45
> > +class ThreadableCallbacksBridgeWrapper;
> Out of order.
> > WebKit/chromium/src/WorkerFileSystemCallbacksBridge.h:48
> > +class WebWorkerBase;
> Out of order.

Fixed.

> > WebKit/chromium/src/WorkerFileSystemCallbacksBridge.h:63
> > +class WorkerFileSystemCallbacksBridge : public WebCore::ActiveDOMObject, public RefCounted<WorkerFileSystemCallbacksBridge> {
> 
> Interesting idea about the ActiveDOMObject. I've discussed it with dimich and he indicated that you get a lot of extra stuff that you may not want when you have an ActiveDOMObject.
> 
> There are two ideas:
> 1. If this could be owned by a JS class.
> 2. If that isn't possible, then perhaps adding a callback to ~WorkerContext. You can find several similar things in ~ScriptExecutionContext (WebKit/WebCore/dom/ScriptExecutionContext.cpp)

Added a WorkerContext::Observer class whose instances are notified when the WorkerThread is stopping.  (The class might be a bit overkilling)

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