>+++ b/netwerk/ipc/NeckoMessageUtils.h >+ // The contained URI is already addrefed on creation. We don't want another >+ // addref when passing it off to its actual owner. >+ operator nsCOMPtr<nsIURI>() const { return already_AddRefed<nsIURI>(mURI); } This is leak-prone. For example, just in this patch we leak URIs if CookieServiceParent::RecvGetCookieString happens when !mCookieService. Not only that, but f you make the mistake of assigning to two different nsCOMPtrs you end up double-releasing. Are we really that worried about the performance impact of refcounting the URIs when passing them across the ipdl boundary? As implemented right now we have no addref/release on the content side, but on the chrome side we have: 1) An addref when creating. 2) An addref/release pair when assigning to an nsCOMPtr 3) A release when that nsCOMPtr goes out of scope. If we made the member an nsCOMPtr and just had the operator() return nsIURI*, we would have an addref/release on the content side, but no change in number of addrefs/releases on the chrome side. If we made operator() return already_AddRefed<nsIURI> (by returning mURI.forget()), we would eliminate one addref/release pair on the chrome side. In either case, Read() would swap() into mURI. I'd prefer one or the other of the above solutions to what we have now. >+ URI& operator=(URI&); This is purposefully unimplemented, right? Should that be documented? Should the copy ctor similarly be private and unimplemented?

Comments addressed. The |nsCOMPtr<nsIURI> uri = ipcuri| pattern no longer compiles, so all future code must be reworked to use either |nsCOMPtr<nsIURI> uri; uri = ipcuri| or |nsCOMPtr<nsIURI> uri(ipcuri)|.

Comment on attachment 460312 [details] [diff] [review] Patch Looks good! r=dwitte

The previous version caused build failures on OSX and Maemo 5 due to an odd compiler decision. Apparently on these platforms, anytime an IPC::URI is constructed from an nsIURI the copy constructor |URI(const URI&)| is also invoked on the temporary. This obviously makes it impossible to make the copy constructor private and unimplemented, so I've reverted that change. Ehsan also pointed out that the IPC::URI calls are implicit, so I've removed them for any uses that solely involve nsIURIs (as opposed to nsCOMPtrs, which are not automatically converted for some mysterious reason). Anyways, I'm carrying forward the r+ because the changes are basically superficial.