Bug 418615 - Neuter the code which tries to reframe existing plugin instances when navigator.plugins.refresh(true) is called. Instead, only scan for new plugins, unload unused plugins. The DOM code will continue to refresh the current page which calls navigator.plugins.refresh(true). r=jschoenick sr=bz a=lsblakk
authorBenjamin Smedberg <benjamin@smedbergs.us>
Tue, 23 Apr 2013 16:02:12 -0400
changeset 137491 66ff4f31d731259adf27588345a3addaa704bb21
parent 137490 bf17ffb21f5e302147db26a433988ea7bd1f51b3
child 137492 175e1e444db4819eb92bea1d8a671fbe05f3e2f6
push id2452
push userlsblakk@mozilla.com
push dateMon, 13 May 2013 16:59:38 +0000
treeherdermozilla-beta@d4b152d29d8d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjschoenick, bz, lsblakk
bugs418615
milestone22.0a2
Bug 418615 - Neuter the code which tries to reframe existing plugin instances when navigator.plugins.refresh(true) is called. Instead, only scan for new plugins, unload unused plugins. The DOM code will continue to refresh the current page which calls navigator.plugins.refresh(true). r=jschoenick sr=bz a=lsblakk
docshell/base/nsWebNavigationInfo.cpp
dom/base/nsPluginArray.cpp
dom/plugins/base/nsIPluginHost.idl
dom/plugins/base/nsNPAPIPlugin.cpp
dom/plugins/base/nsPluginHost.cpp
dom/plugins/base/nsPluginHost.h
dom/plugins/test/unit/head_plugins.js
dom/plugins/test/unit/test_nice_plugin_name.js
--- a/docshell/base/nsWebNavigationInfo.cpp
+++ b/docshell/base/nsWebNavigationInfo.cpp
@@ -52,17 +52,17 @@ nsWebNavigationInfo::IsTypeSupported(con
   }
   
   // Try reloading plugins in case they've changed.
   nsCOMPtr<nsIPluginHost> pluginHost =
     do_GetService(MOZ_PLUGIN_HOST_CONTRACTID);
   if (pluginHost) {
     // false will ensure that currently running plugins will not
     // be shut down
-    rv = pluginHost->ReloadPlugins(false);
+    rv = pluginHost->ReloadPlugins();
     if (NS_SUCCEEDED(rv)) {
       // OK, we reloaded plugins and there were new ones
       // (otherwise NS_ERROR_PLUGINS_PLUGINSNOTCHANGED would have
       // been returned).  Try checking whether we can handle the
       // content now.
       return IsTypeSupportedInternal(flatType, aIsTypeSupported);
     }
   }
--- a/dom/base/nsPluginArray.cpp
+++ b/dom/base/nsPluginArray.cpp
@@ -185,17 +185,17 @@ nsPluginArray::Refresh(bool aReloadDocum
 
   // NS_ERROR_PLUGINS_PLUGINSNOTCHANGED on reloading plugins indicates
   // that plugins did not change and was not reloaded
   bool pluginsNotChanged = false;
   uint32_t currentPluginCount = 0;
   if(mPluginHost) {
     res = GetLength(&currentPluginCount);
     NS_ENSURE_SUCCESS(res, res);
-    nsresult reloadResult = mPluginHost->ReloadPlugins(aReloadDocuments);
+    nsresult reloadResult = mPluginHost->ReloadPlugins();
     // currentPluginCount is as reported by nsPluginHost. mPluginCount is
     // essentially a cache of this value, and may be out of date.
     pluginsNotChanged = (reloadResult == NS_ERROR_PLUGINS_PLUGINSNOTCHANGED &&
                          currentPluginCount == mPluginCount);
   }
 
   // no need to reload the page if plugins have not been changed
   // in fact, if we do reload we can hit recursive load problem, see bug 93351
--- a/dom/plugins/base/nsIPluginHost.idl
+++ b/dom/plugins/base/nsIPluginHost.idl
@@ -15,27 +15,24 @@
 [scriptable, uuid(f89e7679-0adf-4a30-bda9-1afe1ee270d6)]
 interface nsIPluginPlayPreviewInfo : nsISupports
 {
   readonly attribute AUTF8String mimeType;
   readonly attribute boolean     ignoreCTP;
   readonly attribute AUTF8String redirectURL;
 };
 
-[scriptable, uuid(67ebff01-0dce-48f7-b6a5-6235fc78382b)]
+[scriptable, uuid(15f97490-7bdf-4947-885c-9258072af878)]
 interface nsIPluginHost : nsISupports
 {
   /**
    * Causes the plugins directory to be searched again for new plugin 
    * libraries.
-   *
-   * @param reloadPages - indicates whether currently visible pages should 
-   * also be reloaded
    */
-  void reloadPlugins(in boolean reloadPages);
+  void reloadPlugins();
 
   void getPluginTags([optional] out unsigned long aPluginCount,
     [retval, array, size_is(aPluginCount)] out nsIPluginTag aResults);
 
   /*
    * Flags for use with clearSiteData.
    *
    * FLAG_CLEAR_ALL: clear all data associated with a site.
--- a/dom/plugins/base/nsNPAPIPlugin.cpp
+++ b/dom/plugins/base/nsNPAPIPlugin.cpp
@@ -1140,17 +1140,17 @@ void NP_CALLBACK
   }
   NPN_PLUGIN_LOG(PLUGIN_LOG_NORMAL,
                  ("NPN_ReloadPlugins: reloadPages=%d\n", reloadPages));
 
   nsCOMPtr<nsIPluginHost> pluginHost(do_GetService(MOZ_PLUGIN_HOST_CONTRACTID));
   if (!pluginHost)
     return;
 
-  pluginHost->ReloadPlugins(reloadPages);
+  pluginHost->ReloadPlugins();
 }
 
 void NP_CALLBACK
 _invalidaterect(NPP npp, NPRect *invalidRect)
 {
   if (!NS_IsMainThread()) {
     NPN_PLUGIN_LOG(PLUGIN_LOG_ALWAYS,("NPN_invalidaterect called from the wrong thread\n"));
     return;
--- a/dom/plugins/base/nsPluginHost.cpp
+++ b/dom/plugins/base/nsPluginHost.cpp
@@ -263,62 +263,16 @@ bool ReadSectionHeader(nsPluginManifestL
         break; // it's wrong token
       }
       return true;
     }
   } while (reader.NextLine());
   return false;
 }
 
-// Little helper struct to asynchronously reframe any presentations (embedded)
-// or reload any documents (full-page), that contained plugins
-// which were shutdown as a result of a plugins.refresh(1)
-class nsPluginDocReframeEvent: public nsRunnable {
-public:
-  nsPluginDocReframeEvent(nsTArray<nsCOMPtr<nsIDocument> >& aDocs) { mDocs.SwapElements(aDocs); }
-
-  NS_DECL_NSIRUNNABLE
-
-  nsTArray<nsCOMPtr<nsIDocument> > mDocs;
-};
-
-NS_IMETHODIMP nsPluginDocReframeEvent::Run() {
-  uint32_t c = mDocs.Length();
-
-  // for each document (which previously had a running instance), tell
-  // the frame constructor to rebuild
-  for (uint32_t i = 0; i < c; i++) {
-    nsIDocument* doc = mDocs[i];
-    if (doc) {
-      nsIPresShell *shell = doc->GetShell();
-
-      // if this document has a presentation shell, then it has frames and can be reframed
-      if (shell) {
-        /* A reframe will cause a fresh object frame, instance owner, and instance
-         * to be created. Reframing of the entire document is necessary as we may have
-         * recently found new plugins and we want a shot at trying to use them instead
-         * of leaving alternate renderings.
-         * We do not want to completely reload all the documents that had running plugins
-         * because we could possibly trigger a script to run in the unload event handler
-         * which may want to access our defunct plugin and cause us to crash.
-         */
-
-        shell->ReconstructFrames(); // causes reframe of document
-      } else {  // no pres shell --> full-page plugin
-
-        NS_NOTREACHED("all plugins should have a pres shell!");
-
-      }
-    }
-  }
-
-  mDocs.Clear();
-  return NS_OK;
-}
-
 static bool UnloadPluginsASAP()
 {
   return Preferences::GetBool("dom.ipc.plugins.unloadASAP", false);
 }
 
 nsPluginHost::nsPluginHost()
   // No need to initialize members to nullptr, false etc because this class
   // has a zeroing operator new.
@@ -399,21 +353,20 @@ bool nsPluginHost::IsRunningPlugin(nsPlu
         instance->IsRunning()) {
       return true;
     }
   }
 
   return false;
 }
 
-nsresult nsPluginHost::ReloadPlugins(bool reloadPages)
+nsresult nsPluginHost::ReloadPlugins()
 {
   PLUGIN_LOG(PLUGIN_LOG_NORMAL,
-  ("nsPluginHost::ReloadPlugins Begin reloadPages=%d, active_instance_count=%d\n",
-  reloadPages, mInstances.Length()));
+  ("nsPluginHost::ReloadPlugins Begin\n"));
 
   nsresult rv = NS_OK;
 
   // this will create the initial plugin list out of cache
   // if it was not created yet
   if (!mPluginsLoaded)
     return LoadPlugins();
 
@@ -427,24 +380,16 @@ nsresult nsPluginHost::ReloadPlugins(boo
   // look for possible changes
   bool pluginschanged = true;
   FindPlugins(false, &pluginschanged);
 
   // if no changed detected, return an appropriate error code
   if (!pluginschanged)
     return NS_ERROR_PLUGINS_PLUGINSNOTCHANGED;
 
-  nsTArray<nsCOMPtr<nsIDocument> > instsToReload;
-  if (reloadPages) {
-
-    // Then stop any running plugin instances but hold on to the documents in the array
-    // We are going to need to restart the instances in these documents later
-    DestroyRunningInstances(&instsToReload, nullptr);
-  }
-
   // shutdown plugins and kill the list if there are no running plugins
   nsRefPtr<nsPluginTag> prev;
   nsRefPtr<nsPluginTag> next;
 
   for (nsRefPtr<nsPluginTag> p = mPlugins; p != nullptr;) {
     next = p->mNext;
 
     // only remove our plugin from the list if it's not running.
@@ -468,28 +413,18 @@ nsresult nsPluginHost::ReloadPlugins(boo
   }
 
   // set flags
   mPluginsLoaded = false;
 
   // load them again
   rv = LoadPlugins();
 
-  // If we have shut down any plugin instances, we've now got to restart them.
-  // Post an event to do the rest as we are going to be destroying the frame tree and we also want
-  // any posted unload events to finish
-  if (reloadPages && !instsToReload.IsEmpty()){
-    nsCOMPtr<nsIRunnable> ev = new nsPluginDocReframeEvent(instsToReload);
-    if (ev)
-      NS_DispatchToCurrentThread(ev);
-  }
-
   PLUGIN_LOG(PLUGIN_LOG_NORMAL,
-  ("nsPluginHost::ReloadPlugins End active_instance_count=%d\n",
-  mInstances.Length()));
+  ("nsPluginHost::ReloadPlugins End\n"));
 
   return rv;
 }
 
 #define NS_RETURN_UASTRING_SIZE 128
 
 nsresult nsPluginHost::UserAgent(const char **retstring)
 {
@@ -789,17 +724,17 @@ nsresult nsPluginHost::UnloadPlugins()
 {
   PLUGIN_LOG(PLUGIN_LOG_NORMAL, ("nsPluginHost::UnloadPlugins Called\n"));
 
   if (!mPluginsLoaded)
     return NS_OK;
 
   // we should call nsIPluginInstance::Stop and nsIPluginInstance::SetWindow
   // for those plugins who want it
-  DestroyRunningInstances(nullptr, nullptr);
+  DestroyRunningInstances(nullptr);
 
   nsPluginTag *pluginTag;
   for (pluginTag = mPlugins; pluginTag; pluginTag = pluginTag->mNext) {
     pluginTag->TryUnloadPlugin(true);
   }
 
   NS_ITERATIVE_UNREF_LIST(nsRefPtr<nsPluginTag>, mPlugins, mNext);
   NS_ITERATIVE_UNREF_LIST(nsRefPtr<nsPluginTag>, mCachedPlugins, mNext);
@@ -1022,17 +957,17 @@ nsresult nsPluginHost::SetUpPluginInstan
   nsCOMPtr<nsIDocument> currentdocument = do_QueryReferent(mCurrentDocument);
   if (document == currentdocument) {
     return rv;
   }
 
   mCurrentDocument = do_GetWeakReference(document);
 
   // Don't try to set up an instance again if nothing changed.
-  if (ReloadPlugins(false) == NS_ERROR_PLUGINS_PLUGINSNOTCHANGED) {
+  if (ReloadPlugins() == NS_ERROR_PLUGINS_PLUGINSNOTCHANGED) {
     return rv;
   }
 
   return TrySetUpPluginInstance(aMimeType, aURL, aOwner);
 }
 
 nsresult
 nsPluginHost::TrySetUpPluginInstance(const char *aMimeType,
@@ -2387,25 +2322,16 @@ nsPluginHost::UpdatePluginInfo(nsPluginT
   if (obsService)
     obsService->NotifyObservers(nullptr, "plugin-info-updated", nullptr);
 
   // Reload instances if needed
   if (aPluginTag->IsActive()) {
     return NS_OK;
   }
 
-  nsTArray<nsCOMPtr<nsIDocument> > instsToReload;
-  DestroyRunningInstances(&instsToReload, aPluginTag);
-  
-  if (!instsToReload.IsEmpty()) {
-    nsCOMPtr<nsIRunnable> ev = new nsPluginDocReframeEvent(instsToReload);
-    if (ev)
-      NS_DispatchToCurrentThread(ev);
-  }
-
   return NS_OK;
 }
 
 /* static */ bool
 nsPluginHost::IsTypeWhitelisted(const char *aMimeType)
 {
   nsAdoptingCString whitelist = Preferences::GetCString(kPrefWhitelist);
   if (!whitelist.Length()) {
@@ -3755,38 +3681,24 @@ nsPluginHost::StoppedInstanceCount()
 
 nsTArray< nsRefPtr<nsNPAPIPluginInstance> >*
 nsPluginHost::InstanceArray()
 {
   return &mInstances;
 }
 
 void 
-nsPluginHost::DestroyRunningInstances(nsTArray<nsCOMPtr<nsIDocument> >* aReloadDocs,
-                                      nsPluginTag* aPluginTag)
+nsPluginHost::DestroyRunningInstances(nsPluginTag* aPluginTag)
 {
   for (int32_t i = mInstances.Length(); i > 0; i--) {
     nsNPAPIPluginInstance *instance = mInstances[i - 1];
     if (instance->IsRunning() && (!aPluginTag || aPluginTag == TagForPlugin(instance->GetPlugin()))) {
       instance->SetWindow(nullptr);
       instance->Stop();
 
-      // If we've been passed an array to return, lets collect all our documents,
-      // removing duplicates. These will be reframed (embedded) or reloaded (full-page) later
-      // to kickstart our instances.
-      if (aReloadDocs) {
-        nsRefPtr<nsPluginInstanceOwner> owner = instance->GetOwner();
-        if (owner) {
-          nsCOMPtr<nsIDocument> doc;
-          owner->GetDocument(getter_AddRefs(doc));
-          if (doc && !aReloadDocs->Contains(doc))  // don't allow for duplicates
-            aReloadDocs->AppendElement(doc);
-        }
-      }
-
       // Get rid of all the instances without the possibility of caching.
       nsPluginTag* pluginTag = TagForPlugin(instance->GetPlugin());
       instance->SetWindow(nullptr);
 
       nsCOMPtr<nsIDOMElement> domElement;
       instance->GetDOMElement(getter_AddRefs(domElement));
       nsCOMPtr<nsIObjectLoadingContent> objectContent =
         do_QueryInterface(domElement);
--- a/dom/plugins/base/nsPluginHost.h
+++ b/dom/plugins/base/nsPluginHost.h
@@ -170,17 +170,17 @@ public:
                      const nsAString& browserDumpID);
 
   nsNPAPIPluginInstance *FindInstance(const char *mimetype);
   nsNPAPIPluginInstance *FindOldestStoppedInstance();
   uint32_t StoppedInstanceCount();
 
   nsTArray< nsRefPtr<nsNPAPIPluginInstance> > *InstanceArray();
 
-  void DestroyRunningInstances(nsTArray<nsCOMPtr<nsIDocument> >* aReloadDocs, nsPluginTag* aPluginTag);
+  void DestroyRunningInstances(nsPluginTag* aPluginTag);
 
   // Return the tag for |aLibrary| if found, nullptr if not.
   nsPluginTag* FindTagForLibrary(PRLibrary* aLibrary);
 
   // The last argument should be false if we already have an in-flight stream
   // and don't need to set up a new stream.
   nsresult InstantiatePluginInstance(const char *aMimeType, nsIURI* aURL,
                                      nsObjectLoadingContent *aContent,
--- a/dom/plugins/test/unit/head_plugins.js
+++ b/dom/plugins/test/unit/head_plugins.js
@@ -3,17 +3,18 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
 const Cc = Components.classes;
 const Ci = Components.interfaces;
 
 const gIsWindows = ("@mozilla.org/windows-registry-key;1" in Cc);
 const gIsOSX = ("nsILocalFileMac" in Ci);
-const gIsLinux = ("@mozilla.org/gnome-gconf-service;1" in Cc);
+const gIsLinux = ("@mozilla.org/gnome-gconf-service;1" in Cc) ||
+  ("@mozilla.org/gio-service;1" in Cc);
 
 // Finds the test plugin library
 function get_test_plugin() {
   var pluginEnum = gDirSvc.get("APluginsDL", Ci.nsISimpleEnumerator);
   while (pluginEnum.hasMoreElements()) {
     let dir = pluginEnum.getNext().QueryInterface(Ci.nsILocalFile);
     let plugin = dir.clone();
     // OSX plugin
--- a/dom/plugins/test/unit/test_nice_plugin_name.js
+++ b/dom/plugins/test/unit/test_nice_plugin_name.js
@@ -55,19 +55,16 @@ function createAppInfo(id, name, version
   registrar.registerFactory(XULAPPINFO_CID, "XULAppInfo",
                             XULAPPINFO_CONTRACTID, XULAppInfoFactory);
 }
 
 let gDirSvc = Cc["@mozilla.org/file/directory_service;1"]
                 .getService(Ci.nsIProperties);
 
 let gPluginHost = null;
-let gIsWindows = null;
-let gIsOSX = null;
-let gIsLinux = null;
 
 function test_expected_permission_string(aPermString) {
   gPluginHost.reloadPlugins(false);
   let plugin = get_test_plugintag();
   do_check_false(plugin == null);
   do_check_eq(gPluginHost.getPermissionStringForType("application/x-test"),
               aPermString);
 }