Closed Bug 984538 Opened 6 years ago Closed 6 years ago

NS_New*Event should static_cast to outparams, rather than calling through CallQueryInterface

Categories

(Core :: DOM: Events, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: froydnj, Assigned: froydnj)

Details

Attachments

(1 file)

This is similar to bug 983803, but for non-generated events.  Just like in that bug, we should be able to cast to mozilla::dom::Event* and let the compiler do the rest, rather than requiring expensive QueryInterface calls.
Cheaper than CallQueryInterface and maybe even avoids a virtual call if the
appropriate class is MOZ_FINAL.
Attachment #8392400 - Flags: review?(masayuki)
Comment on attachment 8392400 [details] [diff] [review]
use static_cast in NS_NewDOM*Event rather than CallQueryInterface

Looks good to me. But this should be reviewed by smaug.
Attachment #8392400 - Flags: review?(masayuki) → review?(bugs)
Comment on attachment 8392400 [details] [diff] [review]
use static_cast in NS_NewDOM*Event rather than CallQueryInterface

Another option would be
nsRefPtr<FooEvent> it = new ...
*aInstancePtrResult = static_cast<Event*>(it.forget());

But this is ok too.
Attachment #8392400 - Flags: review?(bugs) → review+
(In reply to Olli Pettay [:smaug] from comment #3)
> Comment on attachment 8392400 [details] [diff] [review]
> use static_cast in NS_NewDOM*Event rather than CallQueryInterface
> 
> Another option would be
> nsRefPtr<FooEvent> it = new ...
> *aInstancePtrResult = static_cast<Event*>(it.forget());

I thought about this, but since you can't implicitly cast from already_AddRefed to raw pointers, you need:

*aInstancePtrResult = static_cast<Event*>(it.forget().take());

and that didn't seem better than what is in the patch.
Assignee: nobody → nfroyd
https://hg.mozilla.org/mozilla-central/rev/bff60e1caa23
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.