Closed Bug 1262400 Opened 8 years ago Closed 8 years ago

remove outer pointers from nsCreateInstance* classes

Categories

(Core :: XPCOM, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: froydnj, Assigned: froydnj)

Details

Attachments

(1 file)

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.
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?
(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 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+
(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.
https://hg.mozilla.org/mozilla-central/rev/62f56187fc27
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.