I'm generally happy with the approach you're taking. Like michaeln, this is a lot nicer than I was expecting, and a better solution than what I would have first tried to do myself. :)
======================================================================== http://mondrian.corp.google.com/file/9284351///depot/googleclient/gears/opensource/gears/base/common/js_runner_ff_marshaling.cc?a=1 File //depot/googleclient/gears/opensource/gears/base/common/js_runner_ff_marshaling.cc (snapshot 1) ------------------------------------ Line 554: // a completely bogus value. On 6 Dec, michaeln wrote: > The test on line 501 looks like its trying to accomplish the same thing, > validate that a JSObject refers to a GearsNativeObject. Maybe we can reuse that > test at this site? Yeah, they are trying to accomplish the same thing. However, JsContextWrapper::JsWrapperCaller is a static method and cannot access the Gears-specific (as opposed to a Spidermonkey-specific) JsContextWrapper class (and its shared_js_classes_ member variable). So I'm not sure if you could take the line-501 approach here. But you could change the line-501 code to take the same approach as you've done here. That change might be better as a separate ChangeList, though - perhaps add a TODO(nigeltao)? > Also not I'm not sure what the distinction is between the > JS_GET_CLASS macro and the JS_GetClass function? For Gears' purposes, the JS_GET_CLASS macro just expands to the JS_GetClass function. The difference is if you compile Spidermonkey as single- or multi- threaded (#ifdef JS_THREADSAFE). Gears does the latter. ======================================================================== http://mondrian.corp.google.com/file/9284351///depot/googleclient/gears/opensource/gears/base/common/js_runner_ff_marshaling.h?a=1 File //depot/googleclient/gears/opensource/gears/base/common/js_runner_ff_marshaling.h (snapshot 1) ------------------------------------ Line 143: static std::set<std::string> gears_module_names_; The module names should be known at compile time. It might be possible, then, to create (and populate) this set during the JsContextWrapper constructor, and then you might not need the Mutex. Perhaps this can be another TODO to investigate. ======================================================================== -- To respond, reply to this email or visit http://mondrian.corp.google.com/9284351
