Bug 105528 - [Timeline]Add WebSocket SendFrame, ReceiveFrame and Error message.
Summary: [Timeline]Add WebSocket SendFrame, ReceiveFrame and Error message.
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 38831
  Show dependency treegraph
 
Reported: 2012-12-20 04:07 PST by pdeng6
Modified: 2014-08-03 19:25 PDT (History)
15 users (show)

See Also:


Attachments
Image of WebSocket Frame record in Timeline panel (122.61 KB, image/png)
2013-03-13 20:03 PDT, pdeng6
no flags Details
Patch (22.97 KB, patch)
2013-03-13 20:05 PDT, pdeng6
no flags Details | Formatted Diff | Diff
Patch (24.01 KB, patch)
2013-03-13 22:18 PDT, pdeng6
pfeldman: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description pdeng6 2012-12-20 04:07:45 PST
Sub-bug of #38831
Comment 1 Pavel Feldman 2012-12-20 04:35:22 PST
Handlers are already present in Timeline as "Function call" entries.
Comment 2 pdeng6 2012-12-20 04:43:07 PST
(In reply to comment #1)
> Handlers are already present in Timeline as "Function call" entries.
thanks :)
Yep, I prefer bringing them together as sub-records of "WebSocketCreate", I feel it's not easy to understand the websocket interaction if they fall out here and there.
how do you think of it?
Comment 3 Pavel Feldman 2012-12-20 04:52:11 PST
(In reply to comment #2)
> (In reply to comment #1)
> > Handlers are already present in Timeline as "Function call" entries.
> thanks :)
> Yep, I prefer bringing them together as sub-records of "WebSocketCreate", I feel it's not easy to understand the websocket interaction if they fall out here and there.
> how do you think of it?

My only concern is that timeline instrumentation is currently suboptimal and requires too much plumbing to add (see the patch in bug #38831). I was asking caseq@ if he had plans for simplifying it significantly.
Comment 4 pdeng6 2013-03-13 20:01:40 PDT
To add WebSocket Frame send/receive and Error message to timeline panel.
Comment 5 pdeng6 2013-03-13 20:03:53 PDT
Created attachment 193046 [details]
Image of WebSocket Frame record in Timeline panel
Comment 6 pdeng6 2013-03-13 20:05:49 PDT
Created attachment 193048 [details]
Patch
Comment 7 WebKit Review Bot 2013-03-13 21:14:13 PDT
Comment on attachment 193048 [details]
Patch

Attachment 193048 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17133142

New failing tests:
inspector/timeline/timeline-enum-stability.html
Comment 8 pdeng6 2013-03-13 22:18:05 PDT
Created attachment 193066 [details]
Patch
Comment 9 Pavel Feldman 2013-03-13 22:28:41 PDT
Still too much hand crafting given that there are no requests for this feature.
Comment 10 pdeng6 2013-03-13 23:17:42 PDT
(In reply to comment #9)
> Still too much hand crafting given that there are no requests for this feature.

With this feature, 
1) Websocket frames send/receive can be easily distinguished from other function call(in handlers).
2) If there is no handlers, websocket frames still can be recorded in timeline.
3) Allow the user to understand the continuation frame that cause one "onmessage".
:)

thanks!
Pan
Comment 11 Pavel Feldman 2013-03-13 23:34:57 PDT
Comment on attachment 193066 [details]
Patch

Inspector already contains too many features, they all increase its footprint and complexity, add to maintenance cost and startime time. We can not afford adding random features into it. Only features that are important / have high demand in the field / success story in competing products should get in. I have never seen a request for better websocket inspection in timeline. Given that websockets are rarely used and even less often they are analysed in performance perspective, I tend to think that we should remove websocket support from timeline altogether instead. We already think of removing "glueing" feature from timeline due to its n^2 complexity and lack of scaling.
Comment 12 pdeng6 2013-03-14 00:29:15 PDT
(In reply to comment #11)
> (From update of attachment 193066 [details])
> Inspector already contains too many features, they all increase its footprint and complexity, add to maintenance cost and startime time. We can not afford adding random features into it. Only features that are important / have high demand in the field / success story in competing products should get in. I have never seen a request for better websocket inspection in timeline. Given that websockets are rarely used and even less often they are analysed in performance perspective, I tend to think that we should remove websocket support from timeline altogether instead. We already think of removing "glueing" feature from timeline due to its n^2 complexity and lack of scaling.

Sorry to hear that, but I respect your decision, although sometimes there are different usage opinion, my intent is to make it better, and If I can of any help, just let me know. :)

thanks
Pan