From 3a7ebb4f731fd48d852d91dc031eefcb7a7adc30 Mon Sep 17 00:00:00 2001 From: "sdwilsh@shawnwilsher.com" Date: Thu, 12 Jul 2007 13:01:57 -0700 Subject: [PATCH] Bug 384744 - Download manager should enforce a singleton, not just fail on createInstance. r=gavin.sharp --- .../components/build/nsToolkitCompsModule.cpp | 3 +- .../downloads/src/nsDownloadManager.cpp | 41 ++++++++-------- .../downloads/src/nsDownloadManager.h | 6 ++- .../downloads/test/unit/test_bug_384744.js | 49 +++++++++++++++++++ 4 files changed, 77 insertions(+), 22 deletions(-) create mode 100644 toolkit/components/downloads/test/unit/test_bug_384744.js diff --git a/toolkit/components/build/nsToolkitCompsModule.cpp b/toolkit/components/build/nsToolkitCompsModule.cpp index 07806706d3b4..869789eca0cd 100644 --- a/toolkit/components/build/nsToolkitCompsModule.cpp +++ b/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 diff --git a/toolkit/components/downloads/src/nsDownloadManager.cpp b/toolkit/components/downloads/src/nsDownloadManager.cpp index 24fdb19ac024..7a973e979e91 100644 --- a/toolkit/components/downloads/src/nsDownloadManager.cpp +++ b/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); diff --git a/toolkit/components/downloads/src/nsDownloadManager.h b/toolkit/components/downloads/src/nsDownloadManager.h index b67bc4eef2fd..a817484ab086 100644 --- a/toolkit/components/downloads/src/nsDownloadManager.h +++ b/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 mObserverService; nsCOMPtr mUpdateDownloadStatement; + static nsDownloadManager *gDownloadManagerService; + friend class nsDownload; }; diff --git a/toolkit/components/downloads/test/unit/test_bug_384744.js b/toolkit/components/downloads/test/unit/test_bug_384744.js new file mode 100644 index 000000000000..f76e428d48ba --- /dev/null +++ b/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 . + * 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); +}