Eliminate call to do_CreateInstance(NS_VARIANT_CONTRACTID) in CreateVoidVariant

RESOLVED FIXED in Firefox 44

Status

()

Core
DOM
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: mccr8, Assigned: mccr8)

Tracking

Trunk
mozilla44
Points:
---

Firefox Tracking Flags

(firefox44 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
In bug 1210517, I'm eliminating all calls to do_CreateInstance(NS_VARIANT_CONTRACTID) in favor of calling new nsVariant() directly. The one remaining instance after the patch in that bug is in CreateVoidVariant, because I didn't want to add a new #include to nsGlobalWindow.h. The easiest fix is probably to outline CreateVoidVariant into nsGlobalWindow.cpp. If the performance is even actually an issue here, you'd still be replacing a virtual call with a static one, so I'd think you'd still be ahead. I'm not really sure why this little method actually even exists. It looks like bholley added it in bug 860941.
(Assignee)

Updated

2 years ago
Assignee: nobody → continuation
(Assignee)

Comment 1

2 years ago
Created attachment 8670820 [details] [diff] [review]
Outline CreateVoidVariant and create the variant directly.

I also eliminated the static because a static method in a header seems weird.

try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ea2d0d9c982e
Attachment #8670820 - Flags: review?(bzbarsky)
Comment on attachment 8670820 [details] [diff] [review]
Outline CreateVoidVariant and create the variant directly.

How about you make CreateVoidVariant a static method in nsGlobalWindow.cpp and put DialogValueHolder::Get() out of line?

r=me with that
Attachment #8670820 - Flags: review?(bzbarsky) → review+

Comment 3

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/5e9824ba465b
(Assignee)

Comment 4

2 years ago
(In reply to Boris Zbarsky [:bz] from comment #2)
> How about you make CreateVoidVariant a static method in nsGlobalWindow.cpp
> and put DialogValueHolder::Get() out of line?
That's a lot nicer; thanks for the suggestion.
https://hg.mozilla.org/mozilla-central/rev/5e9824ba465b
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox44: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.