Quantcast

JS 45 problem

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

JS 45 problem

Kent Williams
I recently did the work to migrate from JS38 to JS45.  I can compile all
my code embedding SpiderMonkey -- yay! -- but there's a problem with
adding JSNative classes with JS_InitClass.

The following example (based on the "Hello World" example quits with an
error message "TESTO.good is not a function"  This code worked fine
built against JS38.

http://pastebin.com/H7gVxng1

// This is an example for SpiderMonkey 31.
// For SpiderMonkey 24 or 38, see each comment.

// following code might be needed in some case
// #define __STDC_LIMIT_MACROS
// #include <stdint.h>
#if defined(JSAPI_DEBUG)
#define DEBUG
#endif
#include <iostream>
#include <jsapi.h>
#include <js/Initialization.h>

// TEST CODE FOR JS_InitClass
static bool TESTO_good(JSContext *cx, unsigned int argc, JS::Value *vp) {
     JS::CallArgs      args = JS::CallArgsFromVp(argc, vp);
     JS::RootedObject obj( cx, JS_THIS_OBJECT(cx, vp) );
     args.rval().setBoolean(true);
     return true;
}

static JSFunctionSpec TESTO_methods[] = {
     JS_FS("good", TESTO_good, 1, 0),
     JS_FS_END
};

static void TESTO_dtor(JSFreeOp *, JSObject *obj) {
}

static JSClass TESTO = {
     "TESTO", JSCLASS_HAS_PRIVATE,
     0,                                      // JSPropertyOp addProperty;
     0,                                      // JSDeletePropertyOp
delProperty;
     0,                                      // JSPropertyOp getProperty;
     0,                                      // JSStrictPropertyOp
setProperty;
     0,                                      // JSEnumerateOp enumerate;
     0,                                      // JSResolveOp resolve;
     0,                                      // JSConvertOp convert;
     TESTO_dtor,                  // FinalizeOpType      finalize;
     0,                                      // JSNative call;
     0,                                      // JSHasInstanceOp hasInstance;
     0,                                      // JSNative construct;
     0,      // JSTraceOp           trace
};

static bool TESTO_ctor(JSContext *cx, unsigned int argc, JS::Value *vp) {
     JS::CallArgs      args = JS::CallArgsFromVp(argc, vp);
     JS::RootedObject obj(cx);
     if (args.isConstructing()) {
         obj = JS_NewObject(cx, &TESTO);
         if (!obj)
             return false;
     } else {
         obj = JS_THIS_OBJECT(cx, vp);
     }
     args.rval().setObject(*obj);
     return true;
}

bool init_TESTO(JSContext *cx, JS::HandleObject globs) {
     if(JS_InitClass(cx, globs, nullptr, &TESTO,
                          TESTO_ctor, 0, nullptr, TESTO_methods,
nullptr, nullptr) == nullptr)
         abort();
     return true;
}

// KW ADD -- need JS_GlobalObjectTraceHook
JSClass global_class = {
     "global", JSCLASS_GLOBAL_FLAGS,
     0,                                      // JSPropertyOp addProperty;
     0,                                      // JSDeletePropertyOp
delProperty;
     0,                                      // JSPropertyOp getProperty;
     0,                                      // JSStrictPropertyOp
setProperty;
     0,                                      // JSEnumerateOp enumerate;
     0,                                      // JSResolveOp resolve;
     0,                                      // JSConvertOp convert;
     0,                                      // FinalizeOpType finalize;
     0,                                      // JSNative call;
     0,                                      // JSHasInstanceOp hasInstance;
     0,                                      // JSNative construct;
     JS_GlobalObjectTraceHook,      // JSTraceOp           trace
};

void JSErrorHandler(JSContext *cx, const char *message, JSErrorReport*
what) {
     std::cout << "ERROR: " << message;
     if (what->filename)
         std::cout << ": file: " << what->filename;
     std::cout << ", line:" << (int) what->lineno
                  << ", column:" << (int) what->column
                  << std::endl;
}

int main(int argc, const char *argv[])
{
     // [SpiderMonkey 24] JS_Init does not exist. Remove this line.
     JS_Init();

     // [SpiderMonkey 38] useHelperThreads parameter is removed.
     JSRuntime *rt = JS_NewRuntime(8L * 1024 * 1024);
     // JSRuntime *rt = JS_NewRuntime(8L * 1024 * 1024,
JS_USE_HELPER_THREADS);
     if (!rt)
         return 1;

     JSContext *cx = JS_NewContext(rt, 8192);
     if (!cx)
         return 1;
      // KW ADD -- everything that happens needs to be inside a request.
      JS_BeginRequest(cx);

     // [SpiderMonkey 24] hookOption parameter does not exist.
     // JS::RootedObject global(cx, JS_NewGlobalObject(cx,
&global_class, nullptr));
      // KW ADD -- move global down into brace so it will go out of
     // scope before DestroyContext

      // KW ADD -- move rval inside scope, so
      // it will be destroyed before cleanup at end
     {
          JS::RootedValue rval(cx);
          JS::RootedObject global(cx,
                                          JS_NewGlobalObject(cx,
&global_class, nullptr, JS::FireOnNewGlobalHook));
          if (!global)
              return 1;
         JSAutoCompartment ac(cx, global);
       JS_InitStandardClasses(cx, global);
         init_TESTO(cx, global);
         // set error reporter
         JS_SetErrorReporter(rt, JSErrorHandler);


       const char *script = "'hello'+'world, it is '+new Date();\n"
             "var TESTO = new TESTO(\"testzip.zip\");\n"
             "if (!TESTO || !TESTO.good())\n"
             "throw \"FAIL: cannot create TESTO\";\n";
       const char *filename = "noname";
       int lineno = 1;
       // [SpiderMonkey 24] The type of rval parameter is 'jsval *'.
       // bool ok = JS_EvaluateScript(cx, global, script,
strlen(script), filename, lineno, rval.address());
       // [SpiderMonkey 38] JS_EvaluateScript is replaced with JS::Evaluate.
       JS::CompileOptions opts(cx);
       opts.setFileAndLine(filename, lineno);
       bool ok = JS::Evaluate(cx, opts, script, strlen(script), &rval);
       // bool ok = JS_EvaluateScript(cx, global, script,
strlen(script), filename, lineno, &rval);
       if (!ok)
         return 1;

     JSString *str = rval.toString();
     printf("%s\n", JS_EncodeString(cx, str));
     }
      // KW ADD
      JS_EndRequest(cx);
     JS_DestroyContext(cx);
     JS_DestroyRuntime(rt);
     JS_ShutDown();
     return 0;
}

_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: JS 45 problem

Boris Zbarsky
On 5/24/16 10:32 AM, Kent Williams wrote:
>         obj = JS_NewObject(cx, &TESTO);

Kent,

This line is the problem.  JS_NewObject used to guess at the prototype
to use based on the name of the given class and what that name resolved
to in global scope.

This was all sorts of broken and fragile, and was removed (see
<https://bugzilla.mozilla.org/show_bug.cgi?id=1125567>).  If you use
JS_NewObject, you will get an object whose prototype is Object.prototype.

What you want here is JS_NewObjectWithGivenProto and pass in the thing
you want as your prototype.

I think you have a few options for how to do that:

1)  Save the value returned from JS_InitClass and use that.  Should be
the right thing as long as no one is subclassing you.

2)  In the constructor, get the "prototype" property of args.callee()
and use that.  Again, should be the right thing as long as no one is
subclassing you.

3)  In the constructor, get the "prototype" property of
args.newTarget().  Should do the right thing (as in, use the proto of
the constructor that was actually invoked) even if you're subclassed.

-Boris
_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: JS 45 problem

Kent Williams
Thanks a lot.  I remember talk about this. Luckily we don't (yet!) have
a lot of code using JS_InitClass

On 05/24/2016 09:47 AM, Boris Zbarsky wrote:

> On 5/24/16 10:32 AM, Kent Williams wrote:
>>         obj = JS_NewObject(cx, &TESTO);
>
> Kent,
>
> This line is the problem.  JS_NewObject used to guess at the prototype
> to use based on the name of the given class and what that name
> resolved to in global scope.
>
> This was all sorts of broken and fragile, and was removed (see
> <https://bugzilla.mozilla.org/show_bug.cgi?id=1125567>).  If you use
> JS_NewObject, you will get an object whose prototype is Object.prototype.
>
> What you want here is JS_NewObjectWithGivenProto and pass in the thing
> you want as your prototype.
>
> I think you have a few options for how to do that:
>
> 1)  Save the value returned from JS_InitClass and use that. Should be
> the right thing as long as no one is subclassing you.
>
> 2)  In the constructor, get the "prototype" property of args.callee()
> and use that.  Again, should be the right thing as long as no one is
> subclassing you.
>
> 3)  In the constructor, get the "prototype" property of
> args.newTarget().  Should do the right thing (as in, use the proto of
> the constructor that was actually invoked) even if you're subclassed.
>
> -Boris
> _______________________________________________
> dev-tech-js-engine mailing list
> [hidden email]
> https://lists.mozilla.org/listinfo/dev-tech-js-engine

_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: JS 45 problem

Kent Williams
In reply to this post by Boris Zbarsky
So my change ends up being this:

-        obj = JS_NewObject(cx, &spider_xml);
+        JS::RootedObject newTarget(cx,&args.newTarget().toObject());
+        JS::RootedObject protoObj(cx);
+        if(!JS_GetPrototype(cx,newTarget,&protoObj))
+            return false;
+        obj = JS_NewObjectWithGivenProto(cx, &spider_xml, protoObj);

Is there a more succinct way to accomplish this?  I remember this being
talked about on this mailing list a couple weeks ago, in a slightly
different context.

On 05/24/2016 09:47 AM, Boris Zbarsky wrote:

> On 5/24/16 10:32 AM, Kent Williams wrote:
>>         obj = JS_NewObject(cx, &TESTO);
>
> Kent,
>
> This line is the problem.  JS_NewObject used to guess at the prototype
> to use based on the name of the given class and what that name
> resolved to in global scope.
>
> This was all sorts of broken and fragile, and was removed (see
> <https://bugzilla.mozilla.org/show_bug.cgi?id=1125567>).  If you use
> JS_NewObject, you will get an object whose prototype is Object.prototype.
>
> What you want here is JS_NewObjectWithGivenProto and pass in the thing
> you want as your prototype.
>
> I think you have a few options for how to do that:
>
> 1)  Save the value returned from JS_InitClass and use that. Should be
> the right thing as long as no one is subclassing you.
>
> 2)  In the constructor, get the "prototype" property of args.callee()
> and use that.  Again, should be the right thing as long as no one is
> subclassing you.
>
> 3)  In the constructor, get the "prototype" property of
> args.newTarget().  Should do the right thing (as in, use the proto of
> the constructor that was actually invoked) even if you're subclassed.
>
> -Boris
> _______________________________________________
> dev-tech-js-engine mailing list
> [hidden email]
> https://lists.mozilla.org/listinfo/dev-tech-js-engine

_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: JS 45 problem

Boris Zbarsky
In reply to this post by Boris Zbarsky
On 5/24/16 12:17 PM, Kent Williams wrote:
> +        JS::RootedObject newTarget(cx,&args.newTarget().toObject());
> +        JS::RootedObject protoObj(cx);
> +        if(!JS_GetPrototype(cx,newTarget,&protoObj))

No, that's wrong.  What you want is more like:

   JS::RootedValue protoVal(cx);
   if (!JS_GetProperty(cx, newTarget, "prototype", &protoVal))
     return false;
   JS::RootedObject protoObj(cx);
   if (protoVal.isObject())
     protoObj = &protoVal.toObject();
   else
     protoObj = JS_GetObjectPrototype(cx, newTarget);
   obj = JS_NewObjectWithGivenProto(cx, &spider_xml, protoObj);

(yes, the signature of JS_GetObjectPrototype is dumb; we should fix that).

Basically, implement
http://www.ecma-international.org/ecma-262/6.0/#sec-getprototypefromconstructor 
with the second arg being %ObjectPrototype%.

I don't think there's a more succinct API for this, sadly.  We should
totally consider adding one; something like:

   JS::GetPrototypeFromConstructor(JSContext *cx,
                                   JSObject* ctor,
                                   JSProtoKey key);

or something.  I filed
https://bugzilla.mozilla.org/show_bug.cgi?id=1275312 on that.

-Boris
_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: JS 45 problem

paraboul
Hey,


Out of curiosity, what's the advantage of using |JS_NewObjectWithGivenProto| over |JS_NewObjectForConstructor| within a constructor call?

On Tuesday, May 24, 2016 at 6:29:24 PM UTC+2, Boris Zbarsky wrote:

> On 5/24/16 12:17 PM, Kent Williams wrote:
> > +        JS::RootedObject newTarget(cx,&args.newTarget().toObject());
> > +        JS::RootedObject protoObj(cx);
> > +        if(!JS_GetPrototype(cx,newTarget,&protoObj))
>
> No, that's wrong.  What you want is more like:
>
>    JS::RootedValue protoVal(cx);
>    if (!JS_GetProperty(cx, newTarget, "prototype", &protoVal))
>      return false;
>    JS::RootedObject protoObj(cx);
>    if (protoVal.isObject())
>      protoObj = &protoVal.toObject();
>    else
>      protoObj = JS_GetObjectPrototype(cx, newTarget);
>    obj = JS_NewObjectWithGivenProto(cx, &spider_xml, protoObj);
>
> (yes, the signature of JS_GetObjectPrototype is dumb; we should fix that).
>
> Basically, implement
> http://www.ecma-international.org/ecma-262/6.0/#sec-getprototypefromconstructor 
> with the second arg being %ObjectPrototype%.
>
> I don't think there's a more succinct API for this, sadly.  We should
> totally consider adding one; something like:
>
>    JS::GetPrototypeFromConstructor(JSContext *cx,
>                                    JSObject* ctor,
>                                    JSProtoKey key);
>
> or something.  I filed
> https://bugzilla.mozilla.org/show_bug.cgi?id=1275312 on that.
>
> -Boris

_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: JS 45 problem

Boris Zbarsky
On 11/3/16 5:18 PM, [hidden email] wrote:
> Out of curiosity, what's the advantage of using |JS_NewObjectWithGivenProto| over |JS_NewObjectForConstructor| within a constructor call?

JS_NewObjectForConstructor (which I hadn't actually known about, fwiw)
will always use callee's .prototype as the proto, falling back to
Object.prototype if callee.prototype is not an object.

The code I sent that used JS_NewObjectWithGivenProto was getting
.prototype from the newTarget, not the callee.  This only matters if
your JSNative constructor is being subclassed in script.

We should probably change JS_NewObjectForConstructor to play nice with
subclassing; if we did that it would be the obvious thing to use here.

-Boris
_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Loading...