2009/6/24 Drew Wilson <atwil...@google.com>
> I'm looking at the code in webworker_impl.cc and webworkerclient_impl.cpp,
> vs the code in WebCore/workers/WorkerMessagingProxy.cpp.
> As an example:
>
> Chrome version:
> void WebWorkerClientImpl::postMessageToWorkerContext(
> const WebCore::String& message) {
> // Worker.terminate() could be called from JS before the context is
> started.
> if (asked_to_terminate_)
> return;
>
> ++unconfirmed_message_count_;
>
> if (!WTF::isMainThread()) {
> WebWorkerImpl::DispatchTaskToMainThread(
> WebCore::createCallbackTask(&PostMessageToWorkerContextTask, this,
> message));
> return;
> }
> webworker_->postMessageToWorkerContext(
> webkit_glue::StringToWebString(message));
> }
>
> WebKit version:
> void WorkerMessagingProxy::postMessageToWorkerContext(const String&
> message)
> {
> if (m_askedToTerminate)
> return;
>
> if (m_workerThread) {
> ++m_unconfirmedMessageCount;
>
>
> m_workerThread->runLoop().postTask(MessageWorkerContextTask::create(message));
> } else
>
> m_queuedEarlyTasks.append(MessageWorkerContextTask::create(message));
> }
>
>
> The duplications become even more apparent as I evolve these APIs to
> support MessagePorts. I'm just seeing similar logic (checking for terminated
> tasks, tracking undelivered messages, and soon disentangling message ports
> and dealing with errors) that has to be updated in parallel in both trees.
>
> Not a big deal, either way. Now that the Chrome implementation is pretty
> stable, we might want to refactor the WorkerXXXXProxy APIs to see if we can
> share more code across implementations.
>
It'd be great of course if unconfirmed_message_count_
and asked_to_terminate_ weren't needed, but it's used to avoid a large
number of cross process calls (the equivalent variables in WebKit would be
in a different process). Almost all the rest of the file is converting back
and forth between WebCore and Chrome data types in the glue layer, which is
a necessary pain.
> -atw
>
> 2009/6/24 John Abd-El-Malek <j...@chromium.org>
>
>
>>
>> 2009/6/24 Drew Wilson <atwil...@chromium.org>
>>
>>> BTW, I checked in with IanH - it sounds like WebSockets are also on the
>>> Worker roadmap, so that's something to keep in mind while you iterate on
>>> your design.
>>> +1 to avoiding WebCore/loader, but also +1 to refactoring to enable as
>>> much common code as possible cross-platform - I'm looking at the Chromium
>>> worker code now, and there's a chunk of duplicated logic from the WebKit
>>> worker implementation, which is a bit of a maintenance headache.
>>>
>>
>> I'm curious, which parts of the code are you talking about?
>>
>>
>>>
>>> -atw
>>>
>>> 2009/6/24 Michael Nordman <micha...@google.com>
>>>
>>> Only skimmed thusfar as well... but from what i've seen, looks reasonable
>>>> to me.
>>>> * A version of the diagram you have in the chrome doc would be nice in
>>>> the webkit doc too.
>>>>
>>>> * Does WebSocketHandle really need to be refcounted. I know
>>>> ResourceHandle is a refcounted object and this design looks modeled off of
>>>> that (which may be why you've spec'd it this way). Unless your design
>>>> actually needs refcounting on this class, you may be able to simplify
>>>> things
>>>> without it. From the looks of it, WebSocketChannel 'owns' the
>>>> WebSocketHandle.
>>>>
>>>> > should we reuse WebCore/loader instead of adding new component?
>>>>
>>>> The loader is somewhat notorious for its complexity. I think you've made
>>>> a good decision to keep this out of there and to design the websocket
>>>> system
>>>> in a good clean modular fashion.
>>>>
>>>> > which component is responsible of web socket protocol framing? This
>>>> design assumes WebSocketChannel serializes/deserializes message in web
>>>> socket frame.
>>>>
>>>> Since WebSocketHandle is inevitably going to be platform specific, any
>>>> code you want to be shared code shouldn't be slated for that class. To the
>>>> extent the 'web socket protocol framing' can be done indepedent of the
>>>> 'platform' socket handle (which it looks like it can be), it would be a
>>>> good
>>>> thing to put it in WebSocketChannel so its shared core common code
>>>> goodness.
>>>>
>>>> > Regarding the "WebKit API", note that no WebCore data types can be
>>>> used there. So you'll need to create wrapper classes.
>>>>
>>>> I see you have speced WebKit:: wrapper classes with the same name as the
>>>> corresponding WebCore:: classes.
>>>>
>>>> I wonder if that same naming could be confusingt? The naming convention
>>>> darin has been employing would be WebKit::WebWebSocketHandle, which
>>>> certainly looks odd.
>>>>
>>>> * virtual void didReceiveData(const String& msg) {}
>>>>
>>>> Maybe rename this to channel client api to didReceiveMessage() to help
>>>> distinguish between raw 'data' being surface by the 'handle', and complete
>>>> 'messages' being surfaced by the 'channel'.
>>>>
>>>>
>>>> On Wed, Jun 24, 2009 at 2:32 AM, Fumitoshi Ukai (鵜飼文敏) <
>>>> u...@chromium.org> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> yuzo, tyoshino and I start working to implement HTML5 Web Socket and
>>>>> write design docs
>>>>>
>>>>> WebKit part: http://docs.google.com/View?id=dfm7gfvg_0fpjg22gh
>>>>> Chromium part: http://docs.google.com/View?id=dfm7gfvg_1dm97qxgm
>>>>>
>>>>> We'll send WebKit part to webkit-dev, if it looks ok.
>>>>> We'd welcome if you could give us feedback on our design docs.
>>>>>
>>>>> Thanks,
>>>>> ukai
>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>
>>> >>>
>>>
>>
>
--~--~---------~--~----~------------~-------~--~----~
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~----------~----~----~----~------~----~------~--~---
