The goal here is to leave creation stuff mostly for JS, so we can convert it entirely over to a non-threadsafe cycle-collected version without breaking any existing C++ users. I didn't do this for a remaining use in nsGlobalWindow.h to avoid including nsVariant.h all over the place.

The goal here is to leave creation stuff mostly for JS, so we can convert it entirely over to a non-threadsafe cycle-collected version without breaking any existing C++ users. I didn't do this for a remaining use in nsGlobalWindow.h to avoid including nsVariant.h all over the place. try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=83e2e0f866a3

Comment on attachment 8669137 [details] [diff] [review] Create nsVariant directly rather than via do_CreateInstance(). Review of attachment 8669137 [details] [diff] [review]: ----------------------------------------------------------------- We should probably just shuffle DialogValueHolder and friends around from nsGlobalWindow.h to nsGlobalWindow.cpp so we can alter that final instance. Followup bug? ::: dom/base/nsContentAreaDragDrop.cpp @@ +726,5 @@ > const nsAString& aFlavor, > const nsAString& aData, > nsIPrincipal* aPrincipal) > { > + nsCOMPtr<nsIWritableVariant> variant = new nsVariant(); I have a slight preference for using nsRefPtr<nsVariant> in cases like these, so that we're using concrete types whenever possible. Ditto for all instances below. ::: dom/events/DataTransfer.cpp @@ +434,5 @@ > void > DataTransfer::SetData(const nsAString& aFormat, const nsAString& aData, > ErrorResult& aRv) > { > + nsCOMPtr<nsIWritableVariant> variant = new nsVariant(); I *think* that making this allocation switch from fallible to infallible (along with all the others) is OK, because the mechanism underneath do_CreateInstance probably didn't properly check for fallibility anyway. (Which makes me wonder if we should make it do that...)

I filed bug 1212148 for the use in nsGlobalWindow.h and I replaced all of the nsCOMPtr<> with nsRefPtr<>. There's a lot of funky single platform code in the patch so I'm going to try an all-platform build run to make sure nothing is broken, given all of the problems I've had lately with broken builds in other patches.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=734a970c6cf6 https://treeherder.mozilla.org/#/jobs?repo=try&revision=ea2d0d9c982e