Closed Bug 1493737 Opened 2 years ago Closed 2 years ago

Fix places that make trivial calls to do_QueryInterface

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: mccr8, Assigned: mccr8)

References

Details

Attachments

(1 file)

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.
Priority: -- → P3
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.
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 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.
Attachment #9011985 - Flags: review+
Yeah, thanks for not dropping nsCOMPtr, reviewing would have been quite a bit harder.
Depends on: 1494765
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.
I'm not going to convert any places from the |nsCOMPtr<A> x(foo)| form to |nsCOMPtr<A> x = foo|.
Pushed by amccreight@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/93b4642d28ab
Fix many trivial calls to do_QueryInterface r=smaug
https://hg.mozilla.org/mozilla-central/rev/93b4642d28ab
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.