From e0c7c6670b3a60f98c20e2f88b51f7a4650ef402 Mon Sep 17 00:00:00 2001 From: Paolo Amadini Date: Fri, 20 Jan 2012 13:48:20 +0100 Subject: [PATCH] Bug 672681 - Make nsIDownloadHistory::addDownload asynchronous. r=mak --- toolkit/components/places/History.cpp | 171 +++++++++++++++- toolkit/components/places/History.h | 3 + toolkit/components/places/nsNavHistory.cpp | 86 +------- toolkit/components/places/nsNavHistory.h | 3 - toolkit/components/places/nsPlacesModule.cpp | 2 +- .../components/places/tests/head_common.js | 21 ++ .../tests/unit/test_async_history_api.js | 23 --- .../tests/unit/test_download_history.js | 187 +++++++++++------- 8 files changed, 308 insertions(+), 188 deletions(-) diff --git a/toolkit/components/places/History.cpp b/toolkit/components/places/History.cpp index 300015698b45..4155bc88d417 100644 --- a/toolkit/components/places/History.cpp +++ b/toolkit/components/places/History.cpp @@ -45,6 +45,7 @@ #include "History.h" #include "nsNavHistory.h" #include "nsNavBookmarks.h" +#include "nsAnnotationService.h" #include "Helpers.h" #include "PlaceInfo.h" #include "VisitInfo.h" @@ -79,6 +80,11 @@ namespace places { // Observer event fired after a visit has been registered in the DB. #define URI_VISIT_SAVED "uri-visit-saved" +#define DESTINATIONFILEURI_ANNO \ + NS_LITERAL_CSTRING("downloads/destinationFileURI") +#define DESTINATIONFILENAME_ANNO \ + NS_LITERAL_CSTRING("downloads/destinationFileName") + //////////////////////////////////////////////////////////////////////////////// //// VisitData @@ -1255,6 +1261,109 @@ private: nsRefPtr mHistory; }; +/** + * Adds download-specific annotations to a download page. + */ +class SetDownloadAnnotations : public mozIVisitInfoCallback +{ +public: + NS_DECL_ISUPPORTS + + SetDownloadAnnotations(nsIURI* aDestination) + : mDestination(aDestination) + , mHistory(History::GetService()) + { + MOZ_ASSERT(mDestination); + MOZ_ASSERT(NS_IsMainThread()); + } + + NS_IMETHOD HandleError(nsresult aResultCode, mozIPlaceInfo *aPlaceInfo) + { + // Just don't add the annotations in case the visit isn't added. + return NS_OK; + } + + NS_IMETHOD HandleResult(mozIPlaceInfo *aPlaceInfo) + { + // Exit silently if the download destination is not a local file. + nsCOMPtr destinationFileURL = do_QueryInterface(mDestination); + if (!destinationFileURL) { + return NS_OK; + } + + nsCOMPtr source; + nsresult rv = aPlaceInfo->GetUri(getter_AddRefs(source)); + NS_ENSURE_SUCCESS(rv, rv); + + nsCOMPtr destinationFile; + rv = destinationFileURL->GetFile(getter_AddRefs(destinationFile)); + NS_ENSURE_SUCCESS(rv, rv); + + nsAutoString destinationFileName; + rv = destinationFile->GetLeafName(destinationFileName); + NS_ENSURE_SUCCESS(rv, rv); + + nsCAutoString destinationURISpec; + rv = destinationFileURL->GetSpec(destinationURISpec); + NS_ENSURE_SUCCESS(rv, rv); + + // Use annotations for storing the additional download metadata. + nsAnnotationService* annosvc = nsAnnotationService::GetAnnotationService(); + NS_ENSURE_TRUE(annosvc, NS_ERROR_OUT_OF_MEMORY); + + rv = annosvc->SetPageAnnotationString( + source, + DESTINATIONFILEURI_ANNO, + NS_ConvertUTF8toUTF16(destinationURISpec), + 0, + nsIAnnotationService::EXPIRE_WITH_HISTORY + ); + NS_ENSURE_SUCCESS(rv, rv); + + rv = annosvc->SetPageAnnotationString( + source, + DESTINATIONFILENAME_ANNO, + destinationFileName, + 0, + nsIAnnotationService::EXPIRE_WITH_HISTORY + ); + NS_ENSURE_SUCCESS(rv, rv); + + nsAutoString title; + rv = aPlaceInfo->GetTitle(title); + NS_ENSURE_SUCCESS(rv, rv); + + // In case we are downloading a file that does not correspond to a web + // page for which the title is present, we populate the otherwise empty + // history title with the name of the destination file, to allow it to be + // visible and searchable in history results. + if (title.IsEmpty()) { + rv = mHistory->SetURITitle(source, destinationFileName); + NS_ENSURE_SUCCESS(rv, rv); + } + + return NS_OK; + } + + NS_IMETHOD HandleCompletion() + { + return NS_OK; + } + +private: + nsCOMPtr mDestination; + + /** + * Strong reference to the History object because we do not want it to + * disappear out from under us. + */ + nsRefPtr mHistory; +}; +NS_IMPL_ISUPPORTS1( + SetDownloadAnnotations, + mozIVisitInfoCallback +) + /** * Stores an embed visit, and notifies observers. * @@ -1905,6 +2014,65 @@ History::SetURITitle(nsIURI* aURI, const nsAString& aTitle) return NS_OK; } +//////////////////////////////////////////////////////////////////////////////// +//// nsIDownloadHistory + +NS_IMETHODIMP +History::AddDownload(nsIURI* aSource, nsIURI* aReferrer, + PRTime aStartTime, nsIURI* aDestination) +{ + MOZ_ASSERT(NS_IsMainThread()); + NS_ENSURE_ARG(aSource); + + if (mShuttingDown) { + return NS_OK; + } + + if (XRE_GetProcessType() == GeckoProcessType_Content) { + NS_ERROR("Cannot add downloads to history from content process!"); + return NS_ERROR_NOT_AVAILABLE; + } + + nsNavHistory* navHistory = nsNavHistory::GetHistoryService(); + NS_ENSURE_TRUE(navHistory, NS_ERROR_OUT_OF_MEMORY); + + // Silently return if URI is something we shouldn't add to DB. + bool canAdd; + nsresult rv = navHistory->CanAddURI(aSource, &canAdd); + NS_ENSURE_SUCCESS(rv, rv); + if (!canAdd) { + return NS_OK; + } + + nsTArray placeArray(1); + NS_ENSURE_TRUE(placeArray.AppendElement(VisitData(aSource, aReferrer)), + NS_ERROR_OUT_OF_MEMORY); + VisitData& place = placeArray.ElementAt(0); + NS_ENSURE_FALSE(place.spec.IsEmpty(), NS_ERROR_INVALID_ARG); + + place.visitTime = aStartTime; + place.SetTransitionType(nsINavHistoryService::TRANSITION_DOWNLOAD); + + mozIStorageConnection* dbConn = GetDBConn(); + NS_ENSURE_STATE(dbConn); + + nsCOMPtr callback = aDestination + ? new SetDownloadAnnotations(aDestination) + : nsnull; + + rv = InsertVisitedURIs::Start(dbConn, placeArray, callback); + NS_ENSURE_SUCCESS(rv, rv); + + // Finally, notify that we've been visited. + nsCOMPtr obsService = + mozilla::services::GetObserverService(); + if (obsService) { + obsService->NotifyObservers(aSource, NS_LINK_VISITED_EVENT_TOPIC, nsnull); + } + + return NS_OK; +} + //////////////////////////////////////////////////////////////////////////////// //// mozIAsyncHistory @@ -2101,9 +2269,10 @@ History::Observe(nsISupports* aSubject, const char* aTopic, //////////////////////////////////////////////////////////////////////////////// //// nsISupports -NS_IMPL_THREADSAFE_ISUPPORTS3( +NS_IMPL_THREADSAFE_ISUPPORTS4( History , IHistory +, nsIDownloadHistory , mozIAsyncHistory , nsIObserver ) diff --git a/toolkit/components/places/History.h b/toolkit/components/places/History.h index f2f6b16624f5..2456573fc786 100644 --- a/toolkit/components/places/History.h +++ b/toolkit/components/places/History.h @@ -42,6 +42,7 @@ #include "mozilla/IHistory.h" #include "mozIAsyncHistory.h" +#include "nsIDownloadHistory.h" #include "Database.h" #include "mozilla/dom/Link.h" @@ -62,12 +63,14 @@ struct VisitData; {0x0937a705, 0x91a6, 0x417a, {0x82, 0x92, 0xb2, 0x2e, 0xb1, 0x0d, 0xa8, 0x6c}} class History : public IHistory + , public nsIDownloadHistory , public mozIAsyncHistory , public nsIObserver { public: NS_DECL_ISUPPORTS NS_DECL_IHISTORY + NS_DECL_NSIDOWNLOADHISTORY NS_DECL_MOZIASYNCHISTORY NS_DECL_NSIOBSERVER diff --git a/toolkit/components/places/nsNavHistory.cpp b/toolkit/components/places/nsNavHistory.cpp index 5f09fd533488..0344253e779d 100644 --- a/toolkit/components/places/nsNavHistory.cpp +++ b/toolkit/components/places/nsNavHistory.cpp @@ -151,14 +151,6 @@ static const PRInt64 USECS_PER_DAY = LL_INIT(20, 500654080); // character-set annotation #define CHARSET_ANNO NS_LITERAL_CSTRING("URIProperties/characterSet") -// Download destination file URI annotation -#define DESTINATIONFILEURI_ANNO \ - NS_LITERAL_CSTRING("downloads/destinationFileURI") - -// Download destination file name annotation -#define DESTINATIONFILENAME_ANNO \ - NS_LITERAL_CSTRING("downloads/destinationFileName") - // These macros are used when splitting history by date. // These are the day containers and catch-all final container. #define HISTORY_ADDITIONAL_DATE_CONT_NUM 3 @@ -219,7 +211,6 @@ NS_IMPL_CLASSINFO(nsNavHistory, NULL, nsIClassInfo::SINGLETON, NS_INTERFACE_MAP_BEGIN(nsNavHistory) NS_INTERFACE_MAP_ENTRY(nsINavHistoryService) NS_INTERFACE_MAP_ENTRY(nsIGlobalHistory2) - NS_INTERFACE_MAP_ENTRY(nsIDownloadHistory) NS_INTERFACE_MAP_ENTRY(nsIBrowserHistory) NS_INTERFACE_MAP_ENTRY(nsIObserver) NS_INTERFACE_MAP_ENTRY(nsISupportsWeakReference) @@ -231,11 +222,10 @@ NS_INTERFACE_MAP_BEGIN(nsNavHistory) NS_INTERFACE_MAP_END // We don't care about flattening everything -NS_IMPL_CI_INTERFACE_GETTER4( +NS_IMPL_CI_INTERFACE_GETTER3( nsNavHistory , nsINavHistoryService , nsIGlobalHistory2 -, nsIDownloadHistory , nsIBrowserHistory ) @@ -3783,80 +3773,6 @@ nsNavHistory::OnEndVacuum(bool aSucceeded) } -//////////////////////////////////////////////////////////////////////////////// -//// nsIDownloadHistory - -NS_IMETHODIMP -nsNavHistory::AddDownload(nsIURI* aSource, nsIURI* aReferrer, - PRTime aStartTime, nsIURI* aDestination) -{ - NS_ASSERTION(NS_IsMainThread(), "This can only be called on the main thread"); - NS_ENSURE_ARG(aSource); - - // don't add when history is disabled and silently fail - if (IsHistoryDisabled()) - return NS_OK; - - PRInt64 visitID; - nsresult rv = AddVisit(aSource, aStartTime, aReferrer, TRANSITION_DOWNLOAD, - false, 0, &visitID); - NS_ENSURE_SUCCESS(rv, rv); - - if (!aDestination) { - return NS_OK; - } - - // Exit silently if the download destination is not a local file. - nsCOMPtr destinationFileURL = do_QueryInterface(aDestination); - if (!destinationFileURL) { - return NS_OK; - } - - nsCOMPtr destinationFile; - rv = destinationFileURL->GetFile(getter_AddRefs(destinationFile)); - NS_ENSURE_SUCCESS(rv, rv); - - nsAutoString destinationFileName; - rv = destinationFile->GetLeafName(destinationFileName); - NS_ENSURE_SUCCESS(rv, rv); - - nsCAutoString destinationURISpec; - rv = destinationFileURL->GetSpec(destinationURISpec); - NS_ENSURE_SUCCESS(rv, rv); - - // Use annotations for storing the additional download metadata. - nsAnnotationService* annosvc = nsAnnotationService::GetAnnotationService(); - NS_ENSURE_TRUE(annosvc, NS_ERROR_OUT_OF_MEMORY); - - (void)annosvc->SetPageAnnotationString( - aSource, - DESTINATIONFILEURI_ANNO, - NS_ConvertUTF8toUTF16(destinationURISpec), - 0, - nsIAnnotationService::EXPIRE_WITH_HISTORY - ); - - (void)annosvc->SetPageAnnotationString( - aSource, - DESTINATIONFILENAME_ANNO, - destinationFileName, - 0, - nsIAnnotationService::EXPIRE_WITH_HISTORY - ); - - // In case we are downloading a file that does not correspond to a web - // page for which the title is present, we populate the otherwise empty - // history title with the name of the destination file, to allow it to be - // visible and searchable in history results. - nsAutoString title; - if (NS_SUCCEEDED(GetPageTitle(aSource, title)) && title.IsEmpty()) { - (void)SetPageTitle(aSource, destinationFileName); - } - - return NS_OK; -} - - //////////////////////////////////////////////////////////////////////////////// //// nsPIPlacesDatabase diff --git a/toolkit/components/places/nsNavHistory.h b/toolkit/components/places/nsNavHistory.h index bd9f9cefc83d..d82ce2998c5e 100644 --- a/toolkit/components/places/nsNavHistory.h +++ b/toolkit/components/places/nsNavHistory.h @@ -47,7 +47,6 @@ #include "nsPIPlacesHistoryListenersNotifier.h" #include "nsIBrowserHistory.h" #include "nsIGlobalHistory.h" -#include "nsIDownloadHistory.h" #include "nsINavBookmarksService.h" #include "nsIPrivateBrowsingService.h" #include "nsIFaviconService.h" @@ -109,7 +108,6 @@ class nsNavHistory : public nsSupportsWeakReference , public nsINavHistoryService , public nsIObserver , public nsIBrowserHistory - , public nsIDownloadHistory , public nsPIPlacesDatabase , public nsPIPlacesHistoryListenersNotifier , public mozIStorageVacuumParticipant @@ -122,7 +120,6 @@ public: NS_DECL_ISUPPORTS NS_DECL_NSINAVHISTORYSERVICE NS_DECL_NSIGLOBALHISTORY2 - NS_DECL_NSIDOWNLOADHISTORY NS_DECL_NSIBROWSERHISTORY NS_DECL_NSIOBSERVER NS_DECL_NSPIPLACESDATABASE diff --git a/toolkit/components/places/nsPlacesModule.cpp b/toolkit/components/places/nsPlacesModule.cpp index e47c6c2d0ed4..809e12b23431 100644 --- a/toolkit/components/places/nsPlacesModule.cpp +++ b/toolkit/components/places/nsPlacesModule.cpp @@ -64,7 +64,6 @@ const mozilla::Module::CIDEntry kPlacesCIDs[] = { const mozilla::Module::ContractIDEntry kPlacesContracts[] = { { NS_NAVHISTORYSERVICE_CONTRACTID, &kNS_NAVHISTORYSERVICE_CID }, { NS_GLOBALHISTORY2_CONTRACTID, &kNS_NAVHISTORYSERVICE_CID }, - { NS_DOWNLOADHISTORY_CONTRACTID, &kNS_NAVHISTORYSERVICE_CID }, { NS_ANNOTATIONSERVICE_CONTRACTID, &kNS_ANNOTATIONSERVICE_CID }, { NS_NETWORK_PROTOCOL_CONTRACTID_PREFIX "moz-anno", &kNS_ANNOPROTOCOLHANDLER_CID }, { NS_NAVBOOKMARKSSERVICE_CONTRACTID, &kNS_NAVBOOKMARKSSERVICE_CID }, @@ -74,6 +73,7 @@ const mozilla::Module::ContractIDEntry kPlacesContracts[] = { { NS_IHISTORY_CONTRACTID, &kNS_ANDROIDHISTORY_CID }, #else { NS_IHISTORY_CONTRACTID, &kNS_HISTORYSERVICE_CID }, + { NS_DOWNLOADHISTORY_CONTRACTID, &kNS_HISTORYSERVICE_CID }, #endif { NS_PLACESIMPORTEXPORTSERVICE_CONTRACTID, &kNS_PLACESIMPORTEXPORTSERVICE_CID }, { NULL } diff --git a/toolkit/components/places/tests/head_common.js b/toolkit/components/places/tests/head_common.js index 6c3ceb05dc43..dc585d0b2dcb 100644 --- a/toolkit/components/places/tests/head_common.js +++ b/toolkit/components/places/tests/head_common.js @@ -736,3 +736,24 @@ function do_compare_arrays(a1, a2, sorted) a2.filter(function (e) a1.indexOf(e) == -1).length == 0; } } + +/** + * Generic nsINavHistoryObserver that doesn't implement anything, but provides + * dummy methods to prevent errors about an object not having a certain method. + */ +function NavHistoryObserver() {} + +NavHistoryObserver.prototype = { + onBeginUpdateBatch: function () {}, + onEndUpdateBatch: function () {}, + onVisit: function () {}, + onTitleChanged: function () {}, + onBeforeDeleteURI: function () {}, + onDeleteURI: function () {}, + onClearHistory: function () {}, + onPageChanged: function () {}, + onDeleteVisits: function () {}, + QueryInterface: XPCOMUtils.generateQI([ + Ci.nsINavHistoryObserver, + ]) +}; diff --git a/toolkit/components/places/tests/unit/test_async_history_api.js b/toolkit/components/places/tests/unit/test_async_history_api.js index 6e9822059a73..8d4ece027a6d 100644 --- a/toolkit/components/places/tests/unit/test_async_history_api.js +++ b/toolkit/components/places/tests/unit/test_async_history_api.js @@ -40,29 +40,6 @@ function VisitInfo(aTransitionType, this.visitDate = aVisitTime || Date.now() * 1000; } -/** - * Generic nsINavHistoryObserver that doesn't implement anything, but provides - * dummy methods to prevent errors about an object not having a certain method. - */ -function NavHistoryObserver() -{ -} -NavHistoryObserver.prototype = -{ - onBeginUpdateBatch: function() { }, - onEndUpdateBatch: function() { }, - onVisit: function() { }, - onTitleChanged: function() { }, - onBeforeDeleteURI: function() { }, - onDeleteURI: function() { }, - onClearHistory: function() { }, - onPageChanged: function() { }, - onDeleteVisits: function() { }, - QueryInterface: XPCOMUtils.generateQI([ - Ci.nsINavHistoryObserver, - ]), -}; - /** * Listens for a title change notification, and calls aCallback when it gets it. * diff --git a/toolkit/components/places/tests/unit/test_download_history.js b/toolkit/components/places/tests/unit/test_download_history.js index b6bb57eac656..0bae4d810d6d 100644 --- a/toolkit/components/places/tests/unit/test_download_history.js +++ b/toolkit/components/places/tests/unit/test_download_history.js @@ -8,30 +8,34 @@ //////////////////////////////////////////////////////////////////////////////// /// Globals -const downloadHistory = Cc["@mozilla.org/browser/download-history;1"] - .getService(Ci.nsIDownloadHistory); +XPCOMUtils.defineLazyServiceGetter(this, "gDownloadHistory", + "@mozilla.org/browser/download-history;1", + "nsIDownloadHistory"); -const TEST_URI = NetUtil.newURI("http://google.com/"); -const REFERRER_URI = NetUtil.newURI("http://yahoo.com"); +XPCOMUtils.defineLazyServiceGetter(this, "gHistory", + "@mozilla.org/browser/history;1", + "mozIAsyncHistory"); -const NS_LINK_VISITED_EVENT_TOPIC = "link-visited"; -const ENABLE_HISTORY_PREF = "places.history.enabled"; -const PB_KEEP_SESSION_PREF = "browser.privatebrowsing.keep_current_session"; +const DOWNLOAD_URI = NetUtil.newURI("http://www.example.com/"); +const REFERRER_URI = NetUtil.newURI("http://www.example.org/"); +const PRIVATE_URI = NetUtil.newURI("http://www.example.net/"); /** - * Sets a flag when the link visited notification is received. + * Waits for the first visit notification to be received. + * + * @param aCallback + * This function is called with the same arguments of onVisit. */ -const visitedObserver = { - topicReceived: false, - observe: function VO_observe(aSubject, aTopic, aData) - { - this.topicReceived = true; - } -}; -Services.obs.addObserver(visitedObserver, NS_LINK_VISITED_EVENT_TOPIC, false); -do_register_cleanup(function() { - Services.obs.removeObserver(visitedObserver, NS_LINK_VISITED_EVENT_TOPIC); -}); +function waitForOnVisit(aCallback) { + let historyObserver = { + __proto__: NavHistoryObserver.prototype, + onVisit: function HO_onVisit() { + PlacesUtils.history.removeObserver(this); + aCallback.apply(null, arguments); + } + }; + PlacesUtils.history.addObserver(historyObserver, false); +} /** * Checks to see that a URI is in the database. @@ -59,15 +63,6 @@ function uri_in_db(aURI, aExpected) root.containerOpen = false; } -/** - * Cleanup function called by each individual test if necessary. - */ -function cleanup_and_run_next_test() -{ - visitedObserver.topicReceived = false; - waitForClearHistory(run_next_test); -} - //////////////////////////////////////////////////////////////////////////////// /// Tests @@ -79,72 +74,113 @@ function run_test() add_test(function test_dh_is_from_places() { // Test that this nsIDownloadHistory is the one places implements. - do_check_true(downloadHistory instanceof Ci.nsINavHistoryService); + do_check_true(gDownloadHistory instanceof Ci.mozIAsyncHistory); - run_next_test(); + waitForClearHistory(run_next_test); }); add_test(function test_dh_addDownload() { - // Sanity checks. - uri_in_db(TEST_URI, false); - uri_in_db(REFERRER_URI, false); + waitForOnVisit(function DHAD_onVisit(aURI) { + do_check_true(aURI.equals(DOWNLOAD_URI)); - downloadHistory.addDownload(TEST_URI, REFERRER_URI, Date.now() * 1000); + // Verify that the URI is already available in results at this time. + uri_in_db(DOWNLOAD_URI, true); - do_check_true(visitedObserver.topicReceived); - uri_in_db(TEST_URI, true); - uri_in_db(REFERRER_URI, true); + waitForClearHistory(run_next_test); + }); - cleanup_and_run_next_test(); + gDownloadHistory.addDownload(DOWNLOAD_URI, null, Date.now() * 1000); }); -add_test(function test_dh_privateBrowsing() +add_test(function test_dh_addDownload_referrer() { - // Sanity checks. - uri_in_db(TEST_URI, false); - uri_in_db(REFERRER_URI, false); + waitForOnVisit(function DHAD_prepareReferrer(aURI, aVisitID) { + do_check_true(aURI.equals(REFERRER_URI)); + let referrerVisitId = aVisitID; - var pb = null; - try { - // PrivateBrowsing component is not always available to Places implementers. - pb = Cc["@mozilla.org/privatebrowsing;1"]. - getService(Ci.nsIPrivateBrowsingService); - } catch (ex) { - // Skip this test. + waitForOnVisit(function DHAD_onVisit(aURI, aVisitID, aTime, aSessionID, + aReferringID) { + do_check_true(aURI.equals(DOWNLOAD_URI)); + do_check_eq(aReferringID, referrerVisitId); + + // Verify that the URI is already available in results at this time. + uri_in_db(DOWNLOAD_URI, true); + + waitForClearHistory(run_next_test); + }); + + gDownloadHistory.addDownload(DOWNLOAD_URI, REFERRER_URI, Date.now() * 1000); + }); + + // Note that we don't pass the optional callback argument here because we must + // ensure that we receive the onVisit notification before we call addDownload. + gHistory.updatePlaces({ + uri: REFERRER_URI, + visits: [{ + transitionType: Ci.nsINavHistoryService.TRANSITION_TYPED, + visitDate: Date.now() * 1000 + }] + }); +}); + +add_test(function test_dh_addDownload_privateBrowsing() +{ + if (!("@mozilla.org/privatebrowsing;1" in Cc)) { + todo(false, "PB service is not available, bail out"); run_next_test(); return; } - Services.prefs.setBoolPref(PB_KEEP_SESSION_PREF, true); + + waitForOnVisit(function DHAD_onVisit(aURI) { + // We should only receive the notification for the non-private URI. This + // test is based on the assumption that visit notifications are received in + // the same order of the addDownload calls, which is currently true because + // database access is serialized on the same worker thread. + do_check_true(aURI.equals(DOWNLOAD_URI)); + + uri_in_db(DOWNLOAD_URI, true); + uri_in_db(PRIVATE_URI, false); + + waitForClearHistory(run_next_test); + }); + + let pb = Cc["@mozilla.org/privatebrowsing;1"] + .getService(Ci.nsIPrivateBrowsingService); + Services.prefs.setBoolPref("browser.privatebrowsing.keep_current_session", + true); pb.privateBrowsingEnabled = true; + gDownloadHistory.addDownload(PRIVATE_URI, REFERRER_URI, Date.now() * 1000); - downloadHistory.addDownload(TEST_URI, REFERRER_URI, Date.now() * 1000); - - do_check_false(visitedObserver.topicReceived); - uri_in_db(TEST_URI, false); - uri_in_db(REFERRER_URI, false); - + // The addDownload functions calls CanAddURI synchronously, thus we can exit + // Private Browsing Mode immediately. pb.privateBrowsingEnabled = false; - cleanup_and_run_next_test(); + Services.prefs.clearUserPref("browser.privatebrowsing.keep_current_session"); + gDownloadHistory.addDownload(DOWNLOAD_URI, REFERRER_URI, Date.now() * 1000); }); -add_test(function test_dh_disabledHistory() +add_test(function test_dh_addDownload_disabledHistory() { - // Sanity checks. - uri_in_db(TEST_URI, false); - uri_in_db(REFERRER_URI, false); + waitForOnVisit(function DHAD_onVisit(aURI) { + // We should only receive the notification for the non-private URI. This + // test is based on the assumption that visit notifications are received in + // the same order of the addDownload calls, which is currently true because + // database access is serialized on the same worker thread. + do_check_true(aURI.equals(DOWNLOAD_URI)); - // Disable history. - Services.prefs.setBoolPref(ENABLE_HISTORY_PREF, false); + uri_in_db(DOWNLOAD_URI, true); + uri_in_db(PRIVATE_URI, false); - downloadHistory.addDownload(TEST_URI, REFERRER_URI, Date.now() * 1000); + waitForClearHistory(run_next_test); + }); - do_check_false(visitedObserver.topicReceived); - uri_in_db(TEST_URI, false); - uri_in_db(REFERRER_URI, false); + Services.prefs.setBoolPref("places.history.enabled", false); + gDownloadHistory.addDownload(PRIVATE_URI, REFERRER_URI, Date.now() * 1000); - Services.prefs.setBoolPref(ENABLE_HISTORY_PREF, true); - cleanup_and_run_next_test(); + // The addDownload functions calls CanAddURI synchronously, thus we can reset + // the preference immediately. + Services.prefs.clearUserPref("places.history.enabled"); + gDownloadHistory.addDownload(DOWNLOAD_URI, REFERRER_URI, Date.now() * 1000); }); /** @@ -170,7 +206,7 @@ add_test(function test_dh_details() PlacesUtils.annotations.removeObserver(annoObserver); PlacesUtils.history.removeObserver(historyObserver); - cleanup_and_run_next_test(); + waitForClearHistory(run_next_test); } }; @@ -214,17 +250,18 @@ add_test(function test_dh_details() onDeleteURI: function() {}, onClearHistory: function() {}, onPageChanged: function() {}, - onDeleteVisits: function() {} + onDeleteVisits: function() {} }; PlacesUtils.annotations.addObserver(annoObserver, false); PlacesUtils.history.addObserver(historyObserver, false); // Both null values and remote URIs should not cause errors. - downloadHistory.addDownload(SOURCE_URI, null, Date.now() * 1000); - downloadHistory.addDownload(SOURCE_URI, null, Date.now() * 1000, null); - downloadHistory.addDownload(SOURCE_URI, null, Date.now() * 1000, REMOTE_URI); + gDownloadHistory.addDownload(SOURCE_URI, null, Date.now() * 1000); + gDownloadHistory.addDownload(SOURCE_URI, null, Date.now() * 1000, null); + gDownloadHistory.addDownload(SOURCE_URI, null, Date.now() * 1000, REMOTE_URI); // Valid local file URIs should cause the download details to be saved. - downloadHistory.addDownload(SOURCE_URI, null, Date.now() * 1000, destFileUri); + gDownloadHistory.addDownload(SOURCE_URI, null, Date.now() * 1000, + destFileUri); });