The default bug view has changed. See this FAQ.

Implement JS_CallNonGenericMethodOnProxy

RESOLVED FIXED in mozilla16

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Waldo, Assigned: Waldo)

Tracking

unspecified
mozilla16
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Created attachment 630022 [details] [diff] [review]
Patch with JSAPI test

It's currently not possible for a JSAPI client to implement a method which works both on objects of a given class, and on proxies/wrappers for objects of that class (most often in the cross-compartment case, but also plausibly in the same-compartment-but-wrapped case).  Internally we have HandleNonGenericMethodClassMismatch for this; it's just a matter of exposing it.

The name is kinda fugly, but it's the best Luke and I could come up with.  The functionality offered is also hairy, but hopefully the jsapi.h comment should sufficiently elucidate things.  The method's primarily relevant to embeddings that have multiple interacting globals with similar functionality in each, where functionality must treat an object from one global the same as it'd treat an object from another global.  That's probably mostly the browser embedding, but I could almost imagine Wes or someone hitting this stuff as I know he uses multiple globals at least somewhat.

I need this to implement bug 758912.  I also strongly suspect that the DOM bindings folks will need this as well at some point so that things like this will work (I'd kind of like to know how they're doing it now -- I'm guessing with bad old hacked-up friend APIs at best):

  var desc = Object.getOwnPropertyDescriptor(otherWindow.Node.prototype, "nodeType");
  assertEq(desc.get.call(document.documentElement), Node.ELEMENT_TYPE);
Attachment #630022 - Flags: review?(luke)
Attachment #630022 - Flags: review?(dmandelin)
Without bug 761439 fixed, this patch works in that its test passes, but it won't properly work in cases where |this| is a same-compartment but wrapped object -- curiously, a case which I hit in my patch for bug 758912.  XBL: killing babies as always.
Blocks: 758912
Depends on: 761439

Comment 2

5 years ago
Why couldn't we expose NonGenericMethodGuard instead?  That way JSAPI clients wouldn't have to write the RequireMyClassThis you showed in the jsapi comment.
NonGenericMethodGuard isn't sufficient to meet my need for bug 758912, because in the words of HandleNonGenericMethodClassMismatch, "more than one class is acceptable".  Here's the condition that |this| must satisfy for that code:

http://hg.mozilla.org/mozilla-central/annotate/c76497029f0d/content/xbl/src/nsXBLBinding.cpp#l119

DOM bindings will have the same requirements, actually.  Consider the comment 0 example: the nodeType getter has to work on element nodes, document nodes, comment nodes, and so on, each having a different class.

Comment 4

5 years ago
Comment on attachment 630022 [details] [diff] [review]
Patch with JSAPI test

Very well, then.
Attachment #630022 - Flags: review?(luke) → review+
https://hg.mozilla.org/mozilla-central/rev/5a289ad94bfe
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
Comment on attachment 630022 [details] [diff] [review]
Patch with JSAPI test

Review of attachment 630022 [details] [diff] [review]:
-----------------------------------------------------------------

Looks like you got the review you needed already.
Attachment #630022 - Flags: review?(dmandelin)
You need to log in before you can comment on or make changes to this bug.