Closed
Bug 453331
Opened 16 years ago
Closed 16 years ago
Quick stubs: handle members with the same name
Categories
(Core :: XPConnect, defect)
Core
XPConnect
Tracking
()
RESOLVED
FIXED
People
(Reporter: jorendorff, Assigned: jorendorff)
References
Details
Attachments
(2 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
jst
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
The general problem is, there are cases where two interfaces have scriptable members with the same name and there's a class that implements both interfaces. In these cases, XPConnect first calls NewResolve and lets the scriptable helper (if any) sort it out; and failing that, it exposes whichever member shows up first in the classinfo.
Quick stubs don't have access to scriptable helpers or classinfo, so we have to be careful about stubbing these cases.
Examples:
Two scriptable methods from different interfaces, same name, different signatures.
nsIDOMEventTarget::AddEventListener(
string, nsIDOMEventListener, boolean);
nsIDOMNSEventTarget::AddEventListener(
string, nsIDOMEventListener, boolean, boolean);
imgIDecoderObserver::OnStartRequest(imgIRequest);
nsIRequestObserver::OnStartRequest(nsIRequest, nsISupports*);
imgIDecoderObserver::OnStopRequest(imgIRequest, boolean);
nsIRequestObserver::OnStopRequest(nsIRequest, nsISupports, nsresult);
string nsIDOMJSWindow::Prompt();
string nsIDOMWindowInternal::Prompt(
string, string, string, unsigned int);
nsIDOMDocument nsIDOMWindow::GetDocument();
nsIDOMDocumentView nsIDOMAbstractView::GetDocument();
Two IDL attributes, same name; one is readonly and one is not. Bug 453105 and the bug described in bug 407216 comment 106 are both special cases of this.
nsIDOMHTMLCollection.length (readonly)
nsIDOMHTMLOptionsCollection.length
nsIDOMHTMLOptionElement.text (readonly)
nsIDOMNSHTMLOptionElement.text
Potentially, multiple interfaces implemented by a class might have methods with the same name and signature, but different implementations. There are lots of cases where a class implements two unrelated interfaces that both have a method (of which a few are listed here) but as far as I know, every class that implements both implements them with a single method. So there's no actual conflict. (IIRC, C++ makes it possible but convoluted to implement them differently.)
nsIDOMNode nsIDOMNodeList::Item(unsigned int);
nsIDOMNode nsIDOMHTMLCollection::Item(unsigned int);
unsigned int nsIDOMNodeList::GetLength();
unsigned int nsIDOMHTMLCollection::GetLength();
boolean nsIEditor::CanDrag(nsIDOMEvent);
boolean nsIHTMLEditor::CanDrag(nsIDOMEvent);
Potentially, an XPCOM object might implement multiple interfaces via tearoffs that could have this issue. I don't know an easy way to find these cases. Not being able to find all the cases is scary.
Comment 1•16 years ago
|
||
> Potentially, multiple interfaces implemented by a class might have methods with
> the same name and signature, but different implementations.
Anything like that would already be broken when used from JS (no way to indicate which method you "really" want), so I don't think we should worry about it.
Assignee | ||
Comment 2•16 years ago
|
||
(In reply to comment #1)
> > Potentially, multiple interfaces implemented by a class might have methods with
> > the same name and signature, but different implementations.
>
> Anything like that would already be broken when used from JS (no way to
> indicate which method you "really" want), so I don't think we should worry
> about it.
Right. A scriptable helper could make it nonbroken. But then there shouldn't be quick stubs for anything that has special magic scriptable-helper support.
Assignee | ||
Comment 3•16 years ago
|
||
(Clarifying comment #0)
> Quick stubs don't have access to scriptable helpers or classinfo,
> so we have to be careful about stubbing these cases.
I meant the stuff that generates quick stubs at compile time. (We don't want quick stubs to call scriptable helpers or examine classinfo at run time either, of course.)
Assignee | ||
Comment 4•16 years ago
|
||
This treehydra script checks for name conflicts in classes that implement multiple interfaces. All known potential conflicts are listed.
(This patch disables the stack.js analysis because the tree currently can't pass that one-- it stops the build with errors.)
Assignee: nobody → jorendorff
Assignee | ||
Comment 5•16 years ago
|
||
This also deletes an obsolete comment in qsgen.py. (That bug was fixed before quickstubs landed.)
Attachment #337286 -
Flags: superreview?(jst)
Attachment #337286 -
Flags: review?(jst)
Updated•16 years ago
|
Attachment #337286 -
Flags: superreview?(jst)
Attachment #337286 -
Flags: superreview+
Attachment #337286 -
Flags: review?(jst)
Attachment #337286 -
Flags: review+
Assignee | ||
Comment 6•16 years ago
|
||
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Comment 7•16 years ago
|
||
Comment on attachment 337286 [details] [diff] [review]
v1
>+ # Caution: nsGlobalWindow also implements nsIDOMAbstractView, which *also*
>+ # has an attribute named document (of a different type!). The two getters
>+ # behave differently.
Is this behaviour difference a bug?
Comment 8•16 years ago
|
||
Probably not: the nsIDOMDocument behavior is there for web compat, and since web code doesn't call the other overload we're OK.
You need to log in
before you can comment on or make changes to this bug.
Description
•