1493737 - Fix places that make trivial calls to do_QueryInterface

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P3)

Description

[Andrew McCreight [:mccr8]]

There are a fair number of places this happens, and the actual patch that enforces the static invariant is going to be a little fragile, plus a different reviewer might make sense, so I'm going to split this into a separate bug.

Comment 1

[Andrew McCreight [:mccr8]]

Attachment: Bug 1493737 - Fix many trivial calls to do_QueryInterface

If class A is derived from class B, then an instance of class A can be
converted to B via a static cast, so a slower QI is not needed.

Comment 2

[Andrew McCreight [:mccr8]]

Generally, I just dropped the QI and left alone code like

nsCOMPtr<nsIFoo> foo(bar);

even though in some cases you could maybe replace foo with bar. I did clean up a few cases that were trivial, but in general I was worried about cases where bar might be a raw pointer, or a member variable that could be modified inside a function call.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 9011985 [details]
Bug 1493737 - Fix many trivial calls to do_QueryInterface

Olli Pettay [:smaug] (r- if the bug doesn't explain what the change(s) are about.) has approved the revision.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Yeah, thanks for not dropping nsCOMPtr, reviewing would have been quite a bit harder.

Comment 5

[Andrew McCreight [:mccr8]]

I've updated my patches locally to take advantage of bug 1494765 to use assignment instead of the ctor.

For a few places that were returning to an nsIFoo** via a RefPtr, I had to change the smart pointer from an nsCOMPtr to an nsRefPtr, like this instance in ServiceWorkerRegistrationInfo::GetInstallingWorker():
-  nsCOMPtr<nsIServiceWorkerInfo> info = do_QueryInterface(mInstallingWorker);
+  RefPtr<ServiceWorkerInfo> info = mInstallingWorker;

The problem is that there's no conversion from RefPtr<I> to nsCOMPtr<J>, where I is a subclass of J. I tried a little bit to add one, but I was getting weird errors about ambiguous casts to nsISupports. I think using RefPtr instead of nsCOMPtr is reasonable.

Comment 6

[Andrew McCreight [:mccr8]]

I'm not going to convert any places from the |nsCOMPtr<A> x(foo)| form to |nsCOMPtr<A> x = foo|.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/93b4642d28ab
Fix many trivial calls to do_QueryInterface r=smaug

Comment 8

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/93b4642d28ab