RESOLVED INVALID 105528
[Timeline]Add WebSocket SendFrame, ReceiveFrame and Error message.
https://bugs.webkit.org/show_bug.cgi?id=105528
Summary [Timeline]Add WebSocket SendFrame, ReceiveFrame and Error message.
pdeng6
Reported 2012-12-20 04:07:45 PST
Sub-bug of #38831
Attachments
Image of WebSocket Frame record in Timeline panel (122.61 KB, image/png)
2013-03-13 20:03 PDT, pdeng6
no flags
Patch (22.97 KB, patch)
2013-03-13 20:05 PDT, pdeng6
no flags
Patch (24.01 KB, patch)
2013-03-13 22:18 PDT, pdeng6
pfeldman: review-
Pavel Feldman
Comment 1 2012-12-20 04:35:22 PST
Handlers are already present in Timeline as "Function call" entries.
pdeng6
Comment 2 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?
Pavel Feldman
Comment 3 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.
pdeng6
Comment 4 2013-03-13 20:01:40 PDT
To add WebSocket Frame send/receive and Error message to timeline panel.
pdeng6
Comment 5 2013-03-13 20:03:53 PDT
Created attachment 193046 [details] Image of WebSocket Frame record in Timeline panel
pdeng6
Comment 6 2013-03-13 20:05:49 PDT
WebKit Review Bot
Comment 7 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
pdeng6
Comment 8 2013-03-13 22:18:05 PDT
Pavel Feldman
Comment 9 2013-03-13 22:28:41 PDT
Still too much hand crafting given that there are no requests for this feature.
pdeng6
Comment 10 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
Pavel Feldman
Comment 11 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.
pdeng6
Comment 12 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
Note You need to log in before you can comment on or make changes to this bug.