2009/6/25 Fumitoshi Ukai (鵜飼文敏) <[email protected]>
> Thanks for review.
>
> 2009/6/25 Michael Nordman <[email protected]>
>
>> 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.
>
> > 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?
>
> > Regarding the "WebKit API", note that no WebCore data types can be used
>> there. So you'll need to create wrapper classes.
>>
>
The WebKit API classes still derive from WebCore, which isn't possible. The
WebKit API is an abstraction around WebCore classes, so it can't use any
WebCore types in it.
>> 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.
>
hmm, I actually find WebWeb very unwieldy. I vote for
WebKit::WebSocketHandle.
>
>
>> * 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 (鵜飼文敏) <[email protected]
>> > 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: [email protected]
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~----------~----~----~----~------~----~------~--~---
