[Webkit-unassigned] [Bug 72016] [Chromium] [WebSocket] export WebSocketChannel interface for plugins

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=72016





--- Comment #20 from Takashi Toyoshima <toyoshim at chromium.org>  2011-11-18 01:05:55 PST ---
(From update of attachment 115458)
View in context: https://bugs.webkit.org/attachment.cgi?id=115458&action=review

>> Source/WebKit/chromium/ChangeLog:11
>> +        exported as WebWebSocketChannel because it is the class which implement
> 
> This ChangeLog entry needs to be revised given the renaming.

Sorry. I missed it.

>> Source/WebKit/chromium/public/WebSocket.h:37
>> +namespace WebCore {
> 
> nit: In other header files, we just do this all on one line:
> 
>   namespace WebCore { class WebSocketChannel; }
> 
> I think that might be nicer here too--at least for consistency sake.

Done.

I expanded it because check-webkit-style show a error on that before.
But, now I fixed and re-ran check-webkit-style, then it pass without any error.
Sorry for bothering.

>> Source/WebKit/chromium/public/WebSocket.h:66
>> +    static WebSocket* create(WebDocument*, WebSocketClient*);
> 
> nit: you need to add WEBKIT_EXPORT to static methods so that they get exported from WebKit when it is built as a DLL (to support the component build).
> 
> nit: I recommend putting a new line above the 'create' function to separate it from the enum.
> 
> nit: Pass WebDocument as |const WebDocument&|

Done.

>> Source/WebKit/chromium/public/WebSocketClient.h:46
>> +    };
> 
> same nit: add new line after enum definition.

Done.

>> Source/WebKit/chromium/src/WebSocketImpl.cpp:62
>> +    ScriptExecutionContext* context = static_cast<PassRefPtr<Document> >(*document).get();
> 
> Hmm... too bad WebSocketChannel::create doesn't take a PassRefPtr type...
> 
> How about this instead:
> 
>   m_private = WebSocketChannel::create(PassRefPtr<Document>(document).get(), this);

Thank you for good suggestion.
That work fine and looks better than before.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.