RS by Brady Eidson.

        
        Rolling back in r18786 with leaks fixed, and these renames slightly reworked:

        Because they can return 0:
        rootObjectForImp => findRootObject (overloaded for JSObject* and Interpreter*)
        rootObjectForInterpreter => findRootObject (ditto)
        findReferenceSet => findProtectCountSet



git-svn-id: svn://svn.chromium.org/blink/trunk@18811 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/JavaScriptCore/ChangeLog

+2007-01-12  Geoffrey Garen  <ggaren@apple.com>
+
+        RS by Brady Eidson.
+        
+        Rolling back in r18786 with leaks fixed, and these renames slightly reworked:
+
+        Because they can return 0:
+        rootObjectForImp => findRootObject (overloaded for JSObject* and Interpreter*)
+        rootObjectForInterpreter => findRootObject (ditto)
+        findReferenceSet => findProtectCountSet
+
 2007-01-11  Geoffrey Garen  <ggaren@apple.com>
 
         RS by Brady Eidson.

third_party/WebKit/JavaScriptCore/bindings/c/c_utility.cpp

@@ -126,7 +126,7 @@ void convertValueToNPVariant(ExecState *exec, JSValue *value, NPVariant *result)
             OBJECT_TO_NPVARIANT(obj, *result);
         } else {
             Interpreter *originInterpreter = exec->dynamicInterpreter();
-            const Bindings::RootObject* originRootObject = rootObjectForInterpreter(originInterpreter);
+            const Bindings::RootObject* originRootObject = findRootObject(originInterpreter);
 
             Interpreter *interpreter = 0;
             if (originInterpreter->isGlobalObject(value)) {
@@ -136,7 +136,7 @@ void convertValueToNPVariant(ExecState *exec, JSValue *value, NPVariant *result)
             if (!interpreter)
                 interpreter = originInterpreter;
 
-            const RootObject* rootObject = rootObjectForInterpreter(interpreter);
+            const RootObject* rootObject = findRootObject(interpreter);
             if (!rootObject) {
                 RootObject* newRootObject = new RootObject(0, interpreter);
                 rootObject = newRootObject;

third_party/WebKit/JavaScriptCore/bindings/jni/jni_jsobject.cpp

@@ -79,7 +79,7 @@ jvalue JavaJSObject::invoke (JSObjectCallContext *context)
         }
         else {
             JSObject *imp = jlong_to_impptr(nativeHandle);
-            if (!rootObjectForImp(imp)) {
+            if (!findRootObject(imp)) {
                 fprintf (stderr, "%s:%d:  Attempt to access JavaScript from destroyed applet, type %d.\n", __FILE__, __LINE__, context->type);
                 return result;
             }
@@ -127,14 +127,10 @@ jvalue JavaJSObject::invoke (JSObjectCallContext *context)
     
                 case Finalize: {
                     JSObject *imp = jlong_to_impptr(nativeHandle);
-                    if (findReferenceSet(imp) == 0) {
-                        // We may have received a finalize method call from the VM 
-                        // AFTER removing our last reference to the Java instance.
-                        JS_LOG ("finalize called on instance we have already removed.\n");
-                    }
-                    else {
+                    // We may have received a finalize method call from the VM 
+                    // AFTER removing our last reference to the Java instance.
+                    if (findProtectCountSet(imp))
                         JavaJSObject(nativeHandle).finalize();
-                    }
                     break;
                 }
                 
@@ -158,7 +154,7 @@ JavaJSObject::JavaJSObject(jlong nativeJSObject)
     // terribly wrong.
     assert (_imp != 0);
     
-    _rootObject = rootObjectForImp(_imp);
+    _rootObject = findRootObject(_imp);
     
     // If we can't find the root for the object something is terribly wrong.
     assert (_rootObject != 0);
@@ -300,7 +296,7 @@ jlong JavaJSObject::createNative(jlong nativeHandle)
     if (nativeHandle == UndefinedHandle)
         return nativeHandle;
 
-    if (rootObjectForImp(jlong_to_impptr(nativeHandle)))
+    if (findRootObject(jlong_to_impptr(nativeHandle)))
         return nativeHandle;
 
     CreateRootObjectFunction createRootObject = RootObject::createRootObject();

third_party/WebKit/JavaScriptCore/bindings/objc/objc_runtime.mm

@@ -141,7 +141,7 @@ JSValue* ObjcField::valueFromInstance(ExecState* exec, const Instance* instance)
 
 static id convertValueToObjcObject(ExecState* exec, JSValue* value)
 {
-    const RootObject* rootObject = rootObjectForInterpreter(exec->dynamicInterpreter());
+    const RootObject* rootObject = findRootObject(exec->dynamicInterpreter());
     if (!rootObject) {
         RootObject* newRootObject = new RootObject(0, exec->dynamicInterpreter());
         rootObject = newRootObject;

third_party/WebKit/JavaScriptCore/bindings/objc/objc_utility.mm

@@ -136,7 +136,7 @@ ObjcValue convertValueToObjcValue(ExecState *exec, JSValue *value, ObjcValueType
     switch (type) {
         case ObjcObjectType: {
             Interpreter *originInterpreter = exec->dynamicInterpreter();
-            const RootObject* originRootObject = rootObjectForInterpreter(originInterpreter);
+            const RootObject* originRootObject = findRootObject(originInterpreter);
 
             Interpreter *interpreter = 0;
             if (originInterpreter->isGlobalObject(value))
@@ -145,7 +145,7 @@ ObjcValue convertValueToObjcValue(ExecState *exec, JSValue *value, ObjcValueType
             if (!interpreter)
                 interpreter = originInterpreter;
                 
-            const RootObject* rootObject = rootObjectForInterpreter(interpreter);
+            const RootObject* rootObject = findRootObject(interpreter);
             if (!rootObject) {
                 RootObject* newRootObject = new RootObject(0, interpreter);
                 rootObject = newRootObject;

third_party/WebKit/JavaScriptCore/bindings/runtime_root.cpp

@@ -44,119 +44,93 @@ namespace KJS { namespace Bindings {
 // 1 OR the applet is shutdown we deref the JavaScript instance.  Applet instances
 // are represented by a jlong.
 
-typedef HashMap<const RootObject*, ReferencesSet*> ReferencesByRootMap; 
+typedef HashMap<const RootObject*, ProtectCountSet*> RootObjectMap;
 
-static ReferencesByRootMap* getReferencesByRootMap()
+static RootObjectMap* rootObjectMap()
 {
-    static ReferencesByRootMap* referencesByRootMap = 0;
-    
-    if (!referencesByRootMap)
-        referencesByRootMap = new ReferencesByRootMap;
-    
-    return referencesByRootMap;
+    static RootObjectMap staticRootObjectMap;
+    return &staticRootObjectMap;
 }
 
-static ReferencesSet* getReferencesSet(const RootObject* rootObject)
+static ProtectCountSet* getProtectCountSet(const RootObject* rootObject)
 {
-    ReferencesByRootMap* refsByRoot = getReferencesByRootMap();
-    ReferencesSet* referencesSet = 0;
-    
-    referencesSet = refsByRoot->get(rootObject);
-    if (!referencesSet) {
-        referencesSet  = new ReferencesSet;
-        refsByRoot->add(rootObject, referencesSet);
+    ProtectCountSet* protectCountSet = rootObjectMap()->get(rootObject);
+
+    if (!protectCountSet) {
+        protectCountSet = new ProtectCountSet;
+        rootObjectMap()->add(rootObject, protectCountSet);
     }
-    return referencesSet;
+    return protectCountSet;
 }
 
-// Scan all the dictionary for all the roots to see if any have a 
-// reference to the imp, and if so, return it's reference count
-// dictionary.
-// FIXME:  This is a potential performance bottleneck with many applets.  We could fix be adding a
-// imp to root dictionary.
-ReferencesSet* findReferenceSet(JSObject* imp)
+static void destroyProtectCountSet(const RootObject* rootObject, ProtectCountSet* protectCountSet)
 {
-    ReferencesByRootMap* refsByRoot = getReferencesByRootMap ();
-    if (refsByRoot) {
-        ReferencesByRootMap::const_iterator end = refsByRoot->end();
-        for (ReferencesByRootMap::const_iterator it = refsByRoot->begin(); it != end; ++it) {
-            ReferencesSet* set = it->second;
-            
-            if (set->contains(imp))
-                return set;
-        }
-    }
-    
-    return 0;
+    rootObjectMap()->remove(rootObject);
+    delete protectCountSet;
 }
 
-// FIXME:  This is a potential performance bottleneck with many applets.  We could fix be adding a
-// imp to root dictionary.
-const RootObject* rootObjectForImp (JSObject* imp)
+// FIXME:  These two functions are a potential performance problem.  We could 
+// fix them by adding a JSObject to RootObject dictionary.
+
+ProtectCountSet* findProtectCountSet(JSObject* jsObject)
 {
-    ReferencesByRootMap* refsByRoot = getReferencesByRootMap ();
-    const RootObject* rootObject = 0;
-    
-    if (refsByRoot) {
-        ReferencesByRootMap::const_iterator end = refsByRoot->end();
-        for (ReferencesByRootMap::const_iterator it = refsByRoot->begin(); it != end; ++it) {
-            ReferencesSet* set = it->second;
-            if (set->contains(imp)) {
-                rootObject = it->first;
-                break;
-            }
-        }
+    const RootObject* rootObject = findRootObject(jsObject);
+    return rootObject ? getProtectCountSet(rootObject) : 0;
+}
+
+const RootObject* findRootObject(JSObject* jsObject)
+{
+    RootObjectMap::const_iterator end = rootObjectMap()->end();
+    for (RootObjectMap::const_iterator it = rootObjectMap()->begin(); it != end; ++it) {
+        ProtectCountSet* set = it->second;
+        if (set->contains(jsObject))
+            return it->first;
     }
     
-    return rootObject;
+    return 0;
 }
 
-const RootObject* rootObjectForInterpreter(Interpreter* interpreter)
+const RootObject* findRootObject(Interpreter* interpreter)
 {
-    ReferencesByRootMap* refsByRoot = getReferencesByRootMap ();
-    
-    if (refsByRoot) {
-        ReferencesByRootMap::const_iterator end = refsByRoot->end();
-        for (ReferencesByRootMap::const_iterator it = refsByRoot->begin(); it != end; ++it) {
-            const RootObject* aRootObject = it->first;
-            
-            if (aRootObject->interpreter() == interpreter)
-                return aRootObject;
-        }
+    RootObjectMap::const_iterator end = rootObjectMap()->end();
+    for (RootObjectMap::const_iterator it = rootObjectMap()->begin(); it != end; ++it) {
+        const RootObject* aRootObject = it->first;
+        
+        if (aRootObject->interpreter() == interpreter)
+            return aRootObject;
     }
     
     return 0;
 }
 
-void addNativeReference(const RootObject* rootObject, JSObject* imp)
+void addNativeReference(const RootObject* rootObject, JSObject* jsObject)
 {
-    if (rootObject) {
-        ReferencesSet* referenceMap = getReferencesSet(rootObject);
-        
-        unsigned numReferences = referenceMap->count(imp);
-        if (numReferences == 0) {
-            JSLock lock;
-            gcProtect(imp);
-        }
-        referenceMap->add(imp);
+    if (!rootObject)
+        return;
+    
+    ProtectCountSet* protectCountSet = getProtectCountSet(rootObject);
+    if (!protectCountSet->contains(jsObject)) {
+        JSLock lock;
+        gcProtect(jsObject);
     }
+    protectCountSet->add(jsObject);
 }
 
-void removeNativeReference(JSObject* imp)
+void removeNativeReference(JSObject* jsObject)
 {
-    if (!imp)
+    if (!jsObject)
         return;
 
-    ReferencesSet *referencesSet = findReferenceSet(imp);
-    if (referencesSet) {
-        unsigned numReferences = referencesSet->count(imp);
-        
-        if (numReferences == 1) {
-            JSLock lock;
-            gcUnprotect(imp);
-        }
-        referencesSet->remove(imp);
+    // We might have manually detroyed the root object and its protect set already
+    ProtectCountSet* protectCountSet = findProtectCountSet(jsObject);
+    if (!protectCountSet)
+        return;
+
+    if (protectCountSet->count(jsObject) == 1) {
+        JSLock lock;
+        gcUnprotect(jsObject);
     }
+    protectCountSet->remove(jsObject);
 }
 
 #if PLATFORM(MAC)
@@ -286,18 +260,16 @@ void RootObject::setCreateRootObject(CreateRootObjectFunction createRootObject)
 // Destroys the RootObject and unprotects all JSObjects associated with it.
 void RootObject::destroy()
 {
-    ReferencesSet* referencesSet = getReferencesSet(this);
+    ProtectCountSet* protectCountSet = getProtectCountSet(this);
     
-    if (referencesSet) {
-        ReferencesSet::iterator end = referencesSet->end();
-        for (ReferencesSet::iterator it = referencesSet->begin(); it != end; ++it) {
+    if (protectCountSet) {
+        ProtectCountSet::iterator end = protectCountSet->end();
+        for (ProtectCountSet::iterator it = protectCountSet->begin(); it != end; ++it) {
             JSLock lock;
             gcUnprotect(it->first);            
         }
-        referencesSet->clear();
-        ReferencesByRootMap* refsByRoot = getReferencesByRootMap();
-        refsByRoot->remove(this);
-        delete referencesSet;
+
+        destroyProtectCountSet(this, protectCountSet);
     }
 
     delete this;

third_party/WebKit/JavaScriptCore/bindings/runtime_root.h

@@ -39,13 +39,13 @@ namespace Bindings {
 class RootObject;
 
 typedef RootObject* (*CreateRootObjectFunction)(void* nativeHandle);
-typedef HashCountedSet<JSObject*> ReferencesSet;
+typedef HashCountedSet<JSObject*> ProtectCountSet;
 
-extern ReferencesSet* findReferenceSet(JSObject *imp);
-extern const RootObject* rootObjectForImp(JSObject*);
-extern const RootObject* rootObjectForInterpreter(Interpreter*);
-extern void addNativeReference (const RootObject *root, JSObject *imp);
-extern void removeNativeReference (JSObject *imp);
+extern ProtectCountSet* findProtectCountSet(JSObject*);
+extern const RootObject* findRootObject(JSObject*);
+extern const RootObject* findRootObject(Interpreter*);
+extern void addNativeReference(const RootObject*, JSObject*);
+extern void removeNativeReference(JSObject*);
 
 class RootObject
 {

third_party/WebKit/JavaScriptCore/kjs/collector.cpp

@@ -417,12 +417,12 @@ void Collector::markStackObjectsConservatively()
 #endif
 }
 
-typedef HashCountedSet<JSCell *> ProtectCounts;
+typedef HashCountedSet<JSCell*> ProtectCountSet;
 
-static ProtectCounts& protectedValues()
+static ProtectCountSet& protectedValues()
 {
-    static ProtectCounts pv;
-    return pv;
+    static ProtectCountSet staticProtectCountSet;
+    return staticProtectCountSet;
 }
 
 void Collector::protect(JSValue *k)
@@ -449,9 +449,9 @@ void Collector::unprotect(JSValue *k)
 
 void Collector::markProtectedObjects()
 {
-  ProtectCounts& pv = protectedValues();
-  ProtectCounts::iterator end = pv.end();
-  for (ProtectCounts::iterator it = pv.begin(); it != end; ++it) {
+  ProtectCountSet& protectedValues = KJS::protectedValues();
+  ProtectCountSet::iterator end = protectedValues.end();
+  for (ProtectCountSet::iterator it = protectedValues.begin(); it != end; ++it) {
     JSCell *val = it->first;
     if (!val->marked())
       val->mark();
@@ -669,9 +669,9 @@ HashCountedSet<const char*>* Collector::rootObjectTypeCounts()
 {
     HashCountedSet<const char*>* counts = new HashCountedSet<const char*>;
 
-    ProtectCounts& pv = protectedValues();
-    ProtectCounts::iterator end = pv.end();
-    for (ProtectCounts::iterator it = pv.begin(); it != end; ++it)
+    ProtectCountSet& protectedValues = KJS::protectedValues();
+    ProtectCountSet::iterator end = protectedValues.end();
+    for (ProtectCountSet::iterator it = protectedValues.begin(); it != end; ++it)
         counts->add(typeName(it->first));
 
     return counts;