We need a Websocket Frame Listener API so that we can display WS frames in the devtool Network Inspector. This frame listener initially only needs to be able to access WS connections initiated from a given tab. A nice to have is a pointer back to a listener of the actual WS connection instance so we can listen for events fired by the API and we can overlay it on top of the frame timeline. The most basic use case is that if an error fired, the developer can go back and look at the frame associated with an error. Another use case is to be able to correlate a close event with either a close control frame or lack thereof.

Andrea, this is the frame bug as promised in our discussion this Morning. You and Honza should talk about if the API passed raw frames or they're already parsed and packaged into a clean object, with the basic parts, such as control code, payload, etc... I want to spare Honza from having to rewrite code that parses the frames themselves.

Also, if we don't need 977858 we should close it. For now I am going to put Bug 977858 under this bug.

These 3 patches implement a WebSocketFrameService that allows devtools (or any other chrome code) to add listeners for any WebSocket Frame "related" to a inner window ID. As we discussed, the listener will receive the _real_ webSocket frames: they can be compressed, masked, etc etc. And I guess, at some point, we will need some helper methods to transform those frames in something more 'useful' and quick to be processed. This component is main-thread only, main-process only but it will manage also WebSockets running in workers.

@Andrea: I am seeing the following when compiling with the attached patches: c:/src/mozilla.org/fx-team/netwerk/protocol/websocket/WebSocketChannel.cpp(1267) : error C2664: 'uint32_t NS_FAILED_impl(nsresult)' : cannot convert argument 1 from 'mozilla::DebugOnly<nsresult>' to 'nsresult' Honza

I am still seeing the same, but in WebSocketFrameService.cpp c:/src/mozilla.org/fx-team/netwerk/protocol/websocket/WebSocketFrameService.cpp(238) : error C2664: 'uint32_t NS_FAILED_impl(nsresult)' : cannot convert argument 1 from 'mozilla::DebugOnly<nsresult>' to 'nsresult' Btw. the test.patch has changes, which are now even in part 2 - WebSocketFrameService Honza

Can you try this new set of patches? I pushed to try before uploading.

(In reply to Andrea Marchesini (:baku) from comment #12) > Can you try this new set of patches? I pushed to try before uploading. Yep, works now! I've register a listener and I can get sent and received packets now. Data passed into 'frameReceived' callback seems to be ok, but data in 'frameSent' looks encoded. Could we decode everything passed into the listener? Take a look at my screenshot. Honza

> I've register a listener and I can get sent and received packets now. Data > passed into 'frameReceived' callback seems to be ok, but data in 'frameSent' > looks encoded. Could we decode everything passed into the listener? I guess this is the point of the project. What you see is the 'real' frames. This is what we should show (?) if we want to inspect the WebSocket network traffic. The reason why the sending frames are not so easy to read is because they are masked and containing bit maps (the header). What I would like to do, is to implement some helper method so that you can retrieve data from a 'binary' frame. Otherwise you can do this by yourself following what the spec says. So, basically the point is to find the right balance between what we want to show to the user: the network frames, or the content in a 'readable' way? or both?

I see, so having a helper function that would convert data from binary (raw) format to readable text (hiding all the complexity related to the conversion) would definitely be a step in the right direction. This way we could display both in the UI. Where switching between the binary/text could be done by a single click. Honza

Check how the test works. You have a nsIWebSocketFrame that contains all the information contained in the frame.

Comment on attachment 8661567 [details] [diff] [review] part 1 - WindowID added into WebSocketChannel jduell, this first patch adds a innerWindowID to each WebSocket object.

Comment on attachment 8663366 [details] [diff] [review] part 2 - WebSocketFrameService jduell, this second patch implements a WebSocketFrameService able to send WebSocketFrame objects to listeners.

Comment on attachment 8663366 [details] [diff] [review] part 2 - WebSocketFrameService (In reply to Andrea Marchesini (:baku) from comment #16) > Created attachment 8663366 [details] [diff] [review] > part 2 - WebSocketFrameService > > Check how the test works. You have a nsIWebSocketFrame that contains all the > information contained in the frame. Works great for me so far, I am just using the API in a prototype extension. Honza

@Andrea: I have done first version of working prototype (for anyone interested, all executables including a try build are available here: https://github.com/firebug/devtools-extension-examples/releases/tag/websocketmonitor-0.2.0) Some questions: 1) Does the listeners need to be re-added when the page is reloaded? It seems that the listener stops working when the page is reloaded. 2) Can we get some timing information for every frame? It would be great to display a waterfall diagram (see an example of such diagram here: http://www.softwareishard.com/har/viewer/?path=examples/google.com.har). Not sure how this is feasible, but I am sure that web developer using ws would be interested in timing, e.g. see immediately (on the graph) that some frames takes too long to get through and so, the protocol needs optimization. It could also be a follow up. Also, the patch needs rebase, I can't apply it to the latest fx-team. Honza

> 1) Does the listeners need to be re-added when the page is reloaded? It > seems that the listener stops working when the page is reloaded. Yes. because each reload will create a new inner-window with a unique inner-window-ID. > 2) Can we get some timing information for every frame? It would be great to I can add a a TimeStamp. > Also, the patch needs rebase, I can't apply it to the latest fx-team. Sure!

Excellent! So, what I need in order to display standard waterfall graph is the time when receiving of the frame data starts and time when it ends. Would that be possible? Honza

Comment on attachment 8665432 [details] [diff] [review] part 1 - WindowID added into WebSocketChannel Review of attachment 8665432 [details] [diff] [review]: ----------------------------------------------------------------- Michal has been dealing with our websocket code the most recently, so I'm going to give these to him.

Also, I am not sure if the timeStamp contains correct time. See the attached screenshot showing ws frame flow (it's about 1 frame per 1 sec, roughly), check out the 'Time' column. The displayed values don't make much sense to me. It's computed as follows: var time = new Date(frame.header.timeStamp); var displayedText = time.toLocaleTimeString(); Honza

> It's computed as follows: > var time = new Date(frame.header.timeStamp); the timestamps are in nanoseconds, just do: new Date(frame.header.timeStamp / 1000);

> So, what I need in order to display standard waterfall graph is the time > when receiving of the frame data starts and time when it ends. Would that be > possible? That seems harder to do because when we receive frames, we collect them in buffers and we process them only when they are complete.

(In reply to Andrea Marchesini (:baku) from comment #29) > > It's computed as follows: > > var time = new Date(frame.header.timeStamp); > > the timestamps are in nanoseconds, just do: new Date(frame.header.timeStamp > / 1000); Ah, right, thanks! (In reply to Andrea Marchesini (:baku) from comment #30) > > So, what I need in order to display standard waterfall graph is the time > > when receiving of the frame data starts and time when it ends. Would that be > > possible? > > That seems harder to do because when we receive frames, we collect them in > buffers and we process them only when they are complete. I see, do you think this can be done as part of this bug or a follow up? (this timings will be extremely useful for visualizing the entire communication in a timeline) Btw. what happens if a frame isn't properly received? Is there any error/exception or callback executed, so we can somehow visualize it in the UI? Honza

> I see, do you think this can be done as part of this bug or a follow up? Yep. It sounds like a follow up. > Btw. what happens if a frame isn't properly received? Mainly it's about the payload. It can happen that the frame is big enough so that the header is fully processed, but the payload is partially missing. At that point we wait for more data. > Is there any error/exception or callback executed, so we can somehow > visualize it in the UI? Not really, this is just network buffering. No error is occurred yet. About error handling, the current setup doesn't give you all the possible frames: if, for instance, a header is malformed, that frame is dropped and you don't receive any notification about this. I cannot dispatch the frame to you, so you miss this information - and personally I think it's a corner case that we can ignore for now. But maybe in the future we can implement some better error handling. To be more precise, you don't receive frames in these 2 corner cases: . frame with the 3rd byte 0x80 not null: http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/websocket/WebSocketChannel.cpp#1530 . wrong size for the payload: http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/websocket/WebSocketChannel.cpp#1547

(In reply to Andrea Marchesini (:baku) from comment #32) > > I see, do you think this can be done as part of this bug or a follow up? > > Yep. It sounds like a follow up. Reported here: 1208476 Hope we can have this soon so, I can start prototyping the graph :-) > > Btw. what happens if a frame isn't properly received? > > Mainly it's about the payload. It can happen that the frame is big enough so > that the header is fully processed, but the payload is partially missing. At > that point we wait for more data. > > > Is there any error/exception or callback executed, so we can somehow > > visualize it in the UI? > > Not really, this is just network buffering. No error is occurred yet. > > About error handling, the current setup doesn't give you all the possible > frames: if, for instance, a header is malformed, that frame is dropped and > you don't receive any notification about this. I cannot dispatch the frame > to you, so you miss this information - and personally I think it's a corner > case that we can ignore for now. But maybe in the future we can implement > some better error handling. > > To be more precise, you don't receive frames in these 2 corner cases: > > . frame with the 3rd byte 0x80 not null: > http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/websocket/ > WebSocketChannel.cpp#1530 > > . wrong size for the payload: > http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/websocket/ > WebSocketChannel.cpp#1547 I see, I guess we need some thoughts yet about how to visualize errors in the UI. Thanks for the info! Honza

> Reported here: 1208476 > Hope we can have this soon so, I can start prototyping the graph :-) Well, you can already start prototyping the graph, don't you? Each frame has a timestamp, that can be used to put them into a timeline. The real networking time can be easily done later.

(In reply to Andrea Marchesini (:baku) from comment #34) > > Reported here: 1208476 > > Hope we can have this soon so, I can start prototyping the graph :-) > > Well, you can already start prototyping the graph, don't you? > Each frame has a timestamp, that can be used to put them into a timeline. > The real networking time can be easily done later. Yes, it's just that that the waterfall graph doesn't display the frame just as a point in time, but as something that also spans some time. But, ok, I am starting working on it and I'll wait for the platform API. Honza

This patch exposes something similar to nsIWebSocketListener cross threads. I think would be nice to show when events are dispatched in the timeline.

Looks great Andrea! Here is my feedback: 1) How can I get content of the acknowledge event? There is only ID and size. 2) When is the 'onServerClose' event supposed to be fired? I am seeing it (together with 'onStop') when doing disconnect, but not if I shutdown the server. Also the 'reason' field is always set to an empty string for me. 3) Agree, we could put the events on the timeline, but we need time-stamps passed into all the callbacks. This is because all UI JS is running in the main thread and execution of the JS callbacks can be delayed at any time. So, if we used Date.now() in the callbacks, it can provide inaccurate timing. Honza

I am experiencing a tab crash in debug build with e10s enabled. 1. Open this test page: http://janodvarko.cz/test/websockets/ 2. Open the Toolbox 3. Make sure the WebSockets panel is selected (might be already from the last time) Andrea, let me know if you want me to file separate bug for this. Honza

It seems that we need to support WebSocketFrameService in child processes. I'll submit a patch for this.

IPC. What this patch wants to achieve is to add the support of WebSocketFrameService in child processes.

Andrea, I am not seeing regular builds for OSX, here are build for my try push: http://ftp-ssl.mozilla.org/pub/mozilla.org/firefox/try-builds/jodvarko@mozilla.com-9547af293d4c/ (all the other platforms has regular builds and debug builds seem to be ok too) Could the following be the problem? https://treeherder.mozilla.org/#/jobs?repo=try&revision=9547af293d4c Failed to log stats. Exception = [Errno 1] _ssl.c:504: error:1407742E:SSL routines:SSL23_GET_SERVER_HELLO:tlsv1 alert protocol version CalledProcessError: Command '['hg', 'log', '-r', '9547af293d4cf02420f723940c914c83473ec91f', '--template', '{node}']' returned non-zero exit status 255 /builds/slave/try-m64-0000000000000000000000/build/src/netwerk/protocol/websocket/WebSocketFrameService.cpp:22:1: error: unused function 'IsParentProcess' [-Werror,-Wunused-function] make[6]: *** [Unified_cpp_protocol_websocket0.o] Error 1 make[5]: *** [netwerk/protocol/websocket/target] Error 2 make[5]: *** Waiting for unfinished jobs.... make[4]: *** [compile] Error 2 make[3]: *** [default] Error 2 make[2]: *** [realbuild] Error 2 make[1]: *** [realbuild] Error 2 make: *** [build] Error 2 Return code: 2 'mach build' did not run successfully. Please check log for errors. Running post_fatal callback... Exiting -1 # TBPL FAILURE # # TBPL FAILURE # Honza

Comment on attachment 8668403 [details] [diff] [review] part 2 - WebSocketFrameService Review of attachment 8668403 [details] [diff] [review]: ----------------------------------------------------------------- ::: netwerk/protocol/websocket/WebSocketChannel.cpp @@ +1260,5 @@ > + > + if (NS_IsMainThread()) { > + mFrameService = nullptr; > + } else { > + nsRefPtr<nsRunnable> runnable = new ReleaseFrameService(mFrameService); I don't think this runnable is needed. There is also NS_ReleaseOnMainThread for nsRefPtr in nsProxyRelease.h. ::: netwerk/protocol/websocket/WebSocketFrame.cpp @@ +60,5 @@ > +WebSocketFrame::~WebSocketFrame() > +{} > + > +nsresult > +WebSocketFrame::ProcessHeaderAndPayload(uint8_t* aHeader, uint32_t aHeaderLength, It would be good if we could avoid code duplication between this method and WebSocketChannel::ProcessInput(). The logic could be moved to WebSocketFrame and WebSocketChannel::ProcessInput() would work with the frame directly, i.e. not through the WebSocketFrameService. Once the frame is ready, the channel would pass it to the frame service. @@ +114,5 @@ > + (aPayloadLength + aPayloadInHdrLength) != payloadLength)) { > + return NS_ERROR_ILLEGAL_VALUE; > + } > + > + AutoFreePayload payload(static_cast<uint8_t*>(malloc(payloadLength))); Use nsAutoArrayPtr. @@ +141,5 @@ > + } > + } > + > + if (aPMCECompressor && aPMCECompressor->IsMessageDeflated()) { > + nsresult rv = aPMCECompressor->Inflate(payload.Get(), aPayloadLength, mPayload); This would work only in case server_no_context_takeover param would be negotiated during the handshake. Using inflater on our outgoing messages or twice on the incoming messages corrupts the sliding window of the inflater. The listener doesn't get raw data (masked or compressed), so simply call this method after/before the incoming/outgoing frame is (de)compressed and (de)masked. ::: netwerk/protocol/websocket/WebSocketFrame.h @@ +33,5 @@ > + > + uint8_t mFinBit; > + uint8_t mRsvBit1; > + uint8_t mRsvBit2; > + uint8_t mRsvBit3; Why not to store the flags as bools in a bit field? ::: netwerk/protocol/websocket/WebSocketFrameService.cpp @@ +131,5 @@ > + uint8_t* aPayload, uint32_t aPayloadLength, > + PMCECompression* aPMCECompressor) > +{ > + // This method can be called only from a the network thread. > + MOZ_ASSERT(!NS_IsMainThread()); Check that the thread is socketThread instead of that it isn't a main thread. @@ +201,5 @@ > + // Increasing the number of listeners is the only operation we have to > + // protect. > + { > + MutexAutoLock lock(mMutex); > + ++mCountListeners; IMO no need to have a lock, mozilla::Atomic should be sufficient.

> /builds/slave/try-m64-0000000000000000000000/build/src/netwerk/protocol/ > websocket/WebSocketFrameService.cpp:22:1: error: unused function > 'IsParentProcess' [-Werror,-Wunused-function] I think this is a kind of cached file.

I am still having problems to get builds from the try. See this push https://treeherder.mozilla.org/#/jobs?repo=try&revision=8b5bbe3e81b9 All builds failed. Any tips? Honz

I've been testing e10s and it works great with the new set of patches, no crashes anymore, nice! However, I am experiencing a problem when monitoring sockets on multiple tabs, see my STR: 1) Open two browser tabs and load the following URL on both: http://janodvarko.cz/test/websockets/ 2) Open Toolbox (F12) on the first tab and select the Web Sockets panel 3) Press 'Connect' and 'Send Message' buttons on the page. You should see new entries (frames) in the panel 4) Select the second browser tab, open the Toolbox and select the Web Sockets panel 5) Again, press 'Connect' and 'Send Message' buttons on the page. You should see new entries (frames) in the panel, but it's empty -> BUG I am not seeing the problem when e10s is off. Honza

I rebased all the patches.

new patch to fix some failures on try.

Btw. WebSocket Monitor add-on we are building on top of the API introduce in this report has its own repository on github here: https://github.com/firebug/websocket-monitor Releases are available here: https://github.com/firebug/websocket-monitor/releases (0.3.1 being the latest atm) It can be used to test the platform API (as well as reproduce the issue described in comment #49) Honza

I don't want to ask for a review for this patch until I don't understand why we have the issue from comment 49.

Comment on attachment 8669656 [details] [diff] [review] part 2 - WebSocketFrameService Review of attachment 8669656 [details] [diff] [review]: ----------------------------------------------------------------- ::: netwerk/protocol/websocket/WebSocketChannel.cpp @@ +1071,5 @@ > > mMsg.pStream->Close(); > mMsg.pStream->Release(); > + mMsg.pString.mValue = temp.forget(); > + mMsg.pString.mOrigValue = nullptr; mOrigValue must be already null here @@ +1263,5 @@ > + mFrameService = nullptr; > + > + NS_ReleaseOnMainThread(service); > + MOZ_ASSERT(!service); > + } You can replace this with NS_ReleaseOnMainThread(static_cast<nsIWebSocketFrameService *>(mFrameService.forget().take())); @@ +1741,5 @@ > + if (frame) { > + mFrameService->FrameReceived(mSerial, mInnerWindowID, frame); > + } > + > + if (mListenerMT) { This is already in if (mListenerMT) block. @@ +2214,5 @@ > // handful of bytes and might rotate the mask, so we can just do it locally. > // For real data frames we ship the bulk of the payload off to ApplyMask() > > + uint8_t* payloadPtr = payload; > + uint32_t tMask = mask; IIUC you use tMask here to not change the mask variable that you use below as an parameter for CreateFrameIfNeeded(), right? This wouldn't work if we would have part of the payload in the header and the rest in the mCurrentOut's buffer (which we don't have right now), because ApplyMask() below uses mask and not tMask. So it's better to save the mask to variable maskOrig and pass that to the frame service. Anyway, this is IMO wrong because you create the frame after the data in the header is masked. Also, I don't understand why nsIWebSocketFrameListener should receive mask when it doesn't see the original message. ::: netwerk/protocol/websocket/WebSocketFrame.h @@ +12,5 @@ > + > +namespace mozilla { > +namespace net { > + > +class PMCECompression; This is no longer needed. @@ +32,5 @@ > + bool mRsvBit1; > + bool mRsvBit2; > + bool mRsvBit3; > + uint8_t mOpCode; > + bool mMaskBit; I meant bool mFinBit : 1; bool mRsvBit1 : 1; bool mRsvBit2 : 1; bool mRsvBit3 : 1; bool mMaskBit : 1; ::: netwerk/protocol/websocket/WebSocketFrameService.cpp @@ +341,5 @@ > + if (NS_WARN_IF(!payload)) { > + return nullptr; > + } > + > + if (aPayloadInHdr) { Just a nit, check aPayloadInHdrLength instead of aPayloadInHdr, because payloadPtr which you pass in WebSocketChannel::PrimeNewOutgoingMessage() is always non-null, but the length might be 0.

Thanks for you review. We getting closer :) > > + mMsg.pString.mOrigValue = nullptr; > > mOrigValue must be already null here Why should it be? > Also, I don't understand why nsIWebSocketFrameListener should receive mask > when it doesn't see the original message. Well, I think it would be nice to show the full content of the frame and the mask is an important information. Maybe we want to have the payload masked and unmasked... ?

@Andrea: I am missing two things, which I think are needed for the first version of the tool: 1) Refer to the HTTP request that originated WS connection (upgrade) 2) Get handshake frames Should this be done as a part of this report? Honza

> 1) Refer to the HTTP request that originated WS connection (upgrade) Ok. > 2) Get handshake frames I don't know what you mean with these frames. I think I send you all the frames we send/receive.

(In reply to Andrea Marchesini (:baku) from comment #62) > I don't know what you mean with these frames. I think I send you all the > frames we send/receive. Perhaps it's my wrong understanding of the protocol. I thought there is some kind of a handshake after the upgrade HTTP request and before the first message. If not, than all is ok! Honza

> Perhaps it's my wrong understanding of the protocol. I thought > there is some kind of a handshake after the upgrade HTTP request > and before the first message. If not, than all is ok! Yes, I see. Can this be a follow up? Actually I have my own list of follow ups. I want to shared it: 1. this handshake task 2. I want to dispatch events to devtools when they are dispatched by the WebSocket obj. I don't think it's useful to expose nsIWebSocketListener events because devtools has to implement a similar logic to what WebSocket code does. All of these will be done when the first patches will be reviewed because I don't want to go too far and then update a lot of code applying the reviewer's comments.

Sounds good to me. The only think I am worried about is whether we can land this till the next merge day (2015-11-02). Honza

(In reply to Andrea Marchesini (:baku) from comment #59) > > > + mMsg.pString.mOrigValue = nullptr; > > > > mOrigValue must be already null here > > Why should it be? You're right. I overlooked that mOrigValue isn't initialized in the constructor for the stream message. > Well, I think it would be nice to show the full content of the frame and the > mask is an important information. > Maybe we want to have the payload masked and unmasked... ? It would make sense to provide mask together with the masked payload. But you could also provide compressed payload or even all the separate fragments in case it is fragmented. So it depends what detail will be presented to the user.

Comment on attachment 8670951 [details] [diff] [review] part 2 - WebSocketFrameService Review of attachment 8670951 [details] [diff] [review]: ----------------------------------------------------------------- r+ with the following fixed ::: netwerk/protocol/websocket/WebSocketChannel.cpp @@ +1258,5 @@ > + > + if (NS_IsMainThread()) { > + mFrameService = nullptr; > + } else { > + NS_ReleaseOnMainThread(static_cast<nsIWebSocketFrameService*>(mFrameService.forget().take())); NS_ReleaseOnMainThread will release the frame service immediately if called on the main thread, so no if statement is needed, just this single call. @@ +1259,5 @@ > + if (NS_IsMainThread()) { > + mFrameService = nullptr; > + } else { > + NS_ReleaseOnMainThread(static_cast<nsIWebSocketFrameService*>(mFrameService.forget().take())); > + MOZ_ASSERT(!mFrameService); forget() will nullout the pointer, so this would just test nsRefPtr functionality. Remove it. @@ +2225,5 @@ > + if (frame) { > + mFrameService->FrameSent(mSerial, mInnerWindowID, frame); > + } > + > + uint8_t* payloadPtr = payload; payloadPtr is now unused ::: netwerk/protocol/websocket/WebSocketFrame.h @@ +30,5 @@ > + bool mRsvBit1 : 1; > + bool mRsvBit2 : 1; > + bool mRsvBit3 : 1; > + uint8_t mOpCode; > + bool mMaskBit : 1; Group the bits together, otherwise mOpcode will cause that mMaskBit will start in a new byte.

Comment on attachment 8669589 [details] [diff] [review] part 3 - timeStamp Review of attachment 8669589 [details] [diff] [review]: ----------------------------------------------------------------- ::: netwerk/protocol/websocket/nsIWebSocketFrameService.idl @@ +8,5 @@ > > [scriptable, builtinclass, uuid(e668b6bf-d29d-4861-b1bc-bab70da4da5c)] > interface nsIWebSocketFrame : nsISupports > { > + readonly attribute DOMHighResTimeStamp timeStamp; You need to change uuid.

Comment on attachment 8669590 [details] [diff] [review] part 4 - nsIWebSocketListener Review of attachment 8669590 [details] [diff] [review]: ----------------------------------------------------------------- ::: netwerk/protocol/websocket/WebSocketChannel.cpp @@ +626,5 @@ > mData); > } > } > > + if (mChannel->mFrameService) { You don't have to check if mFrameService is non-null. It is initialized in channel's constructor and nulled out in the destructor. ::: netwerk/protocol/websocket/nsIWebSocketFrameService.idl @@ +41,5 @@ > { > void frameReceived(in unsigned long aWebSocketSerialID, > in nsIWebSocketFrame aFrame); > > void frameSent(in unsigned long aWebSocketSerialID, To keep the interface consistent with the newly added methods, it might be better to rename these to on* methods too. Also better name for this one would be probably onFrameQueued.

Comment on attachment 8669590 [details] [diff] [review] part 4 - nsIWebSocketListener Why did you obsolete this patch by the IPC patch?

> Why did you obsolete this patch by the IPC patch? Sorry, you were reviewing it. The reason why I want to make this patch obsolete is because I want to have IPC support for the first block of patches so we can land them. This nsIWebSocketListener is not exactly what we would like to expose to devtools: I prefer to expose the 'real' events that are dispatched by the WebSocket object. I'm actually modifying that patch so that it's closed to WebSocket instead nsIWebSocketListener. The goal is that devtools can display the messages and their contents almost in sync with what the content page receives.

(In reply to Andrea Marchesini (:baku) from comment #62) > > 1) Refer to the HTTP request that originated WS connection (upgrade) > > Ok. > > > 2) Get handshake frames > > I don't know what you mean with these frames. I think I send you all the > frames we send/receive. I don't think there are any handshake frames. There is an http request for a WS connection, and there is an http request for a protocol upgrade. Those are all http requests. Seems that should be part of a WS discovery API, or, if it's part of this API, maybe it's a special event.

I am getting following failures when applying the patches on the latest fx-team unable to find 'browser/components/loop/modules/LoopCalls.jsm' for patching 1 out of 1 hunks FAILED -- saving rejects to file browser/components/loop/module s/LoopCalls.jsm.rej patching file dom/base/WebSocket.cpp Hunk #3 FAILED at 1316 1 out of 4 hunks FAILED -- saving rejects to file dom/base/WebSocket.cpp.rej patching file netwerk/ipc/NeckoParent.cpp Hunk #2 FAILED at 342 1 out of 2 hunks FAILED -- saving rejects to file netwerk/ipc/NeckoParent.cpp.re j patching file netwerk/protocol/websocket/WebSocketChannel.cpp Hunk #3 succeeded at 624 with fuzz 2 (offset 0 lines). Hunk #4 succeeded at 674 with fuzz 2 (offset 0 lines). Hunk #5 succeeded at 717 with fuzz 2 (offset 0 lines). Hunk #6 succeeded at 758 with fuzz 2 (offset 0 lines). Hunk #28 FAILED at 2305 1 out of 31 hunks FAILED -- saving rejects to file netwerk/protocol/websocket/We bSocketChannel.cpp.rej patch failed, unable to continue (try -v) patch failed, rejects left in working directory errors during apply, please fix and refresh bug1203802-ws Honza

I tried to apply the patches on https://hg.mozilla.org/mozilla-central, but I am still getting failures. Honza

I rebased all the patches on top of the latest m-i.

remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/bf77e2eef0a0 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/1465c8af67d7 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/709a898fdc8b remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/e415ff528bd1

https://hg.mozilla.org/mozilla-central/rev/bf77e2eef0a0 https://hg.mozilla.org/mozilla-central/rev/1465c8af67d7 https://hg.mozilla.org/mozilla-central/rev/709a898fdc8b https://hg.mozilla.org/mozilla-central/rev/e415ff528bd1

Release Note Request (optional, but appreciated) [Why is this notable]: New feature in devtools [Suggested wording]: WebSocket Debugging API [Links (documentation, blog post, etc)]: https://hacks.mozilla.org/2015/11/developer-edition-44-creative-tools-and-more/

Added to FF44 release notes.

Honza, Firefox 44 release notes has an item "WebSocket Debugging API and add-on" which potch suggested we should delete as it's an unsigned add-on. Fx 44 does not enforce add-on signing so I think it is ok to leave it in there. What do you think? Also, are you the author of that add-on and do you plan to sign it sometime soon (as I think Fx46 will enforce signing) ?

(In reply to Ritu Kothari (:ritu) from comment #87) > Honza, Firefox 44 release notes has an item "WebSocket Debugging API and > add-on" which potch suggested we should delete as it's an unsigned add-on. > Fx 44 does not enforce add-on signing so I think it is ok to leave it in > there. What do you think? Also, are you the author of that add-on and do you > plan to sign it sometime soon (as I think Fx46 will enforce signing) ? The add-on has now been reviewed and is signed on AMO: https://addons.mozilla.org/en-US/firefox/addon/websocket-monitor/

(In reply to Ritu Kothari (:ritu) from comment #87) > Honza, Firefox 44 release notes has an item "WebSocket Debugging API and > add-on" which potch suggested we should delete as it's an unsigned add-on. > Fx 44 does not enforce add-on signing so I think it is ok to leave it in > there. What do you think? Also, are you the author of that add-on and do you > plan to sign it sometime soon (as I think Fx46 will enforce signing) ? As Ryan mentioned the add-on is now signed and ready. I'd love to hear a feedback. Let me know if you are missing any features. Honza

@jryan: thanks for the review :-) Honza