Bug 34565 - Web Inspector: Add support for Web Sockets
Summary: Web Inspector: Add support for Web Sockets
Status: RESOLVED DUPLICATE of bug 167520
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Yuta Kitamura
URL:
Keywords: InRadar
Depends on: 38831 40768 40945 105419
Blocks:
  Show dependency treegraph
 
Reported: 2010-02-04 00:43 PST by Yuta Kitamura
Modified: 2017-04-05 16:35 PDT (History)
16 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Yuta Kitamura 2010-02-04 00:43:09 PST
Web Inspector needs to support Web Sockets to help Web Socket application development.
Comment 1 Patrick Mueller 2010-02-04 04:57:14 PST
What kind of support?

I suppose it should show open sockets, and who they're connected to.  Should it track bytes sent, time last message sent, contents of last message?  Where should this information be presented?  Timeline is the first thing I thought of, in terms of where the information should be shown.
Comment 2 Pavel Feldman 2010-02-04 05:04:03 PST
<Copying message I was sending to Yuta offline>

There are two places in the inspector UI that should host websockets-related information: Resources tab and Timeline tab.

1. Resources tab

WebSocket should be an additional resource type. We have two options here: (a) display a row per socket or (b) display a row per socket 'activity'. In case of (a), it is shown as a single named entry with bar chunks reflecting each activity. When selected, web socket displays its metainformation in the headers tab as well as the raw data for all chunks in contents tab. In the (b) case, each socket activity is a row with socket's meta information and contents. I would expect user to like  (a) which in fact is more work since we have no support for 'chunks' in the frontend yet.

There should be a WebSocket filter there (as there is for XHR and other types) so that you would be able to see only sockets information. And we should come up with visual representation of the socket status (open/closed/etc.)

2. Timeline tab

That's where all the native instrumentation goes. It is really easy to provide new types of events there. It's category would be 'Loading' as for HTTP/XHR requests.

Implementation:
You should push WebSocket-related information into the WebCore::InspectorController (see ::willSendRequest, ::didReceiveResponse, ::didFinishLoading that are used for resources and ::resourceRetrievedByXMLHttpRequest that is used for XHRs). Progress::createUniqueIdentifier will create identifiers for your sockets / chunks.

That would cover the resources panel by the 'chunks' modulus. In fact if you start with the (b) approach, it would probably cover it all. [Note that we only have inspector controller instance per page, so don't think about workers now. When workers debugging is in place, we will transition seamlessly].

As for Timeline, just look at the InspectorTimelineAgent usages across the WebCore source base. You will then have good understanding of how to push timeline instrumentation events.
Comment 3 Timothy Hatcher 2010-02-04 07:06:29 PST
I agree with Pavel. And I think it should be a single entry in Resources with a chunked bar (for the Time graph at least).
Comment 4 Patrick Mueller 2010-02-04 07:26:43 PST
(In reply to comment #2)
> There are two places in the inspector UI that should host websockets-related
> information: Resources tab and Timeline tab.

That all sounds about right.  

Some comments:

- For 1., choice (b) would get confusing if you had more than one WebSocket open, I think.  Folks can get the (b) kind of display in the Timeline, right?  

- I'm a little worried about accumulating junk if we keep all the message data.  Perhaps for now, we can set some upper limit, per WebSocket.

- Perhaps for now, for the resources tab - 1.(a) , we can just it be a non-chunked bar - then you'd just see how long it was open.  Visual chunking can come later.  You can figure out the chunking by looking at the Timeline, as well as the detail when you select a WebSocket.
Comment 5 Yuta Kitamura 2010-02-04 17:35:45 PST
Thank you for comments!

(In reply to comment #4)
> Some comments:
> 
> - For 1., choice (b) would get confusing if you had more than one WebSocket
> open, I think.  Folks can get the (b) kind of display in the Timeline, right?  
> 
> - I'm a little worried about accumulating junk if we keep all the message data.
>  Perhaps for now, we can set some upper limit, per WebSocket.

Agreed. 

> 
> - Perhaps for now, for the resources tab - 1.(a) , we can just it be a
> non-chunked bar - then you'd just see how long it was open.  Visual chunking
> can come later.  You can figure out the chunking by looking at the Timeline, as
> well as the detail when you select a WebSocket.

It's reasonable plan. At first I wanted to go with 1.(b) (because of implementation difficulties), but your plan seems better. I'll go this way and give it a try.
Comment 6 Yuta Kitamura 2010-02-04 18:27:53 PST
A list of data which may possibly go into the inspector (for brainstorming):

For each Web Socket connection:
- Connection state (connecting, open, closed)
- Handshake transaction (this is actually HTTP request and response)

For each message sent or received:
- Actual message
- Message direction (send, receive)
- Message size
- (Message frame type -- currently there's only one type (UTF-8 encoded string). In the future binary message might be supported)
- Timestamp
- Location of JS function which handled the message
Comment 7 Patrick Mueller 2010-02-04 22:34:18 PST
Not sure that this is needed, but I guess if you can get that information it would be good.  This is just for received messages - or are you thinking you can capture the sender of messages as well?

> - Location of JS function which handled the message

Even though only UTF8 data is supported at the moment (my understanding), I think we may find people who decide to send binary data encoded in certain ways; for instance, sending a byte of data as one character.  Can't remember what the kids are playing with now.  Anyway, while we don't need to do this right off the bat, it's likely we'll want to be able to have a "hex" view of the data, as well as just the data displayed as a string.  In fact, even when we have support for binary data, and would presumably display that in hex, folks may want to have it displayed as a string to pick data out of it.  Something to keep in mind for the future.
Comment 8 Yuta Kitamura 2010-02-04 23:04:49 PST
(In reply to comment #7)
> Not sure that this is needed, but I guess if you can get that information it
> would be good.  This is just for received messages - or are you thinking you
> can capture the sender of messages as well?
> 
> > - Location of JS function which handled the message
> 

This is just an idea, and I'm not sure if we can get this info. Displaying this info should be optional - but I think it's good if we can make it.

I think the source code location would be useful not only for received messages but for sending messages (location where send() is called). But for now I don't have an idea about the implementation feasibility.

I believe it's easy to get the other info listed above.

> Even though only UTF8 data is supported at the moment (my understanding), I
> think we may find people who decide to send binary data encoded in certain
> ways; for instance, sending a byte of data as one character.  Can't remember
> what the kids are playing with now.  Anyway, while we don't need to do this
> right off the bat, it's likely we'll want to be able to have a "hex" view of
> the data, as well as just the data displayed as a string.  In fact, even when
> we have support for binary data, and would presumably display that in hex,
> folks may want to have it displayed as a string to pick data out of it. 
> Something to keep in mind for the future.

Yes, the current spec says the only message type is UTF-8 encoded string, but it's quite probable that binary message type will be added in the (near) future. Hex dump is a good idea - I think we must have it when the binary messages are available.
Comment 9 Yuta Kitamura 2010-02-09 21:28:01 PST
FYI: websocket-side change https://bugs.webkit.org/show_bug.cgi?id=34784
Comment 10 Yuta Kitamura 2010-02-15 23:05:47 PST
(In reply to comment #9)
> FYI: websocket-side change https://bugs.webkit.org/show_bug.cgi?id=34784

As discussed in the above bug thread, ap isn't happy with generating a ResourceRequest from a WebSocketHandshake object. He didn't want to allow the ResourceRequest to be misused based on incompatibleness between normal HTTP requests and Web Socket handshare requests.

So, I'm currently considering the following:
- Write WebSocketHandshakeRequest class that contains request information.
- Add methods that receive WebSocketHandshakeRequest to InspectorController, InspectorResource and so forth.

Dear inspector folks: How do you think about this idea? Does it sound good or bad?

For completeness' sake, we can do the same thing for handshake responses, while I think it's safe to use ResourceResponse for that purpose.
Comment 11 Pavel Feldman 2010-02-15 23:25:26 PST
(In reply to comment #10)
> (In reply to comment #9)
> > FYI: websocket-side change https://bugs.webkit.org/show_bug.cgi?id=34784
> 
> As discussed in the above bug thread, ap isn't happy with generating a
> ResourceRequest from a WebSocketHandshake object. He didn't want to allow the
> ResourceRequest to be misused based on incompatibleness between normal HTTP
> requests and Web Socket handshare requests.
> 
> So, I'm currently considering the following:
> - Write WebSocketHandshakeRequest class that contains request information.
> - Add methods that receive WebSocketHandshakeRequest to InspectorController,
> InspectorResource and so forth.
> 
> Dear inspector folks: How do you think about this idea? Does it sound good or
> bad?
> 

Inspector can operate any classes or numbers of arbitrary parameters as inputs. It is inspector's responsibility to receive signals from all over the place in various terms and convert them into a user-presentable information. It is important that you spend minimum extra cycles on preparing information for inspector and this code is guarded with #if ENABLE(INSPECTOR). Of course we need some level of abstraction, but only in order to reduce coupling and improve maintainability.

We are likely to wrap the information that you send into a native struct (such as InspectorResource), so there is really no need in intermediate representation. Anyways, if you need this WebSocketHandshakeRequest to make your design more clear, we are definitely open to receiving it as an input. Otherwise, we could receive what you already have (like 4 integers and 2 maps or something).

> For completeness' sake, we can do the same thing for handshake responses, while
> I think it's safe to use ResourceResponse for that purpose.

Symmetry is good.
Comment 12 Yuta Kitamura 2010-02-16 00:22:04 PST
(In reply to comment #11)
> Inspector can operate any classes or numbers of arbitrary parameters as inputs.
> It is inspector's responsibility to receive signals from all over the place in
> various terms and convert them into a user-presentable information. It is
> important that you spend minimum extra cycles on preparing information for
> inspector and this code is guarded with #if ENABLE(INSPECTOR). Of course we
> need some level of abstraction, but only in order to reduce coupling and
> improve maintainability.
> 
> We are likely to wrap the information that you send into a native struct (such
> as InspectorResource), so there is really no need in intermediate
> representation. Anyways, if you need this WebSocketHandshakeRequest to make
> your design more clear, we are definitely open to receiving it as an input.
> Otherwise, we could receive what you already have (like 4 integers and 2 maps
> or something).

Current WebSocketHandshake's interface is so minimum that it's only able to build byte sequence of a handshake request. So anyways we need to add more friendly interface, which probably will be called as WebSocketHandshakeRequest. I came to an agreement on this with Fumitoshi, who originally wrote Web Sockets interface.
Comment 13 Radar WebKit Bug Importer 2014-08-03 19:25:05 PDT
<rdar://problem/17898467>
Comment 14 Nikita Vasilyev 2017-04-05 16:35:49 PDT
Implemented in Bug 167520 - Web Inspector: Show Web Socket connections in Network tab.

*** This bug has been marked as a duplicate of bug 167520 ***