2009/6/25 Fumitoshi Ukai (鵜飼文敏) <u...@chromium.org>

> Thanks for review.
>
> 2009/6/25 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.
>>
>
> Sure.  I've added a diagram in webkit part.
>
>
>> * 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.
>>
>
> Yes.  I missed to add public RefCounted<WebSocketHandle> as base class of
> WebSocketHandle.
>  Thanks.
>

I was suggesting that perhaps the WebSocketHandle does not need to be
refcounted.


>
>
>  > 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.
>>
>
> I see.
> I've one question: Web socket handshaking is also platform independent.
> Should we do the handshaking in WebSocketChannel and WebSocketHandle just
> provides almost raw TCP socket?
> Or Put handshaking in WebSocketHandle as resource loader puts HTTP in
> platform code?
>

I haven't read the web sockets spec, so I don't know what the  'handshaking'
entails?

OS level socket creation/destruction and tcp connection opening/closing
needs to be per-platform. Once you have a full-duplex connection and are
sending application 'protocol' data back and forth (message framing), that
probably wants to be common code. Which camp does the 'handshaking' fall
into?




>
>
> > 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.
>>
>
> Ok.  I follow the naming convention to be WebKit::WebWebSocketHandle.
>
>
>>  * 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'.
>>
>
> Sure. Fixed.
>
> Thanks!
> ukai
>
>
>>
>>
>> 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
-~----------~----~----~----~------~----~------~--~---

Reply via email to