[webkit-reviews] review denied: [Bug 78459] [Chromium] [WebSocket] provide WebFrameClient with a chance of accessing to opening WebSocketStreamHandle : [Attachment 131441] rename some methods to have dispatch prefix, too

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Mar 13 16:11:11 PDT 2012


Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied Takashi Toyoshima
<toyoshim at chromium.org>'s request for review:
Bug 78459: [Chromium] [WebSocket] provide WebFrameClient with a chance of
accessing to opening WebSocketStreamHandle
https://bugs.webkit.org/show_bug.cgi?id=78459

Attachment 131441: rename some methods to have dispatch prefix, too
https://bugs.webkit.org/attachment.cgi?id=131441&action=review

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=131441&action=review


> Source/WebCore/Modules/websockets/WebSocketChannel.cpp:277
> +	 frame->loader()->client()->dispatchWillOpenSocketStream(handle);

nit: indentation

nit: Document::frame() is a very cheap inline getter.  Better to just do:

   if (m_document->frame())
       m_document->frame()->loader()->client()->dispatch...

> Source/WebKit/chromium/public/WebFrameClient.h:392
> +    virtual void dispatchWillOpenSocketStream(WebSocketStreamHandle*) { }

on WebFrameClient, we drop the "dispatch" prefix.  that prefix is for
FrameLoaderClient methods.  it is telling the FrameLoaderClient to 
dispatch the "Foo" event, so if FrameLoaderClient has dispatchFoo,
then WebFrameClient should have a "foo" method.


More information about the webkit-reviews mailing list