Backed out changeset 6fe433fed5fb (bug 1150297) for suspicion of causing widespread test failures.
authorRyan VanderMeulen <ryanvm@gmail.com>
Wed, 10 Jun 2015 18:25:21 -0400
changeset 266230 af8d7ef03660
parent 266229 f137fedd1455
child 266231 60de9fc9408f
push id4793
push userryanvm@gmail.com
push date2015-06-10 22:25 +0000
treeherdermozilla-beta@af8d7ef03660 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1150297
milestone39.0
backs out6fe433fed5fb
Backed out changeset 6fe433fed5fb (bug 1150297) for suspicion of causing widespread test failures.
js/src/builtin/RegExp.cpp
js/src/tests/ecma_5/Object/15.2.3.3-01.js
js/src/tests/ecma_5/Object/15.2.3.4-04.js
js/src/tests/ecma_5/RegExp/instance-property-storage-introspection.js
js/src/tests/ecma_5/strict/15.10.7.js
js/src/tests/ecma_6/RegExp/constructor-regexp.js
js/src/tests/ecma_6/RegExp/descriptor.js
js/src/tests/ecma_6/RegExp/source.js
js/src/tests/ecma_6/TypedArray/from_constructor.js
js/src/tests/js1_8_5/extensions/clone-regexp.js
js/src/vm/RegExpObject.cpp
--- a/js/src/builtin/RegExp.cpp
+++ b/js/src/builtin/RegExp.cpp
@@ -146,17 +146,17 @@ static bool
 CompileRegExpObject(JSContext* cx, RegExpObjectBuilder& builder, CallArgs args,
                     RegExpStaticsUse staticsUse, RegExpCreationMode creationMode)
 {
     if (args.length() == 0) {
         MOZ_ASSERT(staticsUse == UseRegExpStatics);
         RegExpStatics* res = cx->global()->getRegExpStatics(cx);
         if (!res)
             return false;
-        Rooted<JSAtom*> empty(cx, cx->names().emptyRegExp);
+        Rooted<JSAtom*> empty(cx, cx->runtime()->emptyString);
         RegExpObject* reobj = builder.build(empty, res->getFlags());
         if (!reobj)
             return false;
         args.rval().setObject(*reobj);
         return true;
     }
 
     RootedValue sourceValue(cx, args[0]);
@@ -173,25 +173,27 @@ CompileRegExpObject(JSContext* cx, RegEx
 
         /*
          * Beware, sourceObj may be a (transparent) proxy to a RegExp, so only
          * use generic (proxyable) operations on sourceObj that do not assume
          * sourceObj.is<RegExpObject>().
          */
         RootedObject sourceObj(cx, &sourceValue.toObject());
 
+        RootedAtom sourceAtom(cx);
         RegExpFlag flags;
         {
             /*
              * Extract the 'source' from sourceObj; do not reuse the RegExpShared
              * since it may be from a different compartment.
              */
             RegExpGuard g(cx);
             if (!RegExpToShared(cx, sourceObj, &g))
                 return false;
+            sourceAtom = g->getSource();
 
             /*
              * If args[1] is not undefined, then parse the 'flags' from args[1].
              * Otherwise, extract the 'flags' from sourceObj.
              */
             if (args.hasDefined(1)) {
                 flags = RegExpFlag(0);
                 RootedString flagStr(cx, ToString<CanGC>(cx, args[1]));
@@ -199,27 +201,16 @@ CompileRegExpObject(JSContext* cx, RegEx
                     return false;
                 if (!ParseRegExpFlags(cx, flagStr, &flags))
                     return false;
             } else {
                 flags = g->getFlags();
             }
         }
 
-        /*
-         * 'toSource' is a permanent read-only property, so this is equivalent
-         * to executing RegExpObject::getSource on the unwrapped object.
-         */
-        RootedValue v(cx);
-        if (!GetProperty(cx, sourceObj, sourceObj, cx->names().source, &v))
-            return false;
-
-        // For proxies like CPOWs, we can't assume the result of a property get
-        // for 'source' is atomized.
-        Rooted<JSAtom*> sourceAtom(cx, AtomizeString(cx, v.toString()));
         RegExpObject* reobj = builder.build(sourceAtom, flags);
         if (!reobj)
             return false;
 
         args.rval().setObject(*reobj);
         return true;
     }
 
@@ -237,33 +228,29 @@ CompileRegExpObject(JSContext* cx, RegEx
     if (args.hasDefined(1)) {
         RootedString flagStr(cx, ToString<CanGC>(cx, args[1]));
         if (!flagStr)
             return false;
         if (!ParseRegExpFlags(cx, flagStr, &flags))
             return false;
     }
 
-    RootedAtom escapedSourceStr(cx, EscapeRegExpPattern(cx, source));
-    if (!escapedSourceStr)
-        return false;
-
     CompileOptions options(cx);
     frontend::TokenStream dummyTokenStream(cx, options, nullptr, 0, nullptr);
 
-    if (!irregexp::ParsePatternSyntax(dummyTokenStream, cx->tempLifoAlloc(), escapedSourceStr))
+    if (!irregexp::ParsePatternSyntax(dummyTokenStream, cx->tempLifoAlloc(), source))
         return false;
 
     if (staticsUse == UseRegExpStatics) {
         RegExpStatics* res = cx->global()->getRegExpStatics(cx);
         if (!res)
             return false;
         flags = RegExpFlag(flags | res->getFlags());
     }
-    RegExpObject* reobj = builder.build(escapedSourceStr, flags);
+    RegExpObject* reobj = builder.build(source, flags);
     if (!reobj)
         return false;
 
     args.rval().setObject(*reobj);
     return true;
 }
 
 MOZ_ALWAYS_INLINE bool
@@ -380,16 +367,45 @@ regexp_multiline_impl(JSContext* cx, Cal
 static bool
 regexp_multiline(JSContext* cx, unsigned argc, JS::Value* vp)
 {
     /* Steps 1-3. */
     CallArgs args = CallArgsFromVp(argc, vp);
     return CallNonGenericMethod<IsRegExp, regexp_multiline_impl>(cx, args);
 }
 
+/* ES6 draft rev32 21.2.5.10. */
+MOZ_ALWAYS_INLINE bool
+regexp_source_impl(JSContext* cx, CallArgs args)
+{
+    MOZ_ASSERT(IsRegExp(args.thisv()));
+    Rooted<RegExpObject*> reObj(cx, &args.thisv().toObject().as<RegExpObject>());
+
+    /* Step 5. */
+    RootedAtom src(cx, reObj->getSource());
+    if (!src)
+        return false;
+
+    /* Step 7. */
+    RootedString str(cx, EscapeRegExpPattern(cx, src));
+    if (!str)
+        return false;
+
+    args.rval().setString(str);
+    return true;
+}
+
+static bool
+regexp_source(JSContext* cx, unsigned argc, JS::Value* vp)
+{
+    /* Steps 1-4. */
+    CallArgs args = CallArgsFromVp(argc, vp);
+    return CallNonGenericMethod<IsRegExp, regexp_source_impl>(cx, args);
+}
+
 /* ES6 draft rev32 21.2.5.12. */
 MOZ_ALWAYS_INLINE bool
 regexp_sticky_impl(JSContext* cx, CallArgs args)
 {
     MOZ_ASSERT(IsRegExp(args.thisv()));
     Rooted<RegExpObject*> reObj(cx, &args.thisv().toObject().as<RegExpObject>());
 
     /* Steps 4-6. */
@@ -405,16 +421,17 @@ regexp_sticky(JSContext* cx, unsigned ar
     return CallNonGenericMethod<IsRegExp, regexp_sticky_impl>(cx, args);
 }
 
 const JSPropertySpec js::regexp_properties[] = {
     JS_SELF_HOSTED_GET("flags", "RegExpFlagsGetter", 0),
     JS_PSG("global", regexp_global, 0),
     JS_PSG("ignoreCase", regexp_ignoreCase, 0),
     JS_PSG("multiline", regexp_multiline, 0),
+    JS_PSG("source", regexp_source, 0),
     JS_PSG("sticky", regexp_sticky, 0),
     JS_PS_END
 };
 
 const JSFunctionSpec js::regexp_methods[] = {
 #if JS_HAS_TOSOURCE
     JS_SELF_HOSTED_FN(js_toSource_str, "RegExpToString", 0, 0),
 #endif
@@ -548,17 +565,17 @@ js::CreateRegExpPrototype(JSContext* cx,
 {
     MOZ_ASSERT(key == JSProto_RegExp);
 
     Rooted<RegExpObject*> proto(cx, cx->global()->createBlankPrototype<RegExpObject>(cx));
     if (!proto)
         return nullptr;
     proto->NativeObject::setPrivate(nullptr);
 
-    HandlePropertyName empty = cx->names().emptyRegExp;
+    HandlePropertyName empty = cx->names().empty;
     RegExpObjectBuilder builder(cx, proto);
     if (!builder.build(empty, RegExpFlag(0)))
         return nullptr;
 
     return proto;
 }
 
 RegExpRunStatus
--- a/js/src/tests/ecma_5/Object/15.2.3.3-01.js
+++ b/js/src/tests/ecma_5/Object/15.2.3.3-01.js
@@ -262,27 +262,16 @@ expected =
   };
 
 expectDescriptor(pd, expected);
 
 /******************************************************************************/
 
 o = /foo/im;
 
-pd = Object.getOwnPropertyDescriptor(o, "source");
-expected =
-  {
-    value: "foo",
-    writable: false,
-    enumerable: false,
-    configurable: false
-  };
-
-expectDescriptor(pd, expected);
-
 pd = Object.getOwnPropertyDescriptor(o, "lastIndex");
 expected =
   {
     value: 0,
     writable: true,
     enumerable: false,
     configurable: false
   };
--- a/js/src/tests/ecma_5/Object/15.2.3.4-04.js
+++ b/js/src/tests/ecma_5/Object/15.2.3.4-04.js
@@ -11,17 +11,17 @@ var summary = 'Object.getOwnPropertyName
 
 print(BUGNUMBER + ": " + summary);
 
 /**************
  * BEGIN TEST *
  **************/
 
 var actual = Object.getOwnPropertyNames(/a/);
-var expected = ["lastIndex", "source"];
+var expected = ["lastIndex"];
 
 for (var i = 0; i < expected.length; i++)
 {
   reportCompare(actual.indexOf(expected[i]) >= 0, true,
                 expected[i] + " should be a property name on a RegExp");
 }
 
 /******************************************************************************/
--- a/js/src/tests/ecma_5/RegExp/instance-property-storage-introspection.js
+++ b/js/src/tests/ecma_5/RegExp/instance-property-storage-introspection.js
@@ -115,17 +115,16 @@ do
 
   r = /c/g;
   r.lastIndex = 2;
   checkRegExp(r, "/c/g initially", 2);
   Object.defineProperty(r, "lastIndex", { writable: false });
   assertEq(Object.getOwnPropertyDescriptor(r, "lastIndex").writable, false);
   try { r.exec("aabbbba"); } catch (e) { /* swallow error if thrown */ }
   assertEq(Object.getOwnPropertyDescriptor(r, "lastIndex").writable, false);
-  assertEq(Object.getOwnPropertyDescriptor(r, "source").writable, false);
 }
 while (Math.random() > 17); // fake loop to discourage RegExp object caching
 
 /******************************************************************************/
 
 if (typeof reportCompare === "function")
   reportCompare(true, true);
 
--- a/js/src/tests/ecma_5/strict/15.10.7.js
+++ b/js/src/tests/ecma_5/strict/15.10.7.js
@@ -1,22 +1,15 @@
 /* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
 
 /*
  * Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/licenses/publicdomain/
  */
 
-assertEq(testLenientAndStrict('var r = /foo/; r.source = "bar"; r.source',
-                              returns("foo"), raisesException(TypeError)),
-         true);
-assertEq(testLenientAndStrict('var r = /foo/; delete r.source',
-                              returns(false), raisesException(TypeError)),
-         true);
-
 assertEq(testLenientAndStrict('var r = /foo/; r.lastIndex = 42; r.lastIndex',
                               returns(42), returns(42)),
          true);
 assertEq(testLenientAndStrict('var r = /foo/; delete r.lastIndex',
                               returns(false), raisesException(TypeError)),
          true);
 
 reportCompare(true, true);
--- a/js/src/tests/ecma_6/RegExp/constructor-regexp.js
+++ b/js/src/tests/ecma_6/RegExp/constructor-regexp.js
@@ -1,43 +1,61 @@
 var BUGNUMBER = 1130860;
-var summary = "RegExp constructor shouldn't invoke flags getter on argument RegExp instance.";
+var summary = "RegExp constructor shouldn't invoke source/flags getters on argument RegExp instance.";
 
 print(BUGNUMBER + ": " + summary);
 
 // same-compartment
 var a = /foo/;
 var flagsCalled = false;
+var sourceCalled = false;
+Object.defineProperty(a, "source", { get: () => {
+  sourceCalled = true;
+  return "bar";
+}});
 Object.defineProperty(a, "flags", { get: () => {
   flagsCalled = true;
   return "i";
 }});
 
+assertEq(a.source, "bar");
 assertEq(a.flags, "i");
+assertEq(sourceCalled, true);
 assertEq(flagsCalled, true);
 
+sourceCalled = false;
 flagsCalled = false;
-assertEq(new RegExp(a).flags, "");
+assertEq(new RegExp(a).source, "foo");
+assertEq(sourceCalled, false);
 assertEq(flagsCalled, false);
 
 // cross-compartment
 var g = newGlobal();
 var b = g.eval(`
 var b = /foo2/;
 var flagsCalled = false;
+var sourceCalled = false;
+Object.defineProperty(b, "source", { get: () => {
+  sourceCalled = true;
+  return "bar2";
+}});
 Object.defineProperty(b, "flags", { get: () => {
   flagsCalled = true;
   return "i";
 }});
 b;
 `);
 
+assertEq(b.source, "bar2");
 assertEq(b.flags, "i");
+assertEq(g.eval("sourceCalled;"), true);
 assertEq(g.eval("flagsCalled;"), true);
 
 g.eval(`
+sourceCalled = false;
 flagsCalled = false;
 `);
-assertEq(new RegExp(b).flags, "");
+assertEq(new RegExp(b).source, "foo2");
+assertEq(g.eval("sourceCalled;"), false);
 assertEq(g.eval("flagsCalled;"), false);
 
 if (typeof reportCompare === "function")
     reportCompare(true, true);
--- a/js/src/tests/ecma_6/RegExp/descriptor.js
+++ b/js/src/tests/ecma_6/RegExp/descriptor.js
@@ -3,16 +3,17 @@ var summary = "Implement RegExp.prototyp
 
 print(BUGNUMBER + ": " + summary);
 
 var getters = [
   "flags",
   "global",
   "ignoreCase",
   "multiline",
+  "source",
   "sticky",
   //"unicode",
 ];
 
 for (var name of getters) {
   var desc = Object.getOwnPropertyDescriptor(RegExp.prototype, name);
   assertEq(desc.configurable, true);
   assertEq(desc.enumerable, false);
--- a/js/src/tests/ecma_6/RegExp/source.js
+++ b/js/src/tests/ecma_6/RegExp/source.js
@@ -1,21 +1,29 @@
 var BUGNUMBER = 1120169;
 var summary = "Implement RegExp.prototype.source";
 
 print(BUGNUMBER + ": " + summary);
 
-// source is moved to RegExp instance property again (bug 1138325), but keep
-// following behavior.
 assertEq(RegExp.prototype.source, "(?:)");
 assertEq(/foo/.source, "foo");
 assertEq(/foo/iymg.source, "foo");
 assertEq(/\//.source, "\\/");
 assertEq(/\n\r/.source, "\\n\\r");
 assertEq(/\u2028\u2029/.source, "\\u2028\\u2029");
 assertEq(RegExp("").source, "(?:)");
 assertEq(RegExp("", "mygi").source, "(?:)");
 assertEq(RegExp("/").source, "\\/");
 assertEq(RegExp("\n\r").source, "\\n\\r");
 assertEq(RegExp("\u2028\u2029").source, "\\u2028\\u2029");
 
+assertThrowsInstanceOf(() => genericSource(), TypeError);
+assertThrowsInstanceOf(() => genericSource(1), TypeError);
+assertThrowsInstanceOf(() => genericSource(""), TypeError);
+assertThrowsInstanceOf(() => genericSource({}), TypeError);
+assertThrowsInstanceOf(() => genericSource(new Proxy(/foo/, {get(){ return true; }})), TypeError);
+
+function genericSource(obj) {
+    return Object.getOwnPropertyDescriptor(RegExp.prototype, "source").get.call(obj);
+}
+
 if (typeof reportCompare === "function")
     reportCompare(true, true);
--- a/js/src/tests/ecma_6/TypedArray/from_constructor.js
+++ b/js/src/tests/ecma_6/TypedArray/from_constructor.js
@@ -22,17 +22,17 @@ for (var constructor of constructors) {
     assertEq(d[0], "A");
     assertEq(d[1], "B");
 
     // Or RegExp.
     var obj = constructor.from.call(RegExp, [1]);
     assertEq(obj instanceof constructor, false);
     assertEq(Object.getPrototypeOf(obj), RegExp.prototype);
     assertEq(Object.getOwnPropertyNames(obj).join(","),
-             "0,lastIndex,source");
+             "0,lastIndex");
     assertEq(obj.length, undefined);
 
     // Or any JS function.
     function C(arg) {
         this.args = arguments;
     }
     var c = constructor.from.call(C, {length: 1, 0: "zero"});
     assertEq(c instanceof C, true);
--- a/js/src/tests/js1_8_5/extensions/clone-regexp.js
+++ b/js/src/tests/js1_8_5/extensions/clone-regexp.js
@@ -22,16 +22,16 @@ function testRegExp(b, c=b) {
 testRegExp(RegExp(""));
 testRegExp(/(?:)/);
 testRegExp(/^(.*)$/gimy);
 testRegExp(RegExp.prototype);
 
 var re = /\bx\b/gi;
 re.expando = true;
 testRegExp(re);
-// the flag accessors are defined on RegExp.prototype, so they're
+// `source` and the flag accessors are defined on RegExp.prototype, so they're
 // not available after re.__proto__ has been changed. We solve that by passing
 // in an additional copy of the same RegExp to compare the
 // serialized-then-deserialized clone with."
 re.__proto__ = {};
 testRegExp(re, /\bx\b/gi);
 
 reportCompare(0, 0, 'ok');
--- a/js/src/vm/RegExpObject.cpp
+++ b/js/src/vm/RegExpObject.cpp
@@ -341,39 +341,31 @@ RegExpObject::createShared(JSContext* cx
 }
 
 Shape*
 RegExpObject::assignInitialShape(ExclusiveContext* cx, Handle<RegExpObject*> self)
 {
     MOZ_ASSERT(self->empty());
 
     JS_STATIC_ASSERT(LAST_INDEX_SLOT == 0);
-    JS_STATIC_ASSERT(SOURCE_SLOT == LAST_INDEX_SLOT + 1);
 
     /* The lastIndex property alone is writable but non-configurable. */
-    if (!self->addDataProperty(cx, cx->names().lastIndex, LAST_INDEX_SLOT, JSPROP_PERMANENT))
-        return nullptr;
-
-    /* Remaining instance property are non-writable and non-configurable. */
-    unsigned attrs = JSPROP_PERMANENT | JSPROP_READONLY;
-    return self->addDataProperty(cx, cx->names().source, SOURCE_SLOT, attrs);
+    return self->addDataProperty(cx, cx->names().lastIndex, LAST_INDEX_SLOT, JSPROP_PERMANENT);
 }
 
 bool
 RegExpObject::init(ExclusiveContext* cx, HandleAtom source, RegExpFlag flags)
 {
     Rooted<RegExpObject*> self(cx, this);
 
     if (!EmptyShape::ensureInitialCustomShape<RegExpObject>(cx, self))
         return false;
 
     MOZ_ASSERT(self->lookup(cx, NameToId(cx->names().lastIndex))->slot() ==
                LAST_INDEX_SLOT);
-    MOZ_ASSERT(self->lookup(cx, NameToId(cx->names().source))->slot() ==
-               SOURCE_SLOT);
 
     /*
      * If this is a re-initialization with an existing RegExpShared, 'flags'
      * may not match getShared()->flags, so forget the RegExpShared.
      */
     self->NativeObject::setPrivate(nullptr);
 
     self->zeroLastIndex();