We already special-case a lot of interfaces, so this one seems ok too? Otherwise we're starting to get code like the nsOfflineCacheUpdateService.cpp case here. Patch applied over that for bug 794663

now with actual code changes!

Comment on attachment 674149 [details] [diff] [review] v1 for realz Review of attachment 674149 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/base/nsGlobalWindow.cpp @@ +8596,5 @@ > + if (mDocShell) { > + nsCOMPtr<nsILoadContext> loadContext(do_QueryInterface(mDocShell)); > + if (loadContext) { > + *aSink = loadContext; > + NS_ADDREF(((nsISupports *) *aSink)); Make this loadContext.forget(aSink);

Comment on attachment 674149 [details] [diff] [review] v1 for realz r=me with ms2ger's nit. Note that if you do it that way, you don't have to null-check either mDocShell or loadContext. So you can do it like so: FORWARD_TO_OUTER(GetInterface, (aIID, aSink), NS_ERROR_NOT_INITIALIZED); nsCOMPtr<nsILoadContext> loadContext(do_QueryInterface(mDocShell)); loadContext.forget(aSink);

Trivial update: I'm assuming if we want the style in comment 3, we might as well use it elsewhere in the function too. Not even sure this needs review, really...

Comment on attachment 674982 [details] [diff] [review] v2: use forget() convention in rest of GetInterface r=me

I have a vague memory of seeing a patch with some awkward code because it had a window and wanted a load context. If only I could remember where it was...