Fixing bug 781310. Change nsPluginHost::GetInst() to return already_AddRefed<nsPluginHost> to make it harder to write leaky code. r=jschoenick@mozilla.com
authorJohnny Stenback <jst@mozilla.com>
Wed, 20 Mar 2013 11:29:00 -0700
changeset 125722 e3ad8585b8e6862a531bffb5f0a74e429edf9657
parent 125721 07e459058893fa348c127ba2382a8d04fb18192c
child 125723 3d83421e9ca4b535870fe29c149623a70fa9cf05
push id24461
push useremorley@mozilla.com
push dateThu, 21 Mar 2013 11:51:51 +0000
treeherdermozilla-central@a73a2b5c423b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjschoenick
bugs781310
milestone22.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
Fixing bug 781310. Change nsPluginHost::GetInst() to return already_AddRefed<nsPluginHost> to make it harder to write leaky code. r=jschoenick@mozilla.com
content/base/src/nsObjectLoadingContent.cpp
dom/plugins/base/nsNPAPIPlugin.cpp
dom/plugins/base/nsNPAPIPluginInstance.cpp
dom/plugins/base/nsPluginHost.cpp
dom/plugins/base/nsPluginHost.h
dom/plugins/base/nsPluginStreamListenerPeer.cpp
dom/plugins/ipc/PluginModuleParent.cpp
--- a/content/base/src/nsObjectLoadingContent.cpp
+++ b/content/base/src/nsObjectLoadingContent.cpp
@@ -508,18 +508,17 @@ IsPluginEnabledByExtension(nsIURI* uri, 
 {
   nsAutoCString ext;
   GetExtensionFromURI(uri, ext);
 
   if (ext.IsEmpty()) {
     return false;
   }
 
-  nsRefPtr<nsPluginHost> pluginHost =
-    already_AddRefed<nsPluginHost>(nsPluginHost::GetInst());
+  nsRefPtr<nsPluginHost> pluginHost = nsPluginHost::GetInst();
 
   if (!pluginHost) {
     NS_NOTREACHED("No pluginhost");
     return false;
   }
 
   const char* typeFromExt;
   nsresult rv = pluginHost->IsPluginEnabledForExtension(ext.get(), typeFromExt);
@@ -528,18 +527,17 @@ IsPluginEnabledByExtension(nsIURI* uri, 
     return true;
   }
   return false;
 }
 
 nsresult
 IsPluginEnabledForType(const nsCString& aMIMEType)
 {
-  nsRefPtr<nsPluginHost> pluginHost =
-    already_AddRefed<nsPluginHost>(nsPluginHost::GetInst());
+  nsRefPtr<nsPluginHost> pluginHost = nsPluginHost::GetInst();
 
   if (!pluginHost) {
     NS_NOTREACHED("No pluginhost");
     return NS_ERROR_FAILURE;
   }
 
   nsresult rv = pluginHost->IsPluginEnabledForType(aMIMEType.get());
 
@@ -561,18 +559,17 @@ IsPluginEnabledForType(const nsCString& 
 // mFinalListener
 bool
 nsObjectLoadingContent::MakePluginListener()
 {
   if (!mInstanceOwner) {
     NS_NOTREACHED("expecting a spawned plugin");
     return false;
   }
-  nsRefPtr<nsPluginHost> pluginHost =
-    already_AddRefed<nsPluginHost>(nsPluginHost::GetInst());
+  nsRefPtr<nsPluginHost> pluginHost = nsPluginHost::GetInst();
   if (!pluginHost) {
     NS_NOTREACHED("No pluginHost");
     return false;
   }
   NS_ASSERTION(!mFinalListener, "overwriting a final listener");
   nsresult rv;
   nsRefPtr<nsNPAPIPluginInstance> inst;
   nsCOMPtr<nsIStreamListener> finalListener;
@@ -743,18 +740,17 @@ nsObjectLoadingContent::InstantiatePlugi
   doc->FlushPendingNotifications(Flush_Layout);
 
   if (!thisContent->GetPrimaryFrame()) {
     LOG(("OBJLC [%p]: Not instantiating plugin with no frame", this));
     return NS_OK;
   }
 
   nsresult rv = NS_ERROR_FAILURE;
-  nsRefPtr<nsPluginHost> pluginHost =
-    already_AddRefed<nsPluginHost>(nsPluginHost::GetInst());
+  nsRefPtr<nsPluginHost> pluginHost = nsPluginHost::GetInst();
 
   if (!pluginHost) {
     NS_NOTREACHED("No pluginhost");
     return NS_ERROR_FAILURE;
   }
 
   // If you add early return(s), be sure to balance this call to
   // appShell->SuspendNative() with additional call(s) to
@@ -2541,18 +2537,17 @@ nsObjectLoadingContent::DoStopPlugin(nsP
     if (DoDelayedStop(aInstanceOwner, this, aDelayedStop)) {
       return;
     }
 
 #if defined(XP_MACOSX)
     aInstanceOwner->HidePluginWindow();
 #endif
 
-    nsRefPtr<nsPluginHost> pluginHost =
-      already_AddRefed<nsPluginHost>(nsPluginHost::GetInst());
+    nsRefPtr<nsPluginHost> pluginHost = nsPluginHost::GetInst();
     NS_ASSERTION(pluginHost, "No plugin host?");
     pluginHost->StopPluginInstance(inst);
   }
   TeardownProtoChain();
   aInstanceOwner->Destroy();
 
   mIsStopping = false;
 }
@@ -2718,18 +2713,17 @@ nsObjectLoadingContent::CancelPlayPrevie
 
 bool
 nsObjectLoadingContent::ShouldPlay(FallbackType &aReason)
 {
   // mActivated is true if we've been activated via PlayPlugin() (e.g. user has
   // clicked through). Otherwise, only play if click-to-play is off or if page
   // is whitelisted
 
-  nsRefPtr<nsPluginHost> pluginHost =
-    already_AddRefed<nsPluginHost>(nsPluginHost::GetInst());
+  nsRefPtr<nsPluginHost> pluginHost = nsPluginHost::GetInst();
 
   nsCOMPtr<nsIPluginPlayPreviewInfo> playPreviewInfo;
   bool isPlayPreviewSpecified = NS_SUCCEEDED(pluginHost->GetPlayPreviewInfo(
     mContentType, getter_AddRefs(playPreviewInfo)));
   bool ignoreCTP = false;
   if (isPlayPreviewSpecified) {
     playPreviewInfo->GetIgnoreCTP(&ignoreCTP);
   }
--- a/dom/plugins/base/nsNPAPIPlugin.cpp
+++ b/dom/plugins/base/nsNPAPIPlugin.cpp
@@ -244,17 +244,17 @@ nsNPAPIPlugin::~nsNPAPIPlugin()
   delete mLibrary;
   mLibrary = nullptr;
 }
 
 void
 nsNPAPIPlugin::PluginCrashed(const nsAString& pluginDumpID,
                              const nsAString& browserDumpID)
 {
-  nsRefPtr<nsPluginHost> host = dont_AddRef(nsPluginHost::GetInst());
+  nsRefPtr<nsPluginHost> host = nsPluginHost::GetInst();
   host->PluginCrashed(this, pluginDumpID, browserDumpID);
 }
 
 bool
 nsNPAPIPlugin::RunPluginOOP(const nsPluginTag *aPluginTag)
 {
   if (PR_GetEnv("MOZ_DISABLE_OOP_PLUGINS")) {
     return false;
@@ -883,17 +883,17 @@ NPError NP_CALLBACK
   if (!target && relativeURL &&
       (strncmp(relativeURL, "http:", 5) != 0) &&
       (strncmp(relativeURL, "https:", 6) != 0) &&
       (strncmp(relativeURL, "ftp:", 4) != 0)) {
     nsNPAPIPluginInstance *inst = (nsNPAPIPluginInstance *) npp->ndata;
 
     
     const char *name = nullptr;
-    nsRefPtr<nsPluginHost> host = dont_AddRef(nsPluginHost::GetInst());
+    nsRefPtr<nsPluginHost> host = nsPluginHost::GetInst();
     host->GetPluginName(inst, &name);
 
     if (name && strstr(name, "Adobe") && strstr(name, "Acrobat")) {
       return NPERR_NO_ERROR;
     }
   }
 
   return MakeNewNPAPIStreamInternal(npp, relativeURL, target,
@@ -1618,17 +1618,17 @@ bool NP_CALLBACK
   // Also don't pass back a value that Java is likely to mishandle.
 
   nsNPAPIPluginInstance* inst = (nsNPAPIPluginInstance*) npp->ndata;
   if (!inst)
     return false;
   nsNPAPIPlugin* plugin = inst->GetPlugin();
   if (!plugin)
     return false;
-  nsRefPtr<nsPluginHost> host = dont_AddRef(nsPluginHost::GetInst());
+  nsRefPtr<nsPluginHost> host = nsPluginHost::GetInst();
   nsPluginTag* pluginTag = host->TagForPlugin(plugin);
   if (!pluginTag->mIsJavaPlugin)
     return true;
 
   if (!NPVARIANT_IS_STRING(*result))
     return true;
 
   NPUTF8* propertyName = _utf8fromidentifier(property);
--- a/dom/plugins/base/nsNPAPIPluginInstance.cpp
+++ b/dom/plugins/base/nsNPAPIPluginInstance.cpp
@@ -1781,18 +1781,17 @@ nsNPAPIPluginInstance::CheckJavaC2PJSObj
 
   nsPluginTagType tagtype;
   nsresult rv = GetTagType(&tagtype);
   if (NS_FAILED(rv) ||
       (tagtype != nsPluginTagType_Applet)) {
     return;
   }
 
-  nsRefPtr<nsPluginHost> pluginHost =
-    already_AddRefed<nsPluginHost>(nsPluginHost::GetInst());
+  nsRefPtr<nsPluginHost> pluginHost = nsPluginHost::GetInst();
   if (!pluginHost) {
     return;
   }
 
   bool isClickToPlay;
   nsAutoCString mimeType(mMIMEType);
   rv = pluginHost->IsPluginClickToPlayForType(mimeType, &isClickToPlay);
   if (NS_FAILED(rv) || !isClickToPlay) {
--- a/dom/plugins/base/nsPluginHost.cpp
+++ b/dom/plugins/base/nsPluginHost.cpp
@@ -366,17 +366,17 @@ nsPluginHost::~nsPluginHost()
 }
 
 NS_IMPL_ISUPPORTS4(nsPluginHost,
                    nsIPluginHost,
                    nsIObserver,
                    nsITimerCallback,
                    nsISupportsWeakReference)
 
-nsPluginHost*
+already_AddRefed<nsPluginHost>
 nsPluginHost::GetInst()
 {
   if (!sInst) {
     sInst = new nsPluginHost();
     if (!sInst)
       return nullptr;
     NS_ADDREF(sInst);
   }
--- a/dom/plugins/base/nsPluginHost.h
+++ b/dom/plugins/base/nsPluginHost.h
@@ -58,17 +58,17 @@ class nsPluginHost : public nsIPluginHos
                      public nsIObserver,
                      public nsITimerCallback,
                      public nsSupportsWeakReference
 {
 public:
   nsPluginHost();
   virtual ~nsPluginHost();
 
-  static nsPluginHost* GetInst();
+  static already_AddRefed<nsPluginHost> GetInst();
 
   NS_DECL_AND_IMPL_ZEROING_OPERATOR_NEW
 
   NS_DECL_ISUPPORTS
   NS_DECL_NSIPLUGINHOST
   NS_DECL_NSIOBSERVER
   NS_DECL_NSITIMERCALLBACK
 
--- a/dom/plugins/base/nsPluginStreamListenerPeer.cpp
+++ b/dom/plugins/base/nsPluginStreamListenerPeer.cpp
@@ -363,17 +363,17 @@ nsresult nsPluginStreamListenerPeer::Ini
 //
 // TODO? What if we fill up the the dest dir?
 nsresult
 nsPluginStreamListenerPeer::SetupPluginCacheFile(nsIChannel* channel)
 {
   nsresult rv = NS_OK;
   
   bool useExistingCacheFile = false;
-  nsRefPtr<nsPluginHost> pluginHost = dont_AddRef(nsPluginHost::GetInst());
+  nsRefPtr<nsPluginHost> pluginHost = nsPluginHost::GetInst();
 
   // Look for an existing cache file for the URI.
   nsTArray< nsRefPtr<nsNPAPIPluginInstance> > *instances = pluginHost->InstanceArray();
   for (uint32_t i = 0; i < instances->Length(); i++) {
     // most recent streams are at the end of list
     nsTArray<nsPluginStreamListenerPeer*> *streamListeners = instances->ElementAt(i)->FileCachedStreamListeners();
     for (int32_t i = streamListeners->Length() - 1; i >= 0; --i) {
       nsPluginStreamListenerPeer *lp = streamListeners->ElementAt(i);
--- a/dom/plugins/ipc/PluginModuleParent.cpp
+++ b/dom/plugins/ipc/PluginModuleParent.cpp
@@ -189,17 +189,17 @@ PluginModuleParent::WriteExtraDataForMin
         filePos = 0;
     else
         filePos++;
     notes.Put(NS_LITERAL_CSTRING("PluginFilename"), CS(pluginFile.substr(filePos).c_str()));
 
     nsCString pluginName;
     nsCString pluginVersion;
 
-    nsRefPtr<nsPluginHost> ph = already_AddRefed<nsPluginHost>(nsPluginHost::GetInst());
+    nsRefPtr<nsPluginHost> ph = nsPluginHost::GetInst();
     if (ph) {
         nsPluginTag* tag = ph->TagForPlugin(mPlugin);
         if (tag) {
             pluginName = tag->mName;
             pluginVersion = tag->mVersion;
         }
     }
         
@@ -531,18 +531,17 @@ PluginModuleParent::EvaluateHangUIState(
     }
     mIsTimerReset = false;
     SetChildTimeout(autoStopSecs);
 }
 
 bool
 PluginModuleParent::GetPluginName(nsAString& aPluginName)
 {
-    nsRefPtr<nsPluginHost> host = 
-        dont_AddRef<nsPluginHost>(nsPluginHost::GetInst());
+    nsRefPtr<nsPluginHost> host = nsPluginHost::GetInst();
     if (!host) {
         return false;
     }
     nsPluginTag* pluginTag = host->TagForPlugin(mPlugin);
     if (!pluginTag) {
         return false;
     }
     CopyUTF8toUTF16(pluginTag->mName, aPluginName);