Closed Bug 571619 Opened 10 years ago Closed 10 years ago

Can't QueryInterface from the canonical nsISupports pointer of an nsSimpleNestedURI directly to nsINestedURI

Categories

(Core :: Networking, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: neil, Assigned: neil)

References

()

Details

Attachments

(1 file, 2 obsolete files)

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.
Attached patch Possible patch (obsolete) — Splinter Review
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.
Attached patch With test (obsolete) — Splinter Review
Assignee: nobody → neil
Attachment #450766 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #450860 - Flags: review?(cbiesinger)
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-
Attachment #450860 - Attachment is obsolete: true
Attachment #450874 - Flags: review?(cbiesinger)
Attachment #450874 - Flags: review?(cbiesinger) → review+
Pushed changeset 1b9e95be69ab to mozilla-central.
Blocks: 408599
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.