Fix places that make trivial calls to do_QueryInterface

RESOLVED FIXED in Firefox 64

Status

()

enhancement
P3
normal
RESOLVED FIXED
8 months ago
2 months ago

People

(Reporter: mccr8, Assigned: mccr8)

Tracking

unspecified
mozilla64
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox64 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

8 months ago
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
Assignee

Comment 1

8 months ago
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.
Assignee

Comment 2

8 months ago
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.
Assignee

Updated

8 months ago
Depends on: 1494765
Assignee

Comment 5

8 months ago
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.
Assignee

Comment 6

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

Comment 7

8 months ago
Pushed by amccreight@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/93b4642d28ab
Fix many trivial calls to do_QueryInterface r=smaug

Comment 8

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/93b4642d28ab
Status: NEW → RESOLVED
Last Resolved: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.