The objects are usually kept alive when the network connection is still active.

So, I was looking at XHR code. Am I right that if XHR has mChannel, and mChannel's GetNotificationCallbacks returns XHR itself, mChannel owns XHR. But is there any better way to check whether necko owns XHR (or EventSource or WebSocket)?

GetNotificationCallbacks approach wouldn't quite work, since it is addreffing operation, and I need the information at times when addref/release is prohibited.

We talked about this a bit on IRC. For HTTP/FTP (everything other than Websocket channels) we know that necko holds a ref to the listener between a successful AsyncOpen and OnStopRequest. (Callbacks will be held for life of channel IIRC). For websockets Ollie mentioned he might be able to use mKeepingAlive as a proxy for whether we're holding onto CC object, but I'm not sure it's a good metric. For one, there's a window from nsWebSocket::Init until the first UpdateMustKeepAlive call (which is when mKeepingAlive is first set to true). Also, when mKeepingAlive is false, that just means the nsWebSocket removes it's own ref to *itself* not to mOwner. mOwner is held for the duration of a nsWebSocket's lifetime (so maybe it's actually easy: just assume it always holds a ref). Note that this is not the same as the actual necko channel (WebSocketChannel) which can disappear earler/later than nsWebSocket. Hope that helps.

I'm not yet sure how to handle CORS XHR and EventSource

Comment on attachment 596872 [details] [diff] [review] WIP for ws mKeepingAlive is true when nsWebSocket has addrefed itself.

Comment on attachment 596872 [details] [diff] [review] WIP for ws Review of attachment 596872 [details] [diff] [review]: ----------------------------------------------------------------- I'm assuming that if mKeepingAlive is true, that the nsWebSocket must be alive. Interesting case. If I get around to fixing up bug 717500, we could take advantage of that in the GC, too. ::: content/base/src/nsWebSocket.cpp @@ +438,5 @@ > > NS_IMPL_CYCLE_COLLECTION_CLASS(nsWebSocket) > > NS_IMPL_CYCLE_COLLECTION_CAN_SKIP_BEGIN(nsWebSocket) > + bool isBlack = false; Just initialize isBlack to tmp->IsBlack(), no need that I can see for assignment in the if. @@ +447,5 @@ > NS_UNMARK_LISTENER_WRAPPER(Error) > NS_UNMARK_LISTENER_WRAPPER(Message) > NS_UNMARK_LISTENER_WRAPPER(Close) > } > + if (!isBlack) { You don't unmarkGray if it is black because that implies any JS it points to will already be black?

(In reply to Andrew McCreight [:mccr8] from comment #6) > You don't unmarkGray if it is black because that implies any JS it points to > will already be black? yes

This relies on OnStopRequest to be called always after AsyncOpen. Jonas, if I read CORS correctly, it gives the same behavior, right? I'll add some generic UnmarkWrapperCache() in a followup bug. https://tbpl.mozilla.org/?tree=Try&rev=a40b199a54b0

Comment on attachment 597034 [details] [diff] [review] +XHR and ES Review of attachment 597034 [details] [diff] [review]: ----------------------------------------------------------------- Looks reasonable to me, assuming that the object is known to be alive whenever the flag is true, of course, as I'm not familiar with that.

Comment on attachment 597034 [details] [diff] [review] +XHR and ES Review of attachment 597034 [details] [diff] [review]: ----------------------------------------------------------------- I don't know CC well enough to have any comfort being a real reviewer here. I'm fine with the changes if those who do know CC are ok with it, and you verify (via Jonas or someone else) that CORS will also guarantee that OnStop gets called eventually (if it doesn't I assume that's a bug). > mKeepingAlive is true when nsWebSocket has addrefed itself. Yes, while mKeepingAlive is true the WS holds a ref to itself, so I guess that's equiv to black IIUC.

Comment on attachment 596872 [details] [diff] [review] WIP for ws Assuming this patch is obsolete since the code got rolled into the other XHR/ES patch.

I don't really know this part of cycle collection well enough that my review would add a whole lot. But yes, OnStopRequest will always be called, even when CORS listener-proxies are involved.

Comment on attachment 597034 [details] [diff] [review] +XHR and ES r=jst