775317 - Add a JSAPI test for the JSOPTION_VAROBJFIX behavior of JS_EvaluateScript

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Because I'm about to write some code that may depend on it.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Add a test for the JSOPTION_VAROBJFIX behavior of JS_EvaluateScript.

Comment 2

[Jason Orendorff [:jorendorff]]

Comment on attachment 643603 [details] [diff] [review]
Add a test for the JSOPTION_VAROBJFIX behavior of JS_EvaluateScript.

Review of attachment 643603 [details] [diff] [review]:
-----------------------------------------------------------------

Nice test. Thanks very much.

::: js/src/jsapi-tests/testJSEvaluateScript.cpp
@@ +15,5 @@
> +
> +    static const char src[] = "var x = 5;";
> +
> +    JS::Value retval;
> +    CHECK(JS_EvaluateScript(cx, obj, src, sizeof(src) - 1, "", 0, &retval));

Nit: `__FILE__, __LINE__,` rather than `"", 0,` please, just for luck.

@@ +33,5 @@
> +
> +    CHECK(JS_EvaluateScript(cx, obj, src2, sizeof(src2) - 1, "", 0, &retval));
> +
> +    hasProp = JS_FALSE;
> +    CHECK(JS_AlreadyHasOwnProperty(cx, obj, "y", &hasProp));

Use plain JS_HasProperty.  The only special thing about JS_AlreadyHasOwnProperty is that it lets you peek at a particular implementation detail (resolve hooks) that isn't relevant here.

@@ +41,5 @@
> +    CHECK(JS_AlreadyHasOwnProperty(cx, global, "y", &hasProp));
> +    CHECK(!hasProp);
> +
> +    // And restore the context options
> +    JS_SetOptions(cx, options);

Oh, don't bother -- for the same reason that you didn't bother deleting that "x" property you created on the global: all this stuff is 100% throwaway. We'll create a fresh cx for the next test.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Made those changes, landed http://hg.mozilla.org/integration/mozilla-inbound/rev/f142f32a98a3

Comment 4

[Jason Duell]

Backed out due to bustage

 https://hg.mozilla.org/integration/mozilla-inbound/rev/ad5866c81a03

Comment 5

[Boris Zbarsky [:bzbarsky]]

This patch had nothing to do with the bustage... and it continues after the backout.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Relanded as http://hg.mozilla.org/integration/mozilla-inbound/rev/fb0cac0021c5

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/fb0cac0021c5