Closed Bug 899804 Opened 6 years ago Closed 6 years ago

CPOWs don't handle instanceof with WebIDL interfaces

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: billm, Assigned: billm)

Details

Attachments

(2 files)

We handle instanceof correctly for XPCOM interfaces, but not for WebIDL interfaces. A lot more stuff is now using WebIDL, so we need a way to do this properly.
This patch does for WebIDL what we do for XPCOM interfaces (in XPCJSID.cpp). To implement |x instanceof Interface| when |x| is a CPOW and |interface| is in WebIDL, it sends an urgent (synchronous) IPC message to the child process with the interface's prototypeID and depth information. The child does the check on |x|'s counterpart and returns the result.

The normal BindingUtils code for HasInstance also does a proto chain walk. I don't think there's a way we can do that across processes so I left it out. Hopefully it's not too important.
Assignee: general → wmccloskey
Status: NEW → ASSIGNED
Attachment #784501 - Flags: review?(dvander)
Attachment #784501 - Flags: review?(bzbarsky)
Comment on attachment 784501 [details] [diff] [review]
webidl-instanceof

> Hopefully it's not too important.

It can't possibly be if x is a CPOW and Interface is not, since then the proto chain of x in fact will never hit Interface.prototype.

>+++ b/dom/bindings/BindingUtils.cpp
>+  JS::Rooted<JSObject*> unwrapped(cx, js::CheckedUnwrap(instance, true));
>+  if (mozilla::jsipc::JavaScriptParent::IsCPOW(unwrapped)) {

Can't "unwrapped" be null on that second line?  Should probably check that.

r=me as far as it goes, but this leaves broken the instanceof hooks for things like SVGUnitTypes, which don't actually use InterfaceHasInstance...
Attachment #784501 - Flags: review?(bzbarsky) → review+
Comment on attachment 784501 [details] [diff] [review]
webidl-instanceof

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

Nice.
Attachment #784501 - Flags: review?(dvander) → review+
Attached patch cpow-fixSplinter Review
I forgot this piece. It's needed if we send a CPOW back to the child process, and the CPOW was wrapped in a CCW. Interestingly, this happens in the context menu code.
Attachment #784557 - Flags: review?(dvander)
Comment on attachment 784501 [details] [diff] [review]
webidl-instanceof

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

::: dom/bindings/BindingUtils.cpp
@@ +1742,5 @@
>      return true;
>    }
>  
> +  JS::Rooted<JSObject*> unwrapped(cx, js::CheckedUnwrap(instance, true));
> +  if (mozilla::jsipc::JavaScriptParent::IsCPOW(unwrapped)) {

There's no need for the 'mozilla::'
Attachment #784557 - Flags: review?(dvander) → review+
Please do file a followup for the last part of comment 2, if you don't plan to fix it here.  Again, this patch only fixes instanceof for _some_ WebIDL interfaces.
https://hg.mozilla.org/mozilla-central/rev/123ae8924326
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.