From 9679a4ea34bc9ad3590519e4c5d7c9a05e173f43 Mon Sep 17 00:00:00 2001 From: Marco Bonardo Date: Wed, 14 Oct 2009 11:53:13 +0200 Subject: [PATCH] Bug 478718 - Move last Places sync to xpcom-shutdown, r=sdwilsh --HG-- rename : toolkit/components/places/tests/sync/test_database_sync_after_quit_application.js => toolkit/components/places/tests/sync/test_database_sync_after_shutdown.js rename : toolkit/components/places/tests/sync/test_database_sync_after_quit_application_with_removeAllPages.js => toolkit/components/places/tests/sync/test_database_sync_after_shutdown_with_removeAllPages.js --- browser/components/nsBrowserGlue.js | 65 +++++---- .../places/tests/unit/tail_bookmarks.js | 65 --------- .../tests/unit/test_browserGlue_prefs.js | 12 +- .../tests/unit/test_browserGlue_shutdown.js | 2 +- .../test/unit/do_test_removeDataFromDomain.js | 1 - ...st_removeDataFromDomain_activeDownloads.js | 2 - .../test/unit/head_privatebrowsing.js | 9 -- .../places/public/nsINavHistoryService.idl | 6 +- .../places/public/nsPIPlacesDatabase.idl | 15 +- .../components/places/src/nsNavHistory.cpp | 131 +++++++++--------- toolkit/components/places/src/nsNavHistory.h | 10 +- .../places/src/nsPlacesAutoComplete.js | 8 +- .../components/places/src/nsPlacesDBFlush.js | 36 +++-- .../tests/autocomplete/tail_autocomplete.js | 65 --------- .../places/tests/bookmarks/tail_bookmarks.js | 66 --------- .../places/tests/queries/tail_queries.js | 62 --------- .../components/places/tests/sync/head_sync.js | 34 +++-- .../components/places/tests/sync/tail_sync.js | 65 --------- .../sync/test_bookmarks_sorted_by_none.js | 2 +- ...s => test_database_sync_after_shutdown.js} | 10 +- ...ync_after_shutdown_with_removeAllPages.js} | 130 +++++++++-------- .../sync/test_database_sync_embed_visits.js | 2 +- .../test_database_sync_expireAllFavicons.js | 2 +- .../sync/test_database_sync_onitemadded.js | 2 +- ...atabase_sync_with_specialHistoryQueries.js | 2 +- .../test_multiple_bookmarks_around_sync.js | 2 +- .../sync/test_multiple_visits_around_sync.js | 2 +- .../places/tests/unit/head_bookmarks.js | 20 --- .../places/tests/unit/tail_bookmarks.js | 66 --------- .../places/tests/unit/test_248970.js | 13 +- .../places/tests/unit/test_expiration.js | 5 +- .../tests/unit/test_history_removeAllPages.js | 5 +- ...nsPIPlacesDatabase_commitPendingChanges.js | 75 ---------- ...acesDatabase_finalizeInternalStatements.js | 55 -------- 34 files changed, 250 insertions(+), 797 deletions(-) delete mode 100644 browser/components/places/tests/unit/tail_bookmarks.js delete mode 100644 toolkit/components/places/tests/autocomplete/tail_autocomplete.js delete mode 100644 toolkit/components/places/tests/bookmarks/tail_bookmarks.js delete mode 100644 toolkit/components/places/tests/queries/tail_queries.js delete mode 100644 toolkit/components/places/tests/sync/tail_sync.js rename toolkit/components/places/tests/sync/{test_database_sync_after_quit_application.js => test_database_sync_after_shutdown.js} (97%) rename toolkit/components/places/tests/sync/{test_database_sync_after_quit_application_with_removeAllPages.js => test_database_sync_after_shutdown_with_removeAllPages.js} (55%) delete mode 100644 toolkit/components/places/tests/unit/tail_bookmarks.js delete mode 100644 toolkit/components/places/tests/unit/test_nsPIPlacesDatabase_commitPendingChanges.js delete mode 100644 toolkit/components/places/tests/unit/test_nsPIPlacesDatabase_finalizeInternalStatements.js diff --git a/browser/components/nsBrowserGlue.js b/browser/components/nsBrowserGlue.js index 1132877c609e..7c6259949486 100644 --- a/browser/components/nsBrowserGlue.js +++ b/browser/components/nsBrowserGlue.js @@ -78,7 +78,6 @@ const BrowserGlueServiceFactory = { // Constructor function BrowserGlue() { - XPCOMUtils.defineLazyServiceGetter(this, "_prefs", "@mozilla.org/preferences-service;1", "nsIPrefBranch"); @@ -91,14 +90,24 @@ function BrowserGlue() { "@mozilla.org/widget/idleservice;1", "nsIIdleService"); - XPCOMUtils.defineLazyServiceGetter(this, "_observerService", - "@mozilla.org/observer-service;1", - "nsIObserverService"); - XPCOMUtils.defineLazyGetter(this, "_distributionCustomizer", function() { return new DistributionCustomizer(); }); + XPCOMUtils.defineLazyGetter(this, "_sanitizer", + function() { + let sanitizerScope = {}; + Cc["@mozilla.org/moz/jssubscript-loader;1"]. + getService(Ci.mozIJSSubScriptLoader). + loadSubScript("chrome://browser/content/sanitize.js", sanitizerScope); + return sanitizerScope.Sanitizer; + }); + + // The observer service is immediately used in _init(), so there's no reason + // to have a getter. + this._observerService = Cc["@mozilla.org/observer-service;1"]. + getService(Ci.nsIObserverService); + this._init(); } @@ -109,15 +118,17 @@ function BrowserGlue() { #endif BrowserGlue.prototype = { - _saveSession: false, _isIdleObserver: false, _isPlacesInitObserver: false, _isPlacesLockedObserver: false, _isPlacesDatabaseLocked: false, - _setPrefToSaveSession: function() + _setPrefToSaveSession: function(aForce) { + if (!this._saveSession && !aForce) + return; + this._prefs.setBoolPref("browser.sessionstore.resume_session_once", true); // This method can be called via [NSApplication terminate:] on Mac, which @@ -153,12 +164,9 @@ BrowserGlue.prototype = { this._onQuitRequest(subject, data); break; case "quit-application-granted": - if (this._saveSession) { - this._setPrefToSaveSession(); - } - // Everything that uses Places during shutdown should be here, since - // on quit-application Places database connection will be closed - // and history synchronization could fail. + // This pref must be set here because SessionStore will use its value + // on quit-application. + this._setPrefToSaveSession(); this._onProfileShutdown(); break; #ifdef OBSERVE_LASTWINDOW_CLOSE_TOPICS @@ -168,12 +176,11 @@ BrowserGlue.prototype = { this._onQuitRequest(subject, "lastwindow"); break; case "browser-lastwindow-close-granted": - if (this._saveSession) - this._setPrefToSaveSession(); + this._setPrefToSaveSession(); break; #endif case "session-save": - this._setPrefToSaveSession(); + this._setPrefToSaveSession(true); subject.QueryInterface(Ci.nsISupportsPRBool); subject.data = true; break; @@ -191,8 +198,8 @@ BrowserGlue.prototype = { break; case "places-database-locked": this._isPlacesDatabaseLocked = true; - // stop observing, so further attempts to load history service - // do not show the prompt. + // Stop observing, so further attempts to load history service + // will not show the prompt. this._observerService.removeObserver(this, "places-database-locked"); this._isPlacesLockedObserver = false; break; @@ -268,7 +275,7 @@ BrowserGlue.prototype = { // profile startup handler (contains profile initialization routines) _onProfileStartup: function() { - this.Sanitizer.onStartup(); + this._sanitizer.onStartup(); // check if we're in safe mode var app = Cc["@mozilla.org/xre/app-info;1"].getService(Ci.nsIXULAppInfo). QueryInterface(Ci.nsIXULRuntime); @@ -302,7 +309,8 @@ BrowserGlue.prototype = { } } - this._observerService.notifyObservers(null, "browser-ui-startup-complete", ""); + this._observerService + .notifyObservers(null, "browser-ui-startup-complete", ""); }, // profile shutdown handler (contains profile cleanup routines) @@ -311,7 +319,7 @@ BrowserGlue.prototype = { this._shutdownPlaces(); this._idleService.removeIdleObserver(this, BOOKMARKS_BACKUP_IDLE_TIME); this._isIdleObserver = false; - this.Sanitizer.onShutdown(); + this._sanitizer.onShutdown(); }, // Browser startup complete. All initial windows have opened. @@ -572,17 +580,6 @@ BrowserGlue.prototype = { browser.selectedTab = browser.addTab(updateUrl); }, - // returns the (cached) Sanitizer constructor - get Sanitizer() - { - if(typeof(Sanitizer) != "function") { // we should dynamically load the script - Cc["@mozilla.org/moz/jssubscript-loader;1"]. - getService(Ci.mozIJSSubScriptLoader). - loadSubScript("chrome://browser/content/sanitize.js", null); - } - return Sanitizer; - }, - /** * Initialize Places * - imports the bookmarks html file if bookmarks database is empty, try to @@ -907,7 +904,7 @@ BrowserGlue.prototype = { sanitize: function(aParentWindow) { - this.Sanitizer.sanitize(aParentWindow); + this._sanitizer.sanitize(aParentWindow); }, ensurePlacesDefaultQueriesInitialized: function() { @@ -921,7 +918,7 @@ BrowserGlue.prototype = { const SMART_BOOKMARKS_ANNO = "Places/SmartBookmark"; const SMART_BOOKMARKS_PREF = "browser.places.smartBookmarksVersion"; - // XXX should this be a pref? see bug #399268 + // TODO bug 399268: should this be a pref? const MAX_RESULTS = 10; // get current smart bookmarks version diff --git a/browser/components/places/tests/unit/tail_bookmarks.js b/browser/components/places/tests/unit/tail_bookmarks.js deleted file mode 100644 index 6207283cccff..000000000000 --- a/browser/components/places/tests/unit/tail_bookmarks.js +++ /dev/null @@ -1,65 +0,0 @@ -/* ***** 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 Places. - * - * The Initial Developer of the Original Code is - * Mozilla.org - * Portions created by the Initial Developer are Copyright (C) 2006 - * the Initial Developer. All Rights Reserved. - * - * Contributor(s): - * Dietrich Ayala - * - * 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 ***** */ - -// put cleanup of the bookmarks test here. - -// Run the event loop to be more like the browser, which normally runs the -// event loop long before code like this would run. -// Not doing so could cause us to close the connection before all tasks have -// been completed, and that would crash badly. -flush_main_thread_events(); - -// XPCShell doesn't dispatch quit-application, to ensure cleanup we have to -// dispatch it after each test run. -var os = Cc['@mozilla.org/observer-service;1']. - getService(Ci.nsIObserverService); -os.notifyObservers(null, "quit-application-granted", null); -os.notifyObservers(null, "quit-application", null); - -// Run the event loop, since we enqueue some statement finalization. -flush_main_thread_events(); - -// try to close the connection so we can remove places.sqlite -var pip = Cc["@mozilla.org/browser/nav-history-service;1"]. - getService(Ci.nsINavHistoryService). - QueryInterface(Ci.nsPIPlacesDatabase); -if (pip.DBConnection.connectionReady) { - pip.commitPendingChanges(); - pip.finalizeInternalStatements(); - pip.DBConnection.close(); - do_check_false(pip.DBConnection.connectionReady); -} diff --git a/browser/components/places/tests/unit/test_browserGlue_prefs.js b/browser/components/places/tests/unit/test_browserGlue_prefs.js index 12440dd4afb1..d529a6d0f783 100644 --- a/browser/components/places/tests/unit/test_browserGlue_prefs.js +++ b/browser/components/places/tests/unit/test_browserGlue_prefs.js @@ -205,15 +205,16 @@ tests.push({ do_check_false(ps.getBoolPref(PREF_RESTORE_DEFAULT_BOOKMARKS)); do_check_false(ps.getBoolPref(PREF_IMPORT_BOOKMARKS_HTML)); - finish_test(); + do_test_finished(); } }); //------------------------------------------------------------------------------ function finish_test() { - // Simulate application closing to remove the idle observer and avoid leaks. - os.notifyObservers(null, "quit-application-granted", null); + // Clean up database from all bookmarks. + remove_all_bookmarks(); + do_test_finished(); } @@ -222,16 +223,13 @@ function next_test() { // Clean up database from all bookmarks. remove_all_bookmarks(); - // Simulate application closing to remove the idle observer and avoid leaks. - os.notifyObservers(null, "quit-application-granted", null); - // nsBrowserGlue stops observing topics after first notification, // so we add back the observer to test additional runs. os.addObserver(bg, TOPIC_PLACES_INIT_COMPLETE, false); // Execute next test. let test = tests.shift(); - dump("\nTEST " + (++testIndex) + ": " + test.description); + print("\nTEST " + (++testIndex) + ": " + test.description); test.exec(); } diff --git a/browser/components/places/tests/unit/test_browserGlue_shutdown.js b/browser/components/places/tests/unit/test_browserGlue_shutdown.js index 233e1756621e..2a76f9a35655 100644 --- a/browser/components/places/tests/unit/test_browserGlue_shutdown.js +++ b/browser/components/places/tests/unit/test_browserGlue_shutdown.js @@ -139,7 +139,7 @@ tests.push({ do_check_eq(profileBookmarksJSONFile.lastModifiedTime, lastMod); do_check_eq(profileBookmarksJSONFile.fileSize, fileSize); - finish_test(); + do_test_finished(); } }); diff --git a/browser/components/privatebrowsing/test/unit/do_test_removeDataFromDomain.js b/browser/components/privatebrowsing/test/unit/do_test_removeDataFromDomain.js index 6eb2bff486c0..b64e568ada1a 100644 --- a/browser/components/privatebrowsing/test/unit/do_test_removeDataFromDomain.js +++ b/browser/components/privatebrowsing/test/unit/do_test_removeDataFromDomain.js @@ -564,7 +564,6 @@ function test_cache_cleared() observe: function(aSubject, aTopic, aData) { os.removeObserver(observer, "cacheservice:empty-cache"); - shutdownPlaces(); do_test_finished(); } }; diff --git a/browser/components/privatebrowsing/test/unit/do_test_removeDataFromDomain_activeDownloads.js b/browser/components/privatebrowsing/test/unit/do_test_removeDataFromDomain_activeDownloads.js index 27ff386ef7ff..ba772381c197 100644 --- a/browser/components/privatebrowsing/test/unit/do_test_removeDataFromDomain_activeDownloads.js +++ b/browser/components/privatebrowsing/test/unit/do_test_removeDataFromDomain_activeDownloads.js @@ -155,6 +155,4 @@ function do_test() // And check our data for (let i = 0; i < data.length; i++) check_active_download(data[i].source, !data[i].removed); - - shutdownPlaces(); } diff --git a/browser/components/privatebrowsing/test/unit/head_privatebrowsing.js b/browser/components/privatebrowsing/test/unit/head_privatebrowsing.js index 5eb3f2dfe0d0..c579d3609c48 100644 --- a/browser/components/privatebrowsing/test/unit/head_privatebrowsing.js +++ b/browser/components/privatebrowsing/test/unit/head_privatebrowsing.js @@ -110,15 +110,6 @@ function cleanUp() } cleanUp(); -/** - * Finalize Places statements during quit-application in order to prevent leaks - */ -function shutdownPlaces() { - let os = Cc["@mozilla.org/observer-service;1"]. - getService(Ci.nsIObserverService); - os.notifyObservers(null, "quit-application", null); -} - var PRIVATEBROWSING_CONTRACT_ID; function run_test_on_all_services() { var contractIDs = [ diff --git a/toolkit/components/places/public/nsINavHistoryService.idl b/toolkit/components/places/public/nsINavHistoryService.idl index 752a8d970c7a..0b8c05d0cf42 100644 --- a/toolkit/components/places/public/nsINavHistoryService.idl +++ b/toolkit/components/places/public/nsINavHistoryService.idl @@ -40,11 +40,11 @@ * ***** END LICENSE BLOCK ***** */ /** - * Using Places services on quit-application or later is not reliable, so make - * sure to do shutdown work on quit-application-granted, or history + * Using Places services after quit-application is not reliable, so make + * sure to do any shutdown work on quit-application, or history * synchronization could fail, losing latest changes. */ - + #include "nsISupports.idl" #include "nsIArray.idl" #include "nsIURI.idl" diff --git a/toolkit/components/places/public/nsPIPlacesDatabase.idl b/toolkit/components/places/public/nsPIPlacesDatabase.idl index 1d5fe54478d2..ef1922ba9deb 100644 --- a/toolkit/components/places/public/nsPIPlacesDatabase.idl +++ b/toolkit/components/places/public/nsPIPlacesDatabase.idl @@ -46,24 +46,11 @@ interface mozIStorageConnection; * database. If outside consumers wish to use this, they should only read from * the database so they do not break any internal invariants. */ -[scriptable, uuid(8e6d4f8a-4b8e-4026-9fca-517c4494ddb7)] +[scriptable, uuid(5fd91813-229c-4d30-851b-700afa39a987)] interface nsPIPlacesDatabase : nsISupports { /** * The database connection used by Places. */ readonly attribute mozIStorageConnection DBConnection; - - /** - * Finalizes all Places internal statements, allowing to safely close the - * database connection. - */ - void finalizeInternalStatements(); - - /** - * Commits all pending history changes, call this before finalizing - * statements and closing the database connection to ensure safety for all - * history data. - */ - void commitPendingChanges(); }; diff --git a/toolkit/components/places/src/nsNavHistory.cpp b/toolkit/components/places/src/nsNavHistory.cpp index 3df65c12b7e5..c5e881cdfb02 100644 --- a/toolkit/components/places/src/nsNavHistory.cpp +++ b/toolkit/components/places/src/nsNavHistory.cpp @@ -380,7 +380,6 @@ const PRInt32 nsNavHistory::kGetInfoIndex_ItemParentId = 11; const PRInt32 nsNavHistory::kGetInfoIndex_ItemTags = 12; -static const char* gQuitApplicationGrantedMessage = "quit-application-granted"; static const char* gXpcomShutdown = "xpcom-shutdown"; static const char* gAutoCompleteFeedback = "autocomplete-will-enter-text"; static const char* gIdleDaily = "idle-daily"; @@ -547,7 +546,6 @@ nsNavHistory::Init() pbi->AddObserver(PREF_BROWSER_HISTORY_EXPIRE_SITES, this, PR_FALSE); } - observerService->AddObserver(this, gQuitApplicationGrantedMessage, PR_FALSE); observerService->AddObserver(this, gXpcomShutdown, PR_FALSE); observerService->AddObserver(this, gAutoCompleteFeedback, PR_FALSE); observerService->AddObserver(this, gIdleDaily, PR_FALSE); @@ -4862,18 +4860,11 @@ nsNavHistory::RemoveAllPages() // expire everything mExpire->ClearHistory(); - // Compress DB. Currently commented out because compression is very slow. - // Deleted data will be overwritten with 0s by sqlite. -#if 0 - nsresult rv = mDBConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING("VACUUM")); - NS_ENSURE_SUCCESS(rv, rv); -#endif - // privacy cleanup, if there's an old history.dat around, just delete it nsCOMPtr oldHistoryFile; nsresult rv = NS_GetSpecialDirectory(NS_APP_HISTORY_50_FILE, getter_AddRefs(oldHistoryFile)); - if (NS_FAILED(rv)) return rv; + NS_ENSURE_SUCCESS(rv, rv); PRBool fileExists; if (NS_SUCCEEDED(oldHistoryFile->Exists(&fileExists)) && fileExists) { @@ -5499,22 +5490,11 @@ nsNavHistory::GetDBConnection(mozIStorageConnection **_DBConnection) return NS_OK; } -NS_IMETHODIMP +NS_HIDDEN_(nsresult) nsNavHistory::FinalizeInternalStatements() { NS_ASSERTION(NS_IsMainThread(), "This can only be called on the main thread"); -#ifdef LAZY_ADD - // Kill lazy timer or it could fire later when statements won't be valid - // anymore. - // At this point we should have called CommitPendingChanges before the last - // sync, so all data is saved to disk and we can finalize all statements. - if (mLazyTimer) - mLazyTimer->Cancel(); - NS_ABORT_IF_FALSE(mLazyMessages.Length() == 0, - "There are pending lazy messages, did you call CommitPendingChanges()?"); -#endif - // nsNavHistory nsresult rv = FinalizeStatements(); NS_ENSURE_SUCCESS(rv, rv); @@ -5540,36 +5520,6 @@ nsNavHistory::FinalizeInternalStatements() return NS_OK; } -NS_IMETHODIMP -nsNavHistory::CommitPendingChanges() -{ - #ifdef LAZY_ADD - CommitLazyMessages(); - #endif - - // Immediately serve topics we generated, this way they won't try to access - // the database after CommitPendingChanges has been called. - nsCOMPtr os = - do_GetService("@mozilla.org/observer-service;1"); - NS_ENSURE_TRUE(os, NS_ERROR_FAILURE); - nsCOMPtr e; - nsresult rv = os->EnumerateObservers(PLACES_INIT_COMPLETE_TOPIC, - getter_AddRefs(e)); - if (NS_SUCCEEDED(rv) && e) { - nsCOMPtr observer; - PRBool loop = PR_TRUE; - while(NS_SUCCEEDED(e->HasMoreElements(&loop)) && loop) - { - e->GetNext(getter_AddRefs(observer)); - rv = observer->Observe(observer, - PLACES_INIT_COMPLETE_TOPIC, - nsnull); - } - } - - return NS_OK; -} - // nsPIPlacesHistoryListenersNotifier ****************************************** NS_IMETHODIMP @@ -5595,25 +5545,67 @@ nsNavHistory::Observe(nsISupports *aSubject, const char *aTopic, { NS_ASSERTION(NS_IsMainThread(), "This can only be called on the main thread"); - if (strcmp(aTopic, gQuitApplicationGrantedMessage) == 0) { - nsresult rv; + if (strcmp(aTopic, gXpcomShutdown) == 0) { + nsCOMPtr os = + do_GetService("@mozilla.org/observer-service;1"); + if (os) { + os->RemoveObserver(this, NS_PRIVATE_BROWSING_SWITCH_TOPIC); + os->RemoveObserver(this, gIdleDaily); + os->RemoveObserver(this, gXpcomShutdown); + } + + // If xpcom-shutdown is called in the same scope as the service init, we + // should Immediately serve topics we generated, this way they won't try to + // access the database later. + nsCOMPtr e; + nsresult rv = os->EnumerateObservers(PLACES_INIT_COMPLETE_TOPIC, + getter_AddRefs(e)); + if (NS_SUCCEEDED(rv) && e) { + nsCOMPtr observer; + PRBool loop = PR_TRUE; + while(NS_SUCCEEDED(e->HasMoreElements(&loop)) && loop) + { + e->GetNext(getter_AddRefs(observer)); + rv = observer->Observe(observer, + PLACES_INIT_COMPLETE_TOPIC, + nsnull); + } + } + nsCOMPtr prefService = - do_GetService(NS_PREFSERVICE_CONTRACTID, &rv); - if (NS_SUCCEEDED(rv)) + do_GetService(NS_PREFSERVICE_CONTRACTID); + if (prefService) prefService->SavePrefFile(nsnull); // Start shutdown expiration. mExpire->OnQuit(); - } - else if (strcmp(aTopic, gXpcomShutdown) == 0) { - nsresult rv; - nsCOMPtr observerService = - do_GetService("@mozilla.org/observer-service;1", &rv); + +#ifdef LAZY_ADD + // Commit all pending lazy messages. + CommitLazyMessages(PR_TRUE); + + // Kill lazy timer or it could fire later when statements won't be valid + // anymore. + if (mLazyTimer) { + mLazyTimer->Cancel(); + mLazyTimer = 0; + } +#endif + + // Finalize all statements. + rv = FinalizeInternalStatements(); NS_ENSURE_SUCCESS(rv, rv); - observerService->RemoveObserver(this, NS_PRIVATE_BROWSING_SWITCH_TOPIC); - observerService->RemoveObserver(this, gIdleDaily); - observerService->RemoveObserver(this, gXpcomShutdown); - observerService->RemoveObserver(this, gQuitApplicationGrantedMessage); + + // Remove categories. + nsCOMPtr catMan = + do_GetService(NS_CATEGORYMANAGER_CONTRACTID); + if (catMan) { + (void)catMan->DeleteCategory("bookmark-observers"); + (void)catMan->DeleteCategory("history-observers"); + } + + // NOTE: We don't close the connection because the sync service could still + // need it for a final flush. } #ifdef MOZ_XUL else if (strcmp(aTopic, gAutoCompleteFeedback) == 0) { @@ -5927,8 +5919,8 @@ nsNavHistory::LazyTimerCallback(nsITimer* aTimer, void* aClosure) // nsNavHistory::CommitLazyMessages -void -nsNavHistory::CommitLazyMessages() +NS_HIDDEN_(void) +nsNavHistory::CommitLazyMessages(PRBool aIsShutdown) { mozStorageTransaction transaction(mDBConn, PR_TRUE); for (PRUint32 i = 0; i < mLazyMessages.Length(); i ++) { @@ -5942,6 +5934,9 @@ nsNavHistory::CommitLazyMessages() SetPageTitleInternal(message.uri, message.title); break; case LazyMessage::Type_Favicon: { + // Favicons cannot use async channels after xpcom-shutdown. + if (aIsShutdown) + continue; nsFaviconService* faviconService = nsFaviconService::GetFaviconService(); if (faviconService) { faviconService->DoSetAndLoadFaviconForPage(message.uri, @@ -8082,7 +8077,7 @@ nsNavHistory::GetDBBookmarkToUrlResult() return mDBBookmarkToUrlResult; } -nsresult +NS_HIDDEN_(nsresult) nsNavHistory::FinalizeStatements() { mozIStorageStatement* stmts[] = { #ifdef MOZ_XUL diff --git a/toolkit/components/places/src/nsNavHistory.h b/toolkit/components/places/src/nsNavHistory.h index 5c32b8f1ca14..1eed095db6e8 100644 --- a/toolkit/components/places/src/nsNavHistory.h +++ b/toolkit/components/places/src/nsNavHistory.h @@ -433,7 +433,7 @@ protected: /** * Finalize all internal statements. */ - nsresult FinalizeStatements(); + NS_HIDDEN_(nsresult) FinalizeStatements(); /** * Analyzes the database and VACUUM it, if needed. @@ -444,6 +444,12 @@ protected: */ NS_HIDDEN_(nsresult) VacuumDatabase(); + /** + * Finalizes all Places internal statements, allowing to safely close the + * database connection. + */ + NS_HIDDEN_(nsresult) FinalizeInternalStatements(); + // nsICharsetResolver NS_DECL_NSICHARSETRESOLVER @@ -580,7 +586,7 @@ protected: nsresult StartLazyTimer(); nsresult AddLazyMessage(const LazyMessage& aMessage); static void LazyTimerCallback(nsITimer* aTimer, void* aClosure); - void CommitLazyMessages(); + NS_HIDDEN_(void) CommitLazyMessages(PRBool aIsShutdown = PR_FALSE); #endif nsresult ConstructQueryString(const nsCOMArray& aQueries, diff --git a/toolkit/components/places/src/nsPlacesAutoComplete.js b/toolkit/components/places/src/nsPlacesAutoComplete.js index 7cda0e09dc44..cd095dc1db20 100644 --- a/toolkit/components/places/src/nsPlacesAutoComplete.js +++ b/toolkit/components/places/src/nsPlacesAutoComplete.js @@ -72,7 +72,7 @@ const kBookTagSQLFragment = book_tag_sql_fragment("tags", "GROUP_CONCAT(t.title, ',')", true); // observer topics -const kQuitApplication = "quit-application"; +const kXPComShutdown = "xpcom-shutdown"; const kPrefChanged = "nsPref:changed"; // Match type constants. These indicate what type of search function we should @@ -364,7 +364,7 @@ function nsPlacesAutoComplete() // register observers this._os = Cc["@mozilla.org/observer-service;1"]. getService(Ci.nsIObserverService); - this._os.addObserver(this, kQuitApplication, false); + this._os.addObserver(this, kXPComShutdown, false); } @@ -504,8 +504,8 @@ nsPlacesAutoComplete.prototype = { observe: function PAC_observe(aSubject, aTopic, aData) { - if (aTopic == kQuitApplication) { - this._os.removeObserver(this, kQuitApplication); + if (aTopic == kXPComShutdown) { + this._os.removeObserver(this, kXPComShutdown); // Remove our preference observer. this._prefs.removeObserver("", this); diff --git a/toolkit/components/places/src/nsPlacesDBFlush.js b/toolkit/components/places/src/nsPlacesDBFlush.js index fdb74c16a275..58f9950c02e4 100644 --- a/toolkit/components/places/src/nsPlacesDBFlush.js +++ b/toolkit/components/places/src/nsPlacesDBFlush.js @@ -48,7 +48,7 @@ const Ci = Components.interfaces; const Cr = Components.results; const Cu = Components.utils; -const kQuitApplication = "quit-application"; +const kXPComShutdown = "xpcom-shutdown"; const kSyncFinished = "places-sync-finished"; const kDebugStopSync = "places-debug-stop-sync"; const kDebugStartSync = "places-debug-start-sync"; @@ -109,7 +109,7 @@ function nsPlacesDBFlush() // Register observers this._os = Cc["@mozilla.org/observer-service;1"]. getService(Ci.nsIObserverService); - this._os.addObserver(this, kQuitApplication, false); + this._os.addObserver(this, kXPComShutdown, false); this._os.addObserver(this, kDebugStopSync, false); this._os.addObserver(this, kDebugStartSync, false); @@ -149,8 +149,8 @@ nsPlacesDBFlush.prototype = { observe: function DBFlush_observe(aSubject, aTopic, aData) { - if (aTopic == kQuitApplication) { - this._os.removeObserver(this, kQuitApplication); + if (aTopic == kXPComShutdown) { + this._os.removeObserver(this, kXPComShutdown); this._os.removeObserver(this, kDebugStopSync); this._os.removeObserver(this, kDebugStartSync); @@ -158,29 +158,41 @@ nsPlacesDBFlush.prototype = { pb2.removeObserver(kSyncPrefName, this); pb2.removeObserver(kExpireDaysPrefName, this); } - this._timer.cancel(); - this._timer = null; + + if (this._timer) { + this._timer.cancel(); + this._timer = null; + } + // Other components could still make changes to history at this point, // for example to clear private data on shutdown, so here we dispatch // an event to the main thread so that we will sync after - // quit-application ensuring all data have been saved. + // xpcom-shutdown ensuring all data have been saved. let tm = Cc["@mozilla.org/thread-manager;1"]. getService(Ci.nsIThreadManager); tm.mainThread.dispatch({ _self: this, run: function() { - let pip = Cc["@mozilla.org/browser/nav-history-service;1"]. - getService(Ci.nsPIPlacesDatabase); - pip.commitPendingChanges(); + // Flush any remaining change to disk tables. this._self._flushWithQueries([kQuerySyncPlacesId, kQuerySyncHistoryVisitsId]); + + // Ensure we won't act anymore as a category observer, so we stop + // being notified. + let catMan = Cc["@mozilla.org/categorymanager;1"]. + getService(Ci.nsICategoryManager); + catMan.deleteCategoryEntry("bookmark-observers", + this._self.classDescription, + true); + catMan.deleteCategoryEntry("history-observers", + this._self.classDescription, + true); + // Close the database connection, this was the last sync and we can't // ensure database coherence from now on. - pip.finalizeInternalStatements(); this._self._finalizeInternalStatements(); this._self._db.close(); } }, Ci.nsIThread.DISPATCH_NORMAL); - } else if (aTopic == "nsPref:changed" && aData == kSyncPrefName) { // Get the new pref value, and then update our timer diff --git a/toolkit/components/places/tests/autocomplete/tail_autocomplete.js b/toolkit/components/places/tests/autocomplete/tail_autocomplete.js deleted file mode 100644 index 820caf6a79bb..000000000000 --- a/toolkit/components/places/tests/autocomplete/tail_autocomplete.js +++ /dev/null @@ -1,65 +0,0 @@ -/* ***** 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 Places. - * - * The Initial Developer of the Original Code is - * Mozilla.org - * Portions created by the Initial Developer are Copyright (C) 2009 - * the Initial Developer. All Rights Reserved. - * - * Contributor(s): - * Marco Bonardo - * - * 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 ***** */ - -// put cleanup of the bookmarks test here. - -// Run the event loop to be more like the browser, which normally runs the -// event loop long before code like this would run. -// Not doing so could cause us to close the connection before all tasks have -// been completed, and that would crash badly. -flush_main_thread_events(); - -// XPCShell doesn't dispatch quit-application, to ensure cleanup we have to -// dispatch it after each test run. -var os = Cc['@mozilla.org/observer-service;1']. - getService(Ci.nsIObserverService); -os.notifyObservers(null, "quit-application-granted", null); -os.notifyObservers(null, "quit-application", null); - -// Run the event loop, since we enqueue some statement finalization. -flush_main_thread_events(); - -// try to close the connection so we can remove places.sqlite -var pip = Cc["@mozilla.org/browser/nav-history-service;1"]. - getService(Ci.nsINavHistoryService). - QueryInterface(Ci.nsPIPlacesDatabase); -if (pip.DBConnection.connectionReady) { - pip.commitPendingChanges(); - pip.finalizeInternalStatements(); - pip.DBConnection.close(); - do_check_false(pip.DBConnection.connectionReady); -} diff --git a/toolkit/components/places/tests/bookmarks/tail_bookmarks.js b/toolkit/components/places/tests/bookmarks/tail_bookmarks.js deleted file mode 100644 index 116ba5054cb3..000000000000 --- a/toolkit/components/places/tests/bookmarks/tail_bookmarks.js +++ /dev/null @@ -1,66 +0,0 @@ -/* ***** 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 Places. - * - * The Initial Developer of the Original Code is - * Mozilla.org - * Portions created by the Initial Developer are Copyright (C) 2006 - * the Initial Developer. All Rights Reserved. - * - * Contributor(s): - * Dietrich Ayala - * Marco Bonardo - * - * 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 ***** */ - -// put cleanup of the bookmarks test here. - -// Run the event loop to be more like the browser, which normally runs the -// event loop long before code like this would run. -// Not doing so could cause us to close the connection before all tasks have -// been completed, and that would crash badly. -flush_main_thread_events(); - -// XPCShell doesn't dispatch quit-application, to ensure cleanup we have to -// dispatch it after each test run. -var os = Cc['@mozilla.org/observer-service;1']. - getService(Ci.nsIObserverService); -os.notifyObservers(null, "quit-application-granted", null); -os.notifyObservers(null, "quit-application", null); - -// Run the event loop, since we enqueue some statement finalization. -flush_main_thread_events(); - -// try to close the connection so we can remove places.sqlite -var pip = Cc["@mozilla.org/browser/nav-history-service;1"]. - getService(Ci.nsINavHistoryService). - QueryInterface(Ci.nsPIPlacesDatabase); -if (pip.DBConnection.connectionReady) { - pip.commitPendingChanges(); - pip.finalizeInternalStatements(); - pip.DBConnection.close(); - do_check_false(pip.DBConnection.connectionReady); -} diff --git a/toolkit/components/places/tests/queries/tail_queries.js b/toolkit/components/places/tests/queries/tail_queries.js deleted file mode 100644 index bc8f7042da13..000000000000 --- a/toolkit/components/places/tests/queries/tail_queries.js +++ /dev/null @@ -1,62 +0,0 @@ -/* ***** 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 Places. - * - * The Initial Developer of the Original Code is - * Mozilla.org - * Portions created by the Initial Developer are Copyright (C) 2009 - * the Initial Developer. All Rights Reserved. - * - * Contributor(s): - * Marco Bonardo - * - * 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 ***** */ - -// put cleanup of the bookmarks test here. - -// XPCShell doesn't dispatch quit-application, to ensure cleanup we have to -// dispatch it after each test run. -var os = Cc['@mozilla.org/observer-service;1']. - getService(Ci.nsIObserverService); -os.notifyObservers(null, "quit-application-granted", null); -os.notifyObservers(null, "quit-application", null); - -// Run the event loop to be more like the browser, which normally runs the -// event loop long before code like this would run. -// Not doing so could cause us to close the connection before all tasks have -// been completed, and that would crash badly. -flush_main_thread_events(); - -// try to close the connection so we can remove places.sqlite -var pip = Cc["@mozilla.org/browser/nav-history-service;1"]. - getService(Ci.nsINavHistoryService). - QueryInterface(Ci.nsPIPlacesDatabase); -if (pip.DBConnection.connectionReady) { - pip.commitPendingChanges(); - pip.finalizeInternalStatements(); - pip.DBConnection.close(); - do_check_false(pip.DBConnection.connectionReady); -} diff --git a/toolkit/components/places/tests/sync/head_sync.js b/toolkit/components/places/tests/sync/head_sync.js index 9d0ba3f7ee38..fc6be0bddb34 100644 --- a/toolkit/components/places/tests/sync/head_sync.js +++ b/toolkit/components/places/tests/sync/head_sync.js @@ -154,23 +154,10 @@ function dump_table(aName) stmt = null; } -/** - * This dispatches the observer topic "quit-application" to clean up the sync - * component. - */ -function finish_test() -{ - // xpcshell doesn't dispatch shutdown-application - let os = Cc["@mozilla.org/observer-service;1"]. - getService(Ci.nsIObserverService); - os.notifyObservers(null, "quit-application", null); - do_test_finished(); -} - /** * Function tests to see if the place associated with the bookmark with id - * aBookmarkId has the uri aExpectedURI. The event will call finish_test() if - * aFinish is true. + * aBookmarkId has the uri aExpectedURI. The event will call do_test_finished() + * if aFinish is true. * * @param aBookmarkId * The bookmark to check against. @@ -204,12 +191,12 @@ function new_test_bookmark_uri_event(aBookmarkId, aExpectedURI, aExpected, aFini stmt = null; if (aFinish) - finish_test(); + do_test_finished(); } /** * Function tests to see if the place associated with the visit with id aVisitId - * has the uri aExpectedURI. The event will call finish_test() if aFinish is + * has the uri aExpectedURI. The event will call do_test_finished() if aFinish is * true. * * @param aVisitId @@ -244,7 +231,7 @@ function new_test_visit_uri_event(aVisitId, aExpectedURI, aExpected, aFinish) stmt = null; if (aFinish) - finish_test(); + do_test_finished(); } /** @@ -281,3 +268,14 @@ function flush_main_thread_events() while (tm.mainThread.hasPendingEvents()) tm.mainThread.processNextEvent(false); } + +// Simulates a Places shutdown. +function shutdownPlaces() +{ + const TOPIC_XPCOM_SHUTDOWN = "xpcom-shutdown"; + let hs = Cc["@mozilla.org/browser/nav-history-service;1"]. + getService(Ci.nsIObserver); + hs.observe(null, TOPIC_XPCOM_SHUTDOWN, null); + let sync = Cc["@mozilla.org/places/sync;1"].getService(Ci.nsIObserver); + sync.observe(null, TOPIC_XPCOM_SHUTDOWN, null); +} diff --git a/toolkit/components/places/tests/sync/tail_sync.js b/toolkit/components/places/tests/sync/tail_sync.js deleted file mode 100644 index 820caf6a79bb..000000000000 --- a/toolkit/components/places/tests/sync/tail_sync.js +++ /dev/null @@ -1,65 +0,0 @@ -/* ***** 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 Places. - * - * The Initial Developer of the Original Code is - * Mozilla.org - * Portions created by the Initial Developer are Copyright (C) 2009 - * the Initial Developer. All Rights Reserved. - * - * Contributor(s): - * Marco Bonardo - * - * 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 ***** */ - -// put cleanup of the bookmarks test here. - -// Run the event loop to be more like the browser, which normally runs the -// event loop long before code like this would run. -// Not doing so could cause us to close the connection before all tasks have -// been completed, and that would crash badly. -flush_main_thread_events(); - -// XPCShell doesn't dispatch quit-application, to ensure cleanup we have to -// dispatch it after each test run. -var os = Cc['@mozilla.org/observer-service;1']. - getService(Ci.nsIObserverService); -os.notifyObservers(null, "quit-application-granted", null); -os.notifyObservers(null, "quit-application", null); - -// Run the event loop, since we enqueue some statement finalization. -flush_main_thread_events(); - -// try to close the connection so we can remove places.sqlite -var pip = Cc["@mozilla.org/browser/nav-history-service;1"]. - getService(Ci.nsINavHistoryService). - QueryInterface(Ci.nsPIPlacesDatabase); -if (pip.DBConnection.connectionReady) { - pip.commitPendingChanges(); - pip.finalizeInternalStatements(); - pip.DBConnection.close(); - do_check_false(pip.DBConnection.connectionReady); -} diff --git a/toolkit/components/places/tests/sync/test_bookmarks_sorted_by_none.js b/toolkit/components/places/tests/sync/test_bookmarks_sorted_by_none.js index 048ae6f63a98..9e1e030e3564 100644 --- a/toolkit/components/places/tests/sync/test_bookmarks_sorted_by_none.js +++ b/toolkit/components/places/tests/sync/test_bookmarks_sorted_by_none.js @@ -87,7 +87,7 @@ var observer = { // Cleanup. bs.removeFolderChildren(bs.toolbarFolder); - finish_test(); + do_test_finished(); } } } diff --git a/toolkit/components/places/tests/sync/test_database_sync_after_quit_application.js b/toolkit/components/places/tests/sync/test_database_sync_after_shutdown.js similarity index 97% rename from toolkit/components/places/tests/sync/test_database_sync_after_quit_application.js rename to toolkit/components/places/tests/sync/test_database_sync_after_shutdown.js index 9fa61bf0b9e6..6327ca31b263 100644 --- a/toolkit/components/places/tests/sync/test_database_sync_after_quit_application.js +++ b/toolkit/components/places/tests/sync/test_database_sync_after_shutdown.js @@ -56,6 +56,7 @@ var historyObserver = { onVisit: function(aURI, aVisitId, aTime, aSessionId, aReferringId, aTransitionType, aAdded) { observer.visitId = aVisitId; + hs.removeObserver(this); } } hs.addObserver(historyObserver, false); @@ -64,11 +65,11 @@ var observer = { visitId: -1, observe: function(aSubject, aTopic, aData) { if (aTopic == kSyncFinished) { - // visit id must be valid - do_check_neq(this.visitId, -1); // remove the observer, we don't need to observe sync on quit os.removeObserver(this, kSyncFinished); - hs.removeObserver(historyObserver); + + // visit id must be valid + do_check_neq(this.visitId, -1); // Check that tables have been correctly synced new_test_visit_uri_event(this.visitId, TEST_URI, true, true); } @@ -87,7 +88,8 @@ function run_test() hs.TRANSITION_TYPED, false, 0); // Notify that we are quitting the app - we should sync! - os.notifyObservers(null, "quit-application", null); + shutdownPlaces(); + // Test will continue on sync notification. do_test_pending(); } diff --git a/toolkit/components/places/tests/sync/test_database_sync_after_quit_application_with_removeAllPages.js b/toolkit/components/places/tests/sync/test_database_sync_after_shutdown_with_removeAllPages.js similarity index 55% rename from toolkit/components/places/tests/sync/test_database_sync_after_quit_application_with_removeAllPages.js rename to toolkit/components/places/tests/sync/test_database_sync_after_shutdown_with_removeAllPages.js index f0d0097889db..63fa3a088bc6 100644 --- a/toolkit/components/places/tests/sync/test_database_sync_after_quit_application_with_removeAllPages.js +++ b/toolkit/components/places/tests/sync/test_database_sync_after_shutdown_with_removeAllPages.js @@ -45,90 +45,113 @@ var prefs = Cc["@mozilla.org/preferences-service;1"]. var hs = Cc["@mozilla.org/browser/nav-history-service;1"]. getService(Ci.nsINavHistoryService); var bh = hs.QueryInterface(Ci.nsIBrowserHistory); -var mDBConn = hs.QueryInterface(Ci.nsPIPlacesDatabase).DBConnection; -let bs = Cc["@mozilla.org/browser/nav-bookmarks-service;1"]. +var bs = Cc["@mozilla.org/browser/nav-bookmarks-service;1"]. getService(Ci.nsINavBookmarksService); const TEST_URI = "http://test.com/"; -const kSyncPrefName = "syncDBTableIntervalInSecs"; +const PREF_SYNC_INTERVAL = "syncDBTableIntervalInSecs"; const SYNC_INTERVAL = 600; // ten minutes -const kSyncFinished = "places-sync-finished"; -const kQuitApplication = "quit-application"; +const TOPIC_SYNC_FINISHED = "places-sync-finished"; + +// Polling constants to check the connection closed status. +const POLLING_TIMEOUT_MS = 100; +const POLLING_MAX_PASSES = 20; var historyObserver = { + visitId: -1, + cleared: false, onVisit: function(aURI, aVisitId, aTime, aSessionId, aReferringId, aTransitionType, aAdded) { - observer.visitId = aVisitId; + this.visitId = aVisitId; }, onClearHistory: function() { // check browserHistory returns no entries do_check_eq(0, bh.count); + this.cleared = true; + hs.removeObserver(this); } } hs.addObserver(historyObserver, false); var observer = { - visitId: -1, _runCount: 0, observe: function(aSubject, aTopic, aData) { - if (aTopic == kSyncFinished) { - // the first sync is due to the insert bookmark, timings here are really - // constraint, so it's better adding the observer immediately and discard - // first notification. Adding the observer later could result in random - // test failures due to the first sync being delayed. - if (++this._runCount < 2) + if (aTopic == TOPIC_SYNC_FINISHED) { + if (++this._runCount == 1) { + // The first sync is due to the insert bookmark. + // Simulate a clear private data just before shutdown. + bh.removeAllPages(); + // Immediately notify shutdown. + shutdownPlaces(); return; - // visit id must be valid - do_check_neq(this.visitId, -1); - // remove the observer, we don't need to observe sync on quit - os.removeObserver(this, kSyncFinished); - hs.removeObserver(historyObserver); - // Check that tables have been correctly synced - // Check that frecency for not cleared items (bookmarks) has been converted - // to -MAX(visit_count, 1), so we will be able to recalculate frecency - // starting from most frecent bookmarks. - dump_table("moz_places_temp"); - dump_table("moz_places"); - stmt = mDBConn.createStatement( - "SELECT id FROM moz_places WHERE frecency > 0 LIMIT 1"); - do_check_false(stmt.executeStep()); - stmt.finalize(); + } - stmt = mDBConn.createStatement( - "SELECT h.id FROM moz_places h WHERE h.frecency = -2 " + - "AND EXISTS (SELECT id FROM moz_bookmarks WHERE fk = h.id) LIMIT 1"); - do_check_true(stmt.executeStep()); - stmt.finalize(); + // Remove the observer, we don't need it anymore. + os.removeObserver(this, TOPIC_SYNC_FINISHED); - // Check that all visit_counts have been brought to 0 - stmt = mDBConn.createStatement( - "SELECT id FROM moz_places WHERE visit_count <> 0 LIMIT 1"); - do_check_false(stmt.executeStep()); - stmt.finalize(); + // Visit id must be valid. + do_check_neq(historyObserver.visitId, -1); + // History must have been cleared. + do_check_true(historyObserver.cleared); - finish_test(); - } - else if (aTopic == kQuitApplication) { - // simulate a clear private data on shutdown - bh.removeAllPages(); + // The database connection will be closed after this sync, but we can't + // know how much time it will take, so we use a polling strategy. + do_timeout(POLLING_TIMEOUT_MS, "check_results();"); } } } -os.addObserver(observer, kSyncFinished, false); -os.addObserver(observer, kQuitApplication, false); +os.addObserver(observer, TOPIC_SYNC_FINISHED, false); + +var gPasses = 0; +function check_results() { + if (++gPasses >= POLLING_MAX_PASSES) { + do_throw("Maximum time elapsdes waiting for Places database connection to close"); + do_test_finished(); + } + + if (hs.QueryInterface(Ci.nsPIPlacesDatabase).DBConnection.connectionReady) { + do_timeout(POLLING_TIMEOUT_MS, "check_results();"); + return; + } + + dbConn = DBConn(); + do_check_neq(dbConn, null); + do_check_true(dbConn.connectionReady); + + // Check that frecency for not cleared items (bookmarks) has been + // converted to -MAX(visit_count, 1), so we will be able to + // recalculate frecency starting from most frecent bookmarks. + let stmt = dbConn.createStatement( + "SELECT id FROM moz_places WHERE frecency > 0 LIMIT 1"); + do_check_false(stmt.executeStep()); + stmt.finalize(); + + stmt = DBConn().createStatement( + "SELECT h.id FROM moz_places h WHERE h.frecency = -2 " + + "AND EXISTS (SELECT id FROM moz_bookmarks WHERE fk = h.id) LIMIT 1"); + do_check_true(stmt.executeStep()); + stmt.finalize(); + + // Check that all visit_counts have been brought to 0 + stmt = DBConn().createStatement( + "SELECT id FROM moz_places WHERE visit_count <> 0 LIMIT 1"); + do_check_false(stmt.executeStep()); + stmt.finalize(); + + dbConn.close(); + do_check_false(dbConn.connectionReady); + + do_test_finished(); +} function run_test() { - // Run the event loop to be more like the browser, which normally runs the - // event loop long before code like this would run. - let tm = Cc["@mozilla.org/thread-manager;1"].getService(Ci.nsIThreadManager); - while (tm.mainThread.hasPendingEvents()) - tm.mainThread.processNextEvent(false); + do_test_pending(); // Set the preference for the timer to a really large value, so it won't // run before the test finishes. - prefs.setIntPref(kSyncPrefName, SYNC_INTERVAL); + prefs.setIntPref(PREF_SYNC_INTERVAL, SYNC_INTERVAL); // Now add a visit before creating bookmark, and one later hs.addVisit(uri(TEST_URI), Date.now() * 1000, null, @@ -137,9 +160,4 @@ function run_test() bs.DEFAULT_INDEX, "bookmark"); hs.addVisit(uri(TEST_URI), Date.now() * 1000, null, hs.TRANSITION_TYPED, false, 0); - - // Notify that we are quitting the app - we should sync! - os.notifyObservers(null, kQuitApplication, null); - - do_test_pending(); } diff --git a/toolkit/components/places/tests/sync/test_database_sync_embed_visits.js b/toolkit/components/places/tests/sync/test_database_sync_embed_visits.js index 48f421d7d9bf..2c83d051e5e4 100644 --- a/toolkit/components/places/tests/sync/test_database_sync_embed_visits.js +++ b/toolkit/components/places/tests/sync/test_database_sync_embed_visits.js @@ -132,7 +132,7 @@ var observer = { do_check_false(stmt.executeStep()); stmt.finalize(); - finish_test(); + do_test_finished(); } } } diff --git a/toolkit/components/places/tests/sync/test_database_sync_expireAllFavicons.js b/toolkit/components/places/tests/sync/test_database_sync_expireAllFavicons.js index 34fedc1430af..e6218b448a5d 100644 --- a/toolkit/components/places/tests/sync/test_database_sync_expireAllFavicons.js +++ b/toolkit/components/places/tests/sync/test_database_sync_expireAllFavicons.js @@ -109,7 +109,7 @@ var observer = { stmt.finalize(); - finish_test(); + do_test_finished(); } } } diff --git a/toolkit/components/places/tests/sync/test_database_sync_onitemadded.js b/toolkit/components/places/tests/sync/test_database_sync_onitemadded.js index 075ec1dbd9a7..b2159ad9c9ab 100644 --- a/toolkit/components/places/tests/sync/test_database_sync_onitemadded.js +++ b/toolkit/components/places/tests/sync/test_database_sync_onitemadded.js @@ -68,7 +68,7 @@ var syncObserver = { os.removeObserver(this, kSyncFinished); bs.removeObserver(bookmarksObserver, false); - finish_test(); + do_test_finished(); } } } diff --git a/toolkit/components/places/tests/sync/test_database_sync_with_specialHistoryQueries.js b/toolkit/components/places/tests/sync/test_database_sync_with_specialHistoryQueries.js index b00fb4316691..5b9efaa2fb6e 100644 --- a/toolkit/components/places/tests/sync/test_database_sync_with_specialHistoryQueries.js +++ b/toolkit/components/places/tests/sync/test_database_sync_with_specialHistoryQueries.js @@ -98,7 +98,7 @@ var observer = { root.containerOpen = false; os.removeObserver(this, kSyncFinished); - finish_test(); + do_test_finished(); } } } diff --git a/toolkit/components/places/tests/sync/test_multiple_bookmarks_around_sync.js b/toolkit/components/places/tests/sync/test_multiple_bookmarks_around_sync.js index 222e08c0526b..5e518b497bbb 100644 --- a/toolkit/components/places/tests/sync/test_multiple_bookmarks_around_sync.js +++ b/toolkit/components/places/tests/sync/test_multiple_bookmarks_around_sync.js @@ -122,7 +122,7 @@ var observer = { os.removeObserver(this, kSyncFinished); bs.removeObserver(bookmarksObserver); // test ends here - finish_test(); + do_test_finished(); } else do_throw("Too many places sync calls"); diff --git a/toolkit/components/places/tests/sync/test_multiple_visits_around_sync.js b/toolkit/components/places/tests/sync/test_multiple_visits_around_sync.js index 3c77f6ce9eb7..1b99b09a923e 100644 --- a/toolkit/components/places/tests/sync/test_multiple_visits_around_sync.js +++ b/toolkit/components/places/tests/sync/test_multiple_visits_around_sync.js @@ -110,7 +110,7 @@ var observer = { // remove the observers os.removeObserver(this, kSyncFinished); hs.removeObserver(historyObserver, false); - finish_test(); + do_test_finished(); } else do_throw("bad runCount!"); diff --git a/toolkit/components/places/tests/unit/head_bookmarks.js b/toolkit/components/places/tests/unit/head_bookmarks.js index 3c1f47677b86..eb3f9a95f7f9 100644 --- a/toolkit/components/places/tests/unit/head_bookmarks.js +++ b/toolkit/components/places/tests/unit/head_bookmarks.js @@ -177,26 +177,6 @@ function check_no_bookmarks() { root.containerOpen = false; } -var syncSvc = null; -function start_sync() { -// profile-after-change doesn't create components in xpcshell, so we have to do -// it ourselves - syncSvc = Cc["@mozilla.org/places/sync;1"].getService(Ci.nsISupports); -} - -/** - * This dispatches the observer topic "quit-application" to clean up the sync - * component. - */ -function finish_test() -{ - // xpcshell doesn't dispatch shutdown-application - let os = Cc["@mozilla.org/observer-service;1"]. - getService(Ci.nsIObserverService); - os.notifyObservers(null, "quit-application", null); - do_test_finished(); -} - /** * Flushes any events in the event loop of the main thread. */ diff --git a/toolkit/components/places/tests/unit/tail_bookmarks.js b/toolkit/components/places/tests/unit/tail_bookmarks.js deleted file mode 100644 index 116ba5054cb3..000000000000 --- a/toolkit/components/places/tests/unit/tail_bookmarks.js +++ /dev/null @@ -1,66 +0,0 @@ -/* ***** 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 Places. - * - * The Initial Developer of the Original Code is - * Mozilla.org - * Portions created by the Initial Developer are Copyright (C) 2006 - * the Initial Developer. All Rights Reserved. - * - * Contributor(s): - * Dietrich Ayala - * Marco Bonardo - * - * 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 ***** */ - -// put cleanup of the bookmarks test here. - -// Run the event loop to be more like the browser, which normally runs the -// event loop long before code like this would run. -// Not doing so could cause us to close the connection before all tasks have -// been completed, and that would crash badly. -flush_main_thread_events(); - -// XPCShell doesn't dispatch quit-application, to ensure cleanup we have to -// dispatch it after each test run. -var os = Cc['@mozilla.org/observer-service;1']. - getService(Ci.nsIObserverService); -os.notifyObservers(null, "quit-application-granted", null); -os.notifyObservers(null, "quit-application", null); - -// Run the event loop, since we enqueue some statement finalization. -flush_main_thread_events(); - -// try to close the connection so we can remove places.sqlite -var pip = Cc["@mozilla.org/browser/nav-history-service;1"]. - getService(Ci.nsINavHistoryService). - QueryInterface(Ci.nsPIPlacesDatabase); -if (pip.DBConnection.connectionReady) { - pip.commitPendingChanges(); - pip.finalizeInternalStatements(); - pip.DBConnection.close(); - do_check_false(pip.DBConnection.connectionReady); -} diff --git a/toolkit/components/places/tests/unit/test_248970.js b/toolkit/components/places/tests/unit/test_248970.js index 355fba8f3fb0..17434a8c9a49 100644 --- a/toolkit/components/places/tests/unit/test_248970.js +++ b/toolkit/components/places/tests/unit/test_248970.js @@ -102,13 +102,13 @@ function get_PBSvc() { * @returns the place id for aURI. */ function add_visit(aURI, aType) { - var placeID = histsvc.addVisit(uri(aURI), + var visitId = histsvc.addVisit(uri(aURI), Date.now() * 1000, null, // no referrer aType, false, // not redirect 0); - return placeID; + return visitId; } /** @@ -266,13 +266,10 @@ function is_bookmark_A_altered(){ } function run_test() { - // Fetch the private browsing service var pb = get_PBSvc(); - if(pb) { // Private Browsing might not be available - start_sync(); // enable syncing - + if (pb) { // Private Browsing might not be available // need to catch places sync notifications var os = Cc["@mozilla.org/observer-service;1"]. getService(Ci.nsIObserverService); @@ -293,7 +290,7 @@ function run_test() { // Bookmark-A should be bookmarked, data should be retrievable do_check_true(bmsvc.isBookmarked(bookmark_A_URI)); - do_check_eq("google",bmsvc.getKeywordForURI(bookmark_A_URI)); + do_check_eq("google", bmsvc.getKeywordForURI(bookmark_A_URI)); // Enter Private Browsing Mode pb.privateBrowsingEnabled = true; @@ -355,7 +352,7 @@ function run_test() { } prefBranch.clearUserPref("browser.privatebrowsing.keep_current_session"); - finish_test(); + do_test_finished(); } }; diff --git a/toolkit/components/places/tests/unit/test_expiration.js b/toolkit/components/places/tests/unit/test_expiration.js index 4a3dbd43ff29..dc9b7160aa2d 100644 --- a/toolkit/components/places/tests/unit/test_expiration.js +++ b/toolkit/components/places/tests/unit/test_expiration.js @@ -39,9 +39,6 @@ * * ***** END LICENSE BLOCK ***** */ -// execute this test while syncing, this will potentially show possible problems -start_sync(); - // Get services var histsvc = Cc["@mozilla.org/browser/nav-history-service;1"]. getService(Ci.nsINavHistoryService); @@ -827,5 +824,5 @@ function checkExpireBadPrefs() { do_throw(ex); } dump("done incremental expiration test 6\n"); - finish_test(); + do_test_finished(); } diff --git a/toolkit/components/places/tests/unit/test_history_removeAllPages.js b/toolkit/components/places/tests/unit/test_history_removeAllPages.js index c13a848e0ca2..55ce8938c5a0 100644 --- a/toolkit/components/places/tests/unit/test_history_removeAllPages.js +++ b/toolkit/components/places/tests/unit/test_history_removeAllPages.js @@ -37,9 +37,6 @@ * * ***** END LICENSE BLOCK ***** */ -// Enable syncing for this test -start_sync(); - // Get services. let hs = Cc["@mozilla.org/browser/nav-history-service;1"]. getService(Ci.nsINavHistoryService); @@ -224,7 +221,7 @@ let syncObserver = { do_check_false(stmt.executeStep()); stmt.finalize(); - finish_test(); + do_test_finished(); } } os.addObserver(syncObserver, kSyncFinished, false); diff --git a/toolkit/components/places/tests/unit/test_nsPIPlacesDatabase_commitPendingChanges.js b/toolkit/components/places/tests/unit/test_nsPIPlacesDatabase_commitPendingChanges.js deleted file mode 100644 index 34bcce1ae7c2..000000000000 --- a/toolkit/components/places/tests/unit/test_nsPIPlacesDatabase_commitPendingChanges.js +++ /dev/null @@ -1,75 +0,0 @@ -/* -*- Mode: Java; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ -/* vim:set ts=2 sw=2 sts=2 et: */ -/* ***** 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 Places Unit Test Code. - * - * The Initial Developer of the Original Code is Mozilla Corp. - * Portions created by the Initial Developer are Copyright (C) 2008 - * the Initial Developer. All Rights Reserved. - * - * Contributor(s): - * Marco Bonardo (Original Author) - * - * 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 ***** */ - -var hs = Cc["@mozilla.org/browser/nav-history-service;1"]. - getService(Ci.nsINavHistoryService); -var pip = hs.QueryInterface(Ci.nsPIPlacesDatabase); -var mDBConn = pip.DBConnection; -var gh = hs.QueryInterface(Ci.nsIGlobalHistory2); -var iconsvc = Cc["@mozilla.org/browser/favicon-service;1"]. - getService(Ci.nsIFaviconService); - -const TEST_URI = "http://www.mozilla.org/"; -const TEST_TITLE = "testTitle"; - -// main -function run_test() { - var testURI = uri(TEST_URI); - var faviconURI = uri(TEST_URI + "favicon.ico"); - - // Add a uri lazy message - gh.addURI(testURI, false, true, null); - // Add a favicon lazy message - iconsvc.setFaviconUrlForPage(testURI, faviconURI); - // Add a title lazy message - hs.setPageTitle(testURI, TEST_TITLE); - - // Commit all pending changes (lazy messages) - pip.commitPendingChanges(); - - // Check database values, we can't use APIs because them would check - // lazy queue. - let stmt = mDBConn.createStatement( - "SELECT id FROM moz_places_temp WHERE url = :url AND title = :title " + - "AND favicon_id = (SELECT id FROM moz_favicons WHERE url = :favicon_url)"); - stmt.params["url"] = testURI.spec; - stmt.params["title"] = TEST_TITLE; - stmt.params["favicon_url"] = faviconURI.spec; - do_check_true(stmt.executeStep()); - stmt.finalize(); -} diff --git a/toolkit/components/places/tests/unit/test_nsPIPlacesDatabase_finalizeInternalStatements.js b/toolkit/components/places/tests/unit/test_nsPIPlacesDatabase_finalizeInternalStatements.js deleted file mode 100644 index 329a122357ff..000000000000 --- a/toolkit/components/places/tests/unit/test_nsPIPlacesDatabase_finalizeInternalStatements.js +++ /dev/null @@ -1,55 +0,0 @@ -/* -*- Mode: Java; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ -/* vim:set ts=2 sw=2 sts=2 et: */ -/* ***** 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 Places Unit Test Code. - * - * The Initial Developer of the Original Code is Mozilla Corp. - * Portions created by the Initial Developer are Copyright (C) 2008 - * the Initial Developer. All Rights Reserved. - * - * Contributor(s): - * Marco Bonardo (Original Author) - * - * 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 ***** */ - -// main -function run_test() { - var hs = Cc["@mozilla.org/browser/nav-history-service;1"]. - getService(Ci.nsINavHistoryService); - - // Run the event loop to be more like the browser, which normally runs the - // event loop long before code like this would run. - let tm = Cc["@mozilla.org/thread-manager;1"].getService(Ci.nsIThreadManager); - while (tm.mainThread.hasPendingEvents()) - tm.mainThread.processNextEvent(false); - - var mDBConn = hs.QueryInterface(Ci.nsPIPlacesDatabase).DBConnection; - - hs.QueryInterface(Ci.nsPIPlacesDatabase).finalizeInternalStatements(); - mDBConn.close(); - do_check_false(mDBConn.connectionReady); -}