Closed Bug 1073282 Opened 6 years ago Closed 6 years ago
Proxy release of m
In the same way we proxy the release of mLoadInfo in bug 1072316 we have to do the same proxying not only on websocketChannels but also on all other channels that use loadInfol See > https://bugzilla.mozilla.org/show_bug.cgi?id=1072316#c15
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Jason, here is a list of all channels that do have mLoadInfo in the *.h file. For some of them I added questions right underneath, maybe you can help me answer them. * BaseWebSocketChannel.h Do you know if there is a specific reason why the *Base*WebSocketChannel.h holds the mLoadInfo (also mLoadGroup etc) but it's released in the WebSocketChannel code? http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/websocket/WebSocketChannel.cpp#1064 Shouldn't also the dtor of *Base*WebSocketChannel.h release those members? * nsBaseChannel.h * HttpBaseChannel.h * nsWyciwygChannel.h * nsJARChannel.h Currently, all of the above *.h files do not release anything the the dtor. If we add NS_ProxyRelease for mLoadInfo, shouldn't we also add it for mLoadGroup and other nsCOMPtrs? * WyciwygChannelChild.h I am not completely sure why we have mLoadInfo in WyciwygChannelChild.h as well as nsWyciwygChannel, but not in any other child. Since bz added mLoadInfo to the child, I assume there is a plausible reason. Either way, the code in the patch should proxy the release correctly. * mac/nsIconChannel.h * win/nsIconChannel.h For those two, I just added code to proxy the release of mLoadInfo. Other than that, I think the patch is what we want, what do you think?
Attachment #8496902 - Flags: review?(jduell.mcbugs)
Comment on attachment 8496902 [details] [diff] [review] bug_1073282_proxy_mloadinfo_on_channel_dtors.patch Review of attachment 8496902 [details] [diff] [review]: ----------------------------------------------------------------- thanks!
Attachment #8496902 - Flags: review?(jduell.mcbugs) → review+
Target Milestone: --- → mozilla35
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.