Fixing bug 474866. Make the plugin instance peer have a weak reference to its owner, and make the owner tell the peer once it's no longer its owner so we don't have reference cycles, yet the instance can hold on to its peer while being cached between pages. r+sr=mrbkap@gmail.com

layout/generic/nsObjectFrame.cpp

@@ -73,6 +73,7 @@
 #include "nsIURL.h"
 #include "nsNetUtil.h"
 #include "nsIPluginInstanceOwner.h"
+#include "nsIPluginInstancePeer2.h"
 #include "plstr.h"
 #include "nsILinkHandler.h"
 #ifdef OJI
@@ -2277,6 +2278,18 @@ nsPluginInstanceOwner::~nsPluginInstanceOwner()
     pph->DeletePluginNativeWindow(mPluginWindow);
     mPluginWindow = nsnull;
   }
+
+  if (mInstance) {
+    nsCOMPtr<nsIPluginInstancePeer> peer;
+    mInstance->GetPeer(getter_AddRefs(peer));
+
+    nsCOMPtr<nsIPluginInstancePeer2> peer2(do_QueryInterface(peer));
+
+    if (peer2) {
+      // Tell the peer that its owner is going away.
+      peer2->InvalidateOwner();
+    }
+  }
 }
 
 /*
@@ -2306,6 +2319,20 @@ NS_INTERFACE_MAP_END
 
 NS_IMETHODIMP nsPluginInstanceOwner::SetInstance(nsIPluginInstance *aInstance)
 {
+  // XXX: We should probably never already have an instance when we
+  // get here, but in case we do... At some point we should remove
+  // this code and ensure elsewhere that it's not needed.
+  if (mInstance && mInstance != aInstance) {
+    nsCOMPtr<nsIPluginInstancePeer> peer;
+    mInstance->GetPeer(getter_AddRefs(peer));
+
+    nsCOMPtr<nsIPluginInstancePeer2> peer2(do_QueryInterface(peer));
+
+    if (peer2) {
+      peer2->InvalidateOwner();
+    }
+  }
+
   mInstance = aInstance;
 
   return NS_OK;

modules/plugin/base/public/nsIPluginInstancePeer2.idl

@@ -61,30 +61,36 @@ struct JSContext;
  * All functionality in nsIPluginInstancePeer can be mapped to the 4.X
  * plugin API.
  */
-[uuid(e7d48c00-e1f1-11d2-8360-fbc8abc4ae7c)]
+[uuid(79a2d210-55e4-4687-bd87-64b10c2466cc)]
 interface nsIPluginInstancePeer2 : nsIPluginInstancePeer
 {
     /**
      * Get the JavaScript window object corresponding to this plugin instance.
      *
-   * @param aJSWindow - the resulting JavaScript window object
-   * @result          - NS_OK if this operation was successful
+     * @param aJSWindow - the resulting JavaScript window object
+     * @result          - NS_OK if this operation was successful
+     */
+    readonly attribute JSObjectPtr JSWindow;
+
+    /**
+     * Get the JavaScript execution thread corresponding to this
+     * plugin instance.
+     *
+     * @param aJSThread - the resulting JavaScript thread id
+     * @result            - NS_OK if this operation was successful
      */
-  readonly attribute JSObjectPtr JSWindow;
-	
-	/**
-	 * Get the JavaScript execution thread corresponding to this plugin instance.
-	 *
-   * @param aJSThread - the resulting JavaScript thread id
-   * @result            - NS_OK if this operation was successful
-	 */
     readonly attribute unsigned long JSThread;
 
-	/**
-	 * Get the JavaScript context to this plugin instance.
-	 *
-   * @param aJSContext - the resulting JavaScript context
-   * @result           - NS_OK if this operation was successful
-	 */
-  readonly attribute JSContextPtr JSContext;
+    /**
+     * Get the JavaScript context to this plugin instance.
+     *
+     * @param aJSContext - the resulting JavaScript context
+     * @result           - NS_OK if this operation was successful
+     */
+    readonly attribute JSContextPtr JSContext;
+
+    /**
+     * Drop our reference to our owner.
+     */
+    void invalidateOwner();
 };

modules/plugin/base/src/nsNPAPIPluginInstance.cpp

@@ -841,8 +841,6 @@ NS_IMETHODIMP nsNPAPIPluginInstance::Stop(void)
   }
 
   if (!mStarted) {
-    // Break our cycle with the peer that owns us.
-    mPeer = nsnull;
     return NS_OK;
   }
 
@@ -861,8 +859,6 @@ NS_IMETHODIMP nsNPAPIPluginInstance::Stop(void)
   OnPluginDestroy(&fNPP);
 
   if (fCallbacks->destroy == NULL) {
-    // Break our cycle with the peer that owns us.
-    mPeer = nsnull;
     return NS_ERROR_FAILURE;
   }
 
@@ -890,9 +886,6 @@ NS_IMETHODIMP nsNPAPIPluginInstance::Stop(void)
 
   nsJSNPRuntime::OnPluginDestroy(&fNPP);
 
-  // Break our cycle with the peer that owns us.
-  mPeer = nsnull;
-
   if (error != NPERR_NO_ERROR)
     return NS_ERROR_FAILURE;
   else

modules/plugin/base/src/nsPluginInstancePeer.cpp

@@ -819,6 +819,14 @@ nsPluginInstancePeerImpl::GetJSContext(JSContext* *outContext)
   return rv;
 }
 
+NS_IMETHODIMP
+nsPluginInstancePeerImpl::InvalidateOwner()
+{
+  mOwner = nsnull;
+
+  return NS_OK;
+}
+
 nsresult
 nsPluginInstancePeerImpl::Initialize(nsIPluginInstanceOwner *aOwner,
                                      const nsMIMEType aMIMEType)

modules/plugin/base/src/nsPluginInstancePeer.h

@@ -97,7 +97,9 @@ public:
   nsresult SetOwner(nsIPluginInstanceOwner *aOwner);
 
 private:
-  nsCOMPtr<nsIPluginInstanceOwner> mOwner;
+  // Weak pointer to the owner. The owner nulls this out (by calling
+  // InvalidateOwner()) when it's no longer our owner.
+  nsIPluginInstanceOwner  *mOwner;
   nsMIMEType              mMIMEType;
   PRUint32                mThreadID;
   PRBool                  mStopped;