Bug 460941 DIR_Shutdown not always called on shutdown. Patch r=Neil,sr=bienvenu; unit test fix r=Neil
authorMark Banner <bugzilla@standard8.plus.com>
Mon, 22 Dec 2008 13:02:51 +0000
changeset 1481 38e8cf481a5ba7d0df81823db3c698b05a9d0363
parent 1480 de52d877915d99cda142504258d62113460b0f87
child 1482 cf1802da787c12416b7a8c5ba23fb7f2f172aad6
push idunknown
push userunknown
push dateunknown
reviewersNeil, bienvenu, unit, Neil
bugs460941
Bug 460941 DIR_Shutdown not always called on shutdown. Patch r=Neil,sr=bienvenu; unit test fix r=Neil
mailnews/addrbook/build/nsAbFactory.cpp
mailnews/addrbook/src/nsAbManager.cpp
mailnews/addrbook/src/nsAbManager.h
mailnews/addrbook/src/nsDirectoryDataSource.cpp
mailnews/build/nsMailModule.cpp
mailnews/import/test/unit/test_ldif_import.js
--- a/mailnews/addrbook/build/nsAbFactory.cpp
+++ b/mailnews/addrbook/build/nsAbFactory.cpp
@@ -93,17 +93,17 @@
 #endif
 
 #ifdef XP_MACOSX
 #include "nsAbOSXDirectory.h"
 #include "nsAbOSXCard.h"
 #include "nsAbOSXDirFactory.h"
 #endif
 
-NS_GENERIC_FACTORY_CONSTRUCTOR(nsAbManager)
+NS_GENERIC_FACTORY_CONSTRUCTOR_INIT(nsAbManager,Init)
 NS_GENERIC_FACTORY_CONSTRUCTOR(nsAbContentHandler)
 NS_GENERIC_FACTORY_CONSTRUCTOR_INIT(nsAbDirectoryDataSource,Init)
 NS_GENERIC_FACTORY_CONSTRUCTOR(nsAbDirProperty)
 NS_GENERIC_FACTORY_CONSTRUCTOR(nsAbCardProperty)
 NS_GENERIC_FACTORY_CONSTRUCTOR(nsAbBSDirectory)
 NS_GENERIC_FACTORY_CONSTRUCTOR(nsAbMDBDirectory)
 NS_GENERIC_FACTORY_CONSTRUCTOR(nsAbMDBCard)
 NS_GENERIC_FACTORY_CONSTRUCTOR(nsAddrDatabase)
--- a/mailnews/addrbook/src/nsAbManager.cpp
+++ b/mailnews/addrbook/src/nsAbManager.cpp
@@ -62,16 +62,19 @@
 #include "nsVCard.h"
 #include "nsVCardObj.h"
 #include "nsIAbLDAPAttributeMap.h"
 #include "nsICommandLine.h"
 #include "nsILocalFile.h"
 #include "nsIMutableArray.h"
 #include "nsArrayUtils.h"
 #include "nsDirectoryServiceUtils.h"
+#include "nsIObserverService.h"
+#include "nsDirPrefs.h"
+#include "nsThreadUtils.h"
 
 struct ExportAttributesTableStruct
 {
   const char* abPropertyName;
   PRUint32 plainTextStringID;
 };
 
 // our schema is not fixed yet, but we still want some sort of objectclass
@@ -157,16 +160,69 @@ nsAbManager::~nsAbManager()
 }
 
 NS_IMPL_THREADSAFE_ADDREF(nsAbManager)
 NS_IMPL_THREADSAFE_RELEASE(nsAbManager)
 NS_IMPL_QUERY_INTERFACE2(nsAbManager,
                          nsIAbManager,
                          nsICommandLineHandler)
 
+nsresult nsAbManager::Init()
+{
+  NS_ENSURE_TRUE(NS_IsMainThread(), NS_ERROR_FAILURE);
+
+  nsresult rv;
+  nsCOMPtr<nsIObserverService> observerService =
+    do_GetService("@mozilla.org/observer-service;1", &rv);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = observerService->AddObserver(this, "profile-do-change", PR_FALSE);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = observerService->AddObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID,
+                                    PR_FALSE);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  return NS_OK;
+}
+
+NS_IMETHODIMP nsAbManager::Observe(nsISupports *aSubject, const char *aTopic,
+                                   const PRUnichar *someData)
+{
+  // The nsDirPrefs code caches all the directories that it got
+  // from the first profiles prefs.js.
+  // When we profile switch, we need to force it to shut down.
+  // We'll re-load all the directories from the second profiles prefs.js
+  // that happens in nsAbBSDirectory::GetChildNodes()
+  // when we call DIR_GetDirectories().
+  if (!strcmp(aTopic, "profile-do-change"))
+  {
+    DIR_ShutDown();
+    return NS_OK;
+  }
+
+  if (!strcmp(aTopic, NS_XPCOM_SHUTDOWN_OBSERVER_ID))
+  {
+    DIR_ShutDown();
+
+    nsresult rv;
+    nsCOMPtr<nsIObserverService> observerService =
+      do_GetService("@mozilla.org/observer-service;1", &rv);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    rv = observerService->RemoveObserver(this, "profile-do-change");
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    rv = observerService->RemoveObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID);
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+
+  return NS_OK;
+}
+
 //
 // nsIAbManager
 //
 
 NS_IMETHODIMP nsAbManager::GetDirectories(nsISimpleEnumerator **aResult)
 {
   NS_ENSURE_ARG_POINTER(aResult);
 
--- a/mailnews/addrbook/src/nsAbManager.h
+++ b/mailnews/addrbook/src/nsAbManager.h
@@ -37,32 +37,37 @@
 
 #ifndef __nsAbManager_h
 #define __nsAbManager_h
  
 #include "nsIAbManager.h"
 #include "nsTObserverArray.h"
 #include "nsCOMPtr.h"
 #include "nsICommandLineHandler.h"
+#include "nsIObserver.h"
 
 class nsIAbDirectory;
 class nsIAbLDAPAttributeMap;
 
 class nsAbManager : public nsIAbManager,
-                    public nsICommandLineHandler
+                    public nsICommandLineHandler,
+                    public nsIObserver
 {
   
 public:
 	nsAbManager();
 	virtual ~nsAbManager();
 
 	NS_DECL_ISUPPORTS
  	NS_DECL_NSIABMANAGER
+  NS_DECL_NSIOBSERVER
   NS_DECL_NSICOMMANDLINEHANDLER
 
+  nsresult Init();
+
 private:
   nsresult ExportDirectoryToDelimitedText(nsIAbDirectory *aDirectory, const char *aDelim, PRUint32 aDelimLen, nsILocalFile *aLocalFile);
   nsresult ExportDirectoryToLDIF(nsIAbDirectory *aDirectory, nsILocalFile *aLocalFile);
   nsresult AppendLDIFForMailList(nsIAbCard *aCard, nsIAbLDAPAttributeMap *aAttrMap, nsACString &aResult);
   nsresult AppendDNForCard(const char *aProperty, nsIAbCard *aCard, nsIAbLDAPAttributeMap *aAttrMap, nsACString &aResult);
   nsresult AppendBasicLDIFForCard(nsIAbCard *aCard, nsIAbLDAPAttributeMap *aAttrMap, nsACString &aResult);
   nsresult AppendProperty(const char *aProperty, const PRUnichar *aValue, nsACString &aResult);
   PRBool IsSafeLDIFString(const PRUnichar *aStr);
--- a/mailnews/addrbook/src/nsDirectoryDataSource.cpp
+++ b/mailnews/addrbook/src/nsDirectoryDataSource.cpp
@@ -96,31 +96,19 @@ nsresult nsAbDirectoryDataSource::Cleanu
   NS_ENSURE_SUCCESS(rv,rv);
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsAbDirectoryDataSource::Observe(nsISupports *aSubject, const char *aTopic, const PRUnichar *someData)
 {
-  if (!strcmp(aTopic,"profile-do-change")) {
-    /* the nsDirPrefs code caches all the directories that it got
-     * from the first profiles prefs.js
-     * When we profile switch, we need to force it to shut down.
-     * we'll re-load all the directories from the second profiles prefs.js
-     * that happens in nsAbBSDirectory::GetChildNodes()
-     * when we call DIR_GetDirectories()
-     */
-    DIR_ShutDown();
-    return NS_OK;
-  }
-  else if (!strcmp(aTopic,NS_XPCOM_SHUTDOWN_OBSERVER_ID)) {
-    DIR_ShutDown();
+  if (!strcmp(aTopic,NS_XPCOM_SHUTDOWN_OBSERVER_ID))
     return Cleanup();
-  }
+
   return NS_OK;
 }
 
 nsresult
 nsAbDirectoryDataSource::Init()
 {
   nsresult rv;
   nsCOMPtr<nsIAbManager> abManager = do_GetService(NS_ABMANAGER_CONTRACTID, &rv);
@@ -168,18 +156,16 @@ nsAbDirectoryDataSource::Init()
   NS_ENSURE_SUCCESS(rv,rv);
 
   nsCOMPtr<nsIObserverService> observerService = do_GetService("@mozilla.org/observer-service;1", &rv);
   NS_ENSURE_SUCCESS(rv,rv);
 
   // since the observer (this) supports weak ref,
   // and we call AddObserver() with PR_TRUE for ownsWeak
   // we don't need to remove our observer from the from the observer service
-  rv = observerService->AddObserver(this, "profile-do-change", PR_TRUE);
-  NS_ENSURE_SUCCESS(rv,rv);
   rv = observerService->AddObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID, PR_TRUE);
   NS_ENSURE_SUCCESS(rv,rv);
 
   return NS_OK;
 }
 
 NS_IMPL_ISUPPORTS_INHERITED3(nsAbDirectoryDataSource, nsAbRDFDataSource, nsIAbListener, nsIObserver, nsISupportsWeakReference)
 
--- a/mailnews/build/nsMailModule.cpp
+++ b/mailnews/build/nsMailModule.cpp
@@ -350,17 +350,17 @@ NS_GENERIC_FACTORY_CONSTRUCTOR(nsMesseng
 NS_GENERIC_FACTORY_CONSTRUCTOR_INIT(nsMsgContentPolicy, Init)
 #ifdef MOZ_THUNDERBIRD
 NS_GENERIC_FACTORY_CONSTRUCTOR(nsMsgCookiePolicy)
 #endif
 
 ////////////////////////////////////////////////////////////////////////////////
 // addrbook factories
 ////////////////////////////////////////////////////////////////////////////////
-NS_GENERIC_FACTORY_CONSTRUCTOR(nsAbManager)
+NS_GENERIC_FACTORY_CONSTRUCTOR_INIT(nsAbManager,Init)
 NS_GENERIC_FACTORY_CONSTRUCTOR(nsAbContentHandler)
 NS_GENERIC_FACTORY_CONSTRUCTOR_INIT(nsAbDirectoryDataSource,Init)
 NS_GENERIC_FACTORY_CONSTRUCTOR(nsAbDirProperty)
 NS_GENERIC_FACTORY_CONSTRUCTOR(nsAbCardProperty)
 NS_GENERIC_FACTORY_CONSTRUCTOR(nsAbBSDirectory)
 NS_GENERIC_FACTORY_CONSTRUCTOR(nsAbMDBDirectory)
 NS_GENERIC_FACTORY_CONSTRUCTOR(nsAbMDBCard)
 NS_GENERIC_FACTORY_CONSTRUCTOR(nsAddrDatabase)
--- a/mailnews/import/test/unit/test_ldif_import.js
+++ b/mailnews/import/test/unit/test_ldif_import.js
@@ -5,12 +5,16 @@
  *
  * This test also checks for the following bugs:
  *   -Bug 439819: LDIF import does not include mozillahomestreet.
  *   -Bug 182128: Edit Card, Notes on several lines appear on one after
  *                export/import in text format *(only tests the import).
  */
 function run_test()
 {
+  // Due to the import code using nsIAbManager off the main thread, we need
+  // to ensure that it is initialized before we start the main test.
+  var abMgr = Cc["@mozilla.org/abmanager;1"].getService(Ci.nsIAbManager);
+
   var file = do_get_file("../mailnews/import/test/resources/basic_ldif_addressbook.ldif");
   new AbImportHelper(file, "LDIF", "basic_ldif_addressbook",
                      "basic_addressbook").beginImport();
 }