Fixing bug 520639. Make plugin library unloading independent of the lifetime of nsPluginTag objects. Patch and reviews by jst@mozilla.com and joshmoz@gmail.com, a=blocking1.9.2+

modules/plugin/base/src/nsNPAPIPluginInstance.cpp

@@ -1534,6 +1534,13 @@ nsNPAPIPluginInstance::DefineJavaProperties()
   return NS_OK;
 }
 
+nsresult
+nsNPAPIPluginInstance::SetCached(PRBool aCache)
+{
+  mCached = aCache;
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 nsNPAPIPluginInstance::ShouldCache(PRBool* shouldCache)
 {

modules/plugin/base/src/nsNPAPIPluginInstance.h

@@ -110,8 +110,8 @@ public:
   // returns the state of mStarted
   PRBool IsStarted();
 
-  // cache this NPAPI plugin like an XPCOM plugin
-  nsresult SetCached(PRBool aCache) { mCached = aCache; return NS_OK; }
+  // cache this NPAPI plugin
+  nsresult SetCached(PRBool aCache);
 
   already_AddRefed<nsPIDOMWindow> GetDOMWindow();
 

modules/plugin/base/src/nsPluginHost.cpp

@@ -442,12 +442,12 @@ PRBool nsPluginInstanceTagList::remove(nsPluginInstanceTag * plugin)
           rv = pref->GetBoolPref("plugins.unloadASAP", &unloadPluginsASAP);
           if (NS_SUCCEEDED(rv) && unloadPluginsASAP)
             pluginTag->TryUnloadPlugin();
-        } 
-        else
+        } else {
           NS_ASSERTION(pluginTag, "pluginTag was not set, plugin not shutdown");
-      }
-      else
+        }
+      } else {
         delete p;
+      }
 
       mCount--;
       return PR_TRUE;
@@ -598,7 +598,6 @@ nsPluginTag::nsPluginTag(nsPluginTag* aPluginTag)
     mMimeDescriptionArray(aPluginTag->mMimeDescriptionArray),
     mExtensionsArray(nsnull),
     mLibrary(nsnull),
-    mEntryPoint(nsnull),
     mCanUnloadLibrary(PR_TRUE),
     mXPConnected(PR_FALSE),
     mIsJavaPlugin(aPluginTag->mIsJavaPlugin),
@@ -630,7 +629,6 @@ nsPluginTag::nsPluginTag(nsPluginInfo* aPluginInfo)
     mMimeTypeArray(nsnull),
     mExtensionsArray(nsnull),
     mLibrary(nsnull),
-    mEntryPoint(nsnull),
 #ifdef XP_MACOSX
     mCanUnloadLibrary(!aPluginInfo->fBundle),
 #else
@@ -725,7 +723,6 @@ nsPluginTag::nsPluginTag(const char* aName,
     mMimeTypeArray(nsnull),
     mExtensionsArray(nsnull),
     mLibrary(nsnull),
-    mEntryPoint(nsnull),
     mCanUnloadLibrary(aCanUnload),
     mXPConnected(PR_FALSE),
     mIsJavaPlugin(PR_FALSE),
@@ -768,14 +765,6 @@ nsPluginTag::~nsPluginTag()
 {
   NS_ASSERTION(!mNext, "Risk of exhausting the stack space, bug 486349");
 
-  TryUnloadPlugin();
-
-  // Remove mime types added to the category manager
-  // only if we were made 'active' by setting the host
-  if (mPluginHost) {
-    RegisterWithCategoryManager(PR_FALSE, nsPluginTag::ePluginUnregister);
-  }
-
   if (mMimeTypeArray) {
     for (int i = 0; i < mVariants; i++)
       delete[] mMimeTypeArray[i];
@@ -973,7 +962,6 @@ void nsPluginTag::TryUnloadPlugin()
 {
   if (mEntryPoint) {
     mEntryPoint->Shutdown();
-    mEntryPoint->Release();
     mEntryPoint = nsnull;
   }
 
@@ -996,6 +984,12 @@ void nsPluginTag::TryUnloadPlugin()
   // again so the calling code should not be fooled and reload
   // the library fresh
   mLibrary = nsnull;
+
+  // Remove mime types added to the category manager
+  // only if we were made 'active' by setting the host
+  if (mPluginHost) {
+    RegisterWithCategoryManager(PR_FALSE, nsPluginTag::ePluginUnregister);
+  }
 }
 
 void nsPluginTag::Mark(PRUint32 mask)
@@ -2571,6 +2565,10 @@ nsresult nsPluginHost::ReloadPlugins(PRBool reloadPages)
         prev->mNext = next;
 
       p->mNext = nsnull;
+
+      // attempt to unload plugins whenever they are removed from the list
+      p->TryUnloadPlugin();
+
       p = next;
       continue;
     }
@@ -2908,9 +2906,9 @@ NS_IMETHODIMP nsPluginHost::Destroy()
   // at this point nsIPlugin::Shutdown calls will be performed if needed
   mPluginInstanceTagList.shutdown();
 
-  if (mPluginPath) {
-    PR_Free(mPluginPath);
-    mPluginPath = nsnull;
+  nsPluginTag *pluginTag;
+  for (pluginTag = mPlugins; pluginTag; pluginTag = pluginTag->mNext) {
+    pluginTag->TryUnloadPlugin();
   }
 
   NS_ITERATIVE_UNREF_LIST(nsRefPtr<nsPluginTag>, mPlugins, mNext);
@@ -4009,17 +4007,17 @@ NS_IMETHODIMP nsPluginHost::GetPlugin(const char *aMimeType, nsIPlugin** aPlugin
       pluginTag->mLibrary = pluginLibrary;
     }
 
-    nsIPlugin* plugin = pluginTag->mEntryPoint;
+    nsCOMPtr<nsIPlugin> plugin = pluginTag->mEntryPoint;
     if (!plugin) {
       // Now lets try to get the entry point from an NPAPI plugin
-      rv = CreateNPAPIPlugin(pluginTag, &plugin);
+      rv = CreateNPAPIPlugin(pluginTag, getter_AddRefs(plugin));
       if (NS_SUCCEEDED(rv))
         pluginTag->mEntryPoint = plugin;
     }
 
     if (plugin) {
-      *aPlugin = plugin;
-      plugin->AddRef();
+      NS_ADDREF(*aPlugin = plugin);
+
       return NS_OK;
     }
   }

modules/plugin/base/src/nsPluginHost.h

@@ -122,7 +122,7 @@ public:
   nsTArray<nsCString> mMimeDescriptionArray; // UTF-8
   char          **mExtensionsArray;
   PRLibrary     *mLibrary;
-  nsIPlugin     *mEntryPoint;
+  nsCOMPtr<nsIPlugin> mEntryPoint;
   PRPackedBool  mCanUnloadLibrary;
   PRPackedBool  mXPConnected;
   PRPackedBool  mIsJavaPlugin;
@@ -342,7 +342,6 @@ private:
   // calls PostPluginUnloadEvent for each library in mUnusedLibraries
   void UnloadUnusedLibraries();
   
-  char *mPluginPath;
   nsRefPtr<nsPluginTag> mPlugins;
   nsRefPtr<nsPluginTag> mCachedPlugins;
   PRPackedBool mPluginsLoaded;