Closed
Bug 1262400
Opened 8 years ago
Closed 8 years ago
remove outer pointers from nsCreateInstance* classes
Categories
(Core :: XPCOM, defect)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: froydnj, Assigned: froydnj)
Details
Attachments
(1 file)
5.47 KB,
patch
|
erahm
:
review+
|
Details | Diff | Splinter Review |
Outer pointers for object aggregation never get used. Having these always-null pointers around means extra space to store them and extra instructions to deal with them. Let's just remove them.
Assignee | ||
Comment 1•8 years ago
|
||
Attachment #8738506 -
Flags: review?(erahm)
Comment 2•8 years ago
|
||
Do we use aggregation anywhere at all any more? Can we get rid of all the aggregation infrastructure including the outer pointers on nsIComponentManager.createInstance* and the nsAgg.h macrology?
Assignee | ||
Comment 3•8 years ago
|
||
(In reply to Benjamin Smedberg [:bsmedberg] from comment #2) > Do we use aggregation anywhere at all any more? Can we get rid of all the > aggregation infrastructure including the outer pointers on > nsIComponentManager.createInstance* and the nsAgg.h macrology? It looks as though there are a few users of things in nsAgg.h (load groups, some things in rdf/, and nsProperties). I'm not sure how complicated those are to get rid of, but they'd probably be worth doing...
Comment 4•8 years ago
|
||
Comment on attachment 8738506 [details] [diff] [review] remove outer pointers from nsCreateInstance* classes Review of attachment 8738506 [details] [diff] [review]: ----------------------------------------------------------------- Sorry I thought I already r+'d this last week, r=me. Just one point of clarification requested. ::: xpcom/glue/nsComponentManagerUtils.cpp @@ +207,5 @@ > nsresult > nsCreateInstanceFromFactory::operator()(const nsIID& aIID, > void** aInstancePtr) const > { > + nsresult status = mFactory->CreateInstance(nullptr, aIID, aInstancePtr); This is the only piece that's a little confusing to me. Can you explain what's going on here w/ nullptr (and maybe add a comment in the code). I guess it's because |mOuter| was always nullptr?
Attachment #8738506 -
Flags: review?(erahm) → review+
Assignee | ||
Comment 5•8 years ago
|
||
(In reply to Eric Rahm [:erahm] from comment #4) > ::: xpcom/glue/nsComponentManagerUtils.cpp > @@ +207,5 @@ > > nsresult > > nsCreateInstanceFromFactory::operator()(const nsIID& aIID, > > void** aInstancePtr) const > > { > > + nsresult status = mFactory->CreateInstance(nullptr, aIID, aInstancePtr); > > This is the only piece that's a little confusing to me. Can you explain > what's going on here w/ nullptr (and maybe add a comment in the code). I > guess it's because |mOuter| was always nullptr? Right, this is just making explicit what always happened anyway.
Comment 7•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/62f56187fc27
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in
before you can comment on or make changes to this bug.
Description
•