2009/6/26 John Abd-El-Malek <j...@chromium.org> > > > 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. >> >> > 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. >
Ah, Ok. I updated WebKit API part not to use WebCore 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. >> > > hmm, I actually find WebWeb very unwieldy. I vote for > WebKit::WebSocketHandle. > Ok. will use WebKit:;WebSocketHandle. Thanks! ukai > >> >> >>> * 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 -~----------~----~----~----~------~----~------~--~---