2006-03-20 Eric Seidel <eseidel@apple.com>

        Reviewed by adele & ggaren.

        Added new cachePluginDataIfNecessary function to update
        plugins and mimes arrays.  Made sure to call this in
        constructor as well as refresh.  The crash was caused by
        a refresh rendering a "plugins" object invalid.
        Changed existing test case to depend on this new correct behavior.

        <rdar://problem/4480571> Safari crashed at exit at KXMLCore::deleteAllValues + 24

        * khtml/ecma/kjs_navigator.cpp:
        (KJS::PluginBase::cachePluginDataIfNecessary):
        (KJS::PluginBase::PluginBase):
        (KJS::PluginBase::~PluginBase):
        (KJS::PluginBase::refresh):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@13410 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 4c8ea3e8129350431db715d216aa6de4ccedb2b1..ee96787d244ff19916c799397e103aff536f1abb 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,12 @@
+2006-03-20  Eric Seidel  <eseidel@apple.com>
+
+        Reviewed by adele & ggaren.
+
+        Modified test case to cover crasher:
+         <rdar://problem/4480571> Safari crashed at exit at KXMLCore::deleteAllValues + 24
+
+        * plugins/plugin-javascript-access.html:
+
 2006-03-20  Eric Seidel  <eseidel@apple.com>
 
         Reviewed by justing.

diff --git a/LayoutTests/plugins/plugin-javascript-access.html b/LayoutTests/plugins/plugin-javascript-access.html
index f30dfa60180adef3fc184cd771e016189892cc71..223d92881583ec5eaca243b6ba067c64a96c6963 100644 (file)
--- a/LayoutTests/plugins/plugin-javascript-access.html
+++ b/LayoutTests/plugins/plugin-javascript-access.html
@@ -8,12 +8,14 @@
 if (window.layoutTestController)
     layoutTestController.dumpAsText()
 
-navigator.plugins.refresh(false); // Supposedly helps if new plug-ins were added.
+var plugins = navigator.plugins;
+
+plugins.refresh(false); // Supposedly helps if new plug-ins were added.
 
 var foundTestPlugin = false;
 
-for (var pi = 0; pi != navigator.plugins.length; pi++) {
-    var plugin = navigator.plugins[pi];
+for (var pi = 0; pi != plugins.length; pi++) {
+    var plugin = plugins[pi];
     
     // We can only guarantee that the Test PlugIn is installed.
     if (plugin.name != "WebKit Test PlugIn")

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 38ef9c12a77d7e828478a4045c74e70620b0c181..67d36684f7fe4ce9681d6085bfc8db498d499930 100644 (file)
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,21 @@
+2006-03-20  Eric Seidel  <eseidel@apple.com>
+
+        Reviewed by adele & ggaren.
+
+        Added new cachePluginDataIfNecessary function to update
+        plugins and mimes arrays.  Made sure to call this in
+        constructor as well as refresh.  The crash was caused by
+        a refresh rendering a "plugins" object invalid.
+        Changed existing test case to depend on this new correct behavior.
+        
+        <rdar://problem/4480571> Safari crashed at exit at KXMLCore::deleteAllValues + 24
+
+        * khtml/ecma/kjs_navigator.cpp:
+        (KJS::PluginBase::cachePluginDataIfNecessary):
+        (KJS::PluginBase::PluginBase):
+        (KJS::PluginBase::~PluginBase):
+        (KJS::PluginBase::refresh):
+
 2006-03-20  Adele Peterson  <adele@apple.com>
 
         Reviewed by Justin.

diff --git a/WebCore/khtml/ecma/kjs_navigator.cpp b/WebCore/khtml/ecma/kjs_navigator.cpp
index fe4bb30d726e39e894d4e48fdfcd782a17826b16..8b9da0b86cd71d07322c64fe54b70a0655d4922a 100644 (file)
--- a/WebCore/khtml/ecma/kjs_navigator.cpp
+++ b/WebCore/khtml/ecma/kjs_navigator.cpp
@@ -43,11 +43,13 @@ namespace KJS {
         
         void refresh(bool reload);
 
+    protected:
+        static void cachePluginDataIfNecessary();
         static Vector<PluginInfo*> *plugins;
         static Vector<MimeClassInfo*> *mimes;
 
     private:
-        static int m_refCount;
+        static int m_plugInCacheRefCount;
     };
 
 
@@ -117,7 +119,7 @@ const ClassInfo MimeType::info = { "MimeType", 0, &MimeTypeTable, 0 };
 
 Vector<PluginInfo*> *KJS::PluginBase::plugins = 0;
 Vector<MimeClassInfo*> *KJS::PluginBase::mimes = 0;
-int KJS::PluginBase::m_refCount = 0;
+int KJS::PluginBase::m_plugInCacheRefCount = 0;
 
 const ClassInfo Navigator::info = { "Navigator", 0, &NavigatorTable, 0 };
 /*
@@ -196,8 +198,7 @@ JSValue *Navigator::getValueProperty(ExecState *exec, int token) const
 
 /*******************************************************************/
 
-PluginBase::PluginBase(ExecState *exec)
-  : JSObject(exec->lexicalInterpreter()->builtinObjectPrototype() )
+void PluginBase::cachePluginDataIfNecessary()
 {
     if (!plugins) {
         plugins = new Vector<PluginInfo*>;
@@ -220,32 +221,47 @@ PluginBase::PluginBase(ExecState *exec)
                 mimes->append(*itr);
         }
     }
+}
 
-    m_refCount++;
+PluginBase::PluginBase(ExecState *exec)
+  : JSObject(exec->lexicalInterpreter()->builtinObjectPrototype() )
+{
+    cachePluginDataIfNecessary();
+    m_plugInCacheRefCount++;
 }
 
 PluginBase::~PluginBase()
 {
-    m_refCount--;
-    if ( m_refCount==0 ) {
+    m_plugInCacheRefCount--;
+    if (!m_plugInCacheRefCount) {
+        if (plugins) {
+            deleteAllValues(*plugins);
+            delete plugins;
+            plugins = 0;
+        }
+        if (mimes) {
+            deleteAllValues(*mimes);
+            delete mimes;
+            mimes = 0;
+        }
+    }
+}
+
+void PluginBase::refresh(bool reload)
+{
+    if (plugins) {
         deleteAllValues(*plugins);
         delete plugins;
+        plugins = 0;
+    }
+    if (mimes) {
         deleteAllValues(*mimes);
         delete mimes;
-        plugins = 0;
         mimes = 0;
     }
-}
-
-void PluginBase::refresh(bool reload)
-{
-    deleteAllValues(*plugins);
-    delete plugins;
-    deleteAllValues(*mimes);
-    delete mimes;
-    plugins = 0;
-    mimes = 0;
+    
     refreshPlugins(reload);
+    cachePluginDataIfNecessary();
 }