[Webkit-unassigned] [Bug 32214] Add WebSocket feature in Worker

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jan 4 17:20:43 PST 2010


David Levin <levin at chromium.org> changed:

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

--- Comment #19 from David Levin <levin at chromium.org>  2010-01-04 17:20:40 PST ---
(From update of attachment 45264)
Sorry for the delay (I was reviewing it but it is complicated and then there
was a vacation).

My biggest concern is regarding lifetime and cross thread ownership issues.
Specifically, MainThreadClient is owned by two threads, so it is
ThreadsafeShared which means the final delete may happen on any thread.
However, it contains a String which is RefCounted (and not compatible with
being deleted on any thread). It would be best if it would be owned by one
thread (like it wouldn't delete itself until a "final" message came through. If
that doesn't seem possible, then it shouldn't contain any RefCounted objects).

There are some naming things that we probably should address before this gets
checked in. (There may be other things but the ownership issue is the biggest.)

> diff --git a/WebCore/websockets/WorkerThreadableWebSocketChannel.h b/WebCore/websockets/WorkerThreadableWebSocketChannel.h
> +class WorkerThreadableWebSocketChannel : public RefCounted<WorkerThreadableWebSocketChannel>, public ThreadableWebSocketChannel {
> +    class MainThreadClient : public ThreadSafeShared<MainThreadClient>, public WebSocketChannelClient {

I know MainThreadClient and WorkThreadProxy were my suggestions, but I don't
like them that much now that I see them. Any other ideas?

> +    /* Proxy for MainThreadClient.  Running on the worker thread. */

In general WebKit uses // style comments.

> +    class WorkerThreadProxy {
> +        void callSyncMethod();

Consider setMethodNotCompleted (right now it sounds like this does the sync

> +        void waitSyncMethod();

Consider waitForMethodCompletion.

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

More information about the webkit-unassigned mailing list