Closed
Bug 568731
Opened 15 years ago
Closed 14 years ago
TM: rename JSNoopProxyHandler to JSWrapper and move it into jswrapper.[h|cpp]
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: gal, Assigned: gal)
References
Details
(Whiteboard: fixed-in-tracemonkey)
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
cdleary
:
review+
|
Details | Diff | Splinter Review |
This file will hold our new membranes and wrapper code.
Assignee | ||
Comment 1•15 years ago
|
||
Assignee: general → gal
Assignee | ||
Updated•15 years ago
|
Attachment #447899 -
Flags: review?(mozilla+ben)
Assignee | ||
Comment 2•15 years ago
|
||
Review ping.
Assignee | ||
Updated•15 years ago
|
Attachment #447899 -
Flags: review?(mozilla+ben) → review?(cdleary)
Comment 3•15 years ago
|
||
Comment on attachment 447899 [details] [diff] [review]
patch
The singleton wrapper object is a static member of the |JSWrapper| class, so calling |JSWrapper::wrap| will only ever resolve to |JSWrapper::construct| when |singleton.construct| is called in that function -- I don't think that's what we want. The interface that was in there before, |JSWrapper<typename T extends JSWrapper>::wrap| seems to make the most sense, so you can grab |T::singleton| like it was doing. You can expose this kind of thing across C API boundaries by making a helper function per wrapper class that does something like |NopWrap = JSWrapper<JSWrapper>::wrap|.
The interface for JSNoopProxyHandler is still in there, should rip that out too.
The naming is getting confusing -- in ::wrap you have |JSWrapper *handler, JSObject *wrapper|. I get that the wrapper is the JS-land wrapper object, but still makes me double take. :-)
Classes in namespace js don't need a JS prefix, but that's probably for another bug.
Attachment #447899 -
Flags: review?(cdleary)
Assignee | ||
Comment 4•15 years ago
|
||
Attachment #447899 -
Attachment is obsolete: true
Attachment #448302 -
Flags: review?(cdleary)
Assignee | ||
Comment 5•15 years ago
|
||
I will leave a more complex solution to wrap() to a follow-up patch that actually specializes wrappers.
Updated•15 years ago
|
Attachment #448302 -
Flags: review?(cdleary) → review+
Assignee | ||
Comment 6•15 years ago
|
||
Whiteboard: fixed-in-tracemonkey
Updated•14 years ago
|
Blocks: compartments-api
Comment 7•14 years ago
|
||
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•