Bug 1288791 part 3. Ensure that FindAssociatedGlobal never returns a gray global. r=bkelly
authorBoris Zbarsky <bzbarsky@mit.edu>
Wed, 27 Jul 2016 11:05:36 -0400
changeset 306903 45c51ccab098f766e3333c6f4c66c568fa6e79bf
parent 306902 45010c0db1c5ea9bf0c9e059d6fc8f00a9fb5d49
child 306904 dddc81f6047d25c7a7ad85d2dbd0655940332fa6
push id30502
push usercbook@mozilla.com
push dateThu, 28 Jul 2016 15:43:16 +0000
treeherdermozilla-central@9ec789c0ee5b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbkelly
bugs1288791
milestone50.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 1288791 part 3. Ensure that FindAssociatedGlobal never returns a gray global. r=bkelly
dom/base/nsIGlobalObject.h
dom/bindings/BindingUtils.h
dom/bindings/Codegen.py
--- a/dom/base/nsIGlobalObject.h
+++ b/dom/base/nsIGlobalObject.h
@@ -48,16 +48,20 @@ public:
    * a window from going away.
    */
   bool
   IsDying() const
   {
     return mIsDying;
   }
 
+  // GetGlobalJSObject may return a gray object.  If this ever changes so that
+  // it stops doing that, please simplify the code in FindAssociatedGlobal in
+  // BindingUtils.h that does JS::ExposeObjectToActiveJS on the return value of
+  // GetGlobalJSObject.
   virtual JSObject* GetGlobalJSObject() = 0;
 
   // This method is not meant to be overridden.
   nsIPrincipal* PrincipalOrNull();
 
   void RegisterHostObjectURI(const nsACString& aURI);
 
   void UnregisterHostObjectURI(const nsACString& aURI);
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -1519,109 +1519,119 @@ WrapObject(JSContext* cx, JSObject& p, J
 }
 
 // Given an object "p" that inherits from nsISupports, wrap it and return the
 // result.  Null is returned on wrapping failure.  This is somewhat similar to
 // WrapObject() above, but does NOT allow Xrays around the result, since we
 // don't want those for our parent object.
 template<typename T>
 static inline JSObject*
-WrapNativeISupportsParent(JSContext* cx, T* p, nsWrapperCache* cache)
+WrapNativeISupports(JSContext* cx, T* p, nsWrapperCache* cache)
 {
   qsObjectHelper helper(ToSupports(p), cache);
   JS::Rooted<JSObject*> scope(cx, JS::CurrentGlobalOrNull(cx));
   JS::Rooted<JS::Value> v(cx);
   return XPCOMObjectToJsval(cx, scope, helper, nullptr, false, &v) ?
          v.toObjectOrNull() :
          nullptr;
 }
 
 
 // Fallback for when our parent is not a WebIDL binding object.
 template<typename T, bool isISupports=IsBaseOf<nsISupports, T>::value>
-struct WrapNativeParentFallback
+struct WrapNativeFallback
 {
   static inline JSObject* Wrap(JSContext* cx, T* parent, nsWrapperCache* cache)
   {
     return nullptr;
   }
 };
 
 // Fallback for when our parent is not a WebIDL binding object but _is_ an
 // nsISupports object.
 template<typename T >
-struct WrapNativeParentFallback<T, true >
+struct WrapNativeFallback<T, true >
 {
   static inline JSObject* Wrap(JSContext* cx, T* parent, nsWrapperCache* cache)
   {
-    return WrapNativeISupportsParent(cx, parent, cache);
+    return WrapNativeISupports(cx, parent, cache);
   }
 };
 
 // Wrapping of our native parent, for cases when it's a WebIDL object (though
 // possibly preffed off).
 template<typename T, bool hasWrapObject=NativeHasMember<T>::WrapObject>
-struct WrapNativeParentHelper
+struct WrapNativeHelper
 {
   static inline JSObject* Wrap(JSContext* cx, T* parent, nsWrapperCache* cache)
   {
     MOZ_ASSERT(cache);
 
     JSObject* obj;
     if ((obj = cache->GetWrapper())) {
+      // GetWrapper always unmarks gray.
+      MOZ_ASSERT(!JS::ObjectIsMarkedGray(obj));
       return obj;
     }
 
     // Inline this here while we have non-dom objects in wrapper caches.
     if (!CouldBeDOMBinding(parent)) {
-      obj = WrapNativeParentFallback<T>::Wrap(cx, parent, cache);
+      // WrapNativeFallback never returns a gray thing.
+      obj = WrapNativeFallback<T>::Wrap(cx, parent, cache);
+      MOZ_ASSERT_IF(obj, !JS::ObjectIsMarkedGray(obj));
     } else {
+      // WrapObject never returns a gray thing.
       obj = parent->WrapObject(cx, nullptr);
+      MOZ_ASSERT_IF(obj, !JS::ObjectIsMarkedGray(obj));
     }
 
     return obj;
   }
 };
 
 // Wrapping of our native parent, for cases when it's not a WebIDL object.  In
 // this case it must be nsISupports.
 template<typename T>
-struct WrapNativeParentHelper<T, false>
+struct WrapNativeHelper<T, false>
 {
   static inline JSObject* Wrap(JSContext* cx, T* parent, nsWrapperCache* cache)
   {
     JSObject* obj;
     if (cache && (obj = cache->GetWrapper())) {
 #ifdef DEBUG
       JS::Rooted<JSObject*> rootedObj(cx, obj);
-      NS_ASSERTION(WrapNativeISupportsParent(cx, parent, cache) == rootedObj,
+      NS_ASSERTION(WrapNativeISupports(cx, parent, cache) == rootedObj,
                    "Unexpected object in nsWrapperCache");
       obj = rootedObj;
 #endif
+      MOZ_ASSERT(!JS::ObjectIsMarkedGray(obj));
       return obj;
     }
 
-    return WrapNativeISupportsParent(cx, parent, cache);
+    obj = WrapNativeISupports(cx, parent, cache);
+    MOZ_ASSERT_IF(obj, !JS::ObjectIsMarkedGray(obj));
+    return obj;
   }
 };
 
 // Finding the associated global for an object.
 template<typename T>
 static inline JSObject*
 FindAssociatedGlobal(JSContext* cx, T* p, nsWrapperCache* cache,
                      bool useXBLScope = false)
 {
   if (!p) {
     return JS::CurrentGlobalOrNull(cx);
   }
 
-  JSObject* obj = WrapNativeParentHelper<T>::Wrap(cx, p, cache);
+  JSObject* obj = WrapNativeHelper<T>::Wrap(cx, p, cache);
   if (!obj) {
     return nullptr;
   }
+  MOZ_ASSERT(!JS::ObjectIsMarkedGray(obj));
 
   obj = js::GetGlobalForObjectCrossCompartment(obj);
 
   if (!useXBLScope) {
     return obj;
   }
 
   // If useXBLScope is true, it means that the canonical reflector for this
@@ -1647,17 +1657,30 @@ FindAssociatedGlobal(JSContext* cx, cons
 }
 
 // Specialization for the case of nsIGlobalObject, since in that case
 // we can just get the JSObject* directly.
 template<>
 inline JSObject*
 FindAssociatedGlobal(JSContext* cx, nsIGlobalObject* const& p)
 {
-  return p ? p->GetGlobalJSObject() : JS::CurrentGlobalOrNull(cx);
+  if (!p) {
+    return JS::CurrentGlobalOrNull(cx);
+  }
+
+  JSObject* global = p->GetGlobalJSObject();
+  if (!global) {
+    return nullptr;
+  }
+
+  MOZ_ASSERT(JS_IsGlobalObject(global));
+  // This object could be gray if the nsIGlobalObject is the only thing keeping
+  // it alive.
+  JS::ExposeObjectToActiveJS(global);
+  return global;
 }
 
 template<typename T,
          bool hasAssociatedGlobal=NativeHasMember<T>::GetParentObject>
 struct FindAssociatedGlobalForNative
 {
   static JSObject* Get(JSContext* cx, JS::Handle<JSObject*> obj)
   {
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -3582,16 +3582,17 @@ class CGWrapWithCacheMethod(CGAbstractMe
             MOZ_ASSERT(ToSupportsIsOnPrimaryInheritanceChain(aObject, aCache),
                        "nsISupports must be on our primary inheritance chain");
 
             JS::Rooted<JSObject*> global(aCx, FindAssociatedGlobal(aCx, aObject->GetParentObject()));
             if (!global) {
               return false;
             }
             MOZ_ASSERT(JS_IsGlobalObject(global));
+            MOZ_ASSERT(!JS::ObjectIsMarkedGray(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