Bug 1372936 - onManyFrecenciesChanged doesn't get notified for the new Bookmarks eraseEverything and remove APIs. r=mak

MozReview-Commit-ID: sEsJmfaIHk

--HG--
extra : rebase_source : 4f80e2df4f78d6c2f8f50a5557bb5897c943d5ca

toolkit/components/places/Bookmarks.jsm

@@ -1043,17 +1043,17 @@ var Bookmarks = Object.freeze({
 /**
  * Sends a bookmarks notification through the given observers.
  *
- * @param observers
+ * @param {Array} observers
  *        array of nsINavBookmarkObserver objects.
- * @param notification
+ * @param {String} notification
  *        the notification name.
- * @param args
+ * @param {Array} [args]
  *        array of arguments to pass to the notification.
- * @param information
+ * @param {Object} [information]
  *        Information about the notification, so we can filter based
  *        based on the observer's preferences.
  */
-function notify(observers, notification, args, information = {}) {
+function notify(observers, notification, args = [], information = {}) {
   for (let observer of observers) {
     if (information.isTagging && observer.skipTags) {
       continue;

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

@@ -1,6 +1,19 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
+function promiseManyFrecenciesChanged() {
+  return new Promise(resolve => {
+    let obs = new NavHistoryObserver();
+    obs.onManyFrecenciesChanged = () => {
+      Assert.ok(true, "onManyFrecenciesChanged is triggered.");
+      PlacesUtils.history.removeObserver(obs)
+      resolve();
+    };
+
+    PlacesUtils.history.addObserver(obs);
+  });
+}
+
 add_task(async function test_eraseEverything() {
   await PlacesTestUtils.addVisits({ uri: NetUtil.newURI("http://example.com/") });
   await PlacesTestUtils.addVisits({ uri: NetUtil.newURI("http://mozilla.org/") });
@@ -57,8 +70,13 @@ add_task(async function test_eraseEverything() {
   Assert.ok(frecencyForUrl("http://example.com/") > frecencyForExample);
   Assert.ok(frecencyForUrl("http://example.com/") > frecencyForMozilla);
 
+  let manyFrecenciesPromise = promiseManyFrecenciesChanged();
+
   await PlacesUtils.bookmarks.eraseEverything();
 
+  // Ensure we get an onManyFrecenciesChanged notification.
+  await manyFrecenciesPromise;
+
   Assert.equal(frecencyForUrl("http://example.com/"), frecencyForExample);
   Assert.equal(frecencyForUrl("http://example.com/"), frecencyForMozilla);
 

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

@@ -1,6 +1,39 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
+const UNVISITED_BOOKMARK_BONUS = 140;
+
+function promiseFrecencyChanged(expectedURI, expectedFrecency) {
+  return new Promise(resolve => {
+    let obs = new NavHistoryObserver();
+    obs.onFrecencyChanged = (uri, newFrecency) => {
+      Assert.equal(uri.spec, expectedURI, "onFrecencyChanged is triggered for the correct uri.");
+      Assert.equal(newFrecency, expectedFrecency, "onFrecencyChanged has the expected frecency");
+      PlacesUtils.history.removeObserver(obs)
+      resolve();
+    };
+
+    PlacesUtils.history.addObserver(obs);
+  });
+}
+
+function promiseManyFrecenciesChanged() {
+  return new Promise(resolve => {
+    let obs = new NavHistoryObserver();
+    obs.onManyFrecenciesChanged = () => {
+      Assert.ok(true, "onManyFrecenciesChanged is triggered.");
+      PlacesUtils.history.removeObserver(obs)
+      resolve();
+    };
+
+    PlacesUtils.history.addObserver(obs);
+  });
+}
+
+add_task(async function setup() {
+  Services.prefs.setIntPref("places.frecency.unvisitedBookmarkBonus", UNVISITED_BOOKMARK_BONUS);
+});
+
 add_task(async function invalid_input_throws() {
   Assert.throws(() => PlacesUtils.bookmarks.remove(),
                 /Input should be a valid object/);
@@ -66,12 +99,21 @@ add_task(async function remove_normal_folder_under_root_succeeds() {
 });
 
 add_task(async function remove_bookmark() {
+  // When removing a bookmark we need to check the frecency. First we confirm
+  // that there is a normal update when it is inserted.
+  let frecencyChangedPromise = promiseFrecencyChanged("http://example.com/",
+    UNVISITED_BOOKMARK_BONUS);
   let bm1 = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
                                                  type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
                                                  url: "http://example.com/",
                                                  title: "a bookmark" });
   checkBookmarkObject(bm1);
 
+  await frecencyChangedPromise;
+
+  // This second one checks the frecency is changed when we remove the bookmark.
+  frecencyChangedPromise = promiseFrecencyChanged("http://example.com/", 0);
+
   let bm2 = await PlacesUtils.bookmarks.remove(bm1.guid);
   checkBookmarkObject(bm2);
 
@@ -82,6 +124,8 @@ add_task(async function remove_bookmark() {
   Assert.equal(bm2.type, PlacesUtils.bookmarks.TYPE_BOOKMARK);
   Assert.equal(bm2.url.href, "http://example.com/");
   Assert.equal(bm2.title, "a bookmark");
+
+  await frecencyChangedPromise;
 });
 
 
@@ -130,6 +174,8 @@ add_task(async function remove_folder() {
                                                  title: "a folder" });
   checkBookmarkObject(bm1);
 
+  let manyFrencenciesPromise = promiseManyFrecenciesChanged();
+
   let bm2 = await PlacesUtils.bookmarks.remove(bm1.guid);
   checkBookmarkObject(bm2);
 
@@ -140,6 +186,10 @@ add_task(async function remove_folder() {
   Assert.equal(bm2.type, PlacesUtils.bookmarks.TYPE_FOLDER);
   Assert.equal(bm2.title, "a folder");
   Assert.ok(!("url" in bm2));
+
+  // We should get an onManyFrecenciesChanged notification with the remove of
+  // a folder.
+  await manyFrencenciesPromise;
 });
 
 add_task(async function test_nested_contents_removed() {