Fix leaky cycle between nsNPAPIPluginInstance and nsPluginInstancePeer. b=472439 r=josh sr=jst
authorBen Turner <bent.mozilla@gmail.com>
Sat, 10 Jan 2009 12:13:31 -0500
changeset 23529 39a8e0ea892863fa8f362a2b8a197c3e1180ee3e
parent 23528 1203433cd9a7e2694420d1020cd9c82ce0a8db69
child 23530 9d68078d593b99545247194aed7d59adc55f969f
push id4580
push userjosh@mozilla.com
push dateSat, 10 Jan 2009 17:11:56 +0000
treeherdermozilla-central@39a8e0ea8928 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjosh, jst
bugs472439
milestone1.9.2a1pre
Fix leaky cycle between nsNPAPIPluginInstance and nsPluginInstancePeer. b=472439 r=josh sr=jst
modules/plugin/base/public/nsPluginNativeWindow.h
modules/plugin/base/src/nsNPAPIPluginInstance.cpp
modules/plugin/base/src/nsPluginInstancePeer.cpp
modules/plugin/base/src/nsPluginInstancePeer.h
--- a/modules/plugin/base/public/nsPluginNativeWindow.h
+++ b/modules/plugin/base/public/nsPluginNativeWindow.h
@@ -41,25 +41,31 @@
 #ifndef _nsPluginNativeWindow_h_
 #define _nsPluginNativeWindow_h_
 
 #include "nscore.h"
 #include "nsCOMPtr.h"
 #include "nsIPluginInstance.h"
 #include "nsplugindefs.h"
 #include "nsIWidget.h"
+#include "nsTraceRefcnt.h"
 
 /**
  * base class for native plugin window implementations
  */
 class nsPluginNativeWindow : public nsPluginWindow
 {
 public: 
-  nsPluginNativeWindow() : nsPluginWindow() {}
-  virtual ~nsPluginNativeWindow() {}
+  nsPluginNativeWindow() : nsPluginWindow() {
+    MOZ_COUNT_CTOR(nsPluginNativeWindow);
+  }
+
+  virtual ~nsPluginNativeWindow() {
+    MOZ_COUNT_DTOR(nsPluginNativeWindow);
+  }
 
   /**
    *   !!! CAUTION !!!
    *
    * The base class |nsPluginWindow| is defined as a struct in nsplugindefs.h,
    * thus it does not have a destructor of its own.
    * One should never attempt to delete |nsPluginNativeWindow| object instance
    * (or derivatives) using a pointer of |nsPluginWindow *| type. Should such
--- a/modules/plugin/base/src/nsNPAPIPluginInstance.cpp
+++ b/modules/plugin/base/src/nsNPAPIPluginInstance.cpp
@@ -835,35 +835,41 @@ NS_IMETHODIMP nsNPAPIPluginInstance::Sto
   if (mPopupStates.Count() > 0) {
     nsCOMPtr<nsPIDOMWindow> window = GetDOMWindow();
 
     if (window) {
       window->PopPopupControlState(openAbused);
     }
   }
 
-  if (!mStarted)
+  if (!mStarted) {
+    // Break our cycle with the peer that owns us.
+    mPeer = nsnull;
     return NS_OK;
+  }
 
   // If there's code from this plugin instance on the stack, delay the
   // destroy.
   if (PluginDestructionGuard::DelayDestroy(this)) {
     return NS_OK;
   }
 
   // Make sure we lock while we're writing to mStarted after we've
   // started as other threads might be checking that inside a lock.
   EnterAsyncPluginThreadCallLock();
   mStarted = PR_FALSE;
   ExitAsyncPluginThreadCallLock();
 
   OnPluginDestroy(&fNPP);
 
-  if (fCallbacks->destroy == NULL)
+  if (fCallbacks->destroy == NULL) {
+    // Break our cycle with the peer that owns us.
+    mPeer = nsnull;
     return NS_ERROR_FAILURE;
+  }
 
   NPSavedData *sdata = 0;
 
   // clean up open streams
   for (nsInstanceStream *is = mStreams; is != nsnull;) {
     nsNPAPIPluginStreamListener * listener = is->mPluginStreamListener;
 
     nsInstanceStream *next = is->mNext;
@@ -879,16 +885,19 @@ NS_IMETHODIMP nsNPAPIPluginInstance::Sto
 
   NS_TRY_SAFE_CALL_RETURN(error, (*fCallbacks->destroy)(&fNPP, &sdata), fLibrary, this);
 
   NPP_PLUGIN_LOG(PLUGIN_LOG_NORMAL,
   ("NPP Destroy called: this=%p, npp=%p, return=%d\n", this, &fNPP, error));
 
   nsJSNPRuntime::OnPluginDestroy(&fNPP);
 
+  // Break our cycle with the peer that owns us.
+  mPeer = nsnull;
+
   if (error != NPERR_NO_ERROR)
     return NS_ERROR_FAILURE;
   else
     return NS_OK;
 }
 
 already_AddRefed<nsPIDOMWindow>
 nsNPAPIPluginInstance::GetDOMWindow()
--- a/modules/plugin/base/src/nsPluginInstancePeer.cpp
+++ b/modules/plugin/base/src/nsPluginInstancePeer.cpp
@@ -62,26 +62,21 @@
 
 #ifdef XP_WIN
 #include <windows.h>
 #include <winbase.h>
 #endif
 
 nsPluginInstancePeerImpl::nsPluginInstancePeerImpl()
 {
-  mInstance = nsnull;
-  mOwner = nsnull;
   mMIMEType = nsnull;
 }
 
 nsPluginInstancePeerImpl::~nsPluginInstancePeerImpl()
 {
-  mInstance = nsnull;
-  NS_IF_RELEASE(mOwner);
-
   if (nsnull != mMIMEType) {
     PR_Free((void *)mMIMEType);
     mMIMEType = nsnull;
   }
 }
 
 static NS_DEFINE_IID(kIPluginTagInfoIID, NS_IPLUGINTAGINFO_IID); 
 static NS_DEFINE_IID(kIPluginTagInfo2IID, NS_IPLUGINTAGINFO2_IID); 
@@ -824,20 +819,16 @@ nsPluginInstancePeerImpl::GetJSContext(J
   return rv;
 }
 
 nsresult
 nsPluginInstancePeerImpl::Initialize(nsIPluginInstanceOwner *aOwner,
                                      const nsMIMEType aMIMEType)
 {
   mOwner = aOwner;
-  NS_IF_ADDREF(mOwner);
-
-  aOwner->GetInstance(mInstance);
-  NS_IF_RELEASE(mInstance);
 
   if (nsnull != aMIMEType) {
     mMIMEType = (nsMIMEType)PR_Malloc(PL_strlen(aMIMEType) + 1);
 
     if (nsnull != mMIMEType)
       PL_strcpy((char *)mMIMEType, aMIMEType);
   }
   
@@ -845,24 +836,17 @@ nsPluginInstancePeerImpl::Initialize(nsI
   mThreadID = NS_PTR_TO_INT32(PR_GetCurrentThread());
 
   return NS_OK;
 }
 
 nsresult
 nsPluginInstancePeerImpl::SetOwner(nsIPluginInstanceOwner *aOwner)
 {
-  // get rid of the previous owner
-  NS_IF_RELEASE(mOwner);
-
   mOwner = aOwner;
-  NS_IF_ADDREF(mOwner);
-
-  aOwner->GetInstance(mInstance);
-  NS_IF_RELEASE(mInstance);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsPluginInstancePeerImpl::GetOwner(nsIPluginInstanceOwner **aOwner)
 {
   NS_ENSURE_ARG_POINTER(aOwner);
   *aOwner = mOwner;
--- a/modules/plugin/base/src/nsPluginInstancePeer.h
+++ b/modules/plugin/base/src/nsPluginInstancePeer.h
@@ -42,16 +42,18 @@
 #include "nsIWindowlessPlugInstPeer.h"
 #include "nsIPluginTagInfo2.h"
 #include "nsIPluginInstanceOwner.h"
 #ifdef OJI
 #include "nsIJVMPluginTagInfo.h"
 #endif
 #include "nsPIPluginInstancePeer.h"
 
+#include "nsCOMPtr.h"
+
 class nsPluginInstancePeerImpl : public nsIPluginInstancePeer2,
                                  public nsIWindowlessPluginInstancePeer,
                                  public nsIPluginTagInfo2,
 #ifdef OJI
                                  public nsIJVMPluginTagInfo,
 #endif
                                  public nsPIPluginInstancePeer
 								
@@ -90,16 +92,15 @@ public:
   //locals
 
   nsresult Initialize(nsIPluginInstanceOwner *aOwner,
                       const nsMIMEType aMimeType);
 
   nsresult SetOwner(nsIPluginInstanceOwner *aOwner);
 
 private:
-  nsIPluginInstance       *mInstance; //we don't add a ref to this
-  nsIPluginInstanceOwner  *mOwner;    //we don't add a ref to this
+  nsCOMPtr<nsIPluginInstanceOwner> mOwner;
   nsMIMEType              mMIMEType;
   PRUint32                mThreadID;
   PRBool                  mStopped;
 };
 
 #endif