Bug 675221: Notify observers of PSM's background threads on the main thread, r=kaie

security/manager/ssl/src/PSMRunnable.cpp

@@ -65,4 +65,11 @@ SyncRunnableBase::Run()
   return NS_OK;
 }
 
+nsresult
+NotifyObserverRunnable::Run()
+{
+  mObserver->Observe(nsnull, mTopic, nsnull);
+  return NS_OK;
+}
+
 } } // namespace mozilla::psm

security/manager/ssl/src/PSMRunnable.h

@@ -39,6 +39,7 @@
 
 #include "mozilla/Monitor.h"
 #include "nsThreadUtils.h"
+#include "nsIObserver.h"
 
 namespace mozilla { namespace psm {
 
@@ -57,6 +58,18 @@ private:
   mozilla::Monitor monitor;
 };
 
+class NotifyObserverRunnable : public nsRunnable
+{
+public:
+  NotifyObserverRunnable(nsIObserver * observer,
+                         const char * topicStringLiteral)
+    : mObserver(), mTopic(topicStringLiteral) { mObserver = observer; }
+  NS_DECL_NSIRUNNABLE
+private:
+  nsCOMPtr<nsIObserver> mObserver;
+  const char * const mTopic;
+};
+
 } } // namespace mozilla::psm
 
 #endif

security/manager/ssl/src/nsKeygenThread.cpp

@@ -38,13 +38,14 @@
 
 #include "pk11func.h"
 #include "nsCOMPtr.h"
-#include "nsProxiedService.h"
 #include "nsThreadUtils.h"
 #include "nsKeygenThread.h"
 #include "nsIObserver.h"
 #include "nsNSSShutDown.h"
+#include "PSMRunnable.h"
 
 using namespace mozilla;
+using namespace mozilla::psm;
 
 NS_IMPL_THREADSAFE_ISUPPORTS1(nsKeygenThread, nsIKeygenThread)
 
@@ -138,23 +139,23 @@ static void PR_CALLBACK nsKeygenThreadRunner(void *arg)
 
 nsresult nsKeygenThread::StartKeyGeneration(nsIObserver* aObserver)
 {
+  if (!NS_IsMainThread()) {
+    NS_ERROR("nsKeygenThread::StartKeyGeneration called off the main thread");
+    return NS_ERROR_NOT_SAME_THREAD;
+  }
+  
   if (!aObserver)
     return NS_OK;
 
-  nsCOMPtr<nsIObserver> obs;
-  NS_GetProxyForObject( NS_PROXY_TO_MAIN_THREAD,
-                        NS_GET_IID(nsIObserver),
-                        aObserver,
-                        NS_PROXY_SYNC | NS_PROXY_ALWAYS,
-                        getter_AddRefs(obs));
-
   MutexAutoLock lock(mutex);
 
     if (iAmRunning || keygenReady) {
       return NS_OK;
     }
 
-    observer.swap(obs);
+    // We must AddRef aObserver only here on the main thread, because it
+    // probably does not implement a thread-safe AddRef.
+    mNotifyObserver = new NotifyObserverRunnable(aObserver, "keygen-finished");
 
     iAmRunning = true;
 
@@ -213,7 +214,7 @@ void nsKeygenThread::Run(void)
   // As long as key generation can't be canceled, we don't need 
   // to care for cleaning this up.
 
-  nsCOMPtr<nsIObserver> obs;
+  nsCOMPtr<nsIRunnable> notifyObserver;
   {
     MutexAutoLock lock(mutex);
 
@@ -229,14 +230,17 @@ void nsKeygenThread::Run(void)
     params = 0;
     wincx = 0;
 
-    if (!statusDialogClosed)
-      obs = observer;
+    if (!statusDialogClosed && mNotifyObserver)
+      notifyObserver = mNotifyObserver;
 
-    observer = nsnull;
+    mNotifyObserver = nsnull;
   }
 
-  if (obs)
-    obs->Observe(nsnull, "keygen-finished", nsnull);
+  if (notifyObserver) {
+    nsresult rv = NS_DispatchToMainThread(notifyObserver);
+    NS_ASSERTION(NS_SUCCEEDED(rv),
+		 "failed to dispatch keygen thread observer to main thread");
+  }
 }
 
 void nsKeygenThread::Join()

security/manager/ssl/src/nsKeygenThread.h

@@ -46,14 +46,14 @@
 #include "nsIKeygenThread.h"
 #include "nsCOMPtr.h"
 
-class nsIObserver;
+class nsIRunnable;
 
 class nsKeygenThread : public nsIKeygenThread
 {
 private:
   mozilla::Mutex mutex;
   
-  nsCOMPtr<nsIObserver> observer;
+  nsCOMPtr<nsIRunnable> mNotifyObserver;
 
   bool iAmRunning;
   bool keygenReady;

security/manager/ssl/src/nsProtectedAuthThread.cpp

@@ -37,13 +37,14 @@
 #include "pk11func.h"
 #include "nsCOMPtr.h"
 #include "nsAutoPtr.h"
-#include "nsProxiedService.h"
+#include "PSMRunnable.h"
 #include "nsString.h"
 #include "nsReadableUtils.h"
 #include "nsPKCS11Slot.h"
 #include "nsProtectedAuthThread.h"
 
 using namespace mozilla;
+using namespace mozilla::psm;
 
 NS_IMPL_THREADSAFE_ISUPPORTS1(nsProtectedAuthThread, nsIProtectedAuthThread)
 
@@ -56,7 +57,6 @@ static void PR_CALLBACK nsProtectedAuthThreadRunner(void *arg)
 nsProtectedAuthThread::nsProtectedAuthThread()
 : mMutex("nsProtectedAuthThread.mMutex")
 , mIAmRunning(false)
-, mStatusObserverNotified(false)
 , mLoginReady(false)
 , mThreadHandle(nsnull)
 , mSlot(0)
@@ -77,22 +77,19 @@ NS_IMETHODIMP nsProtectedAuthThread::Login(nsIObserver *aObserver)
         // We need pointer to the slot
         return NS_ERROR_FAILURE;
 
-    nsCOMPtr<nsIObserver> observerProxy;
-    nsresult rv = NS_GetProxyForObject(NS_PROXY_TO_MAIN_THREAD,
-                                       NS_GET_IID(nsIObserver),
-                                       aObserver,
-                                       NS_PROXY_SYNC | NS_PROXY_ALWAYS,
-                                       getter_AddRefs(observerProxy));
-    if (NS_FAILED(rv))
-        return rv;
-
     MutexAutoLock lock(mMutex);
     
     if (mIAmRunning || mLoginReady) {
         return NS_OK;
     }
 
-    observerProxy.swap(mStatusObserver);
+    if (aObserver) {
+      // We must AddRef aObserver here on the main thread, because it probably
+      // does not implement a thread-safe AddRef.
+      mNotifyObserver = new NotifyObserverRunnable(aObserver,
+                                                   "operation-completed");
+    }
+
     mIAmRunning = true;
     
     mThreadHandle = PR_CreateThread(PR_USER_THREAD, nsProtectedAuthThreadRunner, static_cast<void*>(this), 
@@ -146,7 +143,7 @@ void nsProtectedAuthThread::Run(void)
     // it is harmless here
     mLoginResult = PK11_CheckUserPassword(mSlot, 0);
 
-    nsCOMPtr<nsIObserver> observer;
+    nsCOMPtr<nsIRunnable> notifyObserver;
     {
         MutexAutoLock lock(mMutex);
 
@@ -160,17 +157,14 @@ void nsProtectedAuthThread::Run(void)
             mSlot = 0;
         }
 
-        if (!mStatusObserverNotified)
-        {
-            observer = mStatusObserver;
-        }
-
-        mStatusObserver = nsnull;
-        mStatusObserverNotified = true;
+        notifyObserver.swap(mNotifyObserver);
     }
     
-    if (observer)
-        observer->Observe(nsnull, "operation-completed", nsnull);
+    if (notifyObserver) {
+        nsresult rv = NS_DispatchToMainThread(notifyObserver);
+	NS_ASSERTION(NS_SUCCEEDED(rv),
+		     "failed to dispatch protected auth observer to main thread");
+    }
 }
 
 void nsProtectedAuthThread::Join()

security/manager/ssl/src/nsProtectedAuthThread.h

@@ -44,15 +44,16 @@
 #include "mozilla/Mutex.h"
 #include "nsIProtectedAuthThread.h"
 
+class nsIRunnable;
+
 class nsProtectedAuthThread : public nsIProtectedAuthThread
 {
 private:
     mozilla::Mutex mMutex;
 
-    nsCOMPtr<nsIObserver> mStatusObserver;
+    nsCOMPtr<nsIRunnable> mNotifyObserver;
 
     bool        mIAmRunning;
-    bool        mStatusObserverNotified;
     bool        mLoginReady;
 
     PRThread    *mThreadHandle;