Bug 105279 – Web Inspector: Document* should be used directly in WebSocket Instrumentation

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[pdeng6]

In inspectorInstrumentation.h,
first arg of some WebSocket instrumentation functions are ScriptExecutionContext, (didCreateWebSocket, willSendWebSocketHandshakeRequest, didReceiveWebSocketHandshakeResponse, didCloseWebSocket), however they are all feed with Document* in their client (WebSocketChannel.cpp) accurately, and handled as Document* again in instrumentingAgentsForContext(). 
in addition, if Document* is adopted as arg, more info can be accessed, frame info for example.

[pdeng6]

Created attachment 179924 [details]
Patch

[Pavel Feldman]

What about web sockets in web workers?

[pdeng6]

(In reply to comment #2)
> What about web sockets in web workers?
Nice consideration, thanks a lot for your comments :)
Actually, "WebSocketChannel" is the backend of "WorkerThreadableWebSocketChannel", and  ScriptExecutionContext* in the former class is always a Document* rather than WorkerContext* (even when play as a backend of worker)

add Toyoshima-san to review it.

thanks
Pan

[Takashi Toyoshima]

(In reply to comment #3)
> Actually, "WebSocketChannel" is the backend of "WorkerThreadableWebSocketChannel", and  ScriptExecutionContext* in the former class is always a Document* rather than WorkerContext* (even when play as a backend of worker)

Right. In the case of Workers, WebSocket objects in the workers have proxy objects in the main thread respectively. The proxy objects always own WebSocketChannel which has Document object as a ScriptExecutionContext.

AFAIK, Inspector code exists in this WebSocketChannel. So, Document object is always available for these interfaces.

[Takashi Toyoshima]

By the way, currently DevTools doesn't handle WebSocket objects in SharedWorkers and (dedicated) Workers seemingly, right?
Also DevTools for Workers doesn't contain Network tab for now. I guess WebSocket is the only object which uses networking in Workers now.

[Takashi Toyoshima]

(In reply to comment #5)
> By the way, currently DevTools doesn't handle WebSocket objects in SharedWorkers and (dedicated) Workers seemingly, right?
> Also DevTools for Workers doesn't contain Network tab for now. I guess WebSocket is the only object which uses networking in Workers now.

https://bugs.webkit.org/show_bug.cgi?id=105419
I filed a bug though I don't have time to work on this for now.

[Pavel Feldman]

> Right. In the case of Workers, WebSocket objects in the workers have proxy objects in the main thread respectively. The proxy objects always own WebSocketChannel which has Document object as a ScriptExecutionContext.

So you are saying that having main thread in the tight loop will prevent workers from dispatching the data? Is this a short term thing or will it be so long term?

[Takashi Toyoshima]

(In reply to comment #7)
> So you are saying that having main thread in the tight loop will prevent workers from dispatching the data? Is this a short term thing or will it be so long term?

Yes. We have no plan to change this design.
In the design was not changed, Chrome port handles all network requests in an IO thread in a browser process. So, distributing requests from workers to threads looks very difficult and could be essential change.

http://www.chromium.org/developers/design-documents/multi-process-resource-loading

[Takashi Toyoshima]

(In reply to comment #8)
> In the design was not changed,

Sorry, it means 'If the design was not changed,'

[WebKit Review Bot]

Comment on attachment 179924 [details]
Patch

Clearing flags on attachment: 179924

Committed r138163: <http://trac.webkit.org/changeset/138163>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.