JSAPI interface break about to land: C++ setter functions for JS properties

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

JSAPI interface break about to land: C++ setter functions for JS properties

Jim Blandy-3
I'm about to land a change to one of JSAPI's widely used public types.
My patch includes fixes for Firefox on all platforms, but no fixes
outside the browser (Thunderbird, Seamonkey, etc). The compiler catches
all code where changes are needed, and the fixes required are minor.

Details:

In order to implement ECMAScript 5's strict mode correctly, some object
property setters need to know whether the assignment they're carrying
out was written in strict mode code or old-style code. For example, the
following must fail:

        var a=[1,2,3]
        Object.defineProperty(a,2,{configurable:false})
        a.length = 2

because the assignment to 'a.length' is unable to delete the last
element of the array. The failure must be silent in 'lenient mode' code,
but throw an exception in strict mode code. The setter for arrays'
length properties needs to be sensitive to the strictness of the code in
which the assignment is written.

As discussed in bug 537873:
https://bugzilla.mozilla.org/show_bug.cgi?id=537873

after some iterations and backouts, the best solution we found was to
simply add a 'JSBool strict' argument to the setter callback functions
for JS object properties. Thus, whereas getters and setters both used to
be JSPropertyOp, setters are now JSStrictPropertyOp. (Getters are
unchanged.)

Fortunately, since we're changing a type, you'll get compiler errors for
all the code that needs to be changed. The fixes I've needed to make
have fallen into three main categories:

1) Use JS_StrictPropertyStub for the default setter in class
definitions, not JS_PropertyStub:

> @@ -144,7 +144,7 @@ nsXULPDGlobalObject_resolve(JSContext *c
>  JSClass nsXULPDGlobalObject::gSharedGlobalClass = {
>      "nsXULPrototypeScript compilation scope",
>      JSCLASS_HAS_PRIVATE | JSCLASS_PRIVATE_IS_NSISUPPORTS | JSCLASS_GLOBAL_FLAGS,
> -    JS_PropertyStub,  JS_PropertyStub, JS_PropertyStub, JS_PropertyStub,
> +    JS_PropertyStub,  JS_PropertyStub, JS_PropertyStub, JS_StrictPropertyStub,
>      JS_EnumerateStub, nsXULPDGlobalObject_resolve,  JS_ConvertStub,
>      nsXULPDGlobalObject_finalize
>  };

2) Use JS_StrictPropertyStub for setters, not JS_PropertyStub, when
defining properties explictly:

> @@ -6905,7 +6913,7 @@ nsWindowSH::NewResolve(nsIXPConnectWrapp
>        JSAutoRequest ar(cx);
>
>        if (!::JS_DefinePropertyById(cx, obj, id, JSVAL_VOID, JS_PropertyStub,
> -                                   JS_PropertyStub, JSPROP_ENUMERATE)) {
> +                                   JS_StrictPropertyStub, JSPROP_ENUMERATE)) {
>          return NS_ERROR_FAILURE;
>        }
>        *objp = obj;

3) Add a 'JSBool strict' argument to real setters. The setters can
ignore this new argument.

@@ -138,7 +138,7 @@ nsXBLDocGlobalObject_getProperty(JSConte

  static JSBool
  nsXBLDocGlobalObject_setProperty(JSContext *cx, JSObject *obj,
-                                 jsid id, jsval *vp)
+                                 jsid id, JSBool strict, jsval *vp)
  {
    return nsXBLDocGlobalObject::
      doCheckAccess(cx, obj, id,
nsIXPCSecurityManager::ACCESS_SET_PROPERTY);

If you'd like to see the details of how I resolved these in the browser
and within SpiderMonkey itself, see these two patches:

https://bugzilla.mozilla.org/attachment.cgi?id=508274&action=diff
https://bugzilla.mozilla.org/attachment.cgi?id=509033&action=diff

This is annoying, but it's the best way to ES5 compliance, and it's
actually pretty quick to go through these and fix them, and it doesn't
require any deep understanding of the code in question.
_______________________________________________
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
|

Re: JSAPI interface break about to land: C++ setter functions for JS properties

Mark Banner-2
On 09/02/2011 19:26, Jim Blandy wrote:
> I'm about to land a change to one of JSAPI's widely used public types.

How soon is "about" ?

Standard8
_______________________________________________
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
|

Re: JSAPI interface break about to land: C++ setter functions for JS properties

Jim Blandy-3
On 02/09/2011 12:03 PM, Mark Banner wrote:
> On 09/02/2011 19:26, Jim Blandy wrote:
>> I'm about to land a change to one of JSAPI's widely used public types.
>
> How soon is "about" ?

It is currently landed in TraceMonkey, and will appear in Mozilla
Central via the next merge.

_______________________________________________
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
|

Re: JSAPI interface break about to land: C++ setter functions for JS properties

Mark Banner-2
On 09/02/2011 22:25, Jim Blandy wrote:
> On 02/09/2011 12:03 PM, Mark Banner wrote:
>> On 09/02/2011 19:26, Jim Blandy wrote:
>>> I'm about to land a change to one of JSAPI's widely used public types.
>>
>> How soon is "about" ?
>
> It is currently landed in TraceMonkey, and will appear in Mozilla
> Central via the next merge.
>
Thanks. I ran the latest tracemonkey through Thunderbird's try server
yesterday and Thunderbird came out all green. I suspect SeaMonkey would
do as well.

So unless there's hidden problems not covered by our tests, we should be
reasonably fine.

Not sure about Lightning, I know that does have some different
interactions with javascript in there.

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