I think it doesn't matter currently, since only Window wants the Finalize, and it doesn't return NS_SUCCESS_ALLOW_SLIM_WRAPPERS in PreCreate. But if some other code starts to use this....
Created attachment 624816 [details] [diff] [review] cargo culted patch This is a straightforward bringing over of the code from XPC_WN_NoHelper_Finalize. Although, if nothing actually uses this code, as Olli said, maybe a better approach would be to just remove that entire clause and assert if there's a slim wrapper. I think that would just cause a leak, but I'm not sure.
Created attachment 624948 [details] [diff] [review] more cleanups This version of the patch fixes some additional problems with XPC_WN_Helper_Finalize I noticed. - XPC_WN_Helper_Finalize doesn't do a null-check before the slim wrapper case, so we'll end up QIing and then releasing a null pointer in that case, so I moved the check before the slim wrapper case. As with the base bug, apparently this case isn't used in practice right now, so it doesn't matter. - Bill pointed out that when we finalize we must be in a GC, so we should have an XPCJSRuntime sitting around, so we can eliminate the check and replace it with an assertion. - Helper_Finalize uses IS_SLIM_WRAPPER instead of IS_SLIM_WRAPPER_OBJECT which I think is unnecessary. - I replaced a C-style cast with a C++ one. With these fixes in place, plus the deferred release for slim wrappers, XPC_WN_NoHelper_Finalize and XPC_WN_Helper_Finalize are almost the same, except that in the Helper case we perform a callback. I factored out the bodies of both functions into a common function WrappedNativeFinalize that takes a flag indicating whether we're in the Helper or NoHelper case. This should help prevent one version from rotting again.
Comment on attachment 624948 [details] [diff] [review] more cleanups Try run looks good-ish. I think the burning builds are the result of some build infrastructure funkiness. https://tbpl.mozilla.org/?tree=Try&rev=8f08edae73e3
Comment on attachment 624948 [details] [diff] [review] more cleanups Huzzah! :-)