Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upAdd proxy handler deriving from DirectWrapper and related methods, along... #52
Conversation
|
It sucks to introduce js::Class here, but so it goes. |
| defineProperty: extern "C" fn(*JSContext, *JSObject, jsid, *JSPropertyDescriptor) -> JSBool, | ||
| getPropertyDescriptor: Option<extern "C" fn(*JSContext, *JSObject, jsid, c_bool, *mut JSPropertyDescriptor) -> c_bool>, | ||
| getOwnPropertyDescriptor: Option<extern "C" fn(*JSContext, *JSObject, jsid, JSBool, *mut JSPropertyDescriptor) -> JSBool>, | ||
| defineProperty: Option<extern "C" fn(*JSContext, *JSObject, jsid, *JSPropertyDescriptor) -> JSBool>, | ||
| getOwnPropertyNames: *u8, |
This comment has been minimized.
This comment has been minimized.
bholley
Mar 19, 2014
I don't grok the logic behind what gets stubbed out as *u8 and what gets a signature. In particular, any proxy implementation that does something special with |hasOwn| almost certainly needs to override getOwnPropertyNames.
This comment has been minimized.
This comment has been minimized.
jdm
Mar 19, 2014
Author
Member
The *u8 stubs are there because writing out each type is annoying and I haven't needed them yet. That's all.
| class WrapperProxyHandler : public js::DirectWrapper | ||
| { | ||
| ProxyTraps mTraps; | ||
| void* mExtra; |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| return mTraps.getPropertyDescriptor ? | ||
| mTraps.getPropertyDescriptor(cx, proxy, id, set, desc) : | ||
| DirectWrapper::getPropertyDescriptor(cx, proxy, id, set, desc); | ||
| } |
This comment has been minimized.
This comment has been minimized.
bholley
Mar 19, 2014
In general, anybody implemented a wrapper-ish proxy (like outer window) is going to want to bounce to the base handler after potentially doing some special handing. It looks like this setup is either-or, which isn't going to work very well.
This comment has been minimized.
This comment has been minimized.
jdm
Mar 19, 2014
Author
Member
Yeah, there are methods further down for invoking other base handlers; I can add more for the rest of them.
…ong with extended js::Class FFI.
Upgrade Rust.
Better static linking r? @metajack @mbrubeck CC @alexcrichton <!-- Reviewable:start --> [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/mozjs/52) <!-- Reviewable:end -->
jdm commentedMar 5, 2014
... with extended js::Class FFI.
Needed for servo/servo#1818.