Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-11-27 23:02:20 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1678618: Apply page-removed event. r=mak

Browse Source 
Depends on D101114

Differential Revision: https://phabricator.services.mozilla.com/D101115
This commit is contained in:
Daisuke Akatsuka 2021-02-15 02:47:38 +00:00
parent a0b93f756d
commit 687e052ffb
25 changed files with 435 additions and 232 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
browser/components/downloads/test/browser/browser_library_clearall.js
View File

@ -50,15 +50,19 @@ async function testClearingDownloads(clearCallback) {
await promiseLength;
let receivedNotifications = [];
let promiseNotification = PlacesTestUtils.waitForNotification(
"onDeleteURI",
uri => {
if (DOWNLOAD_DATA.includes(uri.spec)) {
receivedNotifications.push(uri.spec);
const promiseNotification = PlacesTestUtils.waitForNotification(
"page-removed",
events => {
for (const { url, isRemovedFromStore } of events) {
Assert.ok(isRemovedFromStore);
if (DOWNLOAD_DATA.includes(url)) {
receivedNotifications.push(url);
}
}
return receivedNotifications.length == DOWNLOAD_DATA.length;
},
"history"
"places"
);
promiseLength = waitForChildrenLength(listbox, 0);

25
browser/components/extensions/parent/ext-history.js
View File

@ -261,18 +261,37 @@ this.history = class extends ExtensionAPI {
fire.sync(data);
};
const historyClearedListener = events => {
fire.sync({ allHistory: true, urls: [] });
const removedURLs = [];
for (const event of events) {
switch (event.type) {
case "history-cleared": {
fire.sync({ allHistory: true, urls: [] });
break;
}
case "page-removed": {
if (!event.isPartialVisistsRemoval) {
removedURLs.push(event.url);
}
break;
}
}
}
if (removedURLs.length) {
fire.sync({ allHistory: false, urls: removedURLs });
}
};
getHistoryObserver().on("visitRemoved", listener);
PlacesUtils.observers.addListener(
["history-cleared"],
["history-cleared", "page-removed"],
historyClearedListener
);
return () => {
getHistoryObserver().off("visitRemoved", listener);
PlacesUtils.observers.removeListener(
["history-cleared"],
["history-cleared", "page-removed"],
historyClearedListener
);
};

7
browser/components/extensions/test/xpcshell/test_ext_history.js
View File

@ -32,12 +32,7 @@ add_task(async function test_delete() {
"onVisitRemoved received an empty urls array"
);
} else {
browser.test.assertEq(
1,
data.urls.length,
"onVisitRemoved received one URL"
);
removedUrls.push(data.urls[0]);
removedUrls.push(...data.urls);
}
});

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

@ -116,6 +116,7 @@ class PlacesObserver extends Observer {
guid,
title,
url,
isRemovedFromStore,
isTagging,
type,
} of events) {
@ -123,6 +124,15 @@ class PlacesObserver extends Observer {
case "history-cleared":
this.dispatch({ type: at.PLACES_HISTORY_CLEARED });
break;
case "page-removed":
if (isRemovedFromStore) {
this.dispatch({ type: at.PLACES_LINKS_CHANGED });
this.dispatch({
type: at.PLACES_LINK_DELETED,
data: { url },
});
}
break;
case "bookmark-added":
// Skips items that are not bookmarks (like folders), about:* pages or
// default bookmarks, added when the profile is created.
@ -192,7 +202,7 @@ class PlacesFeed {
.getService(Ci.nsINavBookmarksService)
.addObserver(this.bookmarksObserver, true);
PlacesUtils.observers.addListener(
["bookmark-added", "bookmark-removed", "history-cleared"],
["bookmark-added", "bookmark-removed", "history-cleared", "page-removed"],
this.placesObserver.handlePlacesEvent
);
@ -236,7 +246,7 @@ class PlacesFeed {
PlacesUtils.history.removeObserver(this.historyObserver);
PlacesUtils.bookmarks.removeObserver(this.bookmarksObserver);
PlacesUtils.observers.removeListener(
["bookmark-added", "bookmark-removed", "history-cleared"],
["bookmark-added", "bookmark-removed", "history-cleared", "page-removed"],
this.placesObserver.handlePlacesEvent
);
Services.obs.removeObserver(this, LINK_BLOCKED_EVENT);

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

@ -127,7 +127,12 @@ describe("PlacesFeed", () => {
);
assert.calledWith(
global.PlacesUtils.observers.addListener,
["bookmark-added", "bookmark-removed", "history-cleared"],
[
"bookmark-added",
"bookmark-removed",
"history-cleared",
"page-removed",
],
feed.placesObserver.handlePlacesEvent
);
assert.calledWith(global.Services.obs.addObserver, feed, BLOCKED_EVENT);
@ -149,7 +154,12 @@ describe("PlacesFeed", () => {
);
assert.calledWith(
global.PlacesUtils.observers.removeListener,
["bookmark-added", "bookmark-removed", "history-cleared"],
[
"bookmark-added",
"bookmark-removed",
"history-cleared",
"page-removed",
],
feed.placesObserver.handlePlacesEvent
);
assert.calledWith(
@ -752,16 +762,6 @@ describe("PlacesFeed", () => {
it("should have a QueryInterface property", () => {
assert.property(observer, "QueryInterface");
});
describe("#onDeleteURI", () => {
it("should dispatch a PLACES_LINK_DELETED action with the right url", async () => {
await observer.onDeleteURI({ spec: "foo.com" });
assert.calledWith(dispatch, {
type: at.PLACES_LINK_DELETED,
data: { url: "foo.com" },
});
});
});
describe("Other empty methods (to keep code coverage happy)", () => {
it("should have a various empty functions for xpconnect happiness", () => {
observer.onBeginUpdateBatch();
@ -850,6 +850,17 @@ describe("PlacesFeed", () => {
});
});
describe("#page-removed", () => {
it("should dispatch a PLACES_LINK_DELETED action with the right url", async () => {
const args = [{ type: "page-removed", url: "foo.com" }];
await observer.handlePlacesEvent(args);
assert.calledWith(dispatch, {
type: at.PLACES_LINK_DELETED,
data: { url: "foo.com" },
});
});
});
describe("#bookmark-added", () => {
it("should dispatch a PLACES_BOOKMARK_ADDED action with the bookmark data - http", async () => {
const args = [

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

@ -81,13 +81,17 @@ add_task(async function test_date_container() {
);
// Execute the delete command and check visit has been removed.
let promiseURIRemoved = PlacesTestUtils.waitForNotification(
"onDeleteURI",
v => TEST_URI.equals(v),
"history"
const promiseURIRemoved = PlacesTestUtils.waitForNotification(
"page-removed",
events => events[0].url === TEST_URI.spec,
"places"
);
PO._places.controller.doCommand("cmd_delete");
await promiseURIRemoved;
const removeEvents = await promiseURIRemoved;
Assert.ok(
removeEvents[0].isRemovedFromStore,
"isRemovedFromStore should be true"
);
// Test live update of "History" query.
Assert.equal(historyNode.childCount, 0, "History node has no more children");

16
browser/components/places/tests/chrome/test_bug549192.xhtml
View File

@ -105,10 +105,20 @@
for (let i = 0; i < rc; i++) {
selection.select(0);
let node = tree.selectedNode;
let promiseDeleted = PlacesTestUtils.waitForNotification("onDeleteURI",
uri => uri.spec == node.uri, "history");
const promiseRemoved = PlacesTestUtils.waitForNotification(
"page-removed",
events => events[0].url === node.uri,
"places"
);
tree.controller.remove();
await promiseDeleted;
const removeEvents = await promiseRemoved;
ok(
removeEvents[0].isRemovedFromStore,
"isRemovedFromStore should be true"
);
ok(treeView.treeIndexForNode(node) == -1, node.uri + " removed.");
is(treeView.rowCount, rc - i - 1, "Rows count decreased");
}

14
browser/components/urlbar/tests/browser/browser_remove_match.js
View File

@ -14,9 +14,9 @@ add_task(async function test_remove_history() {
});
let promiseVisitRemoved = PlacesTestUtils.waitForNotification(
"onDeleteURI",
uri => uri.spec == TEST_URL,
"history"
"page-removed",
events => events[0].url === TEST_URL,
"places"
);
await UrlbarTestUtils.promiseAutocompleteResultPopup({
@ -32,7 +32,13 @@ add_task(async function test_remove_history() {
EventUtils.synthesizeKey("KEY_ArrowDown");
Assert.equal(UrlbarTestUtils.getSelectedRowIndex(window), 1);
EventUtils.synthesizeKey("KEY_Delete", { shiftKey: true });
await promiseVisitRemoved;
const removeEvents = await promiseVisitRemoved;
Assert.ok(
removeEvents[0].isRemovedFromStore,
"isRemovedFromStore should be true"
);
await TestUtils.waitForCondition(
() => UrlbarTestUtils.getResultCount(window) == expectedResultCount,
"Waiting for the result to disappear"

22
services/sync/modules/engines/history.js
View File

@ -526,7 +526,7 @@ HistoryTracker.prototype = {
this.handlePlacesEvents.bind(this)
);
PlacesObservers.addListener(
["page-visited", "history-cleared"],
["page-visited", "history-cleared", "page-removed"],
this._placesObserver
);
},
@ -536,7 +536,7 @@ HistoryTracker.prototype = {
PlacesUtils.history.removeObserver(this);
if (this._placesObserver) {
PlacesObservers.removeListener(
["page-visited", "history-cleared"],
["page-visited", "history-cleared", "page-removed"],
this._placesObserver
);
}
@ -548,7 +548,7 @@ HistoryTracker.prototype = {
]),
async onDeleteAffectsGUID(uri, guid, reason, source, increment) {
if (this.ignoreAll || reason == Ci.nsINavHistoryObserver.REASON_EXPIRED) {
if (this.ignoreAll || reason === PlacesVisitRemoved.REASON_EXPIRED) {
return;
}
this._log.trace(source + ": " + uri.spec + ", reason " + reason);
@ -615,6 +615,22 @@ HistoryTracker.prototype = {
this.score += SCORE_INCREMENT_XLARGE;
break;
}
case "page-removed": {
if (event.reason === PlacesVisitRemoved.REASON_EXPIRED) {
return;
}
this._log.trace(
"page-removed: " + event.url + ", reason " + event.reason
);
const added = await this.addChangedID(event.pageGuid);
if (added) {
this.score += event.isRemovedFromStore
? SCORE_INCREMENT_XLARGE
: SCORE_INCREMENT_SMALL;
}
break;
}
}
}
},

16
services/sync/tests/unit/head_helpers.js
View File

@ -619,7 +619,7 @@ async function promiseVisit(expectedType, expectedURI) {
if (uri == expectedURI.spec && type == expectedType) {
PlacesUtils.history.removeObserver(observer);
PlacesObservers.removeListener(
["page-visited"],
["page-visited", "page-removed"],
observer.handlePlacesEvents
);
resolve();
@ -628,8 +628,13 @@ async function promiseVisit(expectedType, expectedURI) {
let observer = {
handlePlacesEvents(events) {
Assert.equal(events.length, 1);
Assert.equal(events[0].type, "page-visited");
done("added", events[0].url);
if (events[0].type === "page-visited") {
done("added", events[0].url);
} else if (events[0].type === "page-removed") {
Assert.ok(events[0].isRemovedFromStore);
done("removed", events[0].url);
}
},
onBeginUpdateBatch() {},
onEndUpdateBatch() {},
@ -639,7 +644,10 @@ async function promiseVisit(expectedType, expectedURI) {
onDeleteVisits() {},
};
PlacesUtils.history.addObserver(observer, false);
PlacesObservers.addListener(["page-visited"], observer.handlePlacesEvents);
PlacesObservers.addListener(
["page-visited", "page-removed"],
observer.handlePlacesEvents
);
});
}

11
toolkit/components/downloads/DownloadHistory.jsm
View File

@ -196,7 +196,10 @@ var DownloadCache = {
const placesObserver = new PlacesWeakCallbackWrapper(
this.handlePlacesEvents.bind(this)
);
PlacesObservers.addListener(["history-cleared"], placesObserver);
PlacesObservers.addListener(
["history-cleared", "page-removed"],
placesObserver
);
let pageAnnos = await PlacesUtils.history.fetchAnnotatedPages([
METADATA_ANNO,
@ -335,6 +338,12 @@ var DownloadCache = {
this._data.clear();
break;
}
case "page-removed": {
if (event.isRemovedFromStore) {
this._data.delete(event.url);
}
break;
}
}
}
},

13
toolkit/components/downloads/DownloadIntegration.jsm
View File

@ -1233,7 +1233,10 @@ var DownloadHistoryObserver = function(aList) {
const placesObserver = new PlacesWeakCallbackWrapper(
this.handlePlacesEvents.bind(this)
);
PlacesObservers.addListener(["history-cleared"], placesObserver);
PlacesObservers.addListener(
["history-cleared", "page-removed"],
placesObserver
);
};
DownloadHistoryObserver.prototype = {
@ -1251,6 +1254,14 @@ DownloadHistoryObserver.prototype = {
this._list.removeFinished();
break;
}
case "page-removed": {
if (event.isRemovedFromStore) {
this._list.removeFinished(
download => event.url === download.source.url
);
}
break;
}
}
}
},

111
toolkit/components/places/History.jsm
View File

@ -1031,77 +1031,64 @@ function removeOrphanIcons(db) {
* - hasForeign: (boolean) If `true`, the page has at least
* one foreign reference (i.e. a bookmark), so the page should
* be kept and its frecency updated.
* @param transition: (Number)
* @param transitionType: (Number)
* Set to a valid TRANSITIONS value to indicate all transitions of a
* certain type have been removed, otherwise defaults to -1 (unknown value).
* certain type have been removed, otherwise defaults to 0 (unknown value).
* @return (Promise)
*/
var notifyCleanup = async function(db, pages, transition = -1) {
var notifyCleanup = async function(db, pages, transitionType = 0) {
const notifications = [];
let notifiedCount = 0;
let observers = PlacesUtils.history.getObservers();
let bookmarkObservers = PlacesUtils.bookmarks.getObservers();
let reason = Ci.nsINavHistoryObserver.REASON_DELETED;
for (let page of pages) {
let uri = Services.io.newURI(page.url.href);
let guid = page.guid;
if (page.hasVisits || page.hasForeign) {
// We have removed all visits, but the page is still alive, e.g.
// because of a bookmark.
notify(observers, "onDeleteVisits", [
uri,
page.hasVisits > 0,
guid,
reason,
transition,
]);
// Also asynchronously notify bookmarks for this uri if all the visits
// have been removed.
if (!page.hasVisits) {
PlacesUtils.bookmarks
.fetch({ url: page.url }, async bookmark => {
let itemId = await PlacesUtils.promiseItemId(bookmark.guid);
let parentId = await PlacesUtils.promiseItemId(bookmark.parentGuid);
notify(
bookmarkObservers,
"onItemChanged",
[
itemId,
"cleartime",
false,
"",
0,
PlacesUtils.bookmarks.TYPE_BOOKMARK,
parentId,
bookmark.guid,
bookmark.parentGuid,
"",
PlacesUtils.bookmarks.SOURCES.DEFAULT,
],
{ concurrent: true }
);
})
.catch(Cu.reportError);
}
} else {
// The page has been entirely removed.
notify(observers, "onDeleteURI", [uri, guid, reason]);
PlacesObservers.notifyListeners([
new PlacesVisitRemoved({
url: uri.spec,
pageGuid: guid,
reason: PlacesVisitRemoved.REASON_DELETED,
isRemovedFromStore: true,
}),
]);
}
if (++notifiedCount % NOTIFICATION_CHUNK_SIZE == 0) {
// Every few notifications, yield time back to the main
// thread to avoid jank.
await Promise.resolve();
const isRemovedFromStore = !page.hasVisits && !page.hasForeign;
notifications.push(
new PlacesVisitRemoved({
url: Services.io.newURI(page.url.href).spec,
pageGuid: page.guid,
reason: PlacesVisitRemoved.REASON_DELETED,
transitionType,
isRemovedFromStore,
isPartialVisistsRemoval: !isRemovedFromStore && page.hasVisits > 0,
})
);
if (page.hasForeign && !page.hasVisits) {
PlacesUtils.bookmarks
.fetch({ url: page.url }, async bookmark => {
let itemId = await PlacesUtils.promiseItemId(bookmark.guid);
let parentId = await PlacesUtils.promiseItemId(bookmark.parentGuid);
notify(
bookmarkObservers,
"onItemChanged",
[
itemId,
"cleartime",
false,
"",
0,
PlacesUtils.bookmarks.TYPE_BOOKMARK,
parentId,
bookmark.guid,
bookmark.parentGuid,
"",
PlacesUtils.bookmarks.SOURCES.DEFAULT,
],
{ concurrent: true }
);
if (++notifiedCount % NOTIFICATION_CHUNK_SIZE == 0) {
// Every few notifications, yield time back to the main
// thread to avoid jank.
await Promise.resolve();
}
})
.catch(Cu.reportError);
}
}
PlacesObservers.notifyListeners(notifications);
};
/**

49
toolkit/components/places/PlacesExpiration.jsm
View File

@ -399,24 +399,6 @@ const EXPIRATION_QUERIES = {
},
};
/**
* Sends a bookmarks notification through the given observers.
*
* @param observers
* array of nsINavBookmarkObserver objects.
* @param notification
* the notification name.
* @param args
* array of arguments to pass to the notification.
*/
function notify(observers, notification, args = []) {
for (let observer of observers) {
try {
observer[notification](...args);
} catch (ex) {}
}
}
function nsPlacesExpiration() {
// Allows other components to easily access getPagesLimit.
this.wrappedJSObject = this;
@ -611,8 +593,6 @@ nsPlacesExpiration.prototype = {
let mostRecentExpiredVisit = row.getResultByName(
"most_recent_expired_visit"
);
let reason = Ci.nsINavHistoryObserver.REASON_EXPIRED;
let observers = PlacesUtils.history.getObservers();
if (mostRecentExpiredVisit) {
let days = parseInt(
@ -626,25 +606,16 @@ nsPlacesExpiration.prototype = {
}
// Dispatch expiration notifications to history.
if (wholeEntry) {
notify(observers, "onDeleteURI", [uri, guid, reason]);
PlacesObservers.notifyListeners([
new PlacesVisitRemoved({
url: uri.spec,
pageGuid: guid,
reason: PlacesVisitRemoved.REASON_EXPIRED,
isRemovedFromStore: true,
}),
]);
} else {
notify(observers, "onDeleteVisits", [
uri,
visitDate > 0,
guid,
reason,
0,
]);
}
const isRemovedFromStore = !!wholeEntry;
PlacesObservers.notifyListeners([
new PlacesVisitRemoved({
url: uri.spec,
pageGuid: guid,
reason: PlacesVisitRemoved.REASON_EXPIRED,
isRemovedFromStore,
isPartialVisistsRemoval: !isRemovedFromStore && visitDate > 0,
}),
]);
},
_shuttingDown: false,

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

@ -19,6 +19,7 @@
#include "nsQueryObject.h"
#include "mozilla/dom/PlacesObservers.h"
#include "mozilla/dom/PlacesVisit.h"
#include "mozilla/dom/PlacesVisitRemoved.h"
#include "mozilla/dom/PlacesVisitTitle.h"
#include "nsCycleCollectionParticipant.h"
@ -3513,6 +3514,7 @@ void nsNavHistoryResult::StopObserving() {
mIsHistoryObserver = false;
}
events.AppendElement(PlacesEventType::History_cleared);
events.AppendElement(PlacesEventType::Page_removed);
}
if (mIsHistoryDetailsObserver) {
events.AppendElement(PlacesEventType::Page_visited);
@ -3548,6 +3550,7 @@ void nsNavHistoryResult::AddHistoryObserver(
AutoTArray<PlacesEventType, 3> events;
events.AppendElement(PlacesEventType::History_cleared);
events.AppendElement(PlacesEventType::Page_removed);
if (!mIsHistoryDetailsObserver) {
events.AppendElement(PlacesEventType::Page_visited);
events.AppendElement(PlacesEventType::Page_title_changed);
@ -4203,6 +4206,29 @@ void nsNavHistoryResult::HandlePlacesEvent(const PlacesEventSequence& aEvents) {
ENUMERATE_HISTORY_OBSERVERS(OnClearHistory());
break;
}
case PlacesEventType::Page_removed: {
const PlacesVisitRemoved* removeEvent = event->AsPlacesVisitRemoved();
if (NS_WARN_IF(!removeEvent)) {
continue;
}
nsCOMPtr<nsIURI> uri;
MOZ_ALWAYS_SUCCEEDS(NS_NewURI(getter_AddRefs(uri), removeEvent->mUrl));
if (!uri) {
continue;
}
if (removeEvent->mIsRemovedFromStore) {
ENUMERATE_HISTORY_OBSERVERS(
OnDeleteURI(uri, removeEvent->mPageGuid, removeEvent->mReason));
} else {
ENUMERATE_HISTORY_OBSERVERS(
OnDeleteVisits(uri, removeEvent->mIsPartialVisistsRemoval,
removeEvent->mPageGuid, removeEvent->mReason,
removeEvent->mTransitionType));
}
break;
}
default: {
MOZ_ASSERT_UNREACHABLE(
"Receive notification of a type not subscribed to.");

50
toolkit/components/places/tests/expiration/test_notifications_onDeleteVisits.js → toolkit/components/places/tests/expiration/test_notifications_pageRemoved_allVisits.js
View File

@ -8,13 +8,9 @@
* What this is aimed to test:
*
* Expiring only visits for a page, but not the full page, should fire an
* onDeleteVisits notification.
* page-removed for all visits notification.
*/
var hs = Cc["@mozilla.org/browser/nav-history-service;1"].getService(
Ci.nsINavHistoryService
);
var tests = [
{
desc: "Add 1 bookmarked page.",
@ -68,7 +64,7 @@ var tests = [
},
];
add_task(async function test_notifications_onDeleteVisits() {
add_task(async () => {
// Set interval to a large value so we don't expire on it.
setInterval(3600); // 1h
@ -111,32 +107,32 @@ add_task(async function test_notifications_onDeleteVisits() {
}
// Observe history.
let historyObserver = {
onBeginUpdateBatch: function PEX_onBeginUpdateBatch() {},
onEndUpdateBatch: function PEX_onEndUpdateBatch() {},
onDeleteURI(aURI, aGUID, aReason) {
// Check this uri was not bookmarked.
Assert.equal(currentTest.bookmarks.indexOf(aURI.spec), -1);
do_check_valid_places_guid(aGUID);
Assert.equal(aReason, Ci.nsINavHistoryObserver.REASON_EXPIRED);
},
onDeleteVisits(aURI, aPartialRemoval, aGUID, aReason) {
currentTest.receivedNotifications++;
do_check_guid_for_uri(aURI, aGUID);
Assert.equal(
aPartialRemoval,
currentTest.expectedIsPartialRemoval,
"Should have the correct flag setting for partial removal"
);
Assert.equal(aReason, Ci.nsINavHistoryObserver.REASON_EXPIRED);
},
const listener = events => {
for (const event of events) {
Assert.equal(event.type, "page-removed");
Assert.equal(event.reason, PlacesVisitRemoved.REASON_EXPIRED);
if (event.isRemovedFromStore) {
// Check this uri was not bookmarked.
Assert.equal(currentTest.bookmarks.indexOf(event.url), -1);
do_check_valid_places_guid(event.pageGuid);
} else {
currentTest.receivedNotifications++;
do_check_guid_for_uri(Services.io.newURI(event.url), event.pageGuid);
Assert.equal(
event.isPartialVisistsRemoval,
currentTest.expectedIsPartialRemoval,
"Should have the correct flag setting for partial removal"
);
}
}
};
hs.addObserver(historyObserver);
PlacesObservers.addListener(["page-removed"], listener);
// Expire now.
await promiseForceExpirationStep(currentTest.limitExpiration);
hs.removeObserver(historyObserver, false);
PlacesObservers.removeListener(["page-removed"], listener);
Assert.equal(
currentTest.receivedNotifications,

33
toolkit/components/places/tests/expiration/test_notifications_onDeleteURI.js → toolkit/components/places/tests/expiration/test_notifications_pageRemoved_fromStore.js
View File

@ -7,13 +7,9 @@
/**
* What this is aimed to test:
*
* Expiring a full page should fire an onDeleteURI notification.
* Expiring a full page should fire an page-removed event notification.
*/
var hs = Cc["@mozilla.org/browser/nav-history-service;1"].getService(
Ci.nsINavHistoryService
);
var tests = [
{
desc: "Add 1 bookmarked page.",
@ -44,7 +40,7 @@ var tests = [
},
];
add_task(async function test_notifications_onDeleteURI() {
add_task(async () => {
// Set interval to a large value so we don't expire on it.
setInterval(3600); // 1h
@ -76,24 +72,27 @@ add_task(async function test_notifications_onDeleteURI() {
}
// Observe history.
let historyObserver = {
onBeginUpdateBatch: function PEX_onBeginUpdateBatch() {},
onEndUpdateBatch: function PEX_onEndUpdateBatch() {},
onDeleteURI(aURI, aGUID, aReason) {
const listener = events => {
for (const event of events) {
Assert.equal(event.type, "page-removed");
if (!event.isRemovedFromStore) {
continue;
}
currentTest.receivedNotifications++;
// Check this uri was not bookmarked.
Assert.equal(currentTest.bookmarks.indexOf(aURI.spec), -1);
do_check_valid_places_guid(aGUID);
Assert.equal(aReason, Ci.nsINavHistoryObserver.REASON_EXPIRED);
},
onDeleteVisits() {},
Assert.equal(currentTest.bookmarks.indexOf(event.url), -1);
do_check_valid_places_guid(event.pageGuid);
Assert.equal(event.reason, PlacesVisitRemoved.REASON_EXPIRED);
}
};
hs.addObserver(historyObserver);
PlacesObservers.addListener(["page-removed"], listener);
// Expire now.
await promiseForceExpirationStep(-1);
hs.removeObserver(historyObserver, false);
PlacesObservers.removeListener(["page-removed"], listener);
Assert.equal(
currentTest.receivedNotifications,

12
toolkit/components/places/tests/expiration/test_pref_maxpages.js
View File

@ -95,12 +95,24 @@ add_task(async function test_pref_maxpages() {
};
PlacesUtils.history.addObserver(historyObserver);
const listener = events => {
for (const event of events) {
print("page-removed " + event.url);
Assert.equal(event.type, "page-removed");
Assert.ok(event.isRemovedFromStore);
Assert.equal(event.reason, PlacesVisitRemoved.REASON_EXPIRED);
currentTest.receivedNotifications++;
}
};
PlacesObservers.addListener(["page-removed"], listener);
setMaxPages(currentTest.maxPages);
// Expire now.
await promiseForceExpirationStep(-1);
PlacesUtils.history.removeObserver(historyObserver, false);
PlacesObservers.removeListener(["page-removed"], listener);
Assert.equal(
currentTest.receivedNotifications,

4
toolkit/components/places/tests/expiration/xpcshell.ini
View File

@ -7,7 +7,7 @@ skip-if = toolkit == 'android'
[test_debug_expiration.js]
[test_idle_daily.js]
[test_notifications.js]
[test_notifications_onDeleteURI.js]
[test_notifications_onDeleteVisits.js]
[test_notifications_pageRemoved_allVisits.js]
[test_notifications_pageRemoved_fromStore.js]
[test_pref_interval.js]
[test_pref_maxpages.js]

28
toolkit/components/places/tests/history/test_remove.js
View File

@ -87,13 +87,32 @@ add_task(async function test_remove_single() {
}
break;
}
case "page-removed": {
Assert.equal(
event.isRemovedFromStore,
shouldRemove,
"Observe page-removed event with right removal type"
);
Assert.equal(
event.url,
uri.spec,
"Observing effect on the right uri"
);
resolve();
break;
}
}
}
};
});
PlacesUtils.history.addObserver(observer);
PlacesObservers.addListener(
["page-title-changed", "history-cleared", "pages-rank-changed"],
[
"page-title-changed",
"history-cleared",
"pages-rank-changed",
"page-removed",
],
placesEventListener
);
@ -121,7 +140,12 @@ add_task(async function test_remove_single() {
await promiseObserved;
PlacesUtils.history.removeObserver(observer);
PlacesObservers.removeListener(
["page-title-changed", "history-cleared", "pages-rank-changed"],
[
"page-title-changed",
"history-cleared",
"pages-rank-changed",
"page-removed",
],
placesEventListener
);

26
toolkit/components/places/tests/history/test_removeByFilter.js
View File

@ -66,7 +66,7 @@ add_task(async function test_removeByFilter() {
}
if (placesEventListener) {
PlacesObservers.addListener(
["page-title-changed", "history-cleared"],
["page-title-changed", "history-cleared", "page-removed"],
placesEventListener
);
}
@ -98,7 +98,7 @@ add_task(async function test_removeByFilter() {
}
if (placesEventListener) {
PlacesObservers.removeListener(
["page-title-changed", "history-cleared"],
["page-title-changed", "history-cleared", "page-removed"],
placesEventListener
);
}
@ -504,6 +504,28 @@ function getObserverPromise(bookmarkedUri) {
reject(new Error("Unexpected history-cleared event happens"));
break;
}
case "page-removed": {
if (event.isRemovedFromStore) {
Assert.notEqual(
event.url,
bookmarkedUri,
"Bookmarked URI should not be deleted"
);
} else {
Assert.equal(
event.isPartialVisistsRemoval,
false,
"Observing page-removed deletes all visits"
);
Assert.equal(
event.url,
bookmarkedUri,
"Bookmarked URI should have all visits removed but not the page itself"
);
}
resolve();
break;
}
}
}
};

52
toolkit/components/places/tests/history/test_removeMany.js
View File

@ -42,6 +42,10 @@ add_task(async function test_remove_many() {
onDeleteVisitsCalled: false,
// `true` once `onDeleteURI` has been called for this page
onDeleteURICalled: false,
// `true` once page-removed for store has been fired for this page
pageRemovedFromStore: false,
// `true` once page-removed for all visits has been fired for this page
pageRemovedAllVisits: false,
};
info("Pushing: " + uri.spec);
pages.push(page);
@ -126,12 +130,40 @@ add_task(async function test_remove_many() {
onPageRankingChanged = true;
break;
}
case "page-removed": {
const origin = pages.find(x => x.uri.spec === event.url);
Assert.ok(origin);
if (event.isRemovedFromStore) {
Assert.ok(
!origin.hasBookmark,
"Observing page-removed event on a page without a bookmark"
);
Assert.ok(
!origin.pageRemovedFromStore,
"Observing page-removed for store for the first time"
);
origin.pageRemovedFromStore = true;
} else {
Assert.ok(
!origin.pageRemovedAllVisits,
"Observing page-removed for all visits for the first time"
);
origin.pageRemovedAllVisits = true;
}
break;
}
}
}
};
PlacesObservers.addListener(
["page-title-changed", "history-cleared", "pages-rank-changed"],
[
"page-title-changed",
"history-cleared",
"pages-rank-changed",
"page-removed",
],
placesEventListener
);
@ -151,7 +183,12 @@ add_task(async function test_remove_many() {
PlacesUtils.history.removeObserver(observer);
PlacesObservers.removeListener(
["page-title-changed", "history-cleared", "pages-rank-changed"],
[
"page-title-changed",
"history-cleared",
"pages-rank-changed",
"page-removed",
],
placesEventListener
);
@ -173,17 +210,16 @@ add_task(async function test_remove_many() {
"Page is present only if it also has bookmarks"
);
Assert.notEqual(
page.onDeleteURICalled,
page.onDeleteVisitsCalled,
"Either only onDeleteVisits or onDeleteVisitsCalled should be called"
page.pageRemovedFromStore,
page.pageRemovedAllVisits,
"Either only page-removed event for store or all visits should be called"
);
}
Assert.equal(
onPageRankingChanged,
pages.some(p => p.onDeleteVisitsCalled) ||
pages.some(p => p.onDeleteURICalled),
"page-rank-changed was fired if onDeleteVisitsCalled or onDeleteURICalled was called"
pages.some(p => p.pageRemovedFromStore || p.pageRemovedAllVisits),
"page-rank-changed was fired if page-removed was fired"
);
Assert.notEqual(

55
toolkit/components/places/tests/unit/test_history_observer.js
View File

@ -149,32 +149,34 @@ add_task(async function test_multiple_onVisit() {
await promiseNotifications;
});
add_task(async function test_onDeleteURI() {
let promiseNotify = onNotify(function onDeleteURI(aURI, aGUID, aReason) {
Assert.ok(aURI.equals(testuri));
// Can't use do_check_guid_for_uri() here because the visit is already gone.
Assert.equal(aGUID, testguid);
Assert.equal(aReason, Ci.nsINavHistoryObserver.REASON_DELETED);
});
add_task(async function test_pageRemovedFromStore() {
let [testuri] = await task_add_visit();
let testguid = do_get_guid_for_uri(testuri);
const promiseNotify = PlacesTestUtils.waitForNotification(
"page-removed",
() => true,
"places"
);
await PlacesUtils.history.remove(testuri);
await promiseNotify;
const events = await promiseNotify;
Assert.equal(events.length, 1, "Right number of page-removed notified");
Assert.equal(events[0].type, "page-removed");
Assert.ok(events[0].isRemovedFromStore);
Assert.equal(events[0].url, testuri.spec);
Assert.equal(events[0].pageGuid, testguid);
Assert.equal(events[0].reason, PlacesVisitRemoved.REASON_DELETED);
});
add_task(async function test_onDeleteVisits() {
let promiseNotify = onNotify(function onDeleteVisits(
aURI,
aVisitTime,
aGUID,
aReason
) {
Assert.ok(aURI.equals(testuri));
// Can't use do_check_guid_for_uri() here because the visit is already gone.
Assert.equal(aGUID, testguid);
Assert.equal(aReason, Ci.nsINavHistoryObserver.REASON_DELETED);
Assert.equal(aVisitTime, 0); // All visits have been removed.
});
add_task(async function test_pageRemovedAllVisits() {
const promiseNotify = PlacesTestUtils.waitForNotification(
"page-removed",
() => true,
"places"
);
let msecs24hrsAgo = Date.now() - 86400 * 1000;
let [testuri] = await task_add_visit(undefined, msecs24hrsAgo * 1000);
// Add a bookmark so the page is not removed.
@ -185,7 +187,16 @@ add_task(async function test_onDeleteVisits() {
});
let testguid = do_get_guid_for_uri(testuri);
await PlacesUtils.history.remove(testuri);
await promiseNotify;
const events = await promiseNotify;
Assert.equal(events.length, 1, "Right number of page-removed notified");
Assert.equal(events[0].type, "page-removed");
Assert.ok(!events[0].isRemovedFromStore);
Assert.equal(events[0].url, testuri.spec);
// Can't use do_check_guid_for_uri() here because the visit is already gone.
Assert.equal(events[0].pageGuid, testguid);
Assert.equal(events[0].reason, PlacesVisitRemoved.REASON_DELETED);
Assert.ok(!events[0].isPartialVisistsRemoval); // All visits have been removed.
});
add_task(async function test_pageTitleChanged() {

11
toolkit/components/thumbnails/PageThumbs.jsm
View File

@ -122,7 +122,10 @@ var PageThumbs = {
this._placesObserver = new PlacesWeakCallbackWrapper(
this.handlePlacesEvents.bind(this)
);
PlacesObservers.addListener(["history-cleared"], this._placesObserver);
PlacesObservers.addListener(
["history-cleared", "page-removed"],
this._placesObserver
);
// Migrate the underlying storage, if needed.
PageThumbsStorageMigrator.migrate();
@ -137,6 +140,12 @@ var PageThumbs = {
PageThumbsStorage.wipe();
break;
}
case "page-removed": {
if (event.isRemovedFromStore) {
PageThumbsStorage.remove(event.url);
}
break;
}
}
}
},

7
toolkit/modules/NewTabUtils.jsm
View File

@ -611,6 +611,7 @@ var PlacesProvider = {
"page-title-changed",
"history-cleared",
"pages-rank-changed",
"page-removed",
],
this._placesObserver
);
@ -748,6 +749,12 @@ var PlacesProvider = {
this.onManyFrecenciesChanged();
break;
}
case "page-removed": {
if (event.isRemovedFromStore) {
this.onDeleteURI(event.url, event.pageGuid, event.reason);
}
break;
}
}
}
},