Bug 1479764 - Remove nsIAnnotationObserver and annotation notifications. r=mak,lina

MozReview-Commit-ID: I9nNefvgch9

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

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Mark Banner 2018-07-31 21:34:42 +00:00
parent a600a51631
commit 607107cbb6
11 changed files with 5 additions and 240 deletions

View File

@ -2704,8 +2704,7 @@ async function initializeTempMirrorEntities(db) {
isUntagging BOOLEAN NOT NULL DEFAULT 0
) WITHOUT ROWID`);
// Stores properties to pass to `onItemAnnotation{Set, Removed}` anno
// observers.
// Stores properties to pass to `onItemChanged` bookmark observers.
await db.execute(`CREATE TEMP TABLE annosChanged(
itemId INTEGER NOT NULL,
annoName TEXT NOT NULL,
@ -4583,7 +4582,6 @@ class BookmarkObserverRecorder {
this.db = db;
this.maxFrecenciesToRecalculate = maxFrecenciesToRecalculate;
this.bookmarkObserverNotifications = [];
this.annoObserverNotifications = [];
this.shouldInvalidateKeywords = false;
}
@ -4597,7 +4595,6 @@ class BookmarkObserverRecorder {
await PlacesUtils.keywords.invalidateCachedKeywords();
}
await this.notifyBookmarkObservers();
await this.notifyAnnoObservers();
await PlacesUtils.livemarks.invalidateCachedLivemarks();
await this.updateFrecencies();
}
@ -4686,18 +4683,6 @@ class BookmarkObserverRecorder {
info.name != PlacesUtils.LMANNO_SITEURI) {
throw new TypeError("Can't record change for unsupported anno");
}
if (info.wasRemoved) {
this.annoObserverNotifications.push({
name: "onItemAnnotationRemoved",
args: [info.id, info.name, PlacesUtils.bookmarks.SOURCES.SYNC],
});
} else {
this.annoObserverNotifications.push({
name: "onItemAnnotationSet",
args: [info.id, info.name, PlacesUtils.bookmarks.SOURCES.SYNC,
/* dontUpdateLastModified */ true],
});
}
this.bookmarkObserverNotifications.push({
name: "onItemChanged",
isTagging: false,
@ -4723,17 +4708,6 @@ class BookmarkObserverRecorder {
}
}
async notifyAnnoObservers() {
MirrorLog.trace("Notifying anno observers");
let observers = PlacesUtils.annotations.getObservers();
for (let observer of observers) {
let wrapped = yieldingIterator(this.annoObserverNotifications);
for await (let { name, args } of wrapped) {
this.notifyObserver(observer, name, args);
}
}
}
notifyObserver(observer, notification, args = []) {
try {
observer[notification](...args);

View File

@ -31,12 +31,6 @@ using namespace mozilla::places;
NS_ENSURE_TRUE(type == nsIAnnotationService::_type, NS_ERROR_INVALID_ARG); \
PR_END_MACRO
#define NOTIFY_ANNOS_OBSERVERS(_notification) \
PR_BEGIN_MACRO \
for (int32_t i = 0; i < mObservers.Count(); i++) \
mObservers[i]->_notification; \
PR_END_MACRO
const int32_t nsAnnotationService::kAnnoIndex_ID = 0;
const int32_t nsAnnotationService::kAnnoIndex_PageOrItem = 1;
const int32_t nsAnnotationService::kAnnoIndex_NameID = 2;
@ -369,7 +363,6 @@ nsAnnotationService::SetItemAnnotation(int64_t aItemId,
return NS_ERROR_NOT_IMPLEMENTED;
}
NOTIFY_ANNOS_OBSERVERS(OnItemAnnotationSet(aItemId, aName, aSource, aDontUpdateLastModified));
NotifyItemChanged(bookmark, aName, aSource, aDontUpdateLastModified);
return NS_OK;
@ -863,7 +856,6 @@ nsAnnotationService::RemoveItemAnnotation(int64_t aItemId,
nsresult rv = RemoveAnnotationInternal(nullptr, aItemId, &bookmark, aName);
NS_ENSURE_SUCCESS(rv, rv);
NOTIFY_ANNOS_OBSERVERS(OnItemAnnotationRemoved(aItemId, aName, aSource));
NotifyItemChanged(bookmark, aName, aSource, false);
return NS_OK;
@ -871,7 +863,7 @@ nsAnnotationService::RemoveItemAnnotation(int64_t aItemId,
nsresult
nsAnnotationService::RemoveItemAnnotationsWithoutNotifying(int64_t aItemId)
nsAnnotationService::RemoveItemAnnotations(int64_t aItemId)
{
NS_ENSURE_ARG_MIN(aItemId, 1);
@ -892,50 +884,6 @@ nsAnnotationService::RemoveItemAnnotationsWithoutNotifying(int64_t aItemId)
}
NS_IMETHODIMP
nsAnnotationService::AddObserver(nsIAnnotationObserver* aObserver)
{
NS_ENSURE_ARG(aObserver);
if (mObservers.IndexOfObject(aObserver) >= 0)
return NS_ERROR_INVALID_ARG; // Already registered.
if (!mObservers.AppendObject(aObserver))
return NS_ERROR_OUT_OF_MEMORY;
return NS_OK;
}
NS_IMETHODIMP
nsAnnotationService::RemoveObserver(nsIAnnotationObserver* aObserver)
{
NS_ENSURE_ARG(aObserver);
if (!mObservers.RemoveObject(aObserver))
return NS_ERROR_INVALID_ARG;
return NS_OK;
}
NS_IMETHODIMP
nsAnnotationService::GetObservers(uint32_t* _count,
nsIAnnotationObserver*** _observers)
{
NS_ENSURE_ARG_POINTER(_count);
NS_ENSURE_ARG_POINTER(_observers);
*_count = 0;
*_observers = nullptr;
nsCOMArray<nsIAnnotationObserver> observers(mObservers);
if (observers.Count() == 0)
return NS_OK;
*_count = observers.Count();
observers.Forget(_observers);
return NS_OK;
}
NS_IMETHODIMP
nsAnnotationService::ItemHasAnnotation(int64_t aItemId,
const nsACString& aName,

View File

@ -85,8 +85,6 @@ private:
protected:
RefPtr<mozilla::places::Database> mDB;
nsCOMArray<nsIAnnotationObserver> mObservers;
static nsAnnotationService* gAnnotationService;
static const int kAnnoIndex_ID;
@ -155,7 +153,7 @@ protected:
public:
nsresult GetItemAnnotationNamesTArray(int64_t aItemId,
nsTArray<nsCString>* _result);
nsresult RemoveItemAnnotationsWithoutNotifying(int64_t aItemId);
nsresult RemoveItemAnnotations(int64_t aItemId);
};
#endif /* nsAnnotationService_h___ */

View File

@ -21,28 +21,6 @@ interface mozIAnnotatedResult;
*
*/
[scriptable, uuid(63fe98e0-6889-4c2c-ac9f-703e4bc25027)]
interface nsIAnnotationObserver : nsISupports
{
/**
* Called when an annotation value is set. It could be a new annotation,
* or it could be a new value for an existing annotation.
*/
void onItemAnnotationSet(in long long aItemId,
in AUTF8String aName,
in unsigned short aSource,
in boolean aDontUpdateLastModified);
/**
* Called when an annotation is deleted. If aName is empty, then ALL
* annotations for the given URI have been deleted. This is not called when
* annotations are expired (normally happens when the app exits).
*/
void onItemAnnotationRemoved(in long long aItemId,
in AUTF8String aName,
in unsigned short aSource);
};
[scriptable, uuid(D4CDAAB1-8EEC-47A8-B420-AD7CB333056A)]
interface nsIAnnotationService : nsISupports
{
@ -185,24 +163,6 @@ interface nsIAnnotationService : nsISupports
void removeItemAnnotation(in long long aItemId,
in AUTF8String aName,
[optional] in unsigned short aSource);
/**
* Adds an annotation observer. The annotation service will keep an owning
* reference to the observer object.
*/
void addObserver(in nsIAnnotationObserver aObserver);
/**
* Removes an annotaton observer previously registered by addObserver.
*/
void removeObserver(in nsIAnnotationObserver aObserver);
/**
* Gets an array of registered nsIAnnotationObserver objects.
*/
void getObservers([optional] out unsigned long count,
[retval, array, size_is(count)] out nsIAnnotationObserver observers);
};
/**

View File

@ -640,15 +640,13 @@ nsNavBookmarks::RemoveItem(int64_t aItemId, uint16_t aSource)
mozStorageTransaction transaction(mDB->MainConn(), false);
// First, if not a tag, remove item annotations. We remove annos without
// notifying to avoid firing `onItemAnnotationRemoved` for an item that
// we're about to remove.
// First, if not a tag, remove item annotations.
int64_t tagsRootId = TagsRootId();
bool isUntagging = bookmark.grandParentId == tagsRootId;
if (bookmark.parentId != tagsRootId && !isUntagging) {
nsAnnotationService* annosvc = nsAnnotationService::GetAnnotationService();
NS_ENSURE_TRUE(annosvc, NS_ERROR_OUT_OF_MEMORY);
rv = annosvc->RemoveItemAnnotationsWithoutNotifying(bookmark.id);
rv = annosvc->RemoveItemAnnotations(bookmark.id);
NS_ENSURE_SUCCESS(rv, rv);
}

View File

@ -259,28 +259,12 @@ BookmarkObserver.prototype = {
});
},
onItemAnnotationSet(itemId, name, source, dontUpdateLastModified) {
this.notifications.push({
name: "onItemAnnotationSet",
params: { itemId, name, source, dontUpdateLastModified },
});
},
onItemAnnotationRemoved(itemId, name, source) {
this.notifications.push({
name: "onItemAnnotationRemoved",
params: { itemId, name, source },
});
},
QueryInterface: ChromeUtils.generateQI([
Ci.nsINavBookmarkObserver,
Ci.nsIAnnotationObserver,
]),
check(expectedNotifications) {
PlacesUtils.bookmarks.removeObserver(this);
PlacesUtils.annotations.removeObserver(this);
if (!ObjectUtils.deepEqual(this.notifications, expectedNotifications)) {
info(`Expected notifications: ${JSON.stringify(expectedNotifications)}`);
info(`Actual notifications: ${JSON.stringify(this.notifications)}`);
@ -295,7 +279,6 @@ BookmarkObserver.prototype = {
function expectBookmarkChangeNotifications(options) {
let observer = new BookmarkObserver(options);
PlacesUtils.bookmarks.addObserver(observer);
PlacesUtils.annotations.addObserver(observer);
return observer;
}

View File

@ -359,43 +359,6 @@ add_task(async function test_livemarks() {
parentGuid: PlacesUtils.bookmarks.unfiledGuid,
oldValue: "",
source: PlacesUtils.bookmarks.SOURCES.SYNC },
}, {
name: "onItemAnnotationRemoved",
params: { itemId: livemarkA.id, name: PlacesUtils.LMANNO_FEEDURI,
source: PlacesUtils.bookmarks.SOURCES.SYNC },
}, {
name: "onItemAnnotationSet",
params: { itemId: livemarkA.id, name: PlacesUtils.LMANNO_FEEDURI,
source: PlacesUtils.bookmarks.SOURCES.SYNC,
dontUpdateLastModified: true },
}, {
name: "onItemAnnotationSet",
params: { itemId: livemarkC.id, name: PlacesUtils.LMANNO_FEEDURI,
source: PlacesUtils.bookmarks.SOURCES.SYNC,
dontUpdateLastModified: true },
}, {
name: "onItemAnnotationRemoved",
params: { itemId: livemarkB.id, name: PlacesUtils.LMANNO_FEEDURI,
source: PlacesUtils.bookmarks.SOURCES.SYNC },
}, {
name: "onItemAnnotationRemoved",
params: { itemId: livemarkB.id, name: PlacesUtils.LMANNO_SITEURI,
source: PlacesUtils.bookmarks.SOURCES.SYNC },
}, {
name: "onItemAnnotationSet",
params: { itemId: livemarkB.id, name: PlacesUtils.LMANNO_FEEDURI,
source: PlacesUtils.bookmarks.SOURCES.SYNC,
dontUpdateLastModified: true },
}, {
name: "onItemAnnotationSet",
params: { itemId: livemarkE.id, name: PlacesUtils.LMANNO_FEEDURI,
source: PlacesUtils.bookmarks.SOURCES.SYNC,
dontUpdateLastModified: true },
}, {
name: "onItemAnnotationSet",
params: { itemId: livemarkE.id, name: PlacesUtils.LMANNO_SITEURI,
source: PlacesUtils.bookmarks.SOURCES.SYNC,
dontUpdateLastModified: true },
}]);
await buf.finalize();

View File

@ -11,22 +11,6 @@ try {
do_throw("Could not get annotation service\n");
}
var annoObserver = {
ITEM_lastSet_Id: -1,
ITEM_lastSet_AnnoName: "",
onItemAnnotationSet(aItemId, aName) {
this.ITEM_lastSet_Id = aItemId;
this.ITEM_lastSet_AnnoName = aName;
},
ITEM_lastRemoved_Id: -1,
ITEM_lastRemoved_AnnoName: "",
onItemAnnotationRemoved(aItemId, aName) {
this.ITEM_lastRemoved_Id = aItemId;
this.ITEM_lastRemoved_AnnoName = aName;
}
};
add_task(async function test_execute() {
let testURI = uri("http://mozilla.com/");
let testItem = await PlacesUtils.bookmarks.insert({
@ -39,7 +23,6 @@ add_task(async function test_execute() {
let testAnnoVal = "test";
let earlierDate = new Date(Date.now() - 1000);
annosvc.addObserver(annoObserver);
// create new string annotation
try {
annosvc.setPageAnnotation(testURI, testAnnoName, testAnnoVal, 0, annosvc.EXPIRE_NEVER);
@ -72,8 +55,6 @@ add_task(async function test_execute() {
// verify that setting the annotation updates the last modified time
Assert.ok(updatedItem.lastModified > item.lastModified);
Assert.equal(annoObserver.ITEM_lastSet_Id, testItemId);
Assert.equal(annoObserver.ITEM_lastSet_AnnoName, testAnnoName);
try {
var annoVal = annosvc.getItemAnnotation(testItemId, testAnnoName);
@ -168,9 +149,6 @@ add_task(async function test_execute() {
info("lastModified4 = " + lastModified4);
Assert.ok(is_time_ordered(lastModified3, lastModified4));
Assert.equal(annoObserver.ITEM_lastRemoved_Id, testItemId);
Assert.equal(annoObserver.ITEM_lastRemoved_AnnoName, int32Key);
// test that getItems/PagesWithAnnotation returns an empty array after
// removing all items/pages which had the annotation set, see bug 380317.
Assert.equal((await getItemsWithAnnotation(int32Key)).length, 0);
@ -191,8 +169,6 @@ add_task(async function test_execute() {
title: "",
url: testURI,
});
annosvc.removeObserver(annoObserver);
});
add_task(async function test_getAnnotationsHavingName() {

View File

@ -1,33 +0,0 @@
// Test that asking for a livemark in a annotationChanged notification works.
add_task(async function() {
let annoPromise = new Promise(resolve => {
let annoObserver = {
onItemAnnotationSet(id, name) {
if (name == PlacesUtils.LMANNO_FEEDURI) {
PlacesUtils.annotations.removeObserver(this);
resolve();
}
},
onItemAnnotationRemoved() {},
QueryInterface: ChromeUtils.generateQI([
Ci.nsIAnnotationObserver
]),
};
PlacesUtils.annotations.addObserver(annoObserver);
});
let livemark = await PlacesUtils.livemarks.addLivemark(
{ title: "livemark title",
parentGuid: PlacesUtils.bookmarks.unfiledGuid,
index: PlacesUtils.bookmarks.DEFAULT_INDEX,
siteURI: uri("http://example.com/"),
feedURI: uri("http://example.com/rdf")
});
await annoPromise;
livemark = await PlacesUtils.livemarks.getLivemark({ guid: livemark.guid });
Assert.ok(livemark);
await PlacesUtils.livemarks.removeLivemark({ guid: livemark.guid });
});

View File

@ -18,7 +18,6 @@ var testServices = [
["createFolder", "getObservers", "onFrecencyChanged", "onTitleChanged", "onDeleteURI"]
],
["browser/livemark-service;2", ["mozIAsyncLivemarks"], ["reloadLivemarks"]],
["browser/annotation-service;1", ["nsIAnnotationService"], ["getObservers"]],
["browser/favicon-service;1", ["nsIFaviconService"], []],
["browser/tagging-service;1", ["nsITaggingService"], []],
];

View File

@ -54,7 +54,6 @@ skip-if = os == "linux" # Bug 821781
[test_bookmarks_restore_notification.js]
[test_broken_folderShortcut_result.js]
[test_browserhistory.js]
[test_bug636917_isLivemark.js]
[test_childlessTags.js]
[test_frecency.js]
[test_frecency_decay.js]