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 118156 c1bbabd184b33ec21cba783c64760937cffe975f
parent 118155 ba1ece534b748b22e68597b7ef4ba237ba17b143
child 118157 a1c63e59513618b034c4e86941dbdf2af4e55767
push id1997
push userakeybl@mozilla.com
push dateMon, 07 Jan 2013 21:25:26 +0000
treeherdermozilla-beta@4baf45cdcf21 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs799465
milestone19.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
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
 };