Bug 1090308 - Invalidate mDaysOfHistory when getObservers is invoked. r=Mano

--HG--
extra : rebase_source : 83b6c6aaaf1605b469a5023c01c909b9dbca46d4

toolkit/components/places/nsNavHistory.cpp

@@ -2265,6 +2265,10 @@ nsNavHistory::GetObservers(uint32_t* _count,
   *_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;
 

toolkit/components/places/tests/browser/browser_bug248970.js

@@ -29,10 +29,6 @@ add_task(function () {
 
   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(() => {
@@ -53,10 +49,6 @@ add_task(function () {
     { 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,

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

@@ -7,67 +7,59 @@
 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);
+  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);
+  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 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) {}
+  // 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);
-  });
+  // 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++) {
@@ -77,49 +69,61 @@ add_test(function test_removePagesByTimeframe()
     });
   }
 
-  promiseAddVisits(visits).then(function () {
-    // Delete all pages except the first and the last.
-    PlacesUtils.bhistory.removePagesByTimeframe(startDate + 1, startDate + 8);
+  yield promiseAddVisits(visits);
 
-    // 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);
-    }
+  // Delete all pages except the first and the last.
+  PlacesUtils.bhistory.removePagesByTimeframe(startDate + 1, startDate + 8);
 
-    // 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();
-  });
+  // 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_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_task(function* test_removePagesFromHost() {
+  yield promiseAddVisits(TEST_URI);
+  PlacesUtils.bhistory.removePagesFromHost("mozilla.com", true);
+  do_check_eq(0, PlacesUtils.history.hasHistoryEntries);
 });
 
-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_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_test(function test_removeAllPages()
-{
+add_task(function* test_removeAllPages() {
   PlacesUtils.bhistory.removeAllPages();
   do_check_eq(0, PlacesUtils.history.hasHistoryEntries);
-  run_next_test();
 });
 
-function run_test()
-{
+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();
+      }
+    });
+  });
+});
+
+function run_test() {
   run_next_test();
 }