Closed
Bug 899804
Opened 11 years ago
Closed 11 years ago
CPOWs don't handle instanceof with WebIDL interfaces
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla25
People
(Reporter: billm, Assigned: billm)
Details
Attachments
(2 files)
9.70 KB,
patch
|
bzbarsky
:
review+
dvander
:
review+
|
Details | Diff | Splinter Review |
784 bytes,
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•11 years ago
|
||
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 2•11 years ago
|
||
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+
Assignee | ||
Comment 4•11 years ago
|
||
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 5•11 years ago
|
||
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::'
Updated•11 years ago
|
Attachment #784557 -
Flags: review?(dvander) → review+
Assignee | ||
Comment 6•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/70ac9672075b
Assignee | ||
Comment 7•11 years ago
|
||
Backed out for some weird build errors on Android. https://hg.mozilla.org/integration/mozilla-inbound/rev/e42f623fd574
Comment 8•11 years ago
|
||
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.
Assignee | ||
Comment 9•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/123ae8924326 I filed bug 901164 for the SVG stuff.
Comment 10•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/123ae8924326
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in
before you can comment on or make changes to this bug.
Description
•