Bug 1090308 - Invalidate mDaysOfHistory when getObservers is invoked. r=Mano
authorMarco Bonardo <mbonardo@mozilla.com>
Wed, 19 Nov 2014 17:09:04 +0100
changeset 216867 44ebd54f1b9cece005c9d9e8034c1940d727e58b
parent 216866 dc582009548d11d05a5735ff1cc045d7e5ba48e0
child 216868 e8f45cf7b043d717f1b5669b9375b0efafa8356d
push id10095
push usermak77@bonardo.net
push dateFri, 21 Nov 2014 21:35:24 +0000
treeherderfx-team@44ebd54f1b9c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMano
bugs1090308
milestone36.0a1
Bug 1090308 - Invalidate mDaysOfHistory when getObservers is invoked. r=Mano
toolkit/components/places/nsNavHistory.cpp
toolkit/components/places/tests/browser/browser_bug248970.js
toolkit/components/places/tests/unit/test_browserhistory.js
--- a/toolkit/components/places/nsNavHistory.cpp
+++ b/toolkit/components/places/nsNavHistory.cpp
@@ -2260,16 +2260,20 @@ nsNavHistory::GetObservers(uint32_t* _co
                            nsINavHistoryObserver*** _observers)
 {
   NS_ENSURE_ARG_POINTER(_count);
   NS_ENSURE_ARG_POINTER(_observers);
 
   *_count = 0;
   *_observers = nullptr;
 
+  // Clear any cached value, cause it's very likely the consumer has made
+  // changes to history and is now trying to notify them.
+  mDaysOfHistory = -1;
+
   if (!mCanNotify)
     return NS_OK;
 
   nsCOMArray<nsINavHistoryObserver> observers;
 
   // First add the category cache observers.
   mCacheObservers.GetEntries(observers);
 
--- a/toolkit/components/places/tests/browser/browser_bug248970.js
+++ b/toolkit/components/places/tests/browser/browser_bug248970.js
@@ -24,20 +24,16 @@ add_task(function () {
   registerCleanupFunction(function() {
     windowsToClose.forEach(function(win) {
       win.close();
     });
   });
 
   yield promiseClearHistory();
 
-  // History database should be empty
-  is(PlacesUtils.history.hasHistoryEntries, false,
-     "History database should be empty");
-
    // Ensure we wait for the default bookmarks import.
   let bookmarksDeferred = Promise.defer();
   waitForCondition(() => {
     placeItemsCount = getPlacesItemsCount();
     return placeItemsCount > 0
   }, bookmarksDeferred.resolve, "Should have default bookmarks");
   yield bookmarksDeferred.promise;
 
@@ -48,20 +44,16 @@ add_task(function () {
     { uri: visitedURIs[2], transition: TRANSITION_BOOKMARK },
     { uri: visitedURIs[3], transition: TRANSITION_REDIRECT_PERMANENT },
     { uri: visitedURIs[4], transition: TRANSITION_REDIRECT_TEMPORARY },
     { uri: visitedURIs[5], transition: TRANSITION_EMBED },
     { uri: visitedURIs[6], transition: TRANSITION_FRAMED_LINK },
     { uri: visitedURIs[7], transition: TRANSITION_DOWNLOAD }
   ]);
 
-  // History database should have entries
-  is(PlacesUtils.history.hasHistoryEntries, true,
-     "History database should have entries");
-
   placeItemsCount += 7;
   // We added 7 new items to history.
   is(getPlacesItemsCount(), placeItemsCount,
      "Check the total items count");
 
   function* testOnWindow(aIsPrivate, aCount) {
     let deferred = Promise.defer();
     whenNewWindowLoaded({ private: aIsPrivate }, deferred.resolve);
--- a/toolkit/components/places/tests/unit/test_browserhistory.js
+++ b/toolkit/components/places/tests/unit/test_browserhistory.js
@@ -2,124 +2,128 @@
 /* vim:set ts=2 sw=2 sts=2 et: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 const TEST_URI = NetUtil.newURI("http://mozilla.com/");
 const TEST_SUBDOMAIN_URI = NetUtil.newURI("http://foobar.mozilla.com/");
 
-add_test(function test_addPage()
-{
-  promiseAddVisits(TEST_URI).then(function () {
-    do_check_eq(1, PlacesUtils.history.hasHistoryEntries);
-    run_next_test();
-  });
+add_task(function* test_addPage() {
+  yield promiseAddVisits(TEST_URI);
+  do_check_eq(1, PlacesUtils.history.hasHistoryEntries);
 });
 
-add_test(function test_removePage()
-{
+add_task(function* test_removePage() {
   PlacesUtils.bhistory.removePage(TEST_URI);
   do_check_eq(0, PlacesUtils.history.hasHistoryEntries);
-  run_next_test();
 });
 
-add_test(function test_removePages()
-{
+add_task(function* test_removePages() {
   let pages = [];
   for (let i = 0; i < 8; i++) {
     pages.push(NetUtil.newURI(TEST_URI.spec + i));
   }
 
-  promiseAddVisits(pages.map(function (uri) ({ uri: uri }))).then(function () {
-    // Bookmarked item should not be removed from moz_places.
-    const ANNO_INDEX = 1;
-    const ANNO_NAME = "testAnno";
-    const ANNO_VALUE = "foo";
-    const BOOKMARK_INDEX = 2;
-    PlacesUtils.annotations.setPageAnnotation(pages[ANNO_INDEX],
-                                              ANNO_NAME, ANNO_VALUE, 0,
-                                              Ci.nsIAnnotationService.EXPIRE_NEVER);
-    PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
-                                         pages[BOOKMARK_INDEX],
-                                         PlacesUtils.bookmarks.DEFAULT_INDEX,
-                                         "test bookmark");
-    PlacesUtils.annotations.setPageAnnotation(pages[BOOKMARK_INDEX],
-                                              ANNO_NAME, ANNO_VALUE, 0,
-                                              Ci.nsIAnnotationService.EXPIRE_NEVER);
-  
-    PlacesUtils.bhistory.removePages(pages, pages.length);
-    do_check_eq(0, PlacesUtils.history.hasHistoryEntries);
-  
-    // Check that the bookmark and its annotation still exist.
-    do_check_true(PlacesUtils.bookmarks.getIdForItemAt(PlacesUtils.unfiledBookmarksFolderId, 0) > 0);
-    do_check_eq(PlacesUtils.annotations.getPageAnnotation(pages[BOOKMARK_INDEX], ANNO_NAME),
-                ANNO_VALUE);
-  
-    // Check the annotation on the non-bookmarked page does not exist anymore.
-    try {
-      PlacesUtils.annotations.getPageAnnotation(pages[ANNO_INDEX], ANNO_NAME);
-      do_throw("did not expire expire_never anno on a not bookmarked item");
-    } catch(ex) {}
-  
-    // Cleanup.
-    PlacesUtils.bookmarks.removeFolderChildren(PlacesUtils.unfiledBookmarksFolderId);
-    promiseClearHistory().then(run_next_test);
-  });
+  yield promiseAddVisits(pages.map(function (uri) ({ uri: uri })));
+  // Bookmarked item should not be removed from moz_places.
+  const ANNO_INDEX = 1;
+  const ANNO_NAME = "testAnno";
+  const ANNO_VALUE = "foo";
+  const BOOKMARK_INDEX = 2;
+  PlacesUtils.annotations.setPageAnnotation(pages[ANNO_INDEX],
+                                            ANNO_NAME, ANNO_VALUE, 0,
+                                            Ci.nsIAnnotationService.EXPIRE_NEVER);
+  PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
+                                       pages[BOOKMARK_INDEX],
+                                       PlacesUtils.bookmarks.DEFAULT_INDEX,
+                                       "test bookmark");
+  PlacesUtils.annotations.setPageAnnotation(pages[BOOKMARK_INDEX],
+                                            ANNO_NAME, ANNO_VALUE, 0,
+                                            Ci.nsIAnnotationService.EXPIRE_NEVER);
+
+  PlacesUtils.bhistory.removePages(pages, pages.length);
+  do_check_eq(0, PlacesUtils.history.hasHistoryEntries);
+
+  // Check that the bookmark and its annotation still exist.
+  do_check_true(PlacesUtils.bookmarks.getIdForItemAt(PlacesUtils.unfiledBookmarksFolderId, 0) > 0);
+  do_check_eq(PlacesUtils.annotations.getPageAnnotation(pages[BOOKMARK_INDEX], ANNO_NAME),
+              ANNO_VALUE);
+
+  // Check the annotation on the non-bookmarked page does not exist anymore.
+  try {
+    PlacesUtils.annotations.getPageAnnotation(pages[ANNO_INDEX], ANNO_NAME);
+    do_throw("did not expire expire_never anno on a not bookmarked item");
+  } catch(ex) {}
+
+  // Cleanup.
+  PlacesUtils.bookmarks.removeFolderChildren(PlacesUtils.unfiledBookmarksFolderId);
+  yield promiseClearHistory();
 });
 
-add_test(function test_removePagesByTimeframe()
-{
+add_task(function* test_removePagesByTimeframe() {
   let visits = [];
   let startDate = Date.now() * 1000;
   for (let i = 0; i < 10; i++) {
     visits.push({
       uri: NetUtil.newURI(TEST_URI.spec + i),
       visitDate: startDate + i
     });
   }
 
-  promiseAddVisits(visits).then(function () {
-    // Delete all pages except the first and the last.
-    PlacesUtils.bhistory.removePagesByTimeframe(startDate + 1, startDate + 8);
-  
-    // Check that we have removed the correct pages.
-    for (let i = 0; i < 10; i++) {
-      do_check_eq(page_in_database(NetUtil.newURI(TEST_URI.spec + i)) == 0,
-                  i > 0 && i < 9);
-    }
-  
-    // Clear remaining items and check that all pages have been removed.
-    PlacesUtils.bhistory.removePagesByTimeframe(startDate, startDate + 9);
-    do_check_eq(0, PlacesUtils.history.hasHistoryEntries);
-    run_next_test();
+  yield promiseAddVisits(visits);
+
+  // Delete all pages except the first and the last.
+  PlacesUtils.bhistory.removePagesByTimeframe(startDate + 1, startDate + 8);
+
+  // Check that we have removed the correct pages.
+  for (let i = 0; i < 10; i++) {
+    do_check_eq(page_in_database(NetUtil.newURI(TEST_URI.spec + i)) == 0,
+                i > 0 && i < 9);
+  }
+
+  // Clear remaining items and check that all pages have been removed.
+  PlacesUtils.bhistory.removePagesByTimeframe(startDate, startDate + 9);
+  do_check_eq(0, PlacesUtils.history.hasHistoryEntries);
+});
+
+add_task(function* test_removePagesFromHost() {
+  yield promiseAddVisits(TEST_URI);
+  PlacesUtils.bhistory.removePagesFromHost("mozilla.com", true);
+  do_check_eq(0, PlacesUtils.history.hasHistoryEntries);
+});
+
+add_task(function* test_removePagesFromHost_keepSubdomains() {
+  yield promiseAddVisits([{ uri: TEST_URI }, { uri: TEST_SUBDOMAIN_URI }]);
+  PlacesUtils.bhistory.removePagesFromHost("mozilla.com", false);
+  do_check_eq(1, PlacesUtils.history.hasHistoryEntries);
+});
+
+add_task(function* test_removeAllPages() {
+  PlacesUtils.bhistory.removeAllPages();
+  do_check_eq(0, PlacesUtils.history.hasHistoryEntries);
+});
+
+add_task(function* test_getObservers() {
+  // Ensure that getObservers() invalidates the hasHistoryEntries cache.
+  yield promiseAddVisits(TEST_URI);
+  do_check_eq(1, PlacesUtils.history.hasHistoryEntries);
+  // This is just for testing purposes, never do it.
+  return new Promise((resolve, reject) => {
+    DBConn().executeSimpleSQLAsync("DELETE FROM moz_historyvisits", {
+      handleError: function(error) {
+        reject(error);
+      },
+      handleResult: function(result) {
+      },
+      handleCompletion: function(result) {
+        // Just invoking getObservers should be enough to invalidate the cache.
+        PlacesUtils.history.getObservers();
+        do_check_eq(0, PlacesUtils.history.hasHistoryEntries);
+        resolve();
+      }
+    });
   });
 });
 
-add_test(function test_removePagesFromHost()
-{
-  promiseAddVisits(TEST_URI).then(function () {
-    PlacesUtils.bhistory.removePagesFromHost("mozilla.com", true);
-    do_check_eq(0, PlacesUtils.history.hasHistoryEntries);
-    run_next_test();
-  });
-});
-
-add_test(function test_removePagesFromHost_keepSubdomains()
-{
-  promiseAddVisits([{ uri: TEST_URI }, { uri: TEST_SUBDOMAIN_URI }]).then(function () {
-    PlacesUtils.bhistory.removePagesFromHost("mozilla.com", false);
-    do_check_eq(1, PlacesUtils.history.hasHistoryEntries);
-    run_next_test();
-  });
-});
-
-add_test(function test_removeAllPages()
-{
-  PlacesUtils.bhistory.removeAllPages();
-  do_check_eq(0, PlacesUtils.history.hasHistoryEntries);
-  run_next_test();
-});
-
-function run_test()
-{
+function run_test() {
   run_next_test();
 }