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.
> 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.
>
> 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
-~----------~----~----~----~------~----~------~--~---
