Closed
Bug 984538
Opened 10 years ago
Closed 10 years ago
NS_New*Event should static_cast to outparams, rather than calling through CallQueryInterface
Categories
(Core :: DOM: Events, defect)
Core
DOM: Events
Tracking
()
RESOLVED
FIXED
mozilla31
People
(Reporter: froydnj, Assigned: froydnj)
Details
Attachments
(1 file)
15.26 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•10 years ago
|
||
Cheaper than CallQueryInterface and maybe even avoids a virtual call if the appropriate class is MOZ_FINAL.
Attachment #8392400 -
Flags: review?(masayuki)
Comment 2•10 years ago
|
||
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 3•10 years ago
|
||
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+
Assignee | ||
Comment 4•10 years ago
|
||
(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 | ||
Updated•10 years ago
|
Assignee: nobody → nfroyd
Assignee | ||
Comment 5•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/bff60e1caa23
Flags: in-testsuite-
Comment 6•10 years ago
|
||
You should also fix generated events. https://mxr.mozilla.org/mozilla-central/source/js/xpconnect/src/event_impl_gen.py?rev=8b872592e441#488
Comment 7•10 years ago
|
||
See Bug 983803
Comment 8•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/bff60e1caa23
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in
before you can comment on or make changes to this bug.
Description
•