diff --git a/toolkit/components/places/Bookmarks.jsm b/toolkit/components/places/Bookmarks.jsm index 2fd775ad56fb..f34ab797ef25 100644 --- a/toolkit/components/places/Bookmarks.jsm +++ b/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; diff --git a/toolkit/components/places/tests/bookmarks/test_bookmarks_eraseEverything.js b/toolkit/components/places/tests/bookmarks/test_bookmarks_eraseEverything.js index ec960ff687ae..600a0621aa0a 100644 --- a/toolkit/components/places/tests/bookmarks/test_bookmarks_eraseEverything.js +++ b/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); diff --git a/toolkit/components/places/tests/bookmarks/test_bookmarks_remove.js b/toolkit/components/places/tests/bookmarks/test_bookmarks_remove.js index d34d7f882b47..4ed559edd058 100644 --- a/toolkit/components/places/tests/bookmarks/test_bookmarks_remove.js +++ b/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() {