Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-12-11 08:13:35 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1511700 - Use the new notification system (PlacesObserver) for bookmark removed notifications. r=Standard8,mak

Browse Source 
Phasing out the old notification system for OnItemRemoved events.

Differential Revision: https://phabricator.services.mozilla.com/D17753

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Siddhant085 2020-01-13 16:25:39 +00:00
parent 6bd7bafb72
commit d48342d7db
52 changed files with 1285 additions and 988 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

48
browser/base/content/browser-places.js
View File

@ -1602,7 +1602,7 @@ var BookmarkingUI = {
if (this._hasBookmarksObserver) {
PlacesUtils.bookmarks.removeObserver(this);
PlacesUtils.observers.removeListener(
["bookmark-added"],
["bookmark-added", "bookmark-removed"],
this.handlePlacesEvents
);
}
@ -1653,7 +1653,7 @@ var BookmarkingUI = {
PlacesUtils.bookmarks.addObserver(this);
this.handlePlacesEvents = this.handlePlacesEvents.bind(this);
PlacesUtils.observers.addListener(
["bookmark-added"],
["bookmark-added", "bookmark-removed"],
this.handlePlacesEvents
);
this._hasBookmarksObserver = true;
@ -1946,28 +1946,30 @@ var BookmarkingUI = {
},
handlePlacesEvents(aEvents) {
// Only need to update the UI if it wasn't marked as starred before:
if (this._itemGuids.size == 0) {
for (let { url, guid } of aEvents) {
if (url && url == this._uri.spec) {
// If a new bookmark has been added to the tracked uri, register it.
if (!this._itemGuids.has(guid)) {
this._itemGuids.add(guid);
this._updateStar();
for (let ev of aEvents) {
switch (ev.type) {
case "bookmark-added":
// Only need to update the UI if it wasn't marked as starred before:
if (this._itemGuids.size == 0) {
if (ev.url && ev.url == this._uri.spec) {
// If a new bookmark has been added to the tracked uri, register it.
if (!this._itemGuids.has(ev.guid)) {
this._itemGuids.add(ev.guid);
this._updateStar();
}
}
}
}
}
}
},
// nsINavBookmarkObserver
onItemRemoved(aItemId, aParentId, aIndex, aItemType, aURI, aGuid) {
// If one of the tracked bookmarks has been removed, unregister it.
if (this._itemGuids.has(aGuid)) {
this._itemGuids.delete(aGuid);
// Only need to update the UI if the page is no longer starred
if (this._itemGuids.size == 0) {
this._updateStar();
break;
case "bookmark-removed":
// If one of the tracked bookmarks has been removed, unregister it.
if (this._itemGuids.has(ev.guid)) {
this._itemGuids.delete(ev.guid);
// Only need to update the UI if the page is no longer starred
if (this._itemGuids.size == 0) {
this._updateStar();
}
}
break;
}
}
},

5
browser/base/content/test/general/browser_bug581253.js
View File

@ -55,8 +55,9 @@ add_task(async function test_remove_bookmark_with_tag_via_edit_bookmark() {
);
let removeNotification = PlacesTestUtils.waitForNotification(
"onItemRemoved",
(id, parentId, index, type, itemUrl) => testURL == unescape(itemUrl.spec)
"bookmark-removed",
events => events.some(event => unescape(event.url) == testURL),
"places"
);
let removeButton = document.getElementById("editBookmarkPanelRemoveButton");

5
browser/base/content/test/pageActions/browser_page_action_menu.js
View File

@ -97,8 +97,9 @@ add_task(async function bookmark() {
});
let onItemRemovedPromise = PlacesTestUtils.waitForNotification(
"onItemRemoved",
(id, parentId, index, type, itemUrl) => url == itemUrl.spec
"bookmark-removed",
events => events.some(event => event.url == url),
"places"
);
// Click the remove-bookmark button in the panel.

42
browser/components/enterprisepolicies/tests/browser/browser_policy_bookmarks.js
View File

@ -84,23 +84,32 @@ async function promiseAllChangesMade({ itemsToAdd, itemsToRemove }) {
return new Promise(resolve => {
let listener = events => {
is(events.length, 1, "Should only have 1 event.");
itemsToAdd--;
if (itemsToAdd == 0 && itemsToRemove == 0) {
PlacesUtils.bookmarks.removeObserver(bmObserver);
PlacesUtils.observers.removeListener(["bookmark-added"], listener);
resolve();
switch (events[0].type) {
case "bookmark-added":
itemsToAdd--;
if (itemsToAdd == 0 && itemsToRemove == 0) {
PlacesUtils.bookmarks.removeObserver(bmObserver);
PlacesUtils.observers.removeListener(
["bookmark-added", "bookmark-removed"],
listener
);
resolve();
}
break;
case "bookmark-removed":
itemsToRemove--;
if (itemsToAdd == 0 && itemsToRemove == 0) {
PlacesUtils.bookmarks.removeObserver(bmObserver);
PlacesUtils.observers.removeListener(
["bookmark-added", "bookmark-removed"],
listener
);
resolve();
}
break;
}
};
let bmObserver = {
onItemRemoved() {
itemsToRemove--;
if (itemsToAdd == 0 && itemsToRemove == 0) {
PlacesUtils.bookmarks.removeObserver(bmObserver);
PlacesUtils.observers.removeListener(["bookmark-added"], listener);
resolve();
}
},
onBeginUpdateBatch() {},
onEndUpdateBatch() {},
onItemChanged() {},
@ -108,7 +117,10 @@ async function promiseAllChangesMade({ itemsToAdd, itemsToRemove }) {
onItemMoved() {},
};
PlacesUtils.bookmarks.addObserver(bmObserver);
PlacesUtils.observers.addListener(["bookmark-added"], listener);
PlacesUtils.observers.addListener(
["bookmark-added", "bookmark-removed"],
listener
);
});
}

69
browser/components/extensions/parent/ext-bookmarks.js
View File

@ -122,7 +122,6 @@ let observer = new (class extends EventEmitter {
super();
this.skipTags = true;
this.skipDescendantsOnItemRemoval = true;
this.handlePlacesEvents = this.handlePlacesEvents.bind(this);
}
@ -132,24 +131,44 @@ let observer = new (class extends EventEmitter {
handlePlacesEvents(events) {
for (let event of events) {
if (event.isTagging) {
continue;
}
let bookmark = {
id: event.guid,
parentId: event.parentGuid,
index: event.index,
title: event.title,
dateAdded: event.dateAdded,
type: BOOKMARKS_TYPES_TO_API_TYPES_MAP.get(event.itemType),
url: getUrl(event.itemType, event.url),
};
switch (event.type) {
case "bookmark-added":
if (event.isTagging) {
continue;
}
let bookmark = {
id: event.guid,
parentId: event.parentGuid,
index: event.index,
title: event.title,
dateAdded: event.dateAdded,
type: BOOKMARKS_TYPES_TO_API_TYPES_MAP.get(event.itemType),
url: getUrl(event.itemType, event.url),
};
if (event.itemType == TYPE_FOLDER) {
bookmark.dateGroupModified = bookmark.dateAdded;
}
if (event.itemType == TYPE_FOLDER) {
bookmark.dateGroupModified = bookmark.dateAdded;
}
this.emit("created", bookmark);
this.emit("created", bookmark);
break;
case "bookmark-removed":
if (event.isTagging || event.isDescendantRemoval) {
continue;
}
let node = {
id: event.guid,
parentId: event.parentGuid,
index: event.index,
type: BOOKMARKS_TYPES_TO_API_TYPES_MAP.get(event.itemType),
url: getUrl(event.itemType, event.url),
};
this.emit("removed", {
guid: event.guid,
info: { parentId: event.parentGuid, index: event.index, node },
});
}
}
}
@ -176,18 +195,6 @@ let observer = new (class extends EventEmitter {
this.emit("moved", { guid, info });
}
onItemRemoved(id, parentId, index, itemType, uri, guid, parentGuid, source) {
let node = {
id: guid,
parentId: parentGuid,
index,
type: BOOKMARKS_TYPES_TO_API_TYPES_MAP.get(itemType),
url: getUrl(itemType, uri && uri.spec),
};
this.emit("removed", { guid, info: { parentId: parentGuid, index, node } });
}
onItemChanged(
id,
prop,
@ -220,7 +227,7 @@ const decrementListeners = () => {
if (!listenerCount) {
PlacesUtils.bookmarks.removeObserver(observer);
PlacesUtils.observers.removeListener(
["bookmark-added"],
["bookmark-added", "bookmark-removed"],
observer.handlePlacesEvents
);
}
@ -231,7 +238,7 @@ const incrementListeners = () => {
if (listenerCount == 1) {
PlacesUtils.bookmarks.addObserver(observer);
PlacesUtils.observers.addListener(
["bookmark-added"],
["bookmark-added", "bookmark-removed"],
observer.handlePlacesEvents
);
}

99
browser/components/newtab/lib/PlacesFeed.jsm
View File

@ -95,34 +95,6 @@ class BookmarksObserver extends Observer {
this.skipTags = true;
}
/**
* onItemRemoved - Called when a bookmark is removed
*
* @param {str} id
* @param {str} folderId
* @param {int} index
* @param {int} type Indicates if the bookmark is an actual bookmark,
* a folder, or a separator.
* @param {str} uri
* @param {str} guid The unique id of the bookmark
*/
// eslint-disable-next-line max-params
onItemRemoved(id, folderId, index, type, uri, guid, parentGuid, source) {
if (
type === PlacesUtils.bookmarks.TYPE_BOOKMARK &&
source !== PlacesUtils.bookmarks.SOURCES.IMPORT &&
source !== PlacesUtils.bookmarks.SOURCES.RESTORE &&
source !== PlacesUtils.bookmarks.SOURCES.RESTORE_ON_STARTUP &&
source !== PlacesUtils.bookmarks.SOURCES.SYNC
) {
this.dispatch({ type: at.PLACES_LINKS_CHANGED });
this.dispatch({
type: at.PLACES_BOOKMARK_REMOVED,
data: { url: uri.spec, bookmarkGuid: guid },
});
}
}
// Empty functions to make xpconnect happy
onBeginUpdateBatch() {}
@ -155,31 +127,52 @@ class PlacesObserver extends Observer {
title,
url,
isTagging,
type,
} of events) {
// Skips items that are not bookmarks (like folders), about:* pages or
// default bookmarks, added when the profile is created.
if (
isTagging ||
itemType !== PlacesUtils.bookmarks.TYPE_BOOKMARK ||
source === PlacesUtils.bookmarks.SOURCES.IMPORT ||
source === PlacesUtils.bookmarks.SOURCES.RESTORE ||
source === PlacesUtils.bookmarks.SOURCES.RESTORE_ON_STARTUP ||
source === PlacesUtils.bookmarks.SOURCES.SYNC ||
(!url.startsWith("http://") && !url.startsWith("https://"))
) {
return;
}
switch (type) {
case "bookmark-added":
// Skips items that are not bookmarks (like folders), about:* pages or
// default bookmarks, added when the profile is created.
if (
isTagging ||
itemType !== PlacesUtils.bookmarks.TYPE_BOOKMARK ||
source === PlacesUtils.bookmarks.SOURCES.IMPORT ||
source === PlacesUtils.bookmarks.SOURCES.RESTORE ||
source === PlacesUtils.bookmarks.SOURCES.RESTORE_ON_STARTUP ||
source === PlacesUtils.bookmarks.SOURCES.SYNC ||
(!url.startsWith("http://") && !url.startsWith("https://"))
) {
return;
}
this.dispatch({ type: at.PLACES_LINKS_CHANGED });
this.dispatch({
type: at.PLACES_BOOKMARK_ADDED,
data: {
bookmarkGuid: guid,
bookmarkTitle: title,
dateAdded: dateAdded * 1000,
url,
},
});
this.dispatch({ type: at.PLACES_LINKS_CHANGED });
this.dispatch({
type: at.PLACES_BOOKMARK_ADDED,
data: {
bookmarkGuid: guid,
bookmarkTitle: title,
dateAdded: dateAdded * 1000,
url,
},
});
break;
case "bookmark-removed":
if (
isTagging ||
(itemType === PlacesUtils.bookmarks.TYPE_BOOKMARK &&
source !== PlacesUtils.bookmarks.SOURCES.IMPORT &&
source !== PlacesUtils.bookmarks.SOURCES.RESTORE &&
source !== PlacesUtils.bookmarks.SOURCES.RESTORE_ON_STARTUP &&
source !== PlacesUtils.bookmarks.SOURCES.SYNC)
) {
this.dispatch({ type: at.PLACES_LINKS_CHANGED });
this.dispatch({
type: at.PLACES_BOOKMARK_REMOVED,
data: { url, bookmarkGuid: guid },
});
}
break;
}
}
}
}
@ -202,7 +195,7 @@ class PlacesFeed {
.getService(Ci.nsINavBookmarksService)
.addObserver(this.bookmarksObserver, true);
PlacesUtils.observers.addListener(
["bookmark-added"],
["bookmark-added", "bookmark-removed"],
this.placesObserver.handlePlacesEvent
);
@ -246,7 +239,7 @@ class PlacesFeed {
PlacesUtils.history.removeObserver(this.historyObserver);
PlacesUtils.bookmarks.removeObserver(this.bookmarksObserver);
PlacesUtils.observers.removeListener(
["bookmark-added"],
["bookmark-added", "bookmark-removed"],
this.placesObserver.handlePlacesEvent
);
Services.obs.removeObserver(this, LINK_BLOCKED_EVENT);

233
browser/components/newtab/test/unit/lib/PlacesFeed.test.js
View File

@ -111,7 +111,7 @@ describe("PlacesFeed", () => {
);
assert.calledWith(
global.PlacesUtils.observers.addListener,
["bookmark-added"],
["bookmark-added", "bookmark-removed"],
feed.placesObserver.handlePlacesEvent
);
assert.calledWith(global.Services.obs.addObserver, feed, BLOCKED_EVENT);
@ -133,7 +133,7 @@ describe("PlacesFeed", () => {
);
assert.calledWith(
global.PlacesUtils.observers.removeListener,
["bookmark-added"],
["bookmark-added", "bookmark-removed"],
feed.placesObserver.handlePlacesEvent
);
assert.calledWith(
@ -708,10 +708,10 @@ describe("PlacesFeed", () => {
"",
SOURCES.DEFAULT,
];
await feed.bookmarksObserver.onItemRemoved(...args);
await feed.bookmarksObserver.onItemRemoved(...args);
await feed.bookmarksObserver.onItemRemoved(...args);
await feed.bookmarksObserver.onItemRemoved(...args);
await feed.bookmarksObserver.handlePlacesEvent(args);
await feed.bookmarksObserver.handlePlacesEvent(args);
await feed.bookmarksObserver.handlePlacesEvent(args);
await feed.bookmarksObserver.handlePlacesEvent(args);
assert.calledOnce(
feed.store.dispatch.withArgs(
@ -733,13 +733,13 @@ describe("PlacesFeed", () => {
});
describe("PlacesObserver", () => {
let dispatch;
let observer;
beforeEach(() => {
dispatch = sandbox.spy();
observer = new PlacesObserver(dispatch);
});
describe("#bookmark-added", () => {
let dispatch;
let observer;
beforeEach(() => {
dispatch = sandbox.spy();
observer = new PlacesObserver(dispatch);
});
it("should dispatch a PLACES_BOOKMARK_ADDED action with the bookmark data - http", async () => {
const args = [
{
@ -774,6 +774,7 @@ describe("PlacesFeed", () => {
title: FAKE_BOOKMARK.bookmarkTitle,
url: "https://www.foo.com",
isTagging: false,
type: "bookmark-added",
},
];
await observer.handlePlacesEvent(args);
@ -798,6 +799,7 @@ describe("PlacesFeed", () => {
title: FAKE_BOOKMARK.bookmarkTitle,
url: "foo.com",
isTagging: false,
type: "bookmark-added",
},
];
await observer.handlePlacesEvent(args);
@ -814,6 +816,7 @@ describe("PlacesFeed", () => {
title: FAKE_BOOKMARK.bookmarkTitle,
url: "foo.com",
isTagging: false,
type: "bookmark-added",
},
];
await observer.handlePlacesEvent(args);
@ -830,6 +833,7 @@ describe("PlacesFeed", () => {
title: FAKE_BOOKMARK.bookmarkTitle,
url: "foo.com",
isTagging: false,
type: "bookmark-added",
},
];
await observer.handlePlacesEvent(args);
@ -846,6 +850,7 @@ describe("PlacesFeed", () => {
title: FAKE_BOOKMARK.bookmarkTitle,
url: "foo.com",
isTagging: false,
type: "bookmark-added",
},
];
await observer.handlePlacesEvent(args);
@ -862,6 +867,7 @@ describe("PlacesFeed", () => {
title: FAKE_BOOKMARK.bookmarkTitle,
url: "foo.com",
isTagging: false,
type: "bookmark-added",
},
];
await observer.handlePlacesEvent(args);
@ -878,6 +884,7 @@ describe("PlacesFeed", () => {
title: FAKE_BOOKMARK.bookmarkTitle,
url: "https://www.foo.com",
isTagging: false,
type: "bookmark-added",
},
];
await observer.handlePlacesEvent(args);
@ -885,6 +892,117 @@ describe("PlacesFeed", () => {
assert.notCalled(dispatch);
});
});
describe("#bookmark-removed", () => {
it("should ignore events that are not of TYPE_BOOKMARK", async () => {
const args = [
{
id: null,
parentId: null,
index: null,
itemType: "nottypebookmark",
url: null,
guid: "123foo",
parentGuid: "",
source: SOURCES.DEFAULT,
type: "bookmark-removed",
},
];
await observer.handlePlacesEvent(args);
assert.notCalled(dispatch);
});
it("should not dispatch a PLACES_BOOKMARK_REMOVED action - has SYNC source", async () => {
const args = [
{
id: null,
parentId: null,
index: null,
itemType: TYPE_BOOKMARK,
url: "foo.com",
guid: "123foo",
parentGuid: "",
source: SOURCES.SYNC,
type: "bookmark-removed",
},
];
await observer.handlePlacesEvent(args);
assert.notCalled(dispatch);
});
it("should not dispatch a PLACES_BOOKMARK_REMOVED action - has IMPORT source", async () => {
const args = [
{
id: null,
parentId: null,
index: null,
itemType: TYPE_BOOKMARK,
url: "foo.com",
guid: "123foo",
parentGuid: "",
source: SOURCES.IMPORT,
type: "bookmark-removed",
},
];
await observer.handlePlacesEvent(args);
assert.notCalled(dispatch);
});
it("should not dispatch a PLACES_BOOKMARK_REMOVED action - has RESTORE source", async () => {
const args = [
{
id: null,
parentId: null,
index: null,
itemType: TYPE_BOOKMARK,
url: "foo.com",
guid: "123foo",
parentGuid: "",
source: SOURCES.RESTORE,
type: "bookmark-removed",
},
];
await observer.handlePlacesEvent(args);
assert.notCalled(dispatch);
});
it("should not dispatch a PLACES_BOOKMARK_REMOVED action - has RESTORE_ON_STARTUP source", async () => {
const args = [
{
id: null,
parentId: null,
index: null,
itemType: TYPE_BOOKMARK,
url: "foo.com",
guid: "123foo",
parentGuid: "",
source: SOURCES.RESTORE_ON_STARTUP,
type: "bookmark-removed",
},
];
await observer.handlePlacesEvent(args);
assert.notCalled(dispatch);
});
it("should dispatch a PLACES_BOOKMARK_REMOVED action with the right URL and bookmarkGuid", async () => {
const args = [
{
id: null,
parentId: null,
index: null,
itemType: TYPE_BOOKMARK,
url: "foo.com",
guid: "123foo",
parentGuid: "",
source: SOURCES.DEFAULT,
type: "bookmark-removed",
},
];
await observer.handlePlacesEvent(args);
assert.calledWith(dispatch, {
type: at.PLACES_BOOKMARK_REMOVED,
data: { bookmarkGuid: "123foo", url: "foo.com" },
});
});
});
});
describe("BookmarksObserver", () => {
@ -897,97 +1015,6 @@ describe("PlacesFeed", () => {
it("should have a QueryInterface property", () => {
assert.property(observer, "QueryInterface");
});
describe("#onItemRemoved", () => {
it("should ignore events that are not of TYPE_BOOKMARK", async () => {
await observer.onItemRemoved(
null,
null,
null,
"nottypebookmark",
null,
"123foo",
"",
SOURCES.DEFAULT
);
assert.notCalled(dispatch);
});
it("should not dispatch a PLACES_BOOKMARK_REMOVED action - has SYNC source", async () => {
const args = [
null,
null,
null,
TYPE_BOOKMARK,
{ spec: "foo.com" },
"123foo",
"",
SOURCES.SYNC,
];
await observer.onItemRemoved(...args);
assert.notCalled(dispatch);
});
it("should not dispatch a PLACES_BOOKMARK_REMOVED action - has IMPORT source", async () => {
const args = [
null,
null,
null,
TYPE_BOOKMARK,
{ spec: "foo.com" },
"123foo",
"",
SOURCES.IMPORT,
];
await observer.onItemRemoved(...args);
assert.notCalled(dispatch);
});
it("should not dispatch a PLACES_BOOKMARK_REMOVED action - has RESTORE source", async () => {
const args = [
null,
null,
null,
TYPE_BOOKMARK,
{ spec: "foo.com" },
"123foo",
"",
SOURCES.RESTORE,
];
await observer.onItemRemoved(...args);
assert.notCalled(dispatch);
});
it("should not dispatch a PLACES_BOOKMARK_REMOVED action - has RESTORE_ON_STARTUP source", async () => {
const args = [
null,
null,
null,
TYPE_BOOKMARK,
{ spec: "foo.com" },
"123foo",
"",
SOURCES.RESTORE_ON_STARTUP,
];
await observer.onItemRemoved(...args);
assert.notCalled(dispatch);
});
it("should dispatch a PLACES_BOOKMARK_REMOVED action with the right URL and bookmarkGuid", () => {
observer.onItemRemoved(
null,
null,
null,
TYPE_BOOKMARK,
{ spec: "foo.com" },
"123foo",
"",
SOURCES.DEFAULT
);
assert.calledWith(dispatch, {
type: at.PLACES_BOOKMARK_REMOVED,
data: { bookmarkGuid: "123foo", url: "foo.com" },
});
});
});
describe("Other empty methods (to keep code coverage happy)", () => {
it("should have a various empty functions for xpconnect happiness", () => {
observer.onBeginUpdateBatch();

1
browser/components/places/content/editBookmark.js
View File

@ -1240,7 +1240,6 @@ var gEditItemOverlay = {
});
},
onItemRemoved() {},
onBeginUpdateBatch() {},
onEndUpdateBatch() {},
onItemVisited() {},

5
browser/components/places/tests/browser/browser_bookmarkProperties_addKeywordForThisSearch.js
View File

@ -6,8 +6,9 @@ const TEST_URL =
function closeHandler(dialogWin) {
let savedItemId = dialogWin.gEditItemOverlay.itemId;
return PlacesTestUtils.waitForNotification(
"onItemRemoved",
itemId => itemId === savedItemId
"bookmark-removed",
events => events.some(event => event.id === savedItemId),
"places"
);
}

5
browser/components/places/tests/browser/browser_bookmarkProperties_bookmarkAllTabs.js
View File

@ -42,8 +42,9 @@ add_task(async function() {
let savedItemId = dialog.gEditItemOverlay.itemId;
Assert.greater(savedItemId, 0, "Found the itemId");
return PlacesTestUtils.waitForNotification(
"onItemRemoved",
id => id === savedItemId
"bookmark-removed",
events => events.some(event => event.id === savedItemId),
"places"
);
}
);

13
browser/components/places/tests/browser/browser_bookmarkProperties_editTagContainer.js
View File

@ -67,9 +67,22 @@ add_task(async function() {
}
);
let promiseTagRemoveNotification = PlacesTestUtils.waitForNotification(
"bookmark-removed",
events =>
events.some(
event => event.parentGuid == PlacesUtils.bookmarks.tagsGuid
),
"places"
);
fillBookmarkTextField("editBMPanel_namePicker", "tag2", dialogWin);
await promiseTagChangeNotification;
await promiseTagRemoveNotification;
// Although we have received the expected notifications, we need
// to let everything resolve to ensure the UI is updated.
await new Promise(resolve => Services.tm.dispatchToMainThread(resolve));
Assert.equal(
namepicker.value,

18
browser/components/places/tests/browser/browser_bookmark_popup.js
View File

@ -126,13 +126,17 @@ async function test_bookmarks_popup({
);
}
let onItemRemovedPromise = Promise.resolve();
let bookmarkRemovedPromise = Promise.resolve();
if (isBookmarkRemoved) {
onItemRemovedPromise = PlacesTestUtils.waitForNotification(
"onItemRemoved",
(id, parentId, index, type, uri, guid, parentGuid) =>
parentGuid == PlacesUtils.bookmarks.unfiledGuid &&
TEST_URL == uri.spec
bookmarkRemovedPromise = PlacesTestUtils.waitForNotification(
"bookmark-removed",
events =>
events.some(
event =>
event.parentGuid == PlacesUtils.bookmarks.unfiledGuid &&
TEST_URL == event.url
),
"places"
);
}
@ -140,7 +144,7 @@ async function test_bookmarks_popup({
if (popupHideFn) {
await popupHideFn();
}
await Promise.all([hiddenPromise, onItemRemovedPromise]);
await Promise.all([hiddenPromise, bookmarkRemovedPromise]);
Assert.equal(
bookmarkStar.hasAttribute("starred"),

6
browser/components/places/tests/browser/browser_bookmarksProperties.js
View File

@ -401,9 +401,9 @@ gTests.push({
self._cleanShutdown = true;
self._removeObserver = PlacesTestUtils.waitForNotification(
"onItemRemoved",
(itemId, parentId, index, type, uri, guid) =>
guid == self._bookmarkGuid
"bookmark-removed",
events => events.some(eve => eve.guid == self._bookmarkGuid),
"places"
);
self.window.document

3
browser/components/places/tests/browser/browser_bug631374_tags_selector_scroll.js
View File

@ -119,6 +119,9 @@ add_task(async function() {
// Remove the second bookmark, then nuke some of the tags.
await PlacesUtils.bookmarks.remove(bm2);
// Allow the tag updates to complete
await PlacesTestUtils.promiseAsyncUpdates();
// Doing this backwords tests more interesting paths.
for (let i = tags.length - 1; i >= 0; i -= 2) {
tagsSelector.selectedIndex = i;

1
browser/components/places/tests/browser/browser_editBookmark_keywords.js
View File

@ -8,7 +8,6 @@ add_task(async function() {
PlacesUtils.bookmarks.addObserver({
onBeginUpdateBatch() {},
onEndUpdateBatch() {},
onItemRemoved() {},
onItemVisited() {},
onItemMoved() {},
onItemChanged(id, property, isAnno, value) {

5
browser/components/places/tests/browser/browser_library_commands.js
View File

@ -177,8 +177,9 @@ add_task(async function test_query_on_toolbar() {
// Execute the delete command and check bookmark has been removed.
let promiseItemRemoved = PlacesTestUtils.waitForNotification(
"onItemRemoved",
(...args) => query.guid == args[5]
"bookmark-removed",
events => events.some(event => query.guid == event.guid),
"places"
);
PO._places.controller.doCommand("cmd_delete");
await promiseItemRemoved;

6
browser/components/places/tests/browser/browser_library_delete.js
View File

@ -69,9 +69,9 @@ add_task(async function test_create_and_remove_bookmarks() {
"Delete command is enabled"
);
let promiseItemRemovedNotification = PlacesTestUtils.waitForNotification(
"onItemRemoved",
(itemId, parentId, index, type, uri, guid) =>
guid == folderNode.bookmarkGuid
"bookmark-removed",
events => events.some(event => event.guid == folderNode.bookmarkGuid),
"places"
);
// Execute the delete command and check bookmark has been removed.

10
browser/components/places/tests/browser/browser_remove_bookmarks.js
View File

@ -54,8 +54,9 @@ add_task(async function test_remove_bookmark_from_toolbar() {
let contextMenuDeleteItem = document.getElementById("placesContext_delete");
let removePromise = PlacesTestUtils.waitForNotification(
"onItemRemoved",
(itemId, parentId, index, type, uri, guid) => uri.spec == TEST_URL
"bookmark-removed",
events => events.some(event => event.url == TEST_URL),
"places"
);
EventUtils.synthesizeMouseAtCenter(contextMenuDeleteItem, {});
@ -138,8 +139,9 @@ add_task(async function test_remove_bookmark_from_library() {
);
let removePromise = PlacesTestUtils.waitForNotification(
"onItemRemoved",
(itemId, parentId, index, type, uri, guid) => uri.spec == uris[0]
"bookmark-removed",
events => events.some(event => event.url == uris[0]),
"places"
);
EventUtils.synthesizeMouseAtCenter(contextMenuDeleteItem, {}, library);

24
browser/components/places/tests/browser/browser_views_liveupdate.js
View File

@ -116,7 +116,7 @@ add_task(async function test() {
);
PlacesUtils.bookmarks.addObserver(bookmarksObserver);
PlacesUtils.observers.addListener(
["bookmark-added"],
["bookmark-added", "bookmark-removed"],
bookmarksObserver.handlePlacesEvents
);
var addedBookmarks = [];
@ -152,7 +152,7 @@ add_task(async function test() {
// Remove observers.
PlacesUtils.bookmarks.removeObserver(bookmarksObserver);
PlacesUtils.observers.removeListener(
["bookmark-added"],
["bookmark-added", "bookmark-removed"],
bookmarksObserver.handlePlacesEvents
);
});
@ -173,15 +173,23 @@ var bookmarksObserver = {
QueryInterface: ChromeUtils.generateQI([Ci.nsINavBookmarkObserver]),
handlePlacesEvents(events) {
for (let { parentGuid, guid, index } of events) {
this._notifications.push(["assertItemAdded", parentGuid, guid, index]);
for (let { type, parentGuid, guid, index } of events) {
switch (type) {
case "bookmark-added":
this._notifications.push([
"assertItemAdded",
parentGuid,
guid,
index,
]);
break;
case "bookmark-removed":
this._notifications.push(["assertItemRemoved", parentGuid, guid]);
break;
}
}
},
onItemRemoved(itemId, folderId, index, itemType, uri, guid, parentGuid) {
this._notifications.push(["assertItemRemoved", parentGuid, guid]);
},
onItemMoved(
itemId,
oldFolderId,

57
dom/base/PlacesBookmarkRemoved.h Normal file
View File

@ -0,0 +1,57 @@
/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
/* vim: set ts=8 sts=2 et sw=2 tw=80: */
/* 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/. */
#ifndef mozilla_dom_PlacesBookmarkRemoved_h
#define mozilla_dom_PlacesBookmarkRemoved_h
#include "mozilla/dom/PlacesBookmark.h"
namespace mozilla {
namespace dom {
class PlacesBookmarkRemoved final : public PlacesBookmark {
public:
explicit PlacesBookmarkRemoved()
: PlacesBookmark(PlacesEventType::Bookmark_removed) {}
static already_AddRefed<PlacesBookmarkRemoved> Constructor(
const GlobalObject& aGlobal, const PlacesBookmarkRemovedInit& aInitDict) {
RefPtr<PlacesBookmarkRemoved> event = new PlacesBookmarkRemoved();
event->mItemType = aInitDict.mItemType;
event->mId = aInitDict.mId;
event->mParentId = aInitDict.mParentId;
event->mIndex = aInitDict.mIndex;
event->mUrl = aInitDict.mUrl;
event->mGuid = aInitDict.mGuid;
event->mParentGuid = aInitDict.mParentGuid;
event->mSource = aInitDict.mSource;
event->mIsTagging = aInitDict.mIsTagging;
event->mIsDescendantRemoval = aInitDict.mIsDescendantRemoval;
return event.forget();
}
JSObject* WrapObject(JSContext* aCx,
JS::Handle<JSObject*> aGivenProto) override {
return PlacesBookmarkRemoved_Binding::Wrap(aCx, this, aGivenProto);
}
const PlacesBookmarkRemoved* AsPlacesBookmarkRemoved() const override {
return this;
}
int32_t Index() { return mIndex; }
bool IsDescendantRemoval() { return mIsDescendantRemoval; }
int32_t mIndex;
bool mIsDescendantRemoval;
private:
~PlacesBookmarkRemoved() = default;
};
} // namespace dom
} // namespace mozilla
#endif

3
dom/base/PlacesEvent.h
View File

@ -37,6 +37,9 @@ class PlacesEvent : public nsWrapperCache {
virtual const PlacesBookmarkAddition* AsPlacesBookmarkAddition() const {
return nullptr;
}
virtual const PlacesBookmarkRemoved* AsPlacesBookmarkRemoved() const {
return nullptr;
}
protected:
virtual ~PlacesEvent() = default;

1
dom/base/moz.build
View File

@ -211,6 +211,7 @@ EXPORTS.mozilla.dom += [
'ParentProcessMessageManager.h',
'PlacesBookmark.h',
'PlacesBookmarkAddition.h',
'PlacesBookmarkRemoved.h',
'PlacesEvent.h',
'PlacesObservers.h',
'PlacesVisit.h',

32
dom/chrome-webidl/PlacesEvent.webidl
View File

@ -10,6 +10,11 @@ enum PlacesEventType {
* (or a bookmark folder/separator) is created.
*/
"bookmark-added",
/**
* data: PlacesBookmarkRemoved. Fired whenever a bookmark
* (or a bookmark folder/separator) is created.
*/
"bookmark-removed",
};
[ChromeOnly, Exposed=Window]
@ -154,3 +159,30 @@ interface PlacesBookmarkAddition : PlacesBookmark {
*/
readonly attribute unsigned long long dateAdded;
};
dictionary PlacesBookmarkRemovedInit {
required long long id;
required long long parentId;
required unsigned short itemType;
required DOMString url;
required ByteString guid;
required ByteString parentGuid;
required unsigned short source;
required long index;
required boolean isTagging;
boolean isDescendantRemoval = false;
};
[ChromeOnly, Exposed=Window]
interface PlacesBookmarkRemoved : PlacesBookmark {
constructor(PlacesBookmarkRemovedInit initDict);
/**
* The item's index in the folder.
*/
readonly attribute long index;
/**
* The item is a descendant of an item whose notification has been sent out.
*/
readonly attribute boolean isDescendantRemoval;
};

38
services/sync/modules/engines/bookmarks.js
View File

@ -1400,7 +1400,10 @@ BookmarksTracker.prototype = {
this._placesListener = new PlacesWeakCallbackWrapper(
this.handlePlacesEvents.bind(this)
);
PlacesUtils.observers.addListener(["bookmark-added"], this._placesListener);
PlacesUtils.observers.addListener(
["bookmark-added", "bookmark-removed"],
this._placesListener
);
Svc.Obs.add("bookmarks-restore-begin", this);
Svc.Obs.add("bookmarks-restore-success", this);
Svc.Obs.add("bookmarks-restore-failed", this);
@ -1409,7 +1412,7 @@ BookmarksTracker.prototype = {
onStop() {
PlacesUtils.bookmarks.removeObserver(this);
PlacesUtils.observers.removeListener(
["bookmark-added"],
["bookmark-added", "bookmark-removed"],
this._placesListener
);
Svc.Obs.remove("bookmarks-restore-begin", this);
@ -1483,24 +1486,27 @@ BookmarksTracker.prototype = {
handlePlacesEvents(events) {
for (let event of events) {
if (IGNORED_SOURCES.includes(event.source)) {
continue;
switch (event.type) {
case "bookmark-added":
if (IGNORED_SOURCES.includes(event.source)) {
continue;
}
this._log.trace("'bookmark-added': " + event.id);
this._upScore();
break;
case "bookmark-removed":
if (IGNORED_SOURCES.includes(event.source)) {
continue;
}
this._log.trace("'bookmark-removed': " + event.id);
this._upScore();
break;
}
this._log.trace("'bookmark-added': " + event.id);
this._upScore();
}
},
onItemRemoved(itemId, parentId, index, type, uri, guid, parentGuid, source) {
if (IGNORED_SOURCES.includes(source)) {
return;
}
this._log.trace("onItemRemoved: " + itemId);
this._upScore();
},
// This method is oddly structured, but the idea is to return as quickly as
// possible -- this handler gets called *every time* a bookmark changes, for
// *each change*.

79
toolkit/components/places/Bookmarks.jsm
View File

@ -1287,26 +1287,24 @@ var Bookmarks = Object.freeze({
// Notify onItemRemoved to listeners.
for (let item of removeItems) {
let observers = PlacesUtils.bookmarks.getObservers();
let uri = item.hasOwnProperty("url")
? PlacesUtils.toURI(item.url)
: null;
let isUntagging = item._grandParentId == PlacesUtils.tagsFolderId;
notify(
observers,
"onItemRemoved",
[
item._id,
item._parentId,
item.index,
item.type,
uri,
item.guid,
item.parentGuid,
options.source,
],
{ isTagging: isUntagging }
);
let url = "";
if (item.type == Bookmarks.TYPE_BOOKMARK) {
url = item.hasOwnProperty("url") ? item.url.href : null;
}
let notification = new PlacesBookmarkRemoved({
id: item._id,
url,
itemType: item.type,
parentId: item._parentId,
index: item.index,
guid: item.guid,
parentGuid: item.parentGuid,
source: options.source,
isTagging: isUntagging,
isDescendantRemoval: false,
});
PlacesObservers.notifyListeners([notification]);
if (isUntagging) {
for (let entry of await fetchBookmarksByURL(item, {
concurrent: true,
@ -1823,7 +1821,6 @@ function notify(observers, notification, args = [], information = {}) {
if (
information.isDescendantRemoval &&
observer.skipDescendantsOnItemRemoval &&
!PlacesUtils.bookmarks.userContentRoots.includes(information.parentGuid)
) {
continue;
@ -3217,30 +3214,28 @@ var removeFoldersContents = async function(db, folderGuids, options) {
// Notify listeners in reverse order to serve children before parents.
let { source = Bookmarks.SOURCES.DEFAULT } = options;
let observers = PlacesUtils.bookmarks.getObservers();
let notification = [];
for (let item of itemsRemoved.reverse()) {
let uri = item.hasOwnProperty("url") ? PlacesUtils.toURI(item.url) : null;
notify(
observers,
"onItemRemoved",
[
item._id,
item._parentId,
item.index,
item.type,
uri,
item.guid,
item.parentGuid,
source,
],
// Notify observers that this item is being
// removed as a descendent.
{
isDescendantRemoval: true,
parentGuid: item.parentGuid,
}
);
let isUntagging = item._grandParentId == PlacesUtils.tagsFolderId;
let url = "";
if (item.type == Bookmarks.TYPE_BOOKMARK) {
url = item.hasOwnProperty("url") ? item.url.href : null;
}
notification = new PlacesBookmarkRemoved({
id: item._id,
url,
parentId: item._parentId,
index: item.index,
itemType: item.type,
guid: item.guid,
parentGuid: item.parentGuid,
source,
isTagging: isUntagging,
isDescendantRemoval: !PlacesUtils.bookmarks.userContentRoots.includes(
item.parentGuid
),
});
PlacesObservers.notifyListeners([notification]);
if (isUntagging) {
for (let entry of await fetchBookmarksByURL(item, true)) {
notify(observers, "onItemChanged", [

79
toolkit/components/places/PlacesUtils.jsm
View File

@ -2925,49 +2925,42 @@ var GuidHelper = {
},
ensureObservingRemovedItems() {
if (!("observer" in this)) {
/**
* This observers serves two purposes:
* (1) Invalidate cached id<->GUID paris on when items are removed.
* (2) Cache GUIDs given us free of charge by onItemAdded/onItemRemoved.
* So, for exmaple, when the NewBookmark needs the new GUID, we already
* have it cached.
*/
let listener = events => {
for (let event of events) {
this.updateCache(event.id, event.guid);
this.updateCache(event.parentId, event.parentGuid);
}
};
this.observer = {
onItemRemoved: (
aItemId,
aParentId,
aIndex,
aItemTyep,
aURI,
aGuid,
aParentGuid
) => {
this.guidsForIds.delete(aItemId);
this.idsForGuids.delete(aGuid);
this.updateCache(aParentId, aParentGuid);
},
QueryInterface: ChromeUtils.generateQI([Ci.nsINavBookmarkObserver]),
onBeginUpdateBatch() {},
onEndUpdateBatch() {},
onItemChanged() {},
onItemVisited() {},
onItemMoved() {},
};
PlacesUtils.bookmarks.addObserver(this.observer);
PlacesUtils.observers.addListener(["bookmark-added"], listener);
PlacesUtils.registerShutdownFunction(() => {
PlacesUtils.bookmarks.removeObserver(this.observer);
PlacesUtils.observers.removeListener(["bookmark-added"], listener);
});
if (this.addListeners) {
return;
}
/**
* This observers serves two purposes:
* (1) Invalidate cached id<->GUID paris on when items are removed.
* (2) Cache GUIDs given us free of charge by onItemAdded/onItemRemoved.
* So, for exmaple, when the NewBookmark needs the new GUID, we already
* have it cached.
*/
let listener = events => {
for (let event of events) {
switch (event.type) {
case "bookmark-added":
this.updateCache(event.id, event.guid);
this.updateCache(event.parentId, event.parentGuid);
break;
case "bookmark-removed":
this.guidsForIds.delete(event.id);
this.idsForGuids.delete(event.guid);
this.updateCache(event.parentId, event.parentGuid);
break;
}
}
};
this.addListeners = true;
PlacesUtils.observers.addListener(
["bookmark-added", "bookmark-removed"],
listener
);
PlacesUtils.registerShutdownFunction(() => {
PlacesUtils.observers.removeListener(
["bookmark-added", "bookmark-removed"],
listener
);
});
},
};

42
toolkit/components/places/SyncedBookmarksMirror.jsm
View File

@ -51,7 +51,6 @@
var EXPORTED_SYMBOLS = ["SyncedBookmarksMirror"];
const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
const { XPCOMUtils } = ChromeUtils.import(
"resource://gre/modules/XPCOMUtils.jsm"
);
@ -2077,7 +2076,6 @@ class BookmarkObserverRecorder {
this.notifyInStableOrder = notifyInStableOrder;
this.signal = signal;
this.placesEvents = [];
this.itemRemovedNotifications = [];
this.guidChangedArgs = [];
this.itemMovedArgs = [];
this.itemChangedArgs = [];
@ -2388,20 +2386,20 @@ class BookmarkObserverRecorder {
}
noteItemRemoved(info) {
let uri = info.urlHref ? Services.io.newURI(info.urlHref) : null;
this.itemRemovedNotifications.push({
isTagging: info.isUntagging,
args: [
info.id,
info.parentId,
info.position,
info.type,
uri,
info.guid,
info.parentGuid,
PlacesUtils.bookmarks.SOURCES.SYNC,
],
});
this.placesEvents.push(
new PlacesBookmarkRemoved({
id: info.id,
parentId: info.parentId,
index: info.position,
url: info.urlHref || "",
guid: info.guid,
parentGuid: info.parentGuid,
source: PlacesUtils.bookmarks.SOURCES.SYNC,
itemType: info.type,
isTagging: info.isUntagging,
isDescendantRemoval: false,
})
);
}
async notifyBookmarkObservers() {
@ -2410,18 +2408,6 @@ class BookmarkObserverRecorder {
for (let observer of observers) {
this.notifyObserver(observer, "onBeginUpdateBatch");
}
await Async.yieldingForEach(
this.itemRemovedNotifications,
info => {
if (this.signal.aborted) {
throw new SyncedBookmarksMirror.InterruptedError(
"Interrupted while notifying observers for removed items"
);
}
this.notifyObserversWithInfo(observers, "onItemRemoved", info);
},
yieldState
);
await Async.yieldingForEach(
this.guidChangedArgs,
args => {

82
toolkit/components/places/TaggingService.jsm
View File

@ -22,7 +22,7 @@ function TaggingService() {
// Observe bookmarks changes.
PlacesUtils.bookmarks.addObserver(this);
PlacesUtils.observers.addListener(
["bookmark-added"],
["bookmark-added", "bookmark-removed"],
this.handlePlacesEvents
);
@ -104,7 +104,6 @@ TaggingService.prototype = {
}
return -1;
},
/**
* Makes a proper array of tag objects like { id: number, name: string }.
*
@ -328,7 +327,7 @@ TaggingService.prototype = {
if (aTopic == TOPIC_SHUTDOWN) {
PlacesUtils.bookmarks.removeObserver(this);
PlacesUtils.observers.removeListener(
["bookmark-added"],
["bookmark-added", "bookmark-removed"],
this.handlePlacesEvents
);
Services.obs.removeObserver(this, TOPIC_SHUTDOWN);
@ -347,7 +346,7 @@ TaggingService.prototype = {
* @returns an array of item ids
*/
_getTaggedItemIdsIfUnbookmarkedURI: function TS__getTaggedItemIdsIfUnbookmarkedURI(
aURI
url
) {
var itemIds = [];
var isBookmarked = false;
@ -360,7 +359,7 @@ TaggingService.prototype = {
FROM moz_bookmarks
WHERE fk = (SELECT id FROM moz_places WHERE url_hash = hash(:page_url) AND url = :page_url)`
);
stmt.params.page_url = aURI.spec;
stmt.params.page_url = url;
try {
while (stmt.executeStep() && !isBookmarked) {
if (this._tagFolders[stmt.row.parent]) {
@ -380,45 +379,46 @@ TaggingService.prototype = {
handlePlacesEvents(events) {
for (let event of events) {
if (
!event.isTagging ||
event.itemType != PlacesUtils.bookmarks.TYPE_FOLDER
) {
continue;
}
switch (event.type) {
case "bookmark-added":
if (
!event.isTagging ||
event.itemType != PlacesUtils.bookmarks.TYPE_FOLDER
) {
continue;
}
this._tagFolders[event.id] = event.title;
}
},
this._tagFolders[event.id] = event.title;
break;
case "bookmark-removed":
// Item is a tag folder.
if (
event.parentId == PlacesUtils.tagsFolderId &&
this._tagFolders[event.id]
) {
delete this._tagFolders[event.id];
break;
}
// nsINavBookmarkObserver
onItemRemoved: function TS_onItemRemoved(
aItemId,
aFolderId,
aIndex,
aItemType,
aURI,
aGuid,
aParentGuid,
aSource
) {
// Item is a tag folder.
if (aFolderId == PlacesUtils.tagsFolderId && this._tagFolders[aItemId]) {
delete this._tagFolders[aItemId];
} else if (aURI && !this._tagFolders[aFolderId]) {
// Item is a bookmark that was removed from a non-tag folder.
// If the only bookmark items now associated with the bookmark's URI are
// contained in tag folders, the URI is no longer properly bookmarked, so
// untag it.
let itemIds = this._getTaggedItemIdsIfUnbookmarkedURI(aURI);
for (let i = 0; i < itemIds.length; i++) {
try {
PlacesUtils.bookmarks.removeItem(itemIds[i], aSource);
} catch (ex) {}
Services.tm.dispatchToMainThread(() => {
if (event.url && !this._tagFolders[event.parentId]) {
// Item is a bookmark that was removed from a non-tag folder.
// If the only bookmark items now associated with the bookmark's URI are
// contained in tag folders, the URI is no longer properly bookmarked, so
// untag it.
let itemIds = this._getTaggedItemIdsIfUnbookmarkedURI(event.url);
for (let i = 0; i < itemIds.length; i++) {
try {
PlacesUtils.bookmarks.removeItem(itemIds[i], event.source);
} catch (ex) {}
}
} else if (event.url && this._tagFolders[event.parentId]) {
// Item is a tag entry. If this was the last entry for this tag, remove it.
this._removeTagIfEmpty(event.parentId, event.source);
}
});
break;
}
} else if (aURI && this._tagFolders[aFolderId]) {
// Item is a tag entry. If this was the last entry for this tag, remove it.
this._removeTagIfEmpty(aFolderId, aSource);
}
},

41
toolkit/components/places/nsINavBookmarksService.idl
View File

@ -21,12 +21,6 @@ interface nsINavBookmarkObserver : nsISupports
*/
readonly attribute boolean skipTags;
/*
* This observer should not be called for descendants when the parent is removed.
* For example when revmoing a folder containing bookmarks.
*/
readonly attribute boolean skipDescendantsOnItemRemoval;
/**
* Notifies that a batch transaction has started.
* Other notifications will be sent during the batch, but the observer is
@ -42,40 +36,6 @@ interface nsINavBookmarkObserver : nsISupports
*/
void onEndUpdateBatch();
/**
* Notifies that an item was removed. Called after the actual remove took
* place.
* When an item is removed, all the items following it in the same folder
* will have their index shifted down, but no additional notifications will
* be sent.
*
* @param aItemId
* The id of the item that was removed.
* @param aParentId
* The id of the folder from which the item was removed.
* @param aIndex
* The bookmark's index in the folder.
* @param aItemType
* The type of the item to be removed (see TYPE_* constants below).
* @param aURI
* The URI of the added item if it was TYPE_BOOKMARK, null otherwise.
* @param aGuid
* The unique ID associated with the item.
* @param aParentGuid
* The unique ID associated with the item's parent.
* @param aSource
* A change source constant from nsINavBookmarksService::SOURCE_*,
* passed to the method that notifies the observer.
*/
void onItemRemoved(in long long aItemId,
in long long aParentId,
in long aIndex,
in unsigned short aItemType,
in nsIURI aURI,
in ACString aGuid,
in ACString aParentGuid,
in unsigned short aSource);
/**
* Notifies that an item's information has changed. This will be called
* whenever any attributes like "title" are changed.
@ -359,6 +319,7 @@ interface nsINavBookmarksService : nsISupports
* The change source, forwarded to all bookmark observers. Defaults
* to SOURCE_DEFAULT.
*/
[can_run_script]
void removeItem(in long long aItemId, [optional] in unsigned short aSource);
/**

55
toolkit/components/places/nsNavBookmarks.cpp
View File

@ -18,6 +18,7 @@
#include "mozilla/Preferences.h"
#include "mozilla/storage.h"
#include "mozilla/dom/PlacesBookmarkAddition.h"
#include "mozilla/dom/PlacesBookmarkRemoved.h"
#include "mozilla/dom/PlacesObservers.h"
#include "mozilla/dom/PlacesVisit.h"
@ -59,11 +60,6 @@ bool SkipTags(nsCOMPtr<nsINavBookmarkObserver> obs) {
(void)obs->GetSkipTags(&skipTags);
return skipTags;
}
bool SkipDescendants(nsCOMPtr<nsINavBookmarkObserver> obs) {
bool skipDescendantsOnItemRemoval = false;
(void)obs->GetSkipDescendantsOnItemRemoval(&skipDescendantsOnItemRemoval);
return skipDescendantsOnItemRemoval;
}
template <typename Method, typename DataType>
class AsyncGetBookmarksForURI : public AsyncStatementCallback {
@ -643,14 +639,27 @@ nsNavBookmarks::RemoveItem(int64_t aItemId, uint16_t aSource) {
NS_WARNING_ASSERTION(uri, "Invalid URI in RemoveItem");
}
NOTIFY_BOOKMARKS_OBSERVERS(
mCanNotify, mObservers,
SKIP_TAGS(bookmark.parentId == tagsRootId ||
bookmark.grandParentId == tagsRootId),
OnItemRemoved(bookmark.id, bookmark.parentId, bookmark.position,
bookmark.type, uri, bookmark.guid, bookmark.parentGuid,
aSource));
if (mCanNotify) {
Sequence<OwningNonNull<PlacesEvent>> events;
RefPtr<PlacesBookmarkRemoved> bookmarkRef = new PlacesBookmarkRemoved();
bookmarkRef->mItemType = bookmark.type;
bookmarkRef->mId = bookmark.id;
bookmarkRef->mParentId = bookmark.parentId;
bookmarkRef->mIndex = bookmark.position;
if (bookmark.type == TYPE_BOOKMARK) {
bookmarkRef->mUrl.Assign(NS_ConvertUTF8toUTF16(bookmark.url));
}
bookmarkRef->mGuid.Assign(bookmark.guid);
bookmarkRef->mParentGuid.Assign(bookmark.parentGuid);
bookmarkRef->mSource = aSource;
bookmarkRef->mIsTagging =
bookmark.parentId == tagsRootId || bookmark.grandParentId == tagsRootId;
bookmarkRef->mIsDescendantRemoval = false;
bool success = !!events.AppendElement(bookmarkRef.forget(), fallible);
MOZ_RELEASE_ASSERT(success);
PlacesObservers::NotifyListeners(events);
}
if (bookmark.type == TYPE_BOOKMARK && bookmark.grandParentId == tagsRootId &&
uri) {
// If the removed bookmark was child of a tag container, notify a tags
@ -930,12 +939,24 @@ nsresult nsNavBookmarks::RemoveFolderChildren(int64_t aFolderId,
NS_WARNING_ASSERTION(uri, "Invalid URI in RemoveFolderChildren");
}
NOTIFY_BOOKMARKS_OBSERVERS(
mCanNotify, mObservers,
((child.grandParentId == tagsRootId) ? SkipTags : SkipDescendants),
OnItemRemoved(child.id, child.parentId, child.position, child.type, uri,
child.guid, child.parentGuid, aSource));
if (mCanNotify) {
Sequence<OwningNonNull<PlacesEvent>> events;
RefPtr<PlacesBookmarkRemoved> bookmark = new PlacesBookmarkRemoved();
bookmark->mItemType = TYPE_BOOKMARK;
bookmark->mId = child.id;
bookmark->mParentId = child.parentId;
bookmark->mIndex = child.position;
bookmark->mUrl.Assign(NS_ConvertUTF8toUTF16(child.url));
bookmark->mGuid.Assign(child.guid);
bookmark->mParentGuid.Assign(child.parentGuid);
bookmark->mSource = aSource;
bookmark->mIsTagging = (child.grandParentId == tagsRootId);
bookmark->mIsDescendantRemoval = (child.grandParentId != tagsRootId);
bool success = !!events.AppendElement(bookmark.forget(), fallible);
MOZ_RELEASE_ASSERT(success);
PlacesObservers::NotifyListeners(events);
}
if (child.type == TYPE_BOOKMARK && child.grandParentId == tagsRootId &&
uri) {
// If the removed bookmark was a child of a tag container, notify all

1
toolkit/components/places/nsNavBookmarks.h
View File

@ -302,6 +302,7 @@ class nsNavBookmarks final
int64_t aSyncChangeDelta, int64_t aItemId,
PRTime aValue);
MOZ_CAN_RUN_SCRIPT
nsresult RemoveFolderChildren(int64_t aFolderId, uint16_t aSource);
// Recursive method to build an array of folder's children

95
toolkit/components/places/nsNavHistoryResult.cpp
View File

@ -1973,13 +1973,6 @@ nsNavHistoryQueryResultNode::GetSkipTags(bool* aSkipTags) {
return NS_OK;
}
NS_IMETHODIMP
nsNavHistoryQueryResultNode::GetSkipDescendantsOnItemRemoval(
bool* aSkipDescendantsOnItemRemoval) {
*aSkipDescendantsOnItemRemoval = false;
return NS_OK;
}
NS_IMETHODIMP
nsNavHistoryQueryResultNode::OnBeginUpdateBatch() { return NS_OK; }
@ -2451,14 +2444,15 @@ nsresult nsNavHistoryQueryResultNode::OnItemAdded(
return NS_OK;
}
NS_IMETHODIMP
nsNavHistoryQueryResultNode::OnItemRemoved(int64_t aItemId, int64_t aParentId,
int32_t aIndex, uint16_t aItemType,
nsIURI* aURI,
const nsACString& aGUID,
const nsACString& aParentGUID,
uint16_t aSource) {
if (aItemType == nsINavBookmarksService::TYPE_BOOKMARK &&
nsresult nsNavHistoryQueryResultNode::OnItemRemoved(
int64_t aItemId, int64_t aParentFolder, int32_t aIndex, uint16_t aItemType,
nsIURI* aURI, const nsACString& aGUID, const nsACString& aParentGUID,
uint16_t aSource) {
if ((aItemType == nsINavBookmarksService::TYPE_BOOKMARK ||
(aItemType == nsINavBookmarksService::TYPE_FOLDER &&
mOptions->ResultType() ==
nsINavHistoryQueryOptions::RESULTS_AS_TAGS_ROOT &&
aParentGUID.EqualsLiteral(TAGS_ROOT_GUID))) &&
mLiveUpdate != QUERYUPDATE_SIMPLE && mLiveUpdate != QUERYUPDATE_TIME &&
mLiveUpdate != QUERYUPDATE_MOBILEPREF) {
nsresult rv = Refresh();
@ -3036,13 +3030,6 @@ nsNavHistoryFolderResultNode::GetSkipTags(bool* aSkipTags) {
return NS_OK;
}
NS_IMETHODIMP
nsNavHistoryFolderResultNode::GetSkipDescendantsOnItemRemoval(
bool* aSkipDescendantsOnItemRemoval) {
*aSkipDescendantsOnItemRemoval = false;
return NS_OK;
}
NS_IMETHODIMP
nsNavHistoryFolderResultNode::OnBeginUpdateBatch() { return NS_OK; }
@ -3167,8 +3154,7 @@ nsresult nsNavHistoryQueryResultNode::OnMobilePrefChanged(bool newValue) {
return RemoveChildAt(existingIndex);
}
NS_IMETHODIMP
nsNavHistoryFolderResultNode::OnItemRemoved(
nsresult nsNavHistoryFolderResultNode::OnItemRemoved(
int64_t aItemId, int64_t aParentFolder, int32_t aIndex, uint16_t aItemType,
nsIURI* aURI, const nsACString& aGUID, const nsACString& aParentGUID,
uint16_t aSource) {
@ -3539,7 +3525,7 @@ nsNavHistoryResult::~nsNavHistoryResult() {
}
void nsNavHistoryResult::StopObserving() {
AutoTArray<PlacesEventType, 2> events;
AutoTArray<PlacesEventType, 3> events;
if (mIsBookmarkFolderObserver || mIsAllBookmarksObserver) {
nsNavBookmarks* bookmarks = nsNavBookmarks::GetBookmarksService();
if (bookmarks) {
@ -3548,6 +3534,7 @@ void nsNavHistoryResult::StopObserving() {
mIsAllBookmarksObserver = false;
}
events.AppendElement(PlacesEventType::Bookmark_added);
events.AppendElement(PlacesEventType::Bookmark_removed);
}
if (mIsMobilePrefObserver) {
Preferences::UnregisterCallback(OnMobilePrefChangedCallback,
@ -3594,8 +3581,9 @@ void nsNavHistoryResult::AddAllBookmarksObserver(
return;
}
bookmarks->AddObserver(this, true);
AutoTArray<PlacesEventType, 1> events;
AutoTArray<PlacesEventType, 2> events;
events.AppendElement(PlacesEventType::Bookmark_added);
events.AppendElement(PlacesEventType::Bookmark_removed);
PlacesObservers::AddListener(events, this);
mIsAllBookmarksObserver = true;
}
@ -3631,8 +3619,9 @@ void nsNavHistoryResult::AddBookmarkFolderObserver(
return;
}
bookmarks->AddObserver(this, true);
AutoTArray<PlacesEventType, 1> events;
AutoTArray<PlacesEventType, 2> events;
events.AppendElement(PlacesEventType::Bookmark_added);
events.AppendElement(PlacesEventType::Bookmark_removed);
PlacesObservers::AddListener(events, this);
mIsBookmarkFolderObserver = true;
}
@ -3823,13 +3812,6 @@ nsNavHistoryResult::GetSkipTags(bool* aSkipTags) {
return NS_OK;
}
NS_IMETHODIMP
nsNavHistoryResult::GetSkipDescendantsOnItemRemoval(
bool* aSkipDescendantsOnItemRemoval) {
*aSkipDescendantsOnItemRemoval = true;
return NS_OK;
}
NS_IMETHODIMP
nsNavHistoryResult::OnBeginUpdateBatch() {
// Since we could be observing both history and bookmarks, it's possible both
@ -3867,26 +3849,6 @@ nsNavHistoryResult::OnEndUpdateBatch() {
return NS_OK;
}
NS_IMETHODIMP
nsNavHistoryResult::OnItemRemoved(int64_t aItemId, int64_t aParentId,
int32_t aIndex, uint16_t aItemType,
nsIURI* aURI, const nsACString& aGUID,
const nsACString& aParentGUID,
uint16_t aSource) {
NS_ENSURE_ARG(aItemType != nsINavBookmarksService::TYPE_BOOKMARK || aURI);
ENUMERATE_BOOKMARK_FOLDER_OBSERVERS(
aParentId, OnItemRemoved(aItemId, aParentId, aIndex, aItemType, aURI,
aGUID, aParentGUID, aSource));
ENUMERATE_ALL_BOOKMARKS_OBSERVERS(OnItemRemoved(aItemId, aParentId, aIndex,
aItemType, aURI, aGUID,
aParentGUID, aSource));
ENUMERATE_HISTORY_OBSERVERS(OnItemRemoved(aItemId, aParentId, aIndex,
aItemType, aURI, aGUID, aParentGUID,
aSource));
return NS_OK;
}
NS_IMETHODIMP
nsNavHistoryResult::OnItemChanged(
int64_t aItemId, const nsACString& aProperty, bool aIsAnnotationProperty,
@ -4113,6 +4075,31 @@ void nsNavHistoryResult::HandlePlacesEvent(const PlacesEventSequence& aEvents) {
item->mGuid, item->mParentGuid, item->mSource));
break;
}
case PlacesEventType::Bookmark_removed: {
const dom::PlacesBookmarkRemoved* item =
event->AsPlacesBookmarkRemoved();
if (NS_WARN_IF(!item)) {
continue;
}
nsCOMPtr<nsIURI> uri;
if (item->mIsDescendantRemoval) {
continue;
}
ENUMERATE_BOOKMARK_FOLDER_OBSERVERS(
item->mParentId,
OnItemRemoved(item->mId, item->mParentId, item->mIndex,
item->mItemType, uri, item->mGuid, item->mParentGuid,
item->mSource));
ENUMERATE_ALL_BOOKMARKS_OBSERVERS(OnItemRemoved(
item->mId, item->mParentId, item->mIndex, item->mItemType, uri,
item->mGuid, item->mParentGuid, item->mSource));
ENUMERATE_HISTORY_OBSERVERS(OnItemRemoved(
item->mId, item->mParentId, item->mIndex, item->mItemType, uri,
item->mGuid, item->mParentGuid, item->mSource));
break;
}
default: {
MOZ_ASSERT_UNREACHABLE(
"Receive notification of a type not subscribed to.");

11
toolkit/components/places/nsNavHistoryResult.h
View File

@ -687,6 +687,12 @@ class nsNavHistoryQueryResultNode final
uint16_t aItemType, nsIURI* aURI, PRTime aDateAdded,
const nsACString& aGUID, const nsACString& aParentGUID,
uint16_t aSource);
nsresult OnItemRemoved(int64_t aItemId, int64_t aParentFolder, int32_t aIndex,
uint16_t aItemType, nsIURI* aURI,
const nsACString& aGUID, const nsACString& aParentGUID,
uint16_t aSource);
// The internal version has an output aAdded parameter, it is incremented by
// query nodes when the visited uri belongs to them. If no such query exists,
// the history result creates a new query node dynamically.
@ -768,7 +774,10 @@ class nsNavHistoryFolderResultNode final
uint16_t aItemType, nsIURI* aURI, PRTime aDateAdded,
const nsACString& aGUID, const nsACString& aParentGUID,
uint16_t aSource);
nsresult OnItemRemoved(int64_t aItemId, int64_t aParentFolder, int32_t aIndex,
uint16_t aItemType, nsIURI* aURI,
const nsACString& aGUID, const nsACString& aParentGUID,
uint16_t aSource);
virtual void OnRemoving() override;
// this indicates whether the folder contents are valid, they don't go away

2
toolkit/components/places/tests/PlacesTestUtils.jsm
View File

@ -417,7 +417,7 @@ var PlacesTestUtils = Object.freeze({
}
};
}
if (name == "skipTags" || name == "skipDescendantsOnItemRemoval") {
if (name == "skipTags") {
return false;
}
return () => false;

72
toolkit/components/places/tests/bookmarks/head_bookmarks.js
View File

@ -15,14 +15,10 @@ var { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
// Put any other stuff relative to this test folder below.
function expectNotifications(skipDescendants, checkAllArgs) {
function expectNotifications(checkAllArgs) {
let notifications = [];
let observer = new Proxy(NavBookmarkObserver, {
get(target, name) {
if (name == "skipDescendantsOnItemRemoval") {
return skipDescendants;
}
if (name == "check") {
PlacesUtils.bookmarks.removeObserver(observer);
return expectedNotifications =>
@ -58,24 +54,60 @@ function expectNotifications(skipDescendants, checkAllArgs) {
return observer;
}
function expectPlacesObserverNotifications(types, checkAllArgs) {
function expectPlacesObserverNotifications(
types,
checkAllArgs = true,
skipDescendants = false
) {
let notifications = [];
let listener = events => {
for (let event of events) {
notifications.push({
type: event.type,
id: event.id,
itemType: event.itemType,
parentId: event.parentId,
index: event.index,
url: event.url || undefined,
title: event.title,
dateAdded: new Date(event.dateAdded),
guid: event.guid,
parentGuid: event.parentGuid,
source: event.source,
isTagging: event.isTagging,
});
switch (event.type) {
case "bookmark-added":
notifications.push({
type: event.type,
id: event.id,
itemType: event.itemType,
parentId: event.parentId,
index: event.index,
url: event.url || undefined,
title: event.title,
dateAdded: new Date(event.dateAdded),
guid: event.guid,
parentGuid: event.parentGuid,
source: event.source,
isTagging: event.isTagging,
});
break;
case "bookmark-removed":
if (
!(
skipDescendants &&
event.isDescendantRemoval &&
!PlacesUtils.bookmarks.userContentRoots.includes(event.parentGuid)
)
) {
if (checkAllArgs) {
notifications.push({
type: event.type,
id: event.id,
itemType: event.itemType,
parentId: event.parentId,
index: event.index,
url: event.url || null,
guid: event.guid,
parentGuid: event.parentGuid,
source: event.source,
isTagging: event.isTagging,
});
} else {
notifications.push({
type: event.type,
guid: event.guid,
});
}
}
}
}
};
PlacesUtils.observers.addListener(types, listener);

29
toolkit/components/places/tests/bookmarks/test_bookmarks_eraseEverything.js
View File

@ -184,23 +184,26 @@ add_task(async function test_notifications() {
],
});
let skipDescendantsObserver = expectNotifications(true);
let receiveAllObserver = expectNotifications(false);
let skipDescendantsObserver = expectPlacesObserverNotifications(
["bookmark-removed"],
false,
true
);
let receiveAllObserver = expectPlacesObserverNotifications(
["bookmark-removed"],
false
);
await PlacesUtils.bookmarks.eraseEverything();
let expectedNotifications = [
{
name: "onItemRemoved",
arguments: {
guid: bms[1].guid,
},
type: "bookmark-removed",
guid: bms[1].guid,
},
{
name: "onItemRemoved",
arguments: {
guid: bms[0].guid,
},
type: "bookmark-removed",
guid: bms[0].guid,
},
];
@ -209,10 +212,8 @@ add_task(async function test_notifications() {
// Note: Items of folders get notified first.
expectedNotifications.unshift({
name: "onItemRemoved",
arguments: {
guid: bms[2].guid,
},
type: "bookmark-removed",
guid: bms[2].guid,
});
receiveAllObserver.check(expectedNotifications);

2
toolkit/components/places/tests/bookmarks/test_bookmarks_moveToFolder.js
View File

@ -198,7 +198,7 @@ async function testMoveToFolder(details) {
let observer;
if (details.notifications) {
observer = expectNotifications(false, true);
observer = expectNotifications(true);
}
let movedItems = await PlacesUtils.bookmarks.moveToFolder(

374
toolkit/components/places/tests/bookmarks/test_bookmarks_notifications.js
View File

@ -437,21 +437,20 @@ add_task(async function remove_bookmark() {
let itemId = await PlacesUtils.promiseItemId(bm.guid);
let parentId = await PlacesUtils.promiseItemId(bm.parentGuid);
let observer = expectNotifications();
let observer = expectPlacesObserverNotifications(["bookmark-removed"]);
await PlacesUtils.bookmarks.remove(bm.guid);
observer.check([
{
name: "onItemRemoved",
arguments: [
itemId,
parentId,
bm.index,
bm.type,
bm.url,
bm.guid,
bm.parentGuid,
Ci.nsINavBookmarksService.SOURCE_DEFAULT,
],
type: "bookmark-removed",
id: itemId,
parentId,
index: bm.index,
url: bm.url,
guid: bm.guid,
parentGuid: bm.parentGuid,
source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
itemType: PlacesUtils.bookmarks.TYPE_BOOKMARK,
isTagging: false,
},
]);
});
@ -472,34 +471,32 @@ add_task(async function remove_multiple_bookmarks() {
let itemId2 = await PlacesUtils.promiseItemId(bm2.guid);
let parentId2 = await PlacesUtils.promiseItemId(bm2.parentGuid);
let observer = expectNotifications();
let observer = expectPlacesObserverNotifications(["bookmark-removed"]);
await PlacesUtils.bookmarks.remove([bm1, bm2]);
observer.check([
{
name: "onItemRemoved",
arguments: [
itemId1,
parentId1,
bm1.index,
bm1.type,
bm1.url,
bm1.guid,
bm1.parentGuid,
Ci.nsINavBookmarksService.SOURCE_DEFAULT,
],
type: "bookmark-removed",
id: itemId1,
parentId: parentId1,
index: bm1.index,
url: bm1.url,
guid: bm1.guid,
parentGuid: bm1.parentGuid,
source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
itemType: PlacesUtils.bookmarks.TYPE_BOOKMARK,
isTagging: false,
},
{
name: "onItemRemoved",
arguments: [
itemId2,
parentId2,
bm2.index - 1,
bm2.type,
bm2.url,
bm2.guid,
bm2.parentGuid,
Ci.nsINavBookmarksService.SOURCE_DEFAULT,
],
type: "bookmark-removed",
id: itemId2,
parentId: parentId2,
index: bm2.index - 1,
url: bm2.url,
guid: bm2.guid,
parentGuid: bm2.parentGuid,
source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
itemType: PlacesUtils.bookmarks.TYPE_BOOKMARK,
isTagging: false,
},
]);
});
@ -512,21 +509,20 @@ add_task(async function remove_folder() {
let itemId = await PlacesUtils.promiseItemId(bm.guid);
let parentId = await PlacesUtils.promiseItemId(bm.parentGuid);
let observer = expectNotifications();
let observer = expectPlacesObserverNotifications(["bookmark-removed"]);
await PlacesUtils.bookmarks.remove(bm.guid);
observer.check([
{
name: "onItemRemoved",
arguments: [
itemId,
parentId,
bm.index,
bm.type,
null,
bm.guid,
bm.parentGuid,
Ci.nsINavBookmarksService.SOURCE_DEFAULT,
],
type: "bookmark-removed",
id: itemId,
parentId,
index: bm.index,
url: null,
guid: bm.guid,
parentGuid: bm.parentGuid,
source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
itemType: PlacesUtils.bookmarks.TYPE_FOLDER,
isTagging: false,
},
]);
});
@ -553,23 +549,25 @@ add_task(async function remove_bookmark_tag_notification() {
let tagId = await PlacesUtils.promiseItemId(tag.guid);
let tagParentId = await PlacesUtils.promiseItemId(tag.parentGuid);
let placesObserver = expectPlacesObserverNotifications(["bookmark-removed"]);
let observer = expectNotifications();
await PlacesUtils.bookmarks.remove(tag.guid);
observer.check([
placesObserver.check([
{
name: "onItemRemoved",
arguments: [
tagId,
tagParentId,
tag.index,
tag.type,
tag.url,
tag.guid,
tag.parentGuid,
Ci.nsINavBookmarksService.SOURCE_DEFAULT,
],
type: "bookmark-removed",
id: tagId,
parentId: tagParentId,
index: tag.index,
url: tag.url,
guid: tag.guid,
parentGuid: tag.parentGuid,
source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
itemType: PlacesUtils.bookmarks.TYPE_BOOKMARK,
isTagging: true,
},
]);
observer.check([
{
name: "onItemChanged",
arguments: [
@ -617,61 +615,57 @@ add_task(async function remove_folder_notification() {
});
let bm2ItemId = await PlacesUtils.promiseItemId(bm2.guid);
let observer = expectNotifications();
let observer = expectPlacesObserverNotifications(["bookmark-removed"]);
await PlacesUtils.bookmarks.remove(folder1.guid);
observer.check([
{
name: "onItemRemoved",
arguments: [
bm2ItemId,
folder2Id,
bm2.index,
bm2.type,
bm2.url,
bm2.guid,
bm2.parentGuid,
Ci.nsINavBookmarksService.SOURCE_DEFAULT,
],
type: "bookmark-removed",
id: bm2ItemId,
parentId: folder2Id,
index: bm2.index,
url: bm2.url,
guid: bm2.guid,
parentGuid: bm2.parentGuid,
source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
itemType: PlacesUtils.bookmarks.TYPE_BOOKMARK,
isTagging: false,
},
{
name: "onItemRemoved",
arguments: [
folder2Id,
folder1Id,
folder2.index,
folder2.type,
null,
folder2.guid,
folder2.parentGuid,
Ci.nsINavBookmarksService.SOURCE_DEFAULT,
],
type: "bookmark-removed",
id: folder2Id,
parentId: folder1Id,
index: folder2.index,
url: null,
guid: folder2.guid,
parentGuid: folder2.parentGuid,
source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
itemType: PlacesUtils.bookmarks.TYPE_FOLDER,
isTagging: false,
},
{
name: "onItemRemoved",
arguments: [
bmItemId,
folder1Id,
bm.index,
bm.type,
bm.url,
bm.guid,
bm.parentGuid,
Ci.nsINavBookmarksService.SOURCE_DEFAULT,
],
type: "bookmark-removed",
id: bmItemId,
parentId: folder1Id,
index: bm.index,
url: bm.url,
guid: bm.guid,
parentGuid: bm.parentGuid,
source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
itemType: PlacesUtils.bookmarks.TYPE_BOOKMARK,
isTagging: false,
},
{
name: "onItemRemoved",
arguments: [
folder1Id,
folder1ParentId,
folder1.index,
folder1.type,
null,
folder1.guid,
folder1.parentGuid,
Ci.nsINavBookmarksService.SOURCE_DEFAULT,
],
type: "bookmark-removed",
id: folder1Id,
parentId: folder1ParentId,
index: folder1.index,
url: null,
guid: folder1.guid,
parentGuid: folder1.parentGuid,
source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
itemType: PlacesUtils.bookmarks.TYPE_FOLDER,
isTagging: false,
},
]);
});
@ -718,75 +712,70 @@ add_task(async function eraseEverything_notification() {
let menuBmId = await PlacesUtils.promiseItemId(menuBm.guid);
let menuBmParentId = await PlacesUtils.promiseItemId(menuBm.parentGuid);
let observer = expectNotifications();
let observer = expectPlacesObserverNotifications(["bookmark-removed"]);
await PlacesUtils.bookmarks.eraseEverything();
// Bookmarks should always be notified before their parents.
observer.check([
{
name: "onItemRemoved",
arguments: [
itemId,
parentId,
bm.index,
bm.type,
bm.url,
bm.guid,
bm.parentGuid,
Ci.nsINavBookmarksService.SOURCE_DEFAULT,
],
type: "bookmark-removed",
id: itemId,
parentId,
index: bm.index,
url: bm.url,
guid: bm.guid,
parentGuid: bm.parentGuid,
source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
itemType: PlacesUtils.bookmarks.TYPE_BOOKMARK,
isTagging: false,
},
{
name: "onItemRemoved",
arguments: [
folder2Id,
folder2ParentId,
folder2.index,
folder2.type,
null,
folder2.guid,
folder2.parentGuid,
Ci.nsINavBookmarksService.SOURCE_DEFAULT,
],
type: "bookmark-removed",
id: folder2Id,
parentId: folder2ParentId,
index: folder2.index,
url: null,
guid: folder2.guid,
parentGuid: folder2.parentGuid,
source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
itemType: PlacesUtils.bookmarks.TYPE_FOLDER,
isTagging: false,
},
{
name: "onItemRemoved",
arguments: [
folder1Id,
folder1ParentId,
folder1.index,
folder1.type,
null,
folder1.guid,
folder1.parentGuid,
Ci.nsINavBookmarksService.SOURCE_DEFAULT,
],
type: "bookmark-removed",
id: folder1Id,
parentId: folder1ParentId,
index: folder1.index,
url: null,
guid: folder1.guid,
parentGuid: folder1.parentGuid,
source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
itemType: PlacesUtils.bookmarks.TYPE_FOLDER,
isTagging: false,
},
{
name: "onItemRemoved",
arguments: [
menuBmId,
menuBmParentId,
menuBm.index,
menuBm.type,
menuBm.url,
menuBm.guid,
menuBm.parentGuid,
Ci.nsINavBookmarksService.SOURCE_DEFAULT,
],
type: "bookmark-removed",
id: menuBmId,
parentId: menuBmParentId,
index: menuBm.index,
url: menuBm.url,
guid: menuBm.guid,
parentGuid: menuBm.parentGuid,
source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
itemType: PlacesUtils.bookmarks.TYPE_BOOKMARK,
isTagging: false,
},
{
name: "onItemRemoved",
arguments: [
toolbarBmId,
toolbarBmParentId,
toolbarBm.index,
toolbarBm.type,
toolbarBm.url,
toolbarBm.guid,
toolbarBm.parentGuid,
Ci.nsINavBookmarksService.SOURCE_DEFAULT,
],
type: "bookmark-removed",
id: toolbarBmId,
parentId: toolbarBmParentId,
index: toolbarBm.index,
url: toolbarBm.url,
guid: toolbarBm.guid,
parentGuid: toolbarBm.parentGuid,
source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
itemType: PlacesUtils.bookmarks.TYPE_BOOKMARK,
isTagging: false,
},
]);
});
@ -820,49 +809,46 @@ add_task(async function eraseEverything_reparented_notification() {
bm = await PlacesUtils.bookmarks.update(bm);
let parentId = await PlacesUtils.promiseItemId(bm.parentGuid);
let observer = expectNotifications();
let observer = expectPlacesObserverNotifications(["bookmark-removed"]);
await PlacesUtils.bookmarks.eraseEverything();
// Bookmarks should always be notified before their parents.
observer.check([
{
name: "onItemRemoved",
arguments: [
itemId,
parentId,
bm.index,
bm.type,
bm.url,
bm.guid,
bm.parentGuid,
Ci.nsINavBookmarksService.SOURCE_DEFAULT,
],
type: "bookmark-removed",
id: itemId,
parentId,
index: bm.index,
url: bm.url,
guid: bm.guid,
parentGuid: bm.parentGuid,
source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
itemType: PlacesUtils.bookmarks.TYPE_BOOKMARK,
isTagging: false,
},
{
name: "onItemRemoved",
arguments: [
folder2Id,
folder2ParentId,
folder2.index,
folder2.type,
null,
folder2.guid,
folder2.parentGuid,
Ci.nsINavBookmarksService.SOURCE_DEFAULT,
],
type: "bookmark-removed",
id: folder2Id,
parentId: folder2ParentId,
index: folder2.index,
url: null,
guid: folder2.guid,
parentGuid: folder2.parentGuid,
source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
itemType: PlacesUtils.bookmarks.TYPE_FOLDER,
isTagging: false,
},
{
name: "onItemRemoved",
arguments: [
folder1Id,
folder1ParentId,
folder1.index,
folder1.type,
null,
folder1.guid,
folder1.parentGuid,
Ci.nsINavBookmarksService.SOURCE_DEFAULT,
],
type: "bookmark-removed",
id: folder1Id,
parentId: folder1ParentId,
index: folder1.index,
url: null,
guid: folder1.guid,
parentGuid: folder1.parentGuid,
source: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
itemType: PlacesUtils.bookmarks.TYPE_FOLDER,
isTagging: false,
},
]);
});

47
toolkit/components/places/tests/bookmarks/test_bookmarks_remove.js
View File

@ -198,11 +198,14 @@ add_task(async function remove_multiple_bookmarks_complex() {
let bmsToRemove = bms.slice(2, 4);
let notifiedIndexes = [];
let notificationPromise = PlacesTestUtils.waitForNotification(
"onItemRemoved",
(itemId, parentId, index, itemType, uri, guid, parentGuid, source) => {
notifiedIndexes.push({ guid, index });
"bookmark-removed",
events => {
for (let event of events) {
notifiedIndexes.push({ guid: event.guid, index: event.index });
}
return notifiedIndexes.length == bmsToRemove.length;
}
},
"places"
);
await PlacesUtils.bookmarks.remove(bmsToRemove);
await notificationPromise;
@ -238,11 +241,14 @@ add_task(async function remove_multiple_bookmarks_complex() {
bmsToRemove = [bms[1], bms[5], bms[6], bms[8]];
notifiedIndexes = [];
notificationPromise = PlacesTestUtils.waitForNotification(
"onItemRemoved",
(itemId, parentId, index, itemType, uri, guid, parentGuid, source) => {
notifiedIndexes.push({ guid, index });
"bookmark-removed",
events => {
for (let event of events) {
notifiedIndexes.push({ guid: event.guid, index: event.index });
}
return notifiedIndexes.length == bmsToRemove.length;
}
},
"places"
);
await PlacesUtils.bookmarks.remove(bmsToRemove);
await notificationPromise;
@ -344,8 +350,16 @@ add_task(async function test_contents_removed() {
title: "",
});
let skipDescendantsObserver = expectNotifications(true);
let receiveAllObserver = expectNotifications(false);
let skipDescendantsObserver = expectPlacesObserverNotifications(
["bookmark-removed"],
false,
true
);
let receiveAllObserver = expectPlacesObserverNotifications(
["bookmark-removed"],
false,
false
);
let frecencyChangedPromise = promiseFrecencyChanged("http://example.com/", 0);
await PlacesUtils.bookmarks.remove(folder1);
Assert.strictEqual(await PlacesUtils.bookmarks.fetch(folder1.guid), null);
@ -355,10 +369,8 @@ add_task(async function test_contents_removed() {
let expectedNotifications = [
{
name: "onItemRemoved",
arguments: {
guid: folder1.guid,
},
type: "bookmark-removed",
guid: folder1.guid,
},
];
@ -367,12 +379,9 @@ add_task(async function test_contents_removed() {
// Note: Items of folders get notified first.
expectedNotifications.unshift({
name: "onItemRemoved",
arguments: {
guid: bm1.guid,
},
type: "bookmark-removed",
guid: bm1.guid,
});
// If we don't skip descendents, we'll be notified of the folder and the
// bookmark.
receiveAllObserver.check(expectedNotifications);

131
toolkit/components/places/tests/bookmarks/test_nsINavBookmarkObserver.js
View File

@ -63,9 +63,6 @@ var gBookmarksObserver = {
onEndUpdateBatch() {
return this.validate("onEndUpdateBatch", arguments);
},
onItemRemoved() {
return this.validate("onItemRemoved", arguments);
},
onItemChanged() {
return this.validate("onItemChanged", arguments);
},
@ -82,7 +79,6 @@ var gBookmarksObserver = {
var gBookmarkSkipObserver = {
skipTags: true,
skipDescendantsOnItemRemoval: true,
expected: null,
setup(expected) {
@ -122,9 +118,6 @@ var gBookmarkSkipObserver = {
onEndUpdateBatch() {
return this.validate("onEndUpdateBatch", arguments);
},
onItemRemoved() {
return this.validate("onItemRemoved", arguments);
},
onItemChanged() {
return this.validate("onItemChanged", arguments);
},
@ -152,11 +145,11 @@ add_task(async function setup() {
gBookmarkSkipObserver
);
PlacesUtils.observers.addListener(
["bookmark-added"],
["bookmark-added", "bookmark-removed"],
gBookmarksObserver.handlePlacesEvents
);
PlacesUtils.observers.addListener(
["bookmark-added"],
["bookmark-added", "bookmark-removed"],
gBookmarkSkipObserver.handlePlacesEvents
);
});
@ -429,42 +422,16 @@ add_task(async function onItemChanged_tags_bookmark() {
],
},
{
name: "onItemRemoved", // This is the tag.
eventType: "bookmark-removed", // This is the tag.
args: [
{ name: "itemId", check: v => typeof v == "number" && v > 0 },
{ name: "id", check: v => typeof v == "number" && v > 0 },
{ name: "parentId", check: v => typeof v == "number" && v > 0 },
{ name: "index", check: v => v === 0 },
{
name: "itemType",
check: v => v === PlacesUtils.bookmarks.TYPE_BOOKMARK,
},
{ name: "uri", check: v => v instanceof Ci.nsIURI && v.equals(uri) },
{
name: "guid",
check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
},
{
name: "parentGuid",
check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
},
{
name: "source",
check: v =>
Object.values(PlacesUtils.bookmarks.SOURCES).includes(v),
},
],
},
{
name: "onItemRemoved", // This is the tag folder.
args: [
{ name: "itemId", check: v => typeof v == "number" && v > 0 },
{ name: "parentId", check: v => v === PlacesUtils.tagsFolderId },
{ name: "index", check: v => v === 0 },
{
name: "itemType",
check: v => v === PlacesUtils.bookmarks.TYPE_FOLDER,
},
{ name: "uri", check: v => v === null },
{ name: "url", check: v => v == uri.spec },
{
name: "guid",
check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
@ -480,6 +447,7 @@ add_task(async function onItemChanged_tags_bookmark() {
},
],
},
{
name: "onItemChanged",
args: [
@ -509,6 +477,32 @@ add_task(async function onItemChanged_tags_bookmark() {
},
],
},
{
eventType: "bookmark-removed", // This is the tag folder.
args: [
{ name: "id", check: v => typeof v == "number" && v > 0 },
{ name: "parentId", check: v => v === PlacesUtils.tagsFolderId },
{ name: "index", check: v => v === 0 },
{
name: "itemType",
check: v => v === PlacesUtils.bookmarks.TYPE_FOLDER,
},
{ name: "url", check: v => v === "" },
{
name: "guid",
check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
},
{
name: "parentGuid",
check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
},
{
name: "source",
check: v =>
Object.values(PlacesUtils.bookmarks.SOURCES).includes(v),
},
],
},
]),
]);
PlacesUtils.tagging.tagURI(uri, [TAG]);
@ -646,26 +640,26 @@ add_task(async function onItemMoved_bookmark() {
await promise;
});
add_task(async function onItemRemoved_bookmark() {
add_task(async function bookmarkItemRemoved_bookmark() {
let bm = await PlacesUtils.bookmarks.fetch({
parentGuid: PlacesUtils.bookmarks.unfiledGuid,
index: 0,
});
let uri = Services.io.newURI(bm.url.href);
let promise = Promise.all([
gBookmarkSkipObserver.setup(["onItemRemoved"]),
gBookmarkSkipObserver.setup(["bookmark-removed"]),
gBookmarksObserver.setup([
{
name: "onItemRemoved",
eventType: "bookmark-removed",
args: [
{ name: "itemId", check: v => typeof v == "number" && v > 0 },
{ name: "id", check: v => typeof v == "number" && v > 0 },
{ name: "parentId", check: v => v === gUnfiledFolderId },
{ name: "index", check: v => v === 0 },
{
name: "itemType",
check: v => v === PlacesUtils.bookmarks.TYPE_BOOKMARK,
},
{ name: "uri", check: v => v instanceof Ci.nsIURI && v.equals(uri) },
{ name: "url", check: v => v === uri.spec },
{
name: "guid",
check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
@ -687,25 +681,25 @@ add_task(async function onItemRemoved_bookmark() {
await promise;
});
add_task(async function onItemRemoved_separator() {
add_task(async function bookmarkItemRemoved_separator() {
let bm = await PlacesUtils.bookmarks.fetch({
parentGuid: PlacesUtils.bookmarks.unfiledGuid,
index: 0,
});
let promise = Promise.all([
gBookmarkSkipObserver.setup(["onItemRemoved"]),
gBookmarkSkipObserver.setup(["bookmark-removed"]),
gBookmarksObserver.setup([
{
name: "onItemRemoved",
eventType: "bookmark-removed",
args: [
{ name: "itemId", check: v => typeof v == "number" && v > 0 },
{ name: "id", check: v => typeof v == "number" && v > 0 },
{ name: "parentId", check: v => typeof v == "number" && v > 0 },
{ name: "index", check: v => v === 0 },
{
name: "itemType",
check: v => v === PlacesUtils.bookmarks.TYPE_SEPARATOR,
},
{ name: "uri", check: v => v === null },
{ name: "url", check: v => v === "" },
{
name: "guid",
check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
@ -727,25 +721,25 @@ add_task(async function onItemRemoved_separator() {
await promise;
});
add_task(async function onItemRemoved_folder() {
add_task(async function bookmarkItemRemoved_folder() {
let bm = await PlacesUtils.bookmarks.fetch({
parentGuid: PlacesUtils.bookmarks.unfiledGuid,
index: 0,
});
let promise = Promise.all([
gBookmarkSkipObserver.setup(["onItemRemoved"]),
gBookmarkSkipObserver.setup(["bookmark-removed"]),
gBookmarksObserver.setup([
{
name: "onItemRemoved",
eventType: "bookmark-removed",
args: [
{ name: "itemId", check: v => typeof v == "number" && v > 0 },
{ name: "id", check: v => typeof v == "number" && v > 0 },
{ name: "parentId", check: v => typeof v == "number" && v > 0 },
{ name: "index", check: v => v === 0 },
{
name: "itemType",
check: v => v === PlacesUtils.bookmarks.TYPE_FOLDER,
},
{ name: "uri", check: v => v === null },
{ name: "url", check: v => v === "" },
{
name: "guid",
check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
@ -767,7 +761,7 @@ add_task(async function onItemRemoved_folder() {
await promise;
});
add_task(async function onItemRemoved_folder_recursive() {
add_task(async function bookmarkItemRemoved_folder_recursive() {
const title = "Folder 3";
const BMTITLE = "Bookmark 1";
let uri = Services.io.newURI("http://1.mozilla.org/");
@ -777,7 +771,10 @@ add_task(async function onItemRemoved_folder_recursive() {
"bookmark-added",
"bookmark-added",
"bookmark-added",
"onItemRemoved",
"bookmark-removed",
"bookmark-removed",
"bookmark-removed",
"bookmark-removed",
]),
gBookmarksObserver.setup([
{
@ -893,16 +890,16 @@ add_task(async function onItemRemoved_folder_recursive() {
],
},
{
name: "onItemRemoved",
eventType: "bookmark-removed",
args: [
{ name: "itemId", check: v => typeof v == "number" && v > 0 },
{ name: "id", check: v => typeof v == "number" && v > 0 },
{ name: "parentId", check: v => typeof v == "number" && v > 0 },
{ name: "index", check: v => v === 0 },
{
name: "itemType",
check: v => v === PlacesUtils.bookmarks.TYPE_BOOKMARK,
},
{ name: "uri", check: v => v instanceof Ci.nsIURI && v.equals(uri) },
{ name: "url", check: v => v === uri.spec },
{
name: "guid",
check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
@ -919,16 +916,16 @@ add_task(async function onItemRemoved_folder_recursive() {
],
},
{
name: "onItemRemoved",
eventType: "bookmark-removed",
args: [
{ name: "itemId", check: v => typeof v == "number" && v > 0 },
{ name: "id", check: v => typeof v == "number" && v > 0 },
{ name: "parentId", check: v => typeof v == "number" && v > 0 },
{ name: "index", check: v => v === 1 },
{
name: "itemType",
check: v => v === PlacesUtils.bookmarks.TYPE_FOLDER,
},
{ name: "uri", check: v => v === null },
{ name: "url", check: v => v === "" },
{
name: "guid",
check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
@ -945,16 +942,16 @@ add_task(async function onItemRemoved_folder_recursive() {
],
},
{
name: "onItemRemoved",
eventType: "bookmark-removed",
args: [
{ name: "itemId", check: v => typeof v == "number" && v > 0 },
{ name: "id", check: v => typeof v == "number" && v > 0 },
{ name: "parentId", check: v => typeof v == "number" && v > 0 },
{ name: "index", check: v => v === 0 },
{
name: "itemType",
check: v => v === PlacesUtils.bookmarks.TYPE_BOOKMARK,
},
{ name: "uri", check: v => v instanceof Ci.nsIURI && v.equals(uri) },
{ name: "url", check: v => v === uri.spec },
{
name: "guid",
check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),
@ -971,16 +968,16 @@ add_task(async function onItemRemoved_folder_recursive() {
],
},
{
name: "onItemRemoved",
eventType: "bookmark-removed",
args: [
{ name: "itemId", check: v => typeof v == "number" && v > 0 },
{ name: "id", check: v => typeof v == "number" && v > 0 },
{ name: "parentId", check: v => typeof v == "number" && v > 0 },
{ name: "index", check: v => v === 0 },
{
name: "itemType",
check: v => v === PlacesUtils.bookmarks.TYPE_FOLDER,
},
{ name: "uri", check: v => v === null },
{ name: "url", check: v => v === "" },
{
name: "guid",
check: v => typeof v == "string" && PlacesUtils.isValidGuid(v),

27
toolkit/components/places/tests/bookmarks/test_removeFolderTransaction_reinsert.js
View File

@ -31,7 +31,7 @@ add_task(async function test_removeFolderTransaction_reinsert() {
let listener = events => {
for (let event of events) {
notifications.push([
"bookmark-added",
event.type,
event.id,
event.parentId,
event.guid,
@ -41,15 +41,18 @@ add_task(async function test_removeFolderTransaction_reinsert() {
};
let observer = {
__proto__: NavBookmarkObserver.prototype,
onItemRemoved(itemId, parentId, index, type, uri, guid, parentGuid) {
notifications.push(["onItemRemoved", itemId, parentId, guid, parentGuid]);
},
};
PlacesUtils.bookmarks.addObserver(observer);
PlacesUtils.observers.addListener(["bookmark-added"], listener);
PlacesUtils.observers.addListener(
["bookmark-added", "bookmark-removed"],
listener
);
PlacesUtils.registerShutdownFunction(function() {
PlacesUtils.bookmarks.removeObserver(observer);
PlacesUtils.observers.removeListener(["bookmark-added"], listener);
PlacesUtils.observers.removeListener(
["bookmark-added", "bookmark-removed"],
listener
);
});
let transaction = PlacesTransactions.Remove({ guid: folder.guid });
@ -62,10 +65,10 @@ add_task(async function test_removeFolderTransaction_reinsert() {
checkNotifications(
[
["onItemRemoved", tbId, folderId, tb.guid, folder.guid],
["onItemRemoved", fxId, folderId, fx.guid, folder.guid],
["bookmark-removed", tbId, folderId, tb.guid, folder.guid],
["bookmark-removed", fxId, folderId, fx.guid, folder.guid],
[
"onItemRemoved",
"bookmark-removed",
folderId,
PlacesUtils.bookmarksMenuFolderId,
folder.guid,
@ -100,10 +103,10 @@ add_task(async function test_removeFolderTransaction_reinsert() {
checkNotifications(
[
["onItemRemoved", tbId, folderId, tb.guid, folder.guid],
["onItemRemoved", fxId, folderId, fx.guid, folder.guid],
["bookmark-removed", tbId, folderId, tb.guid, folder.guid],
["bookmark-removed", fxId, folderId, fx.guid, folder.guid],
[
"onItemRemoved",
"bookmark-removed",
folderId,
PlacesUtils.bookmarksMenuFolderId,
folder.guid,

57
toolkit/components/places/tests/legacy/test_bookmarks.js
View File

@ -12,26 +12,34 @@ var bookmarksObserver = {
handlePlacesEvents(events) {
Assert.equal(events.length, 1);
let event = events[0];
bookmarksObserver._itemAddedId = event.id;
bookmarksObserver._itemAddedParent = event.parentId;
bookmarksObserver._itemAddedIndex = event.index;
bookmarksObserver._itemAddedURI = event.url
? Services.io.newURI(event.url)
: null;
bookmarksObserver._itemAddedTitle = event.title;
switch (event.type) {
case "bookmark-added":
bookmarksObserver._itemAddedId = event.id;
bookmarksObserver._itemAddedParent = event.parentId;
bookmarksObserver._itemAddedIndex = event.index;
bookmarksObserver._itemAddedURI = event.url
? Services.io.newURI(event.url)
: null;
bookmarksObserver._itemAddedTitle = event.title;
// Ensure that we've created a guid for this item.
let stmt = DBConn().createStatement(
`SELECT guid
FROM moz_bookmarks
WHERE id = :item_id`
);
stmt.params.item_id = event.id;
Assert.ok(stmt.executeStep());
Assert.ok(!stmt.getIsNull(0));
do_check_valid_places_guid(stmt.row.guid);
Assert.equal(stmt.row.guid, event.guid);
stmt.finalize();
// Ensure that we've created a guid for this item.
let stmt = DBConn().createStatement(
`SELECT guid
FROM moz_bookmarks
WHERE id = :item_id`
);
stmt.params.item_id = event.id;
Assert.ok(stmt.executeStep());
Assert.ok(!stmt.getIsNull(0));
do_check_valid_places_guid(stmt.row.guid);
Assert.equal(stmt.row.guid, event.guid);
stmt.finalize();
break;
case "bookmark-removed":
bookmarksObserver._itemRemovedId = event.id;
bookmarksObserver._itemRemovedFolder = event.parentId;
bookmarksObserver._itemRemovedIndex = event.index;
}
},
onBeginUpdateBatch() {
@ -40,11 +48,7 @@ var bookmarksObserver = {
onEndUpdateBatch() {
this._endUpdateBatch = true;
},
onItemRemoved(id, folder, index, itemType) {
this._itemRemovedId = id;
this._itemRemovedFolder = folder;
this._itemRemovedIndex = index;
},
onItemChanged(
id,
property,
@ -84,7 +88,10 @@ var bmStartIndex = 0;
add_task(async function test_bookmarks() {
bs.addObserver(bookmarksObserver);
os.addListener(["bookmark-added"], bookmarksObserver.handlePlacesEvents);
os.addListener(
["bookmark-added", "bookmark-removed"],
bookmarksObserver.handlePlacesEvents
);
// test special folders
Assert.ok(bs.placesRoot > 0);

5
toolkit/components/places/tests/queries/test_bookmarks.js
View File

@ -37,7 +37,12 @@ add_task(async function test_eraseEverything() {
}
Assert.equal(root.getChild(0).title, "title 1");
Assert.equal(root.getChild(1).title, "title 2");
await PlacesUtils.bookmarks.eraseEverything();
// Refetch the guid to refresh the data.
unfiled = PlacesUtils.getFolderContents(PlacesUtils.bookmarks.unfiledGuid)
.root;
Assert.equal(unfiled.childCount, 0);
unfiled.containerOpen = false;
});

85
toolkit/components/places/tests/sync/head_sync.js
View File

@ -296,44 +296,59 @@ function BookmarkObserver({ ignoreDates = true, skipTags = false } = {}) {
BookmarkObserver.prototype = {
handlePlacesEvents(events) {
for (let event of events) {
if (this.skipTags && event.isTagging) {
continue;
switch (event.type) {
case "bookmark-added": {
if (this.skipTags && event.isTagging) {
continue;
}
let params = {
itemId: event.id,
parentId: event.parentId,
index: event.index,
type: event.itemType,
urlHref: event.url,
title: event.title,
guid: event.guid,
parentGuid: event.parentGuid,
source: event.source,
};
if (!this.ignoreDates) {
params.dateAdded = event.dateAdded * 1000;
}
this.notifications.push({ name: "bookmark-added", params });
break;
}
case "bookmark-removed": {
if (this.skipTags && event.isTagging) {
continue;
}
// Since we are now skipping tags on the listener side we don't
// prevent unTagging notifications from going out. These events cause empty
// tags folders to be removed which creates another bookmark-removed notification
if (
this.skipTags &&
event.parentGuid == PlacesUtils.bookmarks.tagsGuid
) {
continue;
}
let params = {
itemId: event.id,
parentId: event.parentId,
index: event.index,
type: event.itemType,
urlHref: event.url || null,
guid: event.guid,
parentGuid: event.parentGuid,
source: event.source,
};
this.notifications.push({ name: "bookmark-removed", params });
break;
}
}
let params = {
itemId: event.id,
parentId: event.parentId,
index: event.index,
type: event.itemType,
urlHref: event.url,
title: event.title,
guid: event.guid,
parentGuid: event.parentGuid,
source: event.source,
};
if (!this.ignoreDates) {
params.dateAdded = event.dateAdded * 1000;
}
this.notifications.push({ name: "bookmark-added", params });
}
},
onBeginUpdateBatch() {},
onEndUpdateBatch() {},
onItemRemoved(itemId, parentId, index, type, uri, guid, parentGuid, source) {
let urlHref = uri ? uri.spec : null;
this.notifications.push({
name: "onItemRemoved",
params: {
itemId,
parentId,
index,
type,
urlHref,
guid,
parentGuid,
source,
},
});
},
onItemChanged(
itemId,
property,
@ -401,7 +416,7 @@ BookmarkObserver.prototype = {
check(expectedNotifications) {
PlacesUtils.bookmarks.removeObserver(this);
PlacesUtils.observers.removeListener(
["bookmark-added"],
["bookmark-added", "bookmark-removed"],
this.handlePlacesEvents
);
if (!ObjectUtils.deepEqual(this.notifications, expectedNotifications)) {
@ -419,7 +434,7 @@ function expectBookmarkChangeNotifications(options) {
let observer = new BookmarkObserver(options);
PlacesUtils.bookmarks.addObserver(observer);
PlacesUtils.observers.addListener(
["bookmark-added"],
["bookmark-added", "bookmark-removed"],
observer.handlePlacesEvents
);
return observer;

9
toolkit/components/places/tests/sync/test_bookmark_haschanges.js
View File

@ -210,8 +210,17 @@ add_task(async function test_changes_deleted_bookmark() {
);
await PlacesTestUtils.markBookmarksAsSynced();
let wait = PlacesTestUtils.waitForNotification(
"bookmark-removed",
events =>
events.some(event => event.parentGuid == PlacesUtils.bookmarks.tagsGuid),
"places"
);
await PlacesUtils.bookmarks.remove("mozBmk______");
await wait;
// Wait for everything to be finished
await new Promise(resolve => Services.tm.dispatchToMainThread(resolve));
let controller = new AbortController();
const wasMerged = await buf.merge(controller.signal);
Assert.ok(wasMerged);

26
toolkit/components/places/tests/sync/test_bookmark_observer_recorder.js
View File

@ -433,19 +433,6 @@ add_task(async function test_apply_then_revert() {
]);
observer.check([
{
name: "onItemRemoved",
params: {
itemId: localIdForD,
parentId: PlacesUtils.bookmarksMenuFolderId,
index: 1,
type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
urlHref: "http://example.com/d",
guid: "bookmarkDDDD",
parentGuid: PlacesUtils.bookmarks.menuGuid,
source: PlacesUtils.bookmarks.SOURCES.SYNC,
},
},
{
name: "onItemChanged",
params: {
@ -461,6 +448,19 @@ add_task(async function test_apply_then_revert() {
source: PlacesUtils.bookmarks.SOURCES.SYNC,
},
},
{
name: "bookmark-removed",
params: {
itemId: localIdForD,
parentId: PlacesUtils.bookmarksMenuFolderId,
index: 1,
type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
urlHref: "http://example.com/d",
guid: "bookmarkDDDD",
parentGuid: PlacesUtils.bookmarks.menuGuid,
source: PlacesUtils.bookmarks.SOURCES.SYNC,
},
},
{
name: "bookmark-added",
params: {

11
toolkit/components/places/tests/sync/test_bookmark_value_changes.js
View File

@ -1890,11 +1890,22 @@ add_task(async function test_tags() {
"eleven",
"twelve",
]);
let wait = PlacesTestUtils.waitForNotification(
"bookmark-removed",
events =>
events.some(event => event.parentGuid == PlacesUtils.bookmarks.tagsGuid),
"places"
);
PlacesUtils.tagging.untagURI(
Services.io.newURI("http://example.com/d"),
null
);
await wait;
await new Promise(resolve => Services.tm.dispatchToMainThread(resolve));
info("Apply remote");
let changesToUpload = await buf.apply();
deepEqual(await buf.fetchUnmergedGuids(), [], "Should merge all items");

97
toolkit/components/places/tests/unit/test_async_transactions.js
View File

@ -33,20 +33,34 @@ var observer = {
handlePlacesEvents(events) {
for (let event of events) {
// Ignore tag items.
if (event.isTagging) {
this.tagRelatedGuids.add(event.guid);
return;
}
switch (event.type) {
case "bookmark-added":
// Ignore tag items.
if (event.isTagging) {
this.tagRelatedGuids.add(event.guid);
return;
}
this.itemsAdded.set(event.guid, {
itemId: event.id,
parentGuid: event.parentGuid,
index: event.index,
itemType: event.itemType,
title: event.title,
url: event.url,
});
this.itemsAdded.set(event.guid, {
itemId: event.id,
parentGuid: event.parentGuid,
index: event.index,
itemType: event.itemType,
title: event.title,
url: event.url,
});
break;
case "bookmark-removed":
if (this.tagRelatedGuids.has(event.guid)) {
return;
}
this.itemsRemoved.set(event.guid, {
parentGuid: event.parentGuid,
index: event.index,
itemType: event.itemType,
});
}
}
},
@ -58,26 +72,6 @@ var observer = {
this.endUpdateBatch = true;
},
onItemRemoved(
aItemId,
aParentId,
aIndex,
aItemType,
aURI,
aGuid,
aParentGuid
) {
if (this.tagRelatedGuids.has(aGuid)) {
return;
}
this.itemsRemoved.set(aGuid, {
parentGuid: aParentGuid,
index: aIndex,
itemType: aItemType,
});
},
onItemChanged(
aItemId,
aProperty,
@ -137,10 +131,16 @@ var bmStartIndex = 0;
function run_test() {
bmsvc.addObserver(observer);
observer.handlePlacesEvents = observer.handlePlacesEvents.bind(observer);
obsvc.addListener(["bookmark-added"], observer.handlePlacesEvents);
obsvc.addListener(
["bookmark-added", "bookmark-removed"],
observer.handlePlacesEvents
);
registerCleanupFunction(function() {
bmsvc.removeObserver(observer);
obsvc.removeListener(["bookmark-added"], observer.handlePlacesEvents);
obsvc.removeListener(
["bookmark-added", "bookmark-removed"],
observer.handlePlacesEvents
);
});
run_next_test();
@ -1241,7 +1241,34 @@ add_task(async function test_add_and_remove_bookmarks_with_additional_info() {
observer.reset();
await PT.redo();
// The tag containers are removed in async and take some time
let oldCountTag1 = 0;
let oldCountTag2 = 0;
let allTags = await bmsvc.fetchTags();
for (let i of allTags) {
if (i.name == TAG_1) {
oldCountTag1 = i.count;
}
if (i.name == TAG_2) {
oldCountTag2 = i.count;
}
}
await TestUtils.waitForCondition(async () => {
allTags = await bmsvc.fetchTags();
let newCountTag1 = 0;
let newCountTag2 = 0;
for (let i of allTags) {
if (i.name == TAG_1) {
newCountTag1 = i.count;
}
if (i.name == TAG_2) {
newCountTag2 = i.count;
}
}
return newCountTag1 == oldCountTag1 - 1 && newCountTag2 == oldCountTag2 - 1;
});
await ensureItemsRemoved(b2_info);
ensureTags([]);
observer.reset();

40
toolkit/components/places/tests/unit/test_childlessTags.js
View File

@ -31,9 +31,21 @@ add_task(async function test_removing_tagged_bookmark_removes_tag() {
let tags = ["foo", "bar"];
tagssvc.tagURI(BOOKMARK_URI, tags);
ensureTagsExist(tags);
let root = getTagRoot();
root.containerOpen = true;
let oldCount = root.childCount;
root.containerOpen = false;
print(" Remove the bookmark. The tags should no longer exist.");
let wait = TestUtils.waitForCondition(() => {
root = getTagRoot();
root.containerOpen = true;
let val = root.childCount == oldCount - 2;
root.containerOpen = false;
return val;
});
await PlacesUtils.bookmarks.remove(bookmark.guid);
await wait;
ensureTagsExist([]);
});
@ -58,12 +70,40 @@ add_task(
tagssvc.tagURI(BOOKMARK_URI, tags);
ensureTagsExist(tags);
// The tag containers are removed in async and take some time
let oldCountFoo = await tagCount("foo");
let oldCountBar = await tagCount("bar");
print(" Remove the folder. The tags should no longer exist.");
let wait = TestUtils.waitForCondition(async () => {
let newCountFoo = await tagCount("foo");
let newCountBar = await tagCount("bar");
return newCountFoo == oldCountFoo - 1 && newCountBar == oldCountBar - 1;
});
await PlacesUtils.bookmarks.remove(bookmark.guid);
await wait;
ensureTagsExist([]);
}
);
async function tagCount(aTag) {
let allTags = await PlacesUtils.bookmarks.fetchTags();
for (let i of allTags) {
if (i.name == aTag) {
return i.count;
}
}
return 0;
}
function getTagRoot() {
var query = histsvc.getNewQuery();
var opts = histsvc.getNewQueryOptions();
opts.resultType = opts.RESULTS_AS_TAGS_ROOT;
var resultRoot = histsvc.executeQuery(query, opts).root;
return resultRoot;
}
/**
* Runs a tag query and ensures that the tags returned are those and only those
* in aTags. aTags may be empty, in which case this function ensures that no

26
toolkit/components/places/tests/unit/test_onItemChanged_tags.js
View File

@ -36,12 +36,19 @@ add_task(async function run_test() {
this._changedCount++;
}
},
onItemRemoved(aItemId, aParentId, aIndex, aItemType, aURI, aGuid) {
if (aGuid == bookmark.guid) {
PlacesUtils.bookmarks.removeObserver(this);
Assert.equal(this._changedCount, 2);
promise.resolve();
handlePlacesEvents(events) {
for (let event of events) {
switch (event.type) {
case "bookmark-removed":
if (event.guid == bookmark.guid) {
PlacesUtils.observers.removeListener(
["bookmark-removed"],
this.handlePlacesEvents
);
Assert.equal(this._changedCount, 2);
promise.resolve();
}
}
}
},
@ -51,6 +58,13 @@ add_task(async function run_test() {
onItemMoved() {},
};
PlacesUtils.bookmarks.addObserver(bookmarksObserver);
bookmarksObserver.handlePlacesEvents = bookmarksObserver.handlePlacesEvents.bind(
bookmarksObserver
);
PlacesUtils.observers.addListener(
["bookmark-removed"],
bookmarksObserver.handlePlacesEvents
);
PlacesUtils.tagging.tagURI(uri, ["d"]);
PlacesUtils.tagging.tagURI(uri, ["e"]);

18
toolkit/components/places/tests/unit/test_tagging.js
View File

@ -64,9 +64,16 @@ function run_test() {
tagssvc.untagURI(uri1, ["tag 1"]);
Assert.equal(tag1node.childCount, 1);
let wait = PlacesTestUtils.waitForNotification(
"bookmark-removed",
events => events.some(event => event.id == tagRoot.itemId),
"places"
);
// removing the last uri from a tag should remove the tag-container
tagssvc.untagURI(uri2, ["tag 1"]);
Assert.equal(tagRoot.childCount, 1);
wait.then(() => {
Assert.equal(tagRoot.childCount, 1);
});
// cleanup
tag1node.containerOpen = false;
@ -116,8 +123,15 @@ function run_test() {
do_throw("Passing a sparse array should not throw");
}
try {
wait = PlacesTestUtils.waitForNotification(
"bookmark-removed",
events => events.some(event => event.id == tagRoot.itemId),
"places"
);
tagssvc.untagURI(uri1, [undefined, "tagSparse"]);
Assert.equal(tagRoot.childCount, curChildCount);
wait.then(() => {
Assert.equal(tagRoot.childCount, curChildCount);
});
} catch (ex) {
do_throw("Passing a sparse array should not throw");
}

1
tools/lint/eslint/eslint-plugin-mozilla/lib/environments/privileged.js
View File

@ -419,6 +419,7 @@ module.exports = {
Permissions: false,
PlacesBookmark: false,
PlacesBookmarkAddition: false,
PlacesBookmarkRemoved: false,
PlacesEvent: false,
PlacesObservers: false,
PlacesVisit: false,