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

@@ -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 pref must be set here because SessionStore will use its value
+        // on quit-application.
         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._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();
         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
+        // Stop observing, so further attempts to load history service
-        // do not show the prompt.
+        // 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

browser/components/places/tests/unit/tail_bookmarks.js

@@ -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 <dietrich@mozilla.com>
- *
- * 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);
-}

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.
+  // Clean up database from all bookmarks.
-  os.notifyObservers(null, "quit-application-granted", null);
+  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();
 }
 

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();
   }
 });
 

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();
     }
   };

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();
 }

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 = [

toolkit/components/places/public/nsINavHistoryService.idl

@@ -40,8 +40,8 @@
  * ***** END LICENSE BLOCK ***** */
 
 /**
- * Using Places services on quit-application or later is not reliable, so make
+ * Using Places services after quit-application is not reliable, so make
- * sure to do shutdown work on quit-application-granted, or history
+ * sure to do any shutdown work on quit-application, or history
  * synchronization could fail, losing latest changes.
  */
 

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();
 };

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<nsIFile> 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<nsIObserverService> os =
-    do_GetService("@mozilla.org/observer-service;1");
-  NS_ENSURE_TRUE(os, NS_ERROR_FAILURE);
-  nsCOMPtr<nsISimpleEnumerator> e;
-  nsresult rv = os->EnumerateObservers(PLACES_INIT_COMPLETE_TOPIC,
-                                       getter_AddRefs(e));
-  if (NS_SUCCEEDED(rv) && e) {
-    nsCOMPtr<nsIObserver> 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) {
+  if (strcmp(aTopic, gXpcomShutdown) == 0) {
-    nsresult rv;
+    nsCOMPtr<nsIObserverService> 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<nsISimpleEnumerator> e;
+    nsresult rv = os->EnumerateObservers(PLACES_INIT_COMPLETE_TOPIC,
+                                         getter_AddRefs(e));
+    if (NS_SUCCEEDED(rv) && e) {
+      nsCOMPtr<nsIObserver> 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<nsIPrefService> prefService =
-      do_GetService(NS_PREFSERVICE_CONTRACTID, &rv);
+      do_GetService(NS_PREFSERVICE_CONTRACTID);
-    if (NS_SUCCEEDED(rv))
+    if (prefService)
       prefService->SavePrefFile(nsnull);
 
     // Start shutdown expiration.
     mExpire->OnQuit();
+
+#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;
     }
-  else if (strcmp(aTopic, gXpcomShutdown) == 0) {
+#endif
-    nsresult rv;
+
-    nsCOMPtr<nsIObserverService> observerService =
+    // Finalize all statements.
-      do_GetService("@mozilla.org/observer-service;1", &rv);
+    rv = FinalizeInternalStatements();
     NS_ENSURE_SUCCESS(rv, rv);
-    observerService->RemoveObserver(this, NS_PRIVATE_BROWSING_SWITCH_TOPIC);
+
-    observerService->RemoveObserver(this, gIdleDaily);
+    // Remove categories.
-    observerService->RemoveObserver(this, gXpcomShutdown);
+    nsCOMPtr<nsICategoryManager> catMan =
-    observerService->RemoveObserver(this, gQuitApplicationGrantedMessage);
+      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
+NS_HIDDEN_(void)
-nsNavHistory::CommitLazyMessages()
+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

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<nsNavHistoryQuery>& aQueries, 

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) {
+    if (aTopic == kXPComShutdown) {
-      this._os.removeObserver(this, kQuitApplication);
+      this._os.removeObserver(this, kXPComShutdown);
 
       // Remove our preference observer.
       this._prefs.removeObserver("", this);

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) {
+    if (aTopic == kXPComShutdown) {
-      this._os.removeObserver(this, kQuitApplication);
+      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);
       }
+
+      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"].
+          // Flush any remaining change to disk tables.
-                    getService(Ci.nsPIPlacesDatabase);
-          pip.commitPendingChanges();
           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

toolkit/components/places/tests/autocomplete/tail_autocomplete.js

@@ -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 <mak77@bonardo.net>
- *
- * 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);
-}

toolkit/components/places/tests/bookmarks/tail_bookmarks.js

@@ -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 <dietrich@mozilla.com>
- *  Marco Bonardo <mak77@bonardo.net>
- *
- * 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);
-}

toolkit/components/places/tests/queries/tail_queries.js

@@ -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 <mak77@bonardo.net>
- *
- * 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);
-}

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
+ * aBookmarkId has the uri aExpectedURI.  The event will call do_test_finished()
- * aFinish is true.
+ *  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);
+}

toolkit/components/places/tests/sync/tail_sync.js

@@ -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 <mak77@bonardo.net>
- *
- * 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);
-}

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();
     }
   }
 }

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();
 }

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;
+var bs = Cc["@mozilla.org/browser/nav-bookmarks-service;1"].
-let 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 TOPIC_SYNC_FINISHED = "places-sync-finished";
-const kQuitApplication = "quit-application";
+
+// 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) {
+    if (aTopic == TOPIC_SYNC_FINISHED) {
-      // the first sync is due to the insert bookmark, timings here are really
+      if (++this._runCount == 1) {
-      // constraint, so it's better adding the observer immediately and discard
+        // The first sync is due to the insert bookmark.
-      // first notification. Adding the observer later could result in random
+        // Simulate a clear private data just before shutdown.
-      // test failures due to the first sync being delayed.
+        bh.removeAllPages();
-      if (++this._runCount < 2)
+        // 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
+      // Remove the observer, we don't need it anymore.
-      os.removeObserver(this, kSyncFinished);
+      os.removeObserver(this, TOPIC_SYNC_FINISHED);
-      hs.removeObserver(historyObserver);
+
-      // Check that tables have been correctly synced
+      // Visit id must be valid.
-      // Check that frecency for not cleared items (bookmarks) has been converted
+      do_check_neq(historyObserver.visitId, -1);
-      // to -MAX(visit_count, 1), so we will be able to recalculate frecency
+      // History must have been cleared.
-      // starting from most frecent bookmarks. 
+      do_check_true(historyObserver.cleared);
-      dump_table("moz_places_temp");
+
-      dump_table("moz_places");
+      // The database connection will be closed after this sync, but we can't
-      stmt = mDBConn.createStatement(
+      // know how much time it will take, so we use a polling strategy.
+      do_timeout(POLLING_TIMEOUT_MS, "check_results();");
+    }
+  }
+}
+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 = mDBConn.createStatement(
+    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 = mDBConn.createStatement(
+    stmt = DBConn().createStatement(
       "SELECT id FROM moz_places WHERE visit_count <> 0 LIMIT 1");
     do_check_false(stmt.executeStep());
     stmt.finalize();
 
-      finish_test();
+    dbConn.close();
-    }
+    do_check_false(dbConn.connectionReady);
-    else if (aTopic == kQuitApplication) {
+
-      // simulate a clear private data on shutdown
+    do_test_finished();
-      bh.removeAllPages();
-    }
-  }
 }
-os.addObserver(observer, kSyncFinished, false);
-os.addObserver(observer, kQuitApplication, false);
 
 function run_test()
 {
-  // Run the event loop to be more like the browser, which normally runs the
+  do_test_pending();
-  // 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);
 
   // 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();
 }

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();
     }
   }
 }

toolkit/components/places/tests/sync/test_database_sync_expireAllFavicons.js

@@ -109,7 +109,7 @@ var observer = {
 
       stmt.finalize();
 
-      finish_test();
+      do_test_finished();
     }
   }
 }

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();
     }
   }
 }

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();
     }
   }
 }

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");

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!");

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.
  */

toolkit/components/places/tests/unit/tail_bookmarks.js

@@ -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 <dietrich@mozilla.com>
- *  Marco Bonardo <mak77@bonardo.net>
- *
- * 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);
-}

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
+  if (pb) { // Private Browsing might not be available
-    start_sync(); // enable syncing
-
     // 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();
       }
     };
 

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();
 }

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);

toolkit/components/places/tests/unit/test_nsPIPlacesDatabase_commitPendingChanges.js

@@ -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 <mak77bonardo.net> (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();
-}

toolkit/components/places/tests/unit/test_nsPIPlacesDatabase_finalizeInternalStatements.js

@@ -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 <mak77bonardo.net> (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);
-}