Fix for bug 799465 (Add complete support for non-nsISupports objects in new DOM bindings) - fix CC traversal and wrapper preservation. r=bz.
authorPeter Van der Beken <peterv@propagandism.org>
Wed, 26 Sep 2012 20:12:15 +0200
changeset 110353 c1bbabd184b33ec21cba783c64760937cffe975f
parent 110352 ba1ece534b748b22e68597b7ef4ba237ba17b143
child 110354 a1c63e59513618b034c4e86941dbdf2af4e55767
push id93
push usernmatsakis@mozilla.com
push dateWed, 31 Oct 2012 21:26:57 +0000
reviewersbz
bugs799465
milestone19.0a1
Fix for bug 799465 (Add complete support for non-nsISupports objects in new DOM bindings) - fix CC traversal and wrapper preservation. r=bz.
content/base/public/nsContentUtils.h
content/base/src/nsContentUtils.cpp
dom/base/nsWrapperCache.h
dom/bindings/Codegen.py
dom/bindings/Configuration.py
dom/bindings/DOMJSClass.h
dom/tests/mochitest/chrome/file_bug799299.xul
dom/tests/mochitest/chrome/test_bug799299.xul
js/xpconnect/src/nsXPConnect.cpp
--- a/content/base/public/nsContentUtils.h
+++ b/content/base/public/nsContentUtils.h
@@ -1290,39 +1290,48 @@ public:
    * @param aScriptObjectHolder the object that holds JS objects that we want to
    *                            drop
    */
   static nsresult DropJSObjects(void* aScriptObjectHolder);
 
 #ifdef DEBUG
   static bool AreJSObjectsHeld(void* aScriptObjectHolder); 
 
-  static void CheckCCWrapperTraversal(nsISupports* aScriptObjectHolder,
-                                      nsWrapperCache* aCache);
+  static void CheckCCWrapperTraversal(void* aScriptObjectHolder,
+                                      nsWrapperCache* aCache,
+                                      nsScriptObjectTracer* aTracer);
 #endif
 
   static void PreserveWrapper(nsISupports* aScriptObjectHolder,
                               nsWrapperCache* aCache)
   {
     if (!aCache->PreservingWrapper()) {
       nsISupports *ccISupports;
       aScriptObjectHolder->QueryInterface(NS_GET_IID(nsCycleCollectionISupports),
                                           reinterpret_cast<void**>(&ccISupports));
       MOZ_ASSERT(ccISupports);
       nsXPCOMCycleCollectionParticipant* participant;
       CallQueryInterface(ccISupports, &participant);
-      HoldJSObjects(ccISupports, participant);
+      PreserveWrapper(ccISupports, aCache, participant);
+    }
+  }
+  static void PreserveWrapper(void* aScriptObjectHolder,
+                              nsWrapperCache* aCache,
+                              nsScriptObjectTracer* aTracer)
+  {
+    if (!aCache->PreservingWrapper()) {
+      HoldJSObjects(aScriptObjectHolder, aTracer);
       aCache->SetPreservingWrapper(true);
 #ifdef DEBUG
       // Make sure the cycle collector will be able to traverse to the wrapper.
-      CheckCCWrapperTraversal(ccISupports, aCache);
+      CheckCCWrapperTraversal(aScriptObjectHolder, aCache, aTracer);
 #endif
     }
   }
-  static void ReleaseWrapper(nsISupports* aScriptObjectHolder,
+  static void ReleaseWrapper(void* aScriptObjectHolder,
                              nsWrapperCache* aCache);
   static void TraceWrapper(nsWrapperCache* aCache, TraceCallback aCallback,
                            void *aClosure);
 
   /*
    * Notify when the first XUL menu is opened and when the all XUL menus are
    * closed. At opening, aInstalling should be TRUE, otherwise, it should be
    * FALSE.
--- a/content/base/src/nsContentUtils.cpp
+++ b/content/base/src/nsContentUtils.cpp
@@ -6389,36 +6389,34 @@ DebugWrapperTraceCallback(void *p, const
 {
   DebugWrapperTraversalCallback* callback =
     static_cast<DebugWrapperTraversalCallback*>(closure);
   callback->NoteJSChild(p);
 }
 
 // static
 void
-nsContentUtils::CheckCCWrapperTraversal(nsISupports* aScriptObjectHolder,
-                                        nsWrapperCache* aCache)
+nsContentUtils::CheckCCWrapperTraversal(void* aScriptObjectHolder,
+                                        nsWrapperCache* aCache,
+                                        nsScriptObjectTracer* aTracer)
 {
   JSObject* wrapper = aCache->GetWrapper();
   if (!wrapper) {
     return;
   }
 
-  nsXPCOMCycleCollectionParticipant* participant;
-  CallQueryInterface(aScriptObjectHolder, &participant);
-
   DebugWrapperTraversalCallback callback(wrapper);
 
-  participant->Traverse(aScriptObjectHolder, callback);
+  aTracer->Traverse(aScriptObjectHolder, callback);
   NS_ASSERTION(callback.mFound,
                "Cycle collection participant didn't traverse to preserved "
                "wrapper! This will probably crash.");
 
   callback.mFound = false;
-  participant->Trace(aScriptObjectHolder, DebugWrapperTraceCallback, &callback);
+  aTracer->Trace(aScriptObjectHolder, DebugWrapperTraceCallback, &callback);
   NS_ASSERTION(callback.mFound,
                "Cycle collection participant didn't trace preserved wrapper! "
                "This will probably crash.");
 }
 #endif
 
 // static
 bool
@@ -6927,17 +6925,17 @@ nsContentUtils::GetRootDocument(nsIDocum
   while (doc->GetParentDocument()) {
     doc = doc->GetParentDocument();
   }
   return doc;
 }
 
 // static
 void
-nsContentUtils::ReleaseWrapper(nsISupports* aScriptObjectHolder,
+nsContentUtils::ReleaseWrapper(void* aScriptObjectHolder,
                                nsWrapperCache* aCache)
 {
   if (aCache->PreservingWrapper()) {
     // PreserveWrapper puts new DOM bindings in the JS holders hash, but they
     // can also be in the DOM expando hash, so we need to try to remove them
     // from both here.
     JSObject* obj = aCache->GetWrapperPreserveColor();
     if (aCache->IsDOMBinding() && obj) {
--- a/dom/base/nsWrapperCache.h
+++ b/dom/base/nsWrapperCache.h
@@ -221,16 +221,19 @@ NS_DEFINE_STATIC_IID_ACCESSOR(nsWrapperC
 // Cycle collector macros for wrapper caches.
 
 #define NS_IMPL_CYCLE_COLLECTION_TRACE_PRESERVED_WRAPPER \
   nsContentUtils::TraceWrapper(tmp, aCallback, aClosure);
 
 #define NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER \
   nsContentUtils::ReleaseWrapper(s, tmp);
 
+#define NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER_NATIVE \
+  nsContentUtils::ReleaseWrapper(tmp, tmp);
+
 #define NS_IMPL_CYCLE_COLLECTION_TRACE_WRAPPERCACHE(_class) \
   NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(_class)              \
     NS_IMPL_CYCLE_COLLECTION_TRACE_PRESERVED_WRAPPER        \
   NS_IMPL_CYCLE_COLLECTION_TRACE_END
 
 #define NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE_0(_class) \
   NS_IMPL_CYCLE_COLLECTION_CLASS(_class)                \
   NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(_class)         \
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -88,21 +88,26 @@ def DOMClass(descriptor):
         protoList = ['prototypes::id::' + proto for proto in descriptor.prototypeChain]
         # Pad out the list to the right length with _ID_Count so we
         # guarantee that all the lists are the same length.  _ID_Count
         # is never the ID of any prototype, so it's safe to use as
         # padding.
         protoList.extend(['prototypes::id::_ID_Count'] * (descriptor.config.maxProtoChainLength - len(protoList)))
         prototypeChainString = ', '.join(protoList)
         nativeHooks = "NULL" if descriptor.workers else "&NativeHooks"
+        if descriptor.workers or descriptor.nativeOwnership != 'refcounted':
+            participant = "nullptr"
+        else:
+            participant = "NS_CYCLE_COLLECTION_PARTICIPANT(%s)" % descriptor.nativeType
         return """{
   { %s },
-  %s, %s
+  %s, %s, %s
 }""" % (prototypeChainString, toStringBool(descriptor.nativeOwnership == 'nsisupports'),
-          nativeHooks)
+        nativeHooks,
+        participant)
 
 class CGDOMJSClass(CGThing):
     """
     Generate a DOMJSClass for a given descriptor
     """
     def __init__(self, descriptor):
         CGThing.__init__(self)
         self.descriptor = descriptor
@@ -628,22 +633,23 @@ class CGAddPropertyHook(CGAbstractClassH
     """
     def __init__(self, descriptor):
         args = [Argument('JSContext*', 'cx'), Argument('JSHandleObject', 'obj'),
                 Argument('JSHandleId', 'id'), Argument('JSMutableHandleValue', 'vp')]
         CGAbstractClassHook.__init__(self, descriptor, ADDPROPERTY_HOOK_NAME,
                                      'JSBool', args)
 
     def generate_code(self):
-        # FIXME https://bugzilla.mozilla.org/show_bug.cgi?id=774279
-        # Using a real trace hook might enable us to deal with non-nsISupports
-        # wrappercached things here.
-        assert self.descriptor.nativeOwnership == 'nsisupports'
-        return """  nsContentUtils::PreserveWrapper(reinterpret_cast<nsISupports*>(self), self);
-  return true;"""
+        assert not self.descriptor.workers and self.descriptor.wrapperCache
+        if self.descriptor.nativeOwnership == 'nsisupports':
+            preserveArgs = "reinterpret_cast<nsISupports*>(self), self"
+        else:
+            preserveArgs = "self, self, NS_CYCLE_COLLECTION_PARTICIPANT(%s)" % self.descriptor.nativeType
+        return """  nsContentUtils::PreserveWrapper(%s);
+  return true;""" % preserveArgs
 
 def DeferredFinalizeSmartPtr(descriptor):
     if descriptor.nativeOwnership == 'owned':
         smartPtr = 'nsAutoPtr<%s>'
     else:
         assert descriptor.nativeOwnership == 'refcounted'
         smartPtr = 'nsRefPtr<%s>'
     return smartPtr % descriptor.nativeType
--- a/dom/bindings/Configuration.py
+++ b/dom/bindings/Configuration.py
@@ -261,17 +261,19 @@ class Descriptor(DescriptorProvider):
             self.nativeOwnership = desc.get('nativeOwnership', 'nsisupports')
             if not self.nativeOwnership in ['owned', 'refcounted', 'nsisupports']:
                 raise TypeError("Descriptor for %s has unrecognized value (%s) "
                                 "for nativeOwnership" %
                                 (self.interface.identifier.name, self.nativeOwnership))
         self.customTrace = desc.get('customTrace', self.workers)
         self.customFinalize = desc.get('customFinalize', self.workers)
         self.wrapperCache = (not self.interface.isCallback() and
-                             (self.workers or desc.get('wrapperCache', True)))
+                             (self.workers or
+                              (self.nativeOwnership != 'owned' and
+                               desc.get('wrapperCache', True))))
 
         if not self.wrapperCache and self.prefable:
             raise TypeError("Descriptor for %s is prefable but not wrappercached" %
                             self.interface.identifier.name)
 
         def make_name(name):
             return name + "_workers" if self.workers else name
         self.name = make_name(interface.identifier.name)
--- a/dom/bindings/DOMJSClass.h
+++ b/dom/bindings/DOMJSClass.h
@@ -6,16 +6,18 @@
 #ifndef mozilla_dom_DOMJSClass_h
 #define mozilla_dom_DOMJSClass_h
 
 #include "jsapi.h"
 #include "jsfriendapi.h"
 
 #include "mozilla/dom/PrototypeList.h" // auto-generated
 
+class nsCycleCollectionParticipant;
+
 // We use slot 0 for holding the raw object.  This is safe for both
 // globals and non-globals.
 #define DOM_OBJECT_SLOT 0
 
 // We use slot 1 for holding the expando object. This is not safe for globals
 // until bug 760095 is fixed, so that bug blocks converting Window to new
 // bindings.
 #define DOM_XRAY_EXPANDO_SLOT 1
@@ -61,16 +63,21 @@ struct DOMClass
 
   // We store the DOM object in reserved slot with index DOM_OBJECT_SLOT or in
   // the proxy private if we use a proxy object.
   // Sometimes it's an nsISupports and sometimes it's not; this class tells
   // us which it is.
   const bool mDOMObjectIsISupports;
 
   const NativePropertyHooks* mNativeHooks;
+
+  // This stores the CC participant for the native, null if this class is for a
+  // worker or for a native inheriting from nsISupports (we can get the CC
+  // participant by QI'ing in that case).
+  nsCycleCollectionParticipant* mParticipant;
 };
 
 // Special JSClass for reflected DOM objects.
 struct DOMJSClass
 {
   // It would be nice to just inherit from JSClass, but that precludes pure
   // compile-time initialization of the form |DOMJSClass = {...};|, since C++
   // only allows brace initialization for aggregate/POD types.
--- a/js/xpconnect/src/nsXPConnect.cpp
+++ b/js/xpconnect/src/nsXPConnect.cpp
@@ -842,20 +842,26 @@ NoteGCThingXPCOMChildren(js::Class *clas
         NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "xpc_GetJSPrivate(obj)");
         cb.NoteXPCOMChild(static_cast<nsISupports*>(xpc_GetJSPrivate(obj)));
     } else if (oldproxybindings::instanceIsProxy(obj)) {
         NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "js::GetProxyPrivate(obj)");
         nsISupports *identity =
             static_cast<nsISupports*>(js::GetProxyPrivate(obj).toPrivate());
         cb.NoteXPCOMChild(identity);
     } else {
-        nsISupports *identity;
-        if (UnwrapDOMObjectToISupports(obj, identity)) {
+        const DOMClass* domClass;
+        DOMObjectSlot slot = GetDOMClass(obj, domClass);
+        if (slot != eNonDOMObject) {
             NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "UnwrapDOMObject(obj)");
-            cb.NoteXPCOMChild(identity);
+            if (domClass->mDOMObjectIsISupports) {
+                cb.NoteXPCOMChild(UnwrapDOMObject<nsISupports>(obj, slot));
+            } else if (domClass->mParticipant) {
+                cb.NoteNativeChild(UnwrapDOMObject<void>(obj, slot),
+                                   domClass->mParticipant);
+            }
         }
     }
 }
 
 enum TraverseSelect {
     TRAVERSE_CPP,
     TRAVERSE_FULL
 };