Bug 1288791 part 2. Rename WrapNativeParent to FindAssociatedGlobal and update it to actually do that. r=bkelly
authorBoris Zbarsky <bzbarsky@mit.edu>
Wed, 27 Jul 2016 11:05:36 -0400
changeset 331966 45010c0db1c5ea9bf0c9e059d6fc8f00a9fb5d49
parent 331965 d1188f77967db7e9c98010620850ac066172b778
child 331967 45c51ccab098f766e3333c6f4c66c568fa6e79bf
push id9858
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 14:37:10 +0000
treeherdermozilla-aurora@203106ef6cb6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbkelly
bugs1288791
milestone50.0a1
Bug 1288791 part 2. Rename WrapNativeParent to FindAssociatedGlobal and update it to actually do that. r=bkelly
dom/bindings/BindingUtils.h
dom/bindings/Codegen.py
dom/html/nsHTMLDocument.cpp
js/xpconnect/crashtests/786142-iframe.html
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -1598,76 +1598,77 @@ struct WrapNativeParentHelper<T, false>
 #endif
       return obj;
     }
 
     return WrapNativeISupportsParent(cx, parent, cache);
   }
 };
 
-// Wrapping of our native parent.
+// Finding the associated global for an object.
 template<typename T>
 static inline JSObject*
-WrapNativeParent(JSContext* cx, T* p, nsWrapperCache* cache,
-                 bool useXBLScope = false)
+FindAssociatedGlobal(JSContext* cx, T* p, nsWrapperCache* cache,
+                     bool useXBLScope = false)
 {
   if (!p) {
     return JS::CurrentGlobalOrNull(cx);
   }
 
-  JSObject* parent = WrapNativeParentHelper<T>::Wrap(cx, p, cache);
-  if (!parent || !useXBLScope) {
-    return parent;
+  JSObject* obj = WrapNativeParentHelper<T>::Wrap(cx, p, cache);
+  if (!obj) {
+    return nullptr;
+  }
+
+  obj = js::GetGlobalForObjectCrossCompartment(obj);
+
+  if (!useXBLScope) {
+    return obj;
   }
 
   // If useXBLScope is true, it means that the canonical reflector for this
   // native object should live in the content XBL scope. Note that we never put
   // anonymous content inside an add-on scope.
-  if (xpc::IsInContentXBLScope(parent)) {
-    return parent;
+  if (xpc::IsInContentXBLScope(obj)) {
+    return obj;
   }
-  JS::Rooted<JSObject*> rootedParent(cx, parent);
-  JS::Rooted<JSObject*> xblScope(cx, xpc::GetXBLScope(cx, rootedParent));
-  NS_ENSURE_TRUE(xblScope, nullptr);
-  JSAutoCompartment ac(cx, xblScope);
-  if (NS_WARN_IF(!JS_WrapObject(cx, &rootedParent))) {
-    return nullptr;
-  }
-
-  return rootedParent;
+  JS::Rooted<JSObject*> rootedObj(cx, obj);
+  JSObject* xblScope = xpc::GetXBLScope(cx, rootedObj);
+  MOZ_ASSERT_IF(xblScope, JS_IsGlobalObject(xblScope));
+  MOZ_ASSERT_IF(xblScope, !JS::ObjectIsMarkedGray(xblScope));
+  return xblScope;
 }
 
-// Wrapping of our native parent, when we don't want to explicitly pass in
-// things like the nsWrapperCache for it.
+// Finding of the associated global for an object, when we don't want to
+// explicitly pass in things like the nsWrapperCache for it.
 template<typename T>
 static inline JSObject*
-WrapNativeParent(JSContext* cx, const T& p)
+FindAssociatedGlobal(JSContext* cx, const T& p)
 {
-  return WrapNativeParent(cx, GetParentPointer(p), GetWrapperCache(p), GetUseXBLScope(p));
+  return FindAssociatedGlobal(cx, GetParentPointer(p), GetWrapperCache(p), GetUseXBLScope(p));
 }
 
 // Specialization for the case of nsIGlobalObject, since in that case
 // we can just get the JSObject* directly.
 template<>
 inline JSObject*
-WrapNativeParent(JSContext* cx, nsIGlobalObject* const& p)
+FindAssociatedGlobal(JSContext* cx, nsIGlobalObject* const& p)
 {
   return p ? p->GetGlobalJSObject() : JS::CurrentGlobalOrNull(cx);
 }
 
 template<typename T,
          bool hasAssociatedGlobal=NativeHasMember<T>::GetParentObject>
 struct FindAssociatedGlobalForNative
 {
   static JSObject* Get(JSContext* cx, JS::Handle<JSObject*> obj)
   {
     MOZ_ASSERT(js::IsObjectInContextCompartment(obj, cx));
     T* native = UnwrapDOMObject<T>(obj);
-    JSObject* wrappedParent = WrapNativeParent(cx, native->GetParentObject());
-    return wrappedParent ? js::GetGlobalForObjectCrossCompartment(wrappedParent) : nullptr;
+    return FindAssociatedGlobal(cx, native->GetParentObject());
   }
 };
 
 template<typename T>
 struct FindAssociatedGlobalForNative<T, false>
 {
   static JSObject* Get(JSContext* cx, JS::Handle<JSObject*> obj)
   {
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -3577,33 +3577,33 @@ class CGWrapWithCacheMethod(CGAbstractMe
             $*{assertInheritance}
             MOZ_ASSERT(!aCache->GetWrapper(),
                        "You should probably not be using Wrap() directly; use "
                        "GetOrCreateDOMReflector instead");
 
             MOZ_ASSERT(ToSupportsIsOnPrimaryInheritanceChain(aObject, aCache),
                        "nsISupports must be on our primary inheritance chain");
 
-            JS::Rooted<JSObject*> parent(aCx, WrapNativeParent(aCx, aObject->GetParentObject()));
-            if (!parent) {
+            JS::Rooted<JSObject*> global(aCx, FindAssociatedGlobal(aCx, aObject->GetParentObject()));
+            if (!global) {
               return false;
             }
+            MOZ_ASSERT(JS_IsGlobalObject(global));
 
             // That might have ended up wrapping us already, due to the wonders
             // of XBL.  Check for that, and bail out as needed.
             aReflector.set(aCache->GetWrapper());
             if (aReflector) {
             #ifdef DEBUG
               binding_detail::AssertReflectorHasGivenProto(aCx, aReflector, aGivenProto);
             #endif // DEBUG
               return true;
             }
 
-            JSAutoCompartment ac(aCx, parent);
-            JS::Rooted<JSObject*> global(aCx, js::GetGlobalForObjectCrossCompartment(parent));
+            JSAutoCompartment ac(aCx, global);
             $*{declareProto}
 
             $*{createObject}
 
             aCache->SetWrapper(aReflector);
             $*{unforgeable}
             $*{slots}
             creator.InitializationSucceeded();
--- a/dom/html/nsHTMLDocument.cpp
+++ b/dom/html/nsHTMLDocument.cpp
@@ -2230,18 +2230,16 @@ nsHTMLDocument::NamedGetter(JSContext* c
   nsISupports* supp = ResolveName(aName, &cache);
   if (!supp) {
     aFound = false;
     aRetval.set(nullptr);
     return;
   }
 
   JS::Rooted<JS::Value> val(cx);
-  // XXXbz Should we call the (slightly misnamed, really) WrapNativeParent
-  // here?
   if (!dom::WrapObject(cx, supp, cache, nullptr, &val)) {
     rv.Throw(NS_ERROR_OUT_OF_MEMORY);
     return;
   }
   aFound = true;
   aRetval.set(&val.toObject());
 }
 
--- a/js/xpconnect/crashtests/786142-iframe.html
+++ b/js/xpconnect/crashtests/786142-iframe.html
@@ -18,16 +18,19 @@ function doTest() {
   // The gist here is that we want to create an input element that isn't part
   // of a form, so that its PreCreate hook will cause it to be parented to the
   // document. However, there are several considerations that make this more
   // complicted.
   //
   // First, the crashtest becomes non-deterministics if we morph |form| before
   // getting to scope 3 (as explained below). This means that we can't trigger
   // the PreCreate hook for |input|, because that will call WrapNativeParent
+  // (Well... No, it won't: there is no WrapNativeParent, but there are also no
+  // more pre-create hooks, slimwrappers, parenting to the form, or any of the
+  // stuff this test is trying to test.)
   // on the form, which will end up making a cross-compartment wrapper, which
   // will morph form. But this puts us in a pickle, because appendChild returns
   // the apppended child, which will trigger the PreCreate hook in
   // NativeInterface2JSObject. So we do this little hack where we append a buch
   // of dummy <div> children to the form, and use replaceChild (which returns
   // the replacer, not the replacee) to stick the input elements as children of
   // the form.
   //