Bug 1089026 part 1. Eliminate the "parent" argument to JS_CloneFunctionObject to make callers use the scopeChain version if they want something other than the global. r=shu,peterv
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 30 Oct 2014 19:40:28 -0400
changeset 213334 2887e7c320feed62cb66ff49327d37d13ad81276
parent 213333 411d5252365bc2a564e7edfb2991fe538ae41146
child 213335 dd4ec08078387e78ee34a33432a03bdc3675f882
push id27748
push userryanvm@gmail.com
push dateFri, 31 Oct 2014 20:14:33 +0000
treeherdermozilla-central@12ac66e2c016 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersshu, peterv
bugs1089026
milestone36.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1089026 part 1. Eliminate the "parent" argument to JS_CloneFunctionObject to make callers use the scopeChain version if they want something other than the global. r=shu,peterv
dom/xbl/nsXBLProtoImplMethod.cpp
dom/xbl/nsXBLProtoImplProperty.cpp
js/src/jsapi-tests/testCloneScript.cpp
js/src/jsapi.cpp
js/src/jsapi.h
js/src/jsfun.cpp
js/src/shell/js.cpp
--- a/dom/xbl/nsXBLProtoImplMethod.cpp
+++ b/dom/xbl/nsXBLProtoImplMethod.cpp
@@ -99,26 +99,31 @@ nsXBLProtoImplMethod::SetLineNumber(uint
 nsresult
 nsXBLProtoImplMethod::InstallMember(JSContext* aCx,
                                     JS::Handle<JSObject*> aTargetClassObject)
 {
   NS_PRECONDITION(IsCompiled(),
                   "Should not be installing an uncompiled method");
   MOZ_ASSERT(js::IsObjectInContextCompartment(aTargetClassObject, aCx));
 
-  JS::Rooted<JSObject*> globalObject(aCx, JS_GetGlobalForObject(aCx, aTargetClassObject));
-  MOZ_ASSERT(xpc::IsInContentXBLScope(globalObject) ||
-             xpc::IsInAddonScope(globalObject) ||
-             globalObject == xpc::GetXBLScope(aCx, globalObject));
+#ifdef DEBUG
+  {
+    JS::Rooted<JSObject*> globalObject(aCx, JS_GetGlobalForObject(aCx, aTargetClassObject));
+    MOZ_ASSERT(xpc::IsInContentXBLScope(globalObject) ||
+               xpc::IsInAddonScope(globalObject) ||
+               globalObject == xpc::GetXBLScope(aCx, globalObject));
+    MOZ_ASSERT(JS::CurrentGlobalOrNull(aCx) == globalObject);
+  }
+#endif
 
   JS::Rooted<JSObject*> jsMethodObject(aCx, GetCompiledMethod());
   if (jsMethodObject) {
     nsDependentString name(mName);
 
-    JS::Rooted<JSObject*> method(aCx, JS_CloneFunctionObject(aCx, jsMethodObject, globalObject));
+    JS::Rooted<JSObject*> method(aCx, JS::CloneFunctionObject(aCx, jsMethodObject));
     NS_ENSURE_TRUE(method, NS_ERROR_OUT_OF_MEMORY);
 
     if (!::JS_DefineUCProperty(aCx, aTargetClassObject,
                                static_cast<const char16_t*>(mName),
                                name.Length(), method,
                                JSPROP_ENUMERATE)) {
       return NS_ERROR_OUT_OF_MEMORY;
     }
--- a/dom/xbl/nsXBLProtoImplProperty.cpp
+++ b/dom/xbl/nsXBLProtoImplProperty.cpp
@@ -123,31 +123,37 @@ const char* gPropertyArgs[] = { "val" };
 nsresult
 nsXBLProtoImplProperty::InstallMember(JSContext *aCx,
                                       JS::Handle<JSObject*> aTargetClassObject)
 {
   NS_PRECONDITION(mIsCompiled,
                   "Should not be installing an uncompiled property");
   MOZ_ASSERT(mGetter.IsCompiled() && mSetter.IsCompiled());
   MOZ_ASSERT(js::IsObjectInContextCompartment(aTargetClassObject, aCx));
-  JS::Rooted<JSObject*> globalObject(aCx, JS_GetGlobalForObject(aCx, aTargetClassObject));
-  MOZ_ASSERT(xpc::IsInContentXBLScope(globalObject) ||
-             xpc::IsInAddonScope(globalObject) ||
-             globalObject == xpc::GetXBLScope(aCx, globalObject));
+
+#ifdef DEBUG
+  {
+    JS::Rooted<JSObject*> globalObject(aCx, JS_GetGlobalForObject(aCx, aTargetClassObject));
+    MOZ_ASSERT(xpc::IsInContentXBLScope(globalObject) ||
+               xpc::IsInAddonScope(globalObject) ||
+               globalObject == xpc::GetXBLScope(aCx, globalObject));
+    MOZ_ASSERT(JS::CurrentGlobalOrNull(aCx) == globalObject);
+  }
+#endif
 
   JS::Rooted<JSObject*> getter(aCx, mGetter.GetJSFunction());
   JS::Rooted<JSObject*> setter(aCx, mSetter.GetJSFunction());
   if (getter || setter) {
     if (getter) {
-      if (!(getter = ::JS_CloneFunctionObject(aCx, getter, globalObject)))
+      if (!(getter = JS::CloneFunctionObject(aCx, getter)))
         return NS_ERROR_OUT_OF_MEMORY;
     }
 
     if (setter) {
-      if (!(setter = ::JS_CloneFunctionObject(aCx, setter, globalObject)))
+      if (!(setter = JS::CloneFunctionObject(aCx, setter)))
         return NS_ERROR_OUT_OF_MEMORY;
     }
 
     nsDependentString name(mName);
     if (!::JS_DefineUCProperty(aCx, aTargetClassObject,
                                static_cast<const char16_t*>(mName),
                                name.Length(), JS::UndefinedHandleValue, mJSAttributes,
                                JS_DATA_TO_FUNC_PTR(JSNative, getter.get()),
--- a/js/src/jsapi-tests/testCloneScript.cpp
+++ b/js/src/jsapi-tests/testCloneScript.cpp
@@ -38,17 +38,17 @@ BEGIN_TEST(test_cloneScript)
         CHECK(JS_CompileFunction(cx, A, "f", 0, nullptr, source,
                                  strlen(source), options, &fun));
         CHECK(obj = JS_GetFunctionObject(fun));
     }
 
     // clone into B
     {
         JSAutoCompartment b(cx, B);
-        CHECK(JS_CloneFunctionObject(cx, obj, B));
+        CHECK(JS::CloneFunctionObject(cx, obj));
     }
 
     return true;
 }
 END_TEST(test_cloneScript)
 
 static void
 DestroyPrincipals(JSPrincipals *principals)
@@ -120,17 +120,17 @@ BEGIN_TEST(test_cloneScriptWithPrincipal
         CHECK(JS_GetScriptPrincipals(script) == principalsA);
         CHECK(obj = JS_GetFunctionObject(fun));
     }
 
     // Clone into B
     {
         JSAutoCompartment b(cx, B);
         JS::RootedObject cloned(cx);
-        CHECK(cloned = JS_CloneFunctionObject(cx, obj, B));
+        CHECK(cloned = JS::CloneFunctionObject(cx, obj));
 
         JS::RootedFunction fun(cx);
         JS::RootedValue clonedValue(cx, JS::ObjectValue(*cloned));
         CHECK(fun = JS_ValueToFunction(cx, clonedValue));
 
         JSScript *script;
         CHECK(script = JS_GetFunctionScript(cx, fun));
 
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -3864,29 +3864,25 @@ CreateScopeObjectsForScopeChain(JSContex
         dynamicEnclosingScope = dynamicWith;
     }
 
     dynamicScopeObj.set(dynamicEnclosingScope);
     staticScopeObj.set(staticEnclosingScope);
     return true;
 }
 
-JS_PUBLIC_API(JSObject *)
-JS_CloneFunctionObject(JSContext *cx, HandleObject funobj, HandleObject parentArg)
-{
-    RootedObject parent(cx, parentArg);
-
+static JSObject *
+CloneFunctionObject(JSContext *cx, HandleObject funobj, HandleObject dynamicScope)
+{
     AssertHeapIsIdle(cx);
     CHECK_REQUEST(cx);
-    assertSameCompartment(cx, parent);
+    assertSameCompartment(cx, dynamicScope);
+    MOZ_ASSERT(dynamicScope);
     // Note that funobj can be in a different compartment.
 
-    if (!parent)
-        parent = cx->global();
-
     if (!funobj->is<JSFunction>()) {
         AutoCompartment ac(cx, funobj);
         RootedValue v(cx, ObjectValue(*funobj));
         ReportIsNotFunction(cx, v);
         return nullptr;
     }
 
     RootedFunction fun(cx, &funobj->as<JSFunction>());
@@ -3895,46 +3891,52 @@ JS_CloneFunctionObject(JSContext *cx, Ha
         if (!fun->getOrCreateScript(cx))
             return nullptr;
     }
     /*
      * If a function was compiled to be lexically nested inside some other
      * script, we cannot clone it without breaking the compiler's assumptions.
      */
     if (fun->isInterpreted() && (fun->nonLazyScript()->enclosingStaticScope() ||
-        (fun->nonLazyScript()->compileAndGo() && !parent->is<GlobalObject>())))
+        (fun->nonLazyScript()->compileAndGo() && !dynamicScope->is<GlobalObject>())))
     {
         JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_BAD_CLONE_FUNOBJ_SCOPE);
         return nullptr;
     }
 
     if (fun->isBoundFunction()) {
         JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_CANT_CLONE_OBJECT);
         return nullptr;
     }
 
     if (fun->isNative() && IsAsmJSModuleNative(fun->native())) {
         JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_CANT_CLONE_OBJECT);
         return nullptr;
     }
 
-    return CloneFunctionObject(cx, fun, parent, fun->getAllocKind());
+    return CloneFunctionObject(cx, fun, dynamicScope, fun->getAllocKind());
 }
 
 namespace JS {
 
 JS_PUBLIC_API(JSObject *)
+CloneFunctionObject(JSContext *cx, JS::Handle<JSObject*> funobj)
+{
+    return CloneFunctionObject(cx, funobj, cx->global());
+}
+
+extern JS_PUBLIC_API(JSObject *)
 CloneFunctionObject(JSContext *cx, HandleObject funobj, AutoObjectVector &scopeChain)
 {
     RootedObject dynamicScope(cx);
     RootedObject unusedStaticScope(cx);
     if (!CreateScopeObjectsForScopeChain(cx, scopeChain, &dynamicScope, &unusedStaticScope))
         return nullptr;
 
-    return JS_CloneFunctionObject(cx, funobj, dynamicScope);
+    return CloneFunctionObject(cx, funobj, dynamicScope);
 }
 
 } // namespace JS
 
 JS_PUBLIC_API(JSObject *)
 JS_GetFunctionObject(JSFunction *fun)
 {
     return fun;
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -3522,24 +3522,24 @@ extern JS_PUBLIC_API(JSFunction *)
 JS_DefineUCFunction(JSContext *cx, JS::Handle<JSObject*> obj,
                     const char16_t *name, size_t namelen, JSNative call,
                     unsigned nargs, unsigned attrs);
 
 extern JS_PUBLIC_API(JSFunction *)
 JS_DefineFunctionById(JSContext *cx, JS::Handle<JSObject*> obj, JS::Handle<jsid> id, JSNative call,
                       unsigned nargs, unsigned attrs);
 
+namespace JS {
+
 /*
- * Clone a top-level function into a new scope. This function will dynamically
+ * Clone a top-level function into cx's global. This function will dynamically
  * fail if funobj was lexically nested inside some other function.
  */
 extern JS_PUBLIC_API(JSObject *)
-JS_CloneFunctionObject(JSContext *cx, JS::Handle<JSObject*> funobj, JS::Handle<JSObject*> parent);
-
-namespace JS {
+CloneFunctionObject(JSContext *cx, HandleObject funobj);
 
 /*
  * As above, but providing an explicit scope chain.  scopeChain must not include
  * the global object on it; that's implicit.  It needs to contain the other
  * objects that should end up on the clone's scope chain.
  */
 extern JS_PUBLIC_API(JSObject *)
 CloneFunctionObject(JSContext *cx, HandleObject funobj, AutoObjectVector &scopeChain);
--- a/js/src/jsfun.cpp
+++ b/js/src/jsfun.cpp
@@ -2057,17 +2057,17 @@ js::CloneFunctionObject(JSContext *cx, H
         return clone;
     }
 
     RootedFunction cloneRoot(cx, clone);
 
     /*
      * Across compartments we have to clone the script for interpreted
      * functions. Cross-compartment cloning only happens via JSAPI
-     * (JS_CloneFunctionObject) which dynamically ensures that 'script' has
+     * (JS::CloneFunctionObject) which dynamically ensures that 'script' has
      * no enclosing lexical scope (only the global scope).
      */
     if (cloneRoot->isInterpreted() && !CloneFunctionScript(cx, fun, cloneRoot, newKindArg))
         return nullptr;
 
     return cloneRoot;
 }
 
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -2452,17 +2452,22 @@ Clone(JSContext *cx, unsigned argc, jsva
 
     if (args.length() > 1) {
         if (!JS_ValueToObject(cx, args[1], &parent))
             return false;
     } else {
         parent = JS_GetParent(&args.callee());
     }
 
-    JSObject *clone = JS_CloneFunctionObject(cx, funobj, parent);
+    // Should it worry us that we might be getting with wrappers
+    // around with wrappers here?
+    JS::AutoObjectVector scopeChain(cx);
+    if (!parent->is<GlobalObject>() && !scopeChain.append(parent))
+        return false;
+    JSObject *clone = JS::CloneFunctionObject(cx, funobj, scopeChain);
     if (!clone)
         return false;
     args.rval().setObject(*clone);
     return true;
 }
 
 static bool
 GetSLX(JSContext *cx, unsigned argc, jsval *vp)