Skip to content

Commit

Permalink
Bug 1081099 - Implement bookmarks notifications from Bookmarks.jsm. r…
Browse files Browse the repository at this point in the history 
…=mano
  • Loading branch information
@mak77
mak77 committed Oct 25, 2014
1 parent 6abc627 commit 54881cb
Show file tree
Hide file tree
Showing 7 changed files with 683 additions and 77 deletions.
224 changes: 190 additions & 34 deletions toolkit/components/places/Bookmarks.jsm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,38 @@ let Bookmarks = Object.freeze({
}

let item = yield insertBookmark(insertInfo, parent);

// Notify onItemAdded to listeners.
let observers = PlacesUtils.bookmarks.getObservers();
// We need the itemId to notify, though once the switch to guids is
// complete we may stop using it.
let uri = item.hasOwnProperty("url") ? toURI(item.url) : null;
let itemId = yield PlacesUtils.promiseItemId(item.guid);
notify(observers, "onItemAdded", [ itemId, parent._id, item.index,
item.type, uri, item.title || null,
toPRTime(item.dateAdded), item.guid,
item.parentGuid ]);

// If a keyword is defined, notify onItemChanged for it.
if (item.keyword) {
notify(observers, "onItemChanged", [ itemId, "keyword", false,
item.keyword,
toPRTime(item.lastModified),
item.type, parent._id, item.guid,
item.parentGuid ]);
}

// If it's a tag, notify OnItemChanged to all bookmarks for this URL.
let isTagging = parent._parentId == PlacesUtils.tagsFolderId;
if (isTagging) {
for (let entry of (yield fetchBookmarksByURL(item))) {
notify(observers, "onItemChanged", [ entry._id, "tags", false, "",
toPRTime(entry.lastModified),
entry.type, entry._parentId,
entry.guid, entry.parentGuid ]);
}
}

// Remove non-enumerable properties.
return Object.assign({}, item);
}.bind(this));
Expand Down Expand Up @@ -275,6 +307,59 @@ let Bookmarks = Object.freeze({
updateFrecency(db, [updatedItem.url]).then(null, Cu.reportError);
}

// Notify onItemChanged to listeners.
let observers = PlacesUtils.bookmarks.getObservers();
// For lastModified, we only care about the original input, since we
// should not notify implciit lastModified changes.
if (info.hasOwnProperty("lastModified") &&
updateInfo.hasOwnProperty("lastModified") &&
item.lastModified != updatedItem.lastModified) {
notify(observers, "onItemChanged", [ updatedItem._id, "lastModified",
false,
`${toPRTime(updatedItem.lastModified)}`,
toPRTime(updatedItem.lastModified),
updatedItem.type,
updatedItem._parentId,
updatedItem.guid,
updatedItem.parentGuid ]);
}
if (updateInfo.hasOwnProperty("title")) {
notify(observers, "onItemChanged", [ updatedItem._id, "title",
false, updatedItem.title,
toPRTime(updatedItem.lastModified),
updatedItem.type,
updatedItem._parentId,
updatedItem.guid,
updatedItem.parentGuid ]);
}
if (updateInfo.hasOwnProperty("url")) {
notify(observers, "onItemChanged", [ updatedItem._id, "uri",
false, updatedItem.url.href,
toPRTime(updatedItem.lastModified),
updatedItem.type,
updatedItem._parentId,
updatedItem.guid,
updatedItem.parentGuid ]);
}
if (updateInfo.hasOwnProperty("keyword")) {
notify(observers, "onItemChanged", [ updatedItem._id, "keyword",
false, updatedItem.keyword,
toPRTime(updatedItem.lastModified),
updatedItem.type,
updatedItem._parentId,
updatedItem.guid,
updatedItem.parentGuid ]);
}
// If the item was move, notify onItemMoved.
if (item.parentGuid != updatedItem.parentGuid ||
item.index != updatedItem.index) {
notify(observers, "onItemMoved", [ updatedItem._id, item._parentId,
item.index, updatedItem._parentId,
updatedItem.index, updatedItem.type,
updatedItem.guid, item.parentGuid,
updatedItem.newParentGuid ]);
}

// Remove non-enumerable properties.
return Object.assign({}, updatedItem);
}.bind(this));
Expand Down Expand Up @@ -313,8 +398,25 @@ let Bookmarks = Object.freeze({
if (!item._parentId || item._parentId == PlacesUtils.placesRootId)
throw new Error("It's not possible to remove Places root folders.");

let isUntagging = item._grandParentId == PlacesUtils.bookmarks.tagsFolderId;
item = yield removeBookmark(item, isUntagging);
item = yield removeBookmark(item);

// Notify onItemRemoved to listeners.
let observers = PlacesUtils.bookmarks.getObservers();
let uri = item.hasOwnProperty("url") ? toURI(item.url) : null;
notify(observers, "onItemRemoved", [ item._id, item._parentId, item.index,
item.type, uri, item.guid,
item.parentGuid ]);

let isUntagging = item._grandParentId == PlacesUtils.tagsFolderId;
if (isUntagging) {
for (let entry of (yield fetchBookmarksByURL(item))) {
notify(observers, "onItemChanged", [ entry._id, "tags", false, "",
toPRTime(entry.lastModified),
entry.type, entry._parentId,
entry.guid, entry.parentGuid ]);
}
}

// Remove non-enumerable properties.
return Object.assign({}, item);
});
Expand All @@ -335,15 +437,16 @@ let Bookmarks = Object.freeze({
let rows = yield db.executeCached(
`WITH RECURSIVE
descendants(did) AS (
SELECT folder_id FROM moz_bookmarks_roots
WHERE root_name IN ("toolbar", "menu", "unfiled")
SELECT id FROM moz_bookmarks
WHERE parent IN (SELECT folder_id FROM moz_bookmarks_roots
WHERE root_name IN ("toolbar", "menu", "unfiled"))
UNION ALL
SELECT id FROM moz_bookmarks
JOIN descendants ON parent = did
)
SELECT b.id AS _id, b.parent AS _parentId, b.position AS 'index',
b.type, url, b.guid, p.guid AS parentGuid, b.dateAdded,
b.lastModified, b.title, NULL AS _grandParentId,
b.lastModified, b.title, p.parent AS _grandParentId,
NULL AS _childCount, NULL AS keyword
FROM moz_bookmarks b
JOIN moz_bookmarks p ON p.id = b.parent
Expand All @@ -355,8 +458,9 @@ let Bookmarks = Object.freeze({
yield db.executeCached(
`WITH RECURSIVE
descendants(did) AS (
SELECT folder_id FROM moz_bookmarks_roots
WHERE root_name IN ("toolbar", "menu", "unfiled")
SELECT id FROM moz_bookmarks
WHERE parent IN (SELECT folder_id FROM moz_bookmarks_roots
WHERE root_name IN ("toolbar", "menu", "unfiled"))
UNION ALL
SELECT id FROM moz_bookmarks
JOIN descendants ON parent = did
Expand All @@ -380,9 +484,28 @@ let Bookmarks = Object.freeze({
let urls = [for (item of items) if (item.url) item.url];
updateFrecency(db, urls).then(null, Cu.reportError);

// TODO: send notifications
// Send onItemRemoved notifications to listeners.
// TODO (Bug 1087580): this should send a single clear bookmarks
// notification rather than notifying for each bookmark.

// Notify listeners in reverse order to serve children before parents.
let observers = PlacesUtils.bookmarks.getObservers();
for (let item of items.reverse()) {
let uri = item.hasOwnProperty("url") ? toURI(item.url) : null;
notify(observers, "onItemRemoved", [ item._id, item._parentId,
item.index, item.type, uri,
item.guid, item.parentGuid ]);

let isUntagging = item._grandParentId == PlacesUtils.tagsFolderId;
if (isUntagging) {
for (let entry of (yield fetchBookmarksByURL(item))) {
notify(observers, "onItemChanged", [ entry._id, "tags", false, "",
toPRTime(entry.lastModified),
entry.type, entry._parentId,
entry.guid, entry.parentGuid ]);
}
}
}
});
}),

Expand Down Expand Up @@ -582,6 +705,24 @@ let Bookmarks = Object.freeze({
////////////////////////////////////////////////////////////////////////////////
// Globals.

/**
* 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) {}
}
}

XPCOMUtils.defineLazyGetter(this, "DBConnPromised",
() => new Promise((resolve, reject) => {
Sqlite.wrapStorageConnection({ connection: PlacesUtils.history.DBConnection } )
Expand Down Expand Up @@ -706,9 +847,6 @@ function* updateBookmark(info, item, newParent) {
function* insertBookmark(item, parent) {
let db = yield DBConnPromised;

let isTaggingURL = item.hasOwnProperty("url") &&
parent._parentId == PlacesUtils.bookmarks.tagsFolderId;

// If a guid was not provided, generate one, so we won't need to fetch the
// bookmark just after having created it.
if (!item.hasOwnProperty("guid"))
Expand Down Expand Up @@ -752,19 +890,12 @@ function* insertBookmark(item, parent) {
});

// If not a tag recalculate frecency...
if (item.type == Bookmarks.TYPE_BOOKMARK && !isTaggingURL) {
let isTagging = parent._parentId == PlacesUtils.tagsFolderId;
if (item.type == Bookmarks.TYPE_BOOKMARK && !isTagging) {
// ...though we don't wait for the calculation.
updateFrecency(db, [item.url]).then(null, Cu.reportError);
}

// Notify onItemAdded
// TODO

// If it's a tag notify OnItemChanged to all bookmarks for this URL.
if (isTaggingURL) {
// TODO
}

// Don't return an empty title to the caller.
if (item.hasOwnProperty("title") && item.title === null)
delete item.title;
Expand Down Expand Up @@ -858,14 +989,17 @@ function* fetchBookmarksByKeyword(info) {
function* removeBookmark(item) {
let db = yield DBConnPromised;


let isUntagging = item._grandParentId == PlacesUtils.bookmarks.tagsFolderId;
// Remove annotations first. If this is a tag, we can avoid paying that cost.
if (!isUntagging) {
PlacesUtils.annotations.removeItemAnnotations(item._id);
}
let isUntagging = item._grandParentId == PlacesUtils.tagsFolderId;

yield db.executeTransaction(function* transaction() {
// Remove annotations first. If it's a tag, we can avoid paying that cost.
if (!isUntagging) {
// We don't go through the annotations service for this cause otherwise
// we'd get a pointless onItemChanged notification and it would also
// set lastModified to an unexpected value.
yield removeAnnotationsForItem(db, item._id);
}

// Remove the bookmark from the database.
yield db.executeCached(
`DELETE FROM moz_bookmarks WHERE guid = :guid`, { guid: item.guid });
Expand All @@ -889,14 +1023,6 @@ function* removeBookmark(item) {
updateFrecency(db, [item.url]).then(null, Cu.reportError);
}

// Notify onItemRemoved.
// TODO

// If we are untagging a bookmark, notify OnItemChanged.
if (isUntagging) {
// TODO
}

return item;
}

Expand Down Expand Up @@ -953,6 +1079,15 @@ function removeSameValueProperties(dest, src) {
}
}

/**
* Converts an URL object to an nsIURI.
*
* @param url
* the URL object to convert.
* @return nsIURI for the given URL.
*/
function toURI(url) NetUtil.newURI(url.href);

/**
* Reverse a host based on the moz_places algorithm, that is reverse the host
* string and add a trialing period. For example "google.com" becomes
Expand Down Expand Up @@ -1203,6 +1338,27 @@ let removeOrphanAnnotations = Task.async(function* (db) {
`);
});

/**
* Removes annotations for a given item.
*
* @param db
* the Sqlite.jsm connection handle.
* @param itemId
* internal id of the item for which to remove annotations.
*/
let removeAnnotationsForItem = Task.async(function* (db, itemId) {
yield db.executeCached(
`DELETE FROM moz_items_annos
WHERE item_id = :id
`, { id: itemId });
yield db.executeCached(
`DELETE FROM moz_anno_attributes
WHERE id IN (SELECT n.id from moz_anno_attributes n
LEFT JOIN moz_items_annos a ON a.anno_attribute_id = n.id
WHERE a.id ISNULL)
`);
});

/**
* Updates lastModified for all the ancestors of a given folder GUID.
*
Expand Down
8 changes: 7 additions & 1 deletion toolkit/components/places/nsINavBookmarksService.idl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ interface nsINavBookmarkObserver : nsISupports
* folders. A URI in history can be contained in one or more such folders.
*/

[scriptable, uuid(4C309044-B6DA-4511-AF57-E8940DB00045)]
[scriptable, uuid(b0f9a80a-d7f0-4421-8513-444125f0d828)]
interface nsINavBookmarksService : nsISupports
{
/**
Expand Down Expand Up @@ -544,6 +544,12 @@ interface nsINavBookmarksService : nsISupports
*/
void removeObserver(in nsINavBookmarkObserver observer);

/**
* Gets an array of registered nsINavBookmarkObserver objects.
*/
void getObservers([optional] out unsigned long count,
[retval, array, size_is(count)] out nsINavBookmarkObserver observers);

/**
* Runs the passed callback inside of a database transaction.
* Use this when a lot of things are about to change, for example
Expand Down
39 changes: 39 additions & 0 deletions toolkit/components/places/nsNavBookmarks.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2670,6 +2670,45 @@ nsNavBookmarks::RemoveObserver(nsINavBookmarkObserver* aObserver)
return mObservers.RemoveWeakElement(aObserver);
}

NS_IMETHODIMP
nsNavBookmarks::GetObservers(uint32_t* _count,
nsINavBookmarkObserver*** _observers)
{
NS_ENSURE_ARG_POINTER(_observers);
*_count = 0;
*_observers = nullptr;

if (!mCanNotify)
return NS_OK;

nsCOMArray<nsINavBookmarkObserver> observers;

// First add the category cache observers.
mCacheObservers.GetEntries(observers);

// Then add the other observers.
for (uint32_t i = 0; i < mObservers.Length(); ++i) {
const nsCOMPtr<nsINavBookmarkObserver> &observer = mObservers.ElementAt(i);
// Skip nullified weak observers.
if (observer)
observers.AppendElement(observer);
}

if (observers.Count() == 0)
return NS_OK;

*_observers = static_cast<nsINavBookmarkObserver**>
(nsMemory::Alloc(observers.Count() * sizeof(nsINavBookmarkObserver*)));
NS_ENSURE_TRUE(*_observers, NS_ERROR_OUT_OF_MEMORY);

*_count = observers.Count();
for (uint32_t i = 0; i < *_count; ++i) {
NS_ADDREF((*_observers)[i] = observers[i]);
}

return NS_OK;
}

void
nsNavBookmarks::NotifyItemVisited(const ItemVisitData& aData)
{
Expand Down
Loading

0 comments on commit 54881cb

Please sign in to comment.