Bug 384744 - Download manager should enforce a singleton, not just fail on createInstance. r=gavin.sharp

toolkit/components/build/nsToolkitCompsModule.cpp

@@ -76,7 +76,8 @@ NS_GENERIC_FACTORY_CONSTRUCTOR(nsAlertsService)
 #ifndef MOZ_SUITE
 // XXX Suite isn't ready to include this just yet
 #ifdef MOZ_RDF
-NS_GENERIC_FACTORY_CONSTRUCTOR_INIT(nsDownloadManager, Init) 
+NS_GENERIC_FACTORY_SINGLETON_CONSTRUCTOR(nsDownloadManager,
+                                         nsDownloadManager::GetSingleton) 
 NS_GENERIC_FACTORY_CONSTRUCTOR(nsDownloadProxy)
 #endif
 #endif // MOZ_SUITE

toolkit/components/downloads/src/nsDownloadManager.cpp

@@ -88,8 +88,6 @@ static PRBool gStoppingDownloads = PR_FALSE;
 
 static const PRInt64 gUpdateInterval = 400 * PR_USEC_PER_MSEC;
 
-static PRInt32 gRefCnt = 0;
-
 #define DM_SCHEMA_VERSION      1
 #define DM_DB_NAME             NS_LITERAL_STRING("downloads.sqlite")
 #define DM_DB_CORRUPT_FILENAME NS_LITERAL_STRING("downloads.sqlite.corrupt")
@@ -99,21 +97,29 @@ static PRInt32 gRefCnt = 0;
 
 NS_IMPL_ISUPPORTS2(nsDownloadManager, nsIDownloadManager, nsIObserver)
 
+nsDownloadManager *nsDownloadManager::gDownloadManagerService = nsnull;
+
+nsDownloadManager *
+nsDownloadManager::GetSingleton()
+{
+  if (gDownloadManagerService) {
+    NS_ADDREF(gDownloadManagerService);
+    return gDownloadManagerService;
+  }
+
+  gDownloadManagerService = new nsDownloadManager();
+  if (gDownloadManagerService) {
+    NS_ADDREF(gDownloadManagerService);
+    if (NS_FAILED(gDownloadManagerService->Init()))
+      NS_RELEASE(gDownloadManagerService);
+  }
+
+  return gDownloadManagerService;
+}
+
 nsDownloadManager::~nsDownloadManager()
 {
-  if (--gRefCnt != 0)
-    // Either somebody tried to use |CreateInstance| instead of
-    // |GetService| or |Init| failed very early, so there's nothing to
-    // do here.
-    return;
-
-#if 0
-  // Temporary fix for orange regression from bug 328159 until I
-  // understand new protocol following bug 326491.  See bug 315421.
-  mObserverService->RemoveObserver(this, "quit-application");
-  mObserverService->RemoveObserver(this, "quit-application-requested");
-  mObserverService->RemoveObserver(this, "offline-requested");
-#endif
+  gDownloadManagerService = nsnull;
 }
 
 nsresult
@@ -467,11 +473,6 @@ nsDownloadManager::AddDownloadToDB(const nsAString &aName,
 nsresult
 nsDownloadManager::Init()
 {
-  if (gRefCnt++ != 0) {
-    NS_NOTREACHED("download manager should be used as a service");
-    return NS_ERROR_UNEXPECTED; // This will make the |CreateInstance| fail.
-  }
-
   nsresult rv;
   mObserverService = do_GetService("@mozilla.org/observer-service;1", &rv);
   NS_ENSURE_SUCCESS(rv, rv);

toolkit/components/downloads/src/nsDownloadManager.h

@@ -83,6 +83,8 @@ public:
 
   nsresult Init();
 
+  static nsDownloadManager *GetSingleton();
+
   virtual ~nsDownloadManager();
 
 protected:
@@ -94,7 +96,7 @@ protected:
   nsresult CreateTable();
   nsresult ImportDownloadHistory();
   nsresult GetDownloadFromDB(PRUint32 aID, nsDownload **retVal);
-  
+
   inline nsresult AddToCurrentDownloads(nsDownload *aDl)
   {
     if (!mCurrentDownloads.AppendObject(aDl))
@@ -187,6 +189,8 @@ private:
   nsCOMPtr<nsIObserverService> mObserverService;
   nsCOMPtr<mozIStorageStatement> mUpdateDownloadStatement;
 
+  static nsDownloadManager *gDownloadManagerService;
+
   friend class nsDownload;
 };
 

toolkit/components/downloads/test/unit/test_bug_384744.js

@@ -0,0 +1,49 @@
+/* ***** BEGIN LICENSE BLOCK *****
+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Original Code is Download Manager Test Code.
+ *
+ * The Initial Developer of the Original Code is
+ * Shawn Wilsher <me@shawnwilsher.com>.
+ * Portions created by the Initial Developer are Copyright (C) 2007
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either the GNU General Public License Version 2 or later (the "GPL"), or
+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+ * in which case the provisions of the GPL or the LGPL are applicable instead
+ * of those above. If you wish to allow use of your version of this file only
+ * under the terms of either the GPL or the LGPL, and not to allow others to
+ * use your version of this file under the terms of the MPL, indicate your
+ * decision by deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL or the LGPL. If you do not delete
+ * the provisions above, a recipient may use your version of this file under
+ * the terms of any one of the MPL, the GPL or the LGPL.
+ *
+ * ***** END LICENSE BLOCK ***** */
+
+// This file tests Bug 384744 - or specifically that createInstance won't cause
+// a failure with the download manager.  The old practice was bad because if
+// someone called createInstance before anyone called getService, getService
+// would fail, and we would have a non-functional download manager.
+
+function run_test()
+{
+  var dmci = Cc["@mozilla.org/download-manager;1"].
+             createInstance(Ci.nsIDownloadManager);
+  var dmgs = Cc["@mozilla.org/download-manager;1"].
+             getService(Ci.nsIDownloadManager);
+  do_check_eq(dmci, dmgs);
+}