Closed Bug 571619 Opened 10 years ago Closed 10 years ago
Interface from the canonical ns ISupports pointer of an ns Simple Nested URI directly to ns INested URI
nsSimpleNestedURI inherits from nsSimpleURI, which supports aggregation. It does this by containing a dummy object that performs QueryInterface on its behalf. Thus whenever you QueryInterface to an interface that nsSimpleURI knows about, you get the nsSimpleURI itself, otherwise you get whatever the aggregate or dummy object supports. Because the dummy object only supports nsISupports, it cannot QueryInterface to nsINestedURI. Only the real nsSimpleNestedURI object can do that. There appear to be three possibilities: 1. Make nsSimpleNestedURI an object that aggregates nsSimpleURI. Looking at the // XXX comment in StartClone, this would probably break serialisation. 2. Remove the aggregation code from nsSimpleURI. This means that it gets a normal QueryInterface method that nsSimpleNestedURI can inherit. 3. Write a custom QueryInterface method for nsSimpleNestedURI that hides the aggregation. It can do this either by implementing all the interfaces that nsSimpleNestedURI supports or by forwarding all the interfaces except nsISupports and nsINestedURI to nsSimpleURI.
This is one way of implementing option 3. Another way would be to write NS_IMPL_QUERY_INTERFACE5(nsSimpleNestedURI, nsIURI, nsISerializable, nsIClassInfo, nsIMutable, nsINestedURI)
That sems fine... I'd also be fine with solution 2, since I've never understood why aggregation is involved here at all.
OK, after some mxr'ing the aggregation part of nsSimpleURI seems unused, so it's probably best to just remove it and go with option 2.
Comment on attachment 450860 [details] [diff] [review] With test - } else if (aIID.Equals(kThisSimpleURIImplementationCID) || // used by Equals you need to keep this, right?
Attachment #450860 - Flags: review?(cbiesinger) → review-
10 years ago
Attachment #450874 - Flags: review?(cbiesinger) → review+
Pushed changeset 1b9e95be69ab to mozilla-central.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.