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

Bug 1417101 - Remove the annotation observer from the bookmarks service. r=mak

Browse Source 
Since `SetItemAnnotation` already queries `moz_bookmarks`, we can fetch
and pass the changed bookmark's info directly to
`nsNavBookmarks::NotifyItemChanged`, without going through the anno
observer.

This patch refactors the internal `Set*` methods to pass an optional
`BookmarkData` from `SetItemAnnotation`, and fire `OnItemChanged`
notifications after notifying anno observers. `NotifyItemChanged` also
updates the bookmark's last modified time if requested.

MozReview-Commit-ID: Hz5qiOmAjsD

--HG--
extra : rebase_source : 37170f4661341e3a401f8210ceec84cbf439b4b2
This commit is contained in:
Kit Cambridge 2017-11-16 16:49:03 -08:00
parent 3852b33ae4
commit 870acfd373
8 changed files with 344 additions and 312 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

375
toolkit/components/places/nsAnnotationService.cpp
View File

@ -47,6 +47,28 @@ const int32_t nsAnnotationService::kAnnoIndex_Type = 6;
const int32_t nsAnnotationService::kAnnoIndex_DateAdded = 7;
const int32_t nsAnnotationService::kAnnoIndex_LastModified = 8;
namespace {
// Fires `onItemChanged` notifications for bookmark annotation changes.
void
NotifyItemChanged(const BookmarkData& aBookmark, const nsACString& aName,
uint16_t aSource, bool aDontUpdateLastModified)
{
ItemChangeData changeData;
changeData.bookmark = aBookmark;
changeData.isAnnotation = true;
changeData.updateLastModified = !aDontUpdateLastModified;
changeData.source = aSource;
changeData.property = aName;
nsNavBookmarks* bookmarks = nsNavBookmarks::GetBookmarksService();
if (bookmarks) {
bookmarks->NotifyItemChanged(changeData);
}
}
}
namespace mozilla {
namespace places {
@ -151,7 +173,7 @@ nsAnnotationService::Init()
return NS_OK;
}
nsresult
Result<Maybe<BookmarkData>, nsresult>
nsAnnotationService::SetAnnotationStringInternal(nsIURI* aURI,
int64_t aItemId,
const nsACString& aName,
@ -161,22 +183,26 @@ nsAnnotationService::SetAnnotationStringInternal(nsIURI* aURI,
{
mozStorageTransaction transaction(mDB->MainConn(), false);
nsCOMPtr<mozIStorageStatement> statement;
nsresult rv = StartSetAnnotation(aURI, aItemId, aName, aFlags, aExpiration,
nsIAnnotationService::TYPE_STRING,
statement);
NS_ENSURE_SUCCESS(rv, rv);
mozStorageStatementScoper scoper(statement);
rv = statement->BindStringByName(NS_LITERAL_CSTRING("content"), aValue);
NS_ENSURE_SUCCESS(rv, rv);
Result<Maybe<BookmarkData>, nsresult> result =
StartSetAnnotation(aURI, aItemId, aName, aFlags, aExpiration,
nsIAnnotationService::TYPE_STRING,
statement);
if (result.isOk()) {
mozStorageStatementScoper scoper(statement);
rv = statement->Execute();
NS_ENSURE_SUCCESS(rv, rv);
nsresult rv = statement->BindStringByName(NS_LITERAL_CSTRING("content"),
aValue);
NS_ENSURE_SUCCESS(rv, Err(rv));
rv = transaction.Commit();
NS_ENSURE_SUCCESS(rv, rv);
rv = statement->Execute();
NS_ENSURE_SUCCESS(rv, Err(rv));
return NS_OK;
rv = transaction.Commit();
NS_ENSURE_SUCCESS(rv, Err(rv));
}
return result;
}
@ -356,9 +382,12 @@ nsAnnotationService::SetPageAnnotationString(nsIURI* aURI,
{
NS_ENSURE_ARG(aURI);
nsresult rv = SetAnnotationStringInternal(aURI, 0, aName, aValue,
aFlags, aExpiration);
NS_ENSURE_SUCCESS(rv, rv);
Result<Maybe<BookmarkData>, nsresult> result =
SetAnnotationStringInternal(aURI, 0, aName, aValue,
aFlags, aExpiration);
if (result.isErr()) {
return result.unwrapErr();
}
NOTIFY_ANNOS_OBSERVERS(OnPageAnnotationSet(aURI, aName));
@ -380,17 +409,26 @@ nsAnnotationService::SetItemAnnotationString(int64_t aItemId,
if (aExpiration == EXPIRE_WITH_HISTORY)
return NS_ERROR_INVALID_ARG;
nsresult rv = SetAnnotationStringInternal(nullptr, aItemId, aName, aValue,
aFlags, aExpiration);
NS_ENSURE_SUCCESS(rv, rv);
Result<Maybe<BookmarkData>, nsresult> result =
SetAnnotationStringInternal(nullptr, aItemId, aName, aValue,
aFlags, aExpiration);
if (result.isErr()) {
return result.unwrapErr();
}
NOTIFY_ANNOS_OBSERVERS(OnItemAnnotationSet(aItemId, aName, aSource, aDontUpdateLastModified));
Maybe<BookmarkData> maybeBookmark = result.unwrap();
if (maybeBookmark) {
NotifyItemChanged(maybeBookmark.ref(), aName, aSource,
aDontUpdateLastModified);
}
return NS_OK;
}
nsresult
Result<Maybe<BookmarkData>, nsresult>
nsAnnotationService::SetAnnotationInt32Internal(nsIURI* aURI,
int64_t aItemId,
const nsACString& aName,
@ -400,22 +438,25 @@ nsAnnotationService::SetAnnotationInt32Internal(nsIURI* aURI,
{
mozStorageTransaction transaction(mDB->MainConn(), false);
nsCOMPtr<mozIStorageStatement> statement;
nsresult rv = StartSetAnnotation(aURI, aItemId, aName, aFlags, aExpiration,
nsIAnnotationService::TYPE_INT32,
statement);
NS_ENSURE_SUCCESS(rv, rv);
mozStorageStatementScoper scoper(statement);
Result<Maybe<BookmarkData>, nsresult> result =
StartSetAnnotation(aURI, aItemId, aName, aFlags, aExpiration,
nsIAnnotationService::TYPE_INT32,
statement);
if (result.isOk()) {
mozStorageStatementScoper scoper(statement);
rv = statement->BindInt32ByName(NS_LITERAL_CSTRING("content"), aValue);
NS_ENSURE_SUCCESS(rv, rv);
nsresult rv = statement->BindInt32ByName(NS_LITERAL_CSTRING("content"),
aValue);
NS_ENSURE_SUCCESS(rv, Err(rv));
rv = statement->Execute();
NS_ENSURE_SUCCESS(rv, rv);
rv = statement->Execute();
NS_ENSURE_SUCCESS(rv, Err(rv));
rv = transaction.Commit();
NS_ENSURE_SUCCESS(rv, rv);
rv = transaction.Commit();
NS_ENSURE_SUCCESS(rv, Err(rv));
}
return NS_OK;
return result;
}
@ -428,9 +469,12 @@ nsAnnotationService::SetPageAnnotationInt32(nsIURI* aURI,
{
NS_ENSURE_ARG(aURI);
nsresult rv = SetAnnotationInt32Internal(aURI, 0, aName, aValue,
aFlags, aExpiration);
NS_ENSURE_SUCCESS(rv, rv);
Result<Maybe<BookmarkData>, nsresult> result =
SetAnnotationInt32Internal(aURI, 0, aName, aValue,
aFlags, aExpiration);
if (result.isErr()) {
return result.unwrapErr();
}
NOTIFY_ANNOS_OBSERVERS(OnPageAnnotationSet(aURI, aName));
@ -452,17 +496,26 @@ nsAnnotationService::SetItemAnnotationInt32(int64_t aItemId,
if (aExpiration == EXPIRE_WITH_HISTORY)
return NS_ERROR_INVALID_ARG;
nsresult rv = SetAnnotationInt32Internal(nullptr, aItemId, aName, aValue,
aFlags, aExpiration);
NS_ENSURE_SUCCESS(rv, rv);
Result<Maybe<BookmarkData>, nsresult> result =
SetAnnotationInt32Internal(nullptr, aItemId, aName, aValue,
aFlags, aExpiration);
if (result.isErr()) {
return result.unwrapErr();
}
NOTIFY_ANNOS_OBSERVERS(OnItemAnnotationSet(aItemId, aName, aSource, aDontUpdateLastModified));
Maybe<BookmarkData> maybeBookmark = result.unwrap();
if (maybeBookmark) {
NotifyItemChanged(maybeBookmark.ref(), aName, aSource,
aDontUpdateLastModified);
}
return NS_OK;
}
nsresult
Result<Maybe<BookmarkData>, nsresult>
nsAnnotationService::SetAnnotationInt64Internal(nsIURI* aURI,
int64_t aItemId,
const nsACString& aName,
@ -472,22 +525,25 @@ nsAnnotationService::SetAnnotationInt64Internal(nsIURI* aURI,
{
mozStorageTransaction transaction(mDB->MainConn(), false);
nsCOMPtr<mozIStorageStatement> statement;
nsresult rv = StartSetAnnotation(aURI, aItemId, aName, aFlags, aExpiration,
nsIAnnotationService::TYPE_INT64,
statement);
NS_ENSURE_SUCCESS(rv, rv);
mozStorageStatementScoper scoper(statement);
Result<Maybe<BookmarkData>, nsresult> result =
StartSetAnnotation(aURI, aItemId, aName, aFlags, aExpiration,
nsIAnnotationService::TYPE_INT64,
statement);
if (result.isOk()) {
mozStorageStatementScoper scoper(statement);
rv = statement->BindInt64ByName(NS_LITERAL_CSTRING("content"), aValue);
NS_ENSURE_SUCCESS(rv, rv);
nsresult rv = statement->BindInt64ByName(NS_LITERAL_CSTRING("content"),
aValue);
NS_ENSURE_SUCCESS(rv, Err(rv));
rv = statement->Execute();
NS_ENSURE_SUCCESS(rv, rv);
rv = statement->Execute();
NS_ENSURE_SUCCESS(rv, Err(rv));
rv = transaction.Commit();
NS_ENSURE_SUCCESS(rv, rv);
rv = transaction.Commit();
NS_ENSURE_SUCCESS(rv, Err(rv));
}
return NS_OK;
return result;
}
@ -500,9 +556,12 @@ nsAnnotationService::SetPageAnnotationInt64(nsIURI* aURI,
{
NS_ENSURE_ARG(aURI);
nsresult rv = SetAnnotationInt64Internal(aURI, 0, aName, aValue,
aFlags, aExpiration);
NS_ENSURE_SUCCESS(rv, rv);
Result<Maybe<BookmarkData>, nsresult> result =
SetAnnotationInt64Internal(aURI, 0, aName, aValue,
aFlags, aExpiration);
if (result.isErr()) {
return result.unwrapErr();
}
NOTIFY_ANNOS_OBSERVERS(OnPageAnnotationSet(aURI, aName));
@ -524,17 +583,26 @@ nsAnnotationService::SetItemAnnotationInt64(int64_t aItemId,
if (aExpiration == EXPIRE_WITH_HISTORY)
return NS_ERROR_INVALID_ARG;
nsresult rv = SetAnnotationInt64Internal(nullptr, aItemId, aName, aValue,
aFlags, aExpiration);
NS_ENSURE_SUCCESS(rv, rv);
Result<Maybe<BookmarkData>, nsresult> result =
SetAnnotationInt64Internal(nullptr, aItemId, aName, aValue,
aFlags, aExpiration);
if (result.isErr()) {
return result.unwrapErr();
}
NOTIFY_ANNOS_OBSERVERS(OnItemAnnotationSet(aItemId, aName, aSource, aDontUpdateLastModified));
Maybe<BookmarkData> maybeBookmark = result.unwrap();
if (maybeBookmark) {
NotifyItemChanged(maybeBookmark.ref(), aName, aSource,
aDontUpdateLastModified);
}
return NS_OK;
}
nsresult
Result<Maybe<BookmarkData>, nsresult>
nsAnnotationService::SetAnnotationDoubleInternal(nsIURI* aURI,
int64_t aItemId,
const nsACString& aName,
@ -544,22 +612,25 @@ nsAnnotationService::SetAnnotationDoubleInternal(nsIURI* aURI,
{
mozStorageTransaction transaction(mDB->MainConn(), false);
nsCOMPtr<mozIStorageStatement> statement;
nsresult rv = StartSetAnnotation(aURI, aItemId, aName, aFlags, aExpiration,
nsIAnnotationService::TYPE_DOUBLE,
statement);
NS_ENSURE_SUCCESS(rv, rv);
mozStorageStatementScoper scoper(statement);
Result<Maybe<BookmarkData>, nsresult> result =
StartSetAnnotation(aURI, aItemId, aName, aFlags, aExpiration,
nsIAnnotationService::TYPE_DOUBLE,
statement);
if (result.isOk()) {
mozStorageStatementScoper scoper(statement);
rv = statement->BindDoubleByName(NS_LITERAL_CSTRING("content"), aValue);
NS_ENSURE_SUCCESS(rv, rv);
nsresult rv = statement->BindDoubleByName(NS_LITERAL_CSTRING("content"),
aValue);
NS_ENSURE_SUCCESS(rv, Err(rv));
rv = statement->Execute();
NS_ENSURE_SUCCESS(rv, rv);
rv = statement->Execute();
NS_ENSURE_SUCCESS(rv, Err(rv));
rv = transaction.Commit();
NS_ENSURE_SUCCESS(rv, rv);
rv = transaction.Commit();
NS_ENSURE_SUCCESS(rv, Err(rv));
}
return NS_OK;
return result;
}
@ -572,9 +643,12 @@ nsAnnotationService::SetPageAnnotationDouble(nsIURI* aURI,
{
NS_ENSURE_ARG(aURI);
nsresult rv = SetAnnotationDoubleInternal(aURI, 0, aName, aValue,
aFlags, aExpiration);
NS_ENSURE_SUCCESS(rv, rv);
Result<Maybe<BookmarkData>, nsresult> result =
SetAnnotationDoubleInternal(aURI, 0, aName, aValue,
aFlags, aExpiration);
if (result.isErr()) {
return result.unwrapErr();
}
NOTIFY_ANNOS_OBSERVERS(OnPageAnnotationSet(aURI, aName));
@ -596,12 +670,21 @@ nsAnnotationService::SetItemAnnotationDouble(int64_t aItemId,
if (aExpiration == EXPIRE_WITH_HISTORY)
return NS_ERROR_INVALID_ARG;
nsresult rv = SetAnnotationDoubleInternal(nullptr, aItemId, aName, aValue,
aFlags, aExpiration);
NS_ENSURE_SUCCESS(rv, rv);
Result<Maybe<BookmarkData>, nsresult> result =
SetAnnotationDoubleInternal(nullptr, aItemId, aName, aValue,
aFlags, aExpiration);
if (result.isErr()) {
return result.unwrapErr();
}
NOTIFY_ANNOS_OBSERVERS(OnItemAnnotationSet(aItemId, aName, aSource, aDontUpdateLastModified));
Maybe<BookmarkData> maybeBookmark = result.unwrap();
if (maybeBookmark) {
NotifyItemChanged(maybeBookmark.ref(), aName, aSource,
aDontUpdateLastModified);
}
return NS_OK;
}
@ -1364,7 +1447,7 @@ nsAnnotationService::ItemHasAnnotation(int64_t aItemId,
* delete the last item of a given name, that item really should go away.
* It will be cleaned up by expiration.
*/
nsresult
Result<Maybe<BookmarkData>, nsresult>
nsAnnotationService::RemoveAnnotationInternal(nsIURI* aURI,
int64_t aItemId,
const nsACString& aName)
@ -1388,23 +1471,35 @@ nsAnnotationService::RemoveAnnotationInternal(nsIURI* aURI,
"(SELECT id FROM moz_anno_attributes WHERE name = :anno_name)"
);
}
NS_ENSURE_STATE(statement);
NS_ENSURE_TRUE(statement, Err(NS_ERROR_UNEXPECTED));
mozStorageStatementScoper scoper(statement);
nsresult rv;
if (isItemAnnotation)
Maybe<BookmarkData> maybeBookmark;
if (isItemAnnotation) {
rv = statement->BindInt64ByName(NS_LITERAL_CSTRING("item_id"), aItemId);
else
NS_ENSURE_SUCCESS(rv, Err(rv));
nsNavBookmarks* bookmarks = nsNavBookmarks::GetBookmarksService();
if (bookmarks) {
BookmarkData bookmark;
if (NS_SUCCEEDED(bookmarks->FetchItemInfo(aItemId, bookmark))) {
maybeBookmark.emplace(bookmark);
}
}
}
else {
rv = URIBinder::Bind(statement, NS_LITERAL_CSTRING("page_url"), aURI);
NS_ENSURE_SUCCESS(rv, rv);
NS_ENSURE_SUCCESS(rv, Err(rv));
}
rv = statement->BindUTF8StringByName(NS_LITERAL_CSTRING("anno_name"), aName);
NS_ENSURE_SUCCESS(rv, rv);
NS_ENSURE_SUCCESS(rv, Err(rv));
rv = statement->Execute();
NS_ENSURE_SUCCESS(rv, rv);
NS_ENSURE_SUCCESS(rv, Err(rv));
return NS_OK;
return maybeBookmark;
}
@ -1414,8 +1509,11 @@ nsAnnotationService::RemovePageAnnotation(nsIURI* aURI,
{
NS_ENSURE_ARG(aURI);
nsresult rv = RemoveAnnotationInternal(aURI, 0, aName);
NS_ENSURE_SUCCESS(rv, rv);
Result<Maybe<BookmarkData>, nsresult> result =
RemoveAnnotationInternal(aURI, 0, aName);
if (result.isErr()) {
return result.unwrapErr();
}
NOTIFY_ANNOS_OBSERVERS(OnPageAnnotationRemoved(aURI, aName));
@ -1430,11 +1528,19 @@ nsAnnotationService::RemoveItemAnnotation(int64_t aItemId,
{
NS_ENSURE_ARG_MIN(aItemId, 1);
nsresult rv = RemoveAnnotationInternal(nullptr, aItemId, aName);
NS_ENSURE_SUCCESS(rv, rv);
Result<Maybe<BookmarkData>, nsresult> result =
RemoveAnnotationInternal(nullptr, aItemId, aName);
if (result.isErr()) {
return result.unwrapErr();
}
NOTIFY_ANNOS_OBSERVERS(OnItemAnnotationRemoved(aItemId, aName, aSource));
Maybe<BookmarkData> maybeBookmark = result.unwrap();
if (maybeBookmark) {
NotifyItemChanged(maybeBookmark.ref(), aName, aSource, false);
}
return NS_OK;
}
@ -1468,6 +1574,19 @@ nsAnnotationService::RemovePageAnnotations(nsIURI* aURI)
NS_IMETHODIMP
nsAnnotationService::RemoveItemAnnotations(int64_t aItemId,
uint16_t aSource)
{
nsresult rv = RemoveItemAnnotationsWithoutNotifying(aItemId);
NS_ENSURE_SUCCESS(rv, rv);
NOTIFY_ANNOS_OBSERVERS(OnItemAnnotationRemoved(aItemId, EmptyCString(),
aSource));
return NS_OK;
}
nsresult
nsAnnotationService::RemoveItemAnnotationsWithoutNotifying(int64_t aItemId)
{
NS_ENSURE_ARG_MIN(aItemId, 1);
@ -1484,9 +1603,6 @@ nsAnnotationService::RemoveItemAnnotations(int64_t aItemId,
rv = statement->Execute();
NS_ENSURE_SUCCESS(rv, rv);
NOTIFY_ANNOS_OBSERVERS(OnItemAnnotationRemoved(aItemId, EmptyCString(),
aSource));
return NS_OK;
}
@ -1649,6 +1765,14 @@ nsAnnotationService::CopyItemAnnotations(int64_t aSourceItemId,
NS_ENSURE_SUCCESS(rv, rv);
NOTIFY_ANNOS_OBSERVERS(OnItemAnnotationSet(aDestItemId, annoName, aSource, false));
nsNavBookmarks* bookmarks = nsNavBookmarks::GetBookmarksService();
if (bookmarks) {
BookmarkData bookmark;
if (NS_SUCCEEDED(bookmarks->FetchItemInfo(aDestItemId, bookmark))) {
NotifyItemChanged(bookmark, annoName, aSource, false);
}
}
}
rv = transaction.Commit();
@ -1835,7 +1959,7 @@ nsAnnotationService::StartGetAnnotation(nsIURI* aURI,
* vars, which control its scope. DO NOT RELEASE.
* The caller must take care of resetting the statement if this succeeds.
*/
nsresult
Result<Maybe<BookmarkData>, nsresult>
nsAnnotationService::StartSetAnnotation(nsIURI* aURI,
int64_t aItemId,
const nsACString& aName,
@ -1854,13 +1978,13 @@ nsAnnotationService::StartSetAnnotation(nsIURI* aURI,
nsCOMPtr<mozIStorageStatement> addNameStmt = mDB->GetStatement(
"INSERT OR IGNORE INTO moz_anno_attributes (name) VALUES (:anno_name)"
);
NS_ENSURE_STATE(addNameStmt);
NS_ENSURE_TRUE(addNameStmt, Err(NS_ERROR_UNEXPECTED));
mozStorageStatementScoper scoper(addNameStmt);
nsresult rv = addNameStmt->BindUTF8StringByName(NS_LITERAL_CSTRING("anno_name"), aName);
NS_ENSURE_SUCCESS(rv, rv);
NS_ENSURE_SUCCESS(rv, Err(rv));
rv = addNameStmt->Execute();
NS_ENSURE_SUCCESS(rv, rv);
NS_ENSURE_SUCCESS(rv, Err(rv));
// We have to check 2 things:
// - if the annotation already exists we should update it.
@ -1875,8 +1999,9 @@ nsAnnotationService::StartSetAnnotation(nsIURI* aURI,
stmt = mDB->GetStatement(
"SELECT b.id, "
"(SELECT id FROM moz_anno_attributes WHERE name = :anno_name) AS nameid, "
"a.id, a.dateAdded "
"a.id, a.dateAdded, b.parent, b.type, b.lastModified, b.guid, p.guid "
"FROM moz_bookmarks b "
"JOIN moz_bookmarks p ON p.id = b.parent "
"LEFT JOIN moz_items_annos a ON a.item_id = b.id "
"AND a.anno_attribute_id = nameid "
"WHERE b.id = :item_id"
@ -1893,24 +2018,29 @@ nsAnnotationService::StartSetAnnotation(nsIURI* aURI,
"WHERE h.url_hash = hash(:page_url) AND h.url = :page_url"
);
}
NS_ENSURE_STATE(stmt);
if (!stmt) {
return Err(NS_ERROR_UNEXPECTED);
}
mozStorageStatementScoper checkAnnoScoper(stmt);
rv = stmt->BindUTF8StringByName(NS_LITERAL_CSTRING("anno_name"), aName);
NS_ENSURE_SUCCESS(rv, rv);
if (isItemAnnotation)
NS_ENSURE_SUCCESS(rv, Err(rv));
if (isItemAnnotation) {
rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("item_id"), aItemId);
else
NS_ENSURE_SUCCESS(rv, Err(rv));
}
else {
rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("page_url"), aURI);
NS_ENSURE_SUCCESS(rv, rv);
NS_ENSURE_SUCCESS(rv, Err(rv));
}
bool hasResult;
rv = stmt->ExecuteStep(&hasResult);
NS_ENSURE_SUCCESS(rv, rv);
NS_ENSURE_SUCCESS(rv, Err(rv));
if (!hasResult) {
// We are trying to create an annotation on an invalid bookmark
// or history entry.
return NS_ERROR_INVALID_ARG;
return Err(NS_ERROR_INVALID_ARG);
}
int64_t fkId = stmt->AsInt64(0);
@ -1918,6 +2048,7 @@ nsAnnotationService::StartSetAnnotation(nsIURI* aURI,
int64_t oldAnnoId = stmt->AsInt64(2);
int64_t oldAnnoDate = stmt->AsInt64(3);
Maybe<BookmarkData> maybeBookmark;
if (isItemAnnotation) {
aStatement = mDB->GetStatement(
"INSERT OR REPLACE INTO moz_items_annos "
@ -1926,6 +2057,20 @@ nsAnnotationService::StartSetAnnotation(nsIURI* aURI,
"VALUES (:id, :fk, :name_id, :content, :flags, "
":expiration, :type, :date_added, :last_modified)"
);
// Since we're already querying `moz_bookmarks`, we fetch the changed
// bookmark's info here, instead of using `FetchItemInfo`.
BookmarkData bookmark;
bookmark.id = fkId;
bookmark.parentId = stmt->AsInt64(4);
bookmark.type = stmt->AsInt64(5);
bookmark.lastModified = static_cast<PRTime>(stmt->AsInt64(6));
if (NS_SUCCEEDED(stmt->GetUTF8String(7, bookmark.guid)) &&
NS_SUCCEEDED(stmt->GetUTF8String(8, bookmark.parentGuid))) {
maybeBookmark.emplace(bookmark);
}
}
else {
aStatement = mDB->GetStatement(
@ -1936,42 +2081,42 @@ nsAnnotationService::StartSetAnnotation(nsIURI* aURI,
":expiration, :type, :date_added, :last_modified)"
);
}
NS_ENSURE_STATE(aStatement);
NS_ENSURE_TRUE(aStatement, Err(NS_ERROR_UNEXPECTED));
mozStorageStatementScoper setAnnoScoper(aStatement);
// Don't replace existing annotations.
if (oldAnnoId > 0) {
rv = aStatement->BindInt64ByName(NS_LITERAL_CSTRING("id"), oldAnnoId);
NS_ENSURE_SUCCESS(rv, rv);
NS_ENSURE_SUCCESS(rv, Err(rv));
rv = aStatement->BindInt64ByName(NS_LITERAL_CSTRING("date_added"), oldAnnoDate);
NS_ENSURE_SUCCESS(rv, rv);
NS_ENSURE_SUCCESS(rv, Err(rv));
}
else {
rv = aStatement->BindNullByName(NS_LITERAL_CSTRING("id"));
NS_ENSURE_SUCCESS(rv, rv);
NS_ENSURE_SUCCESS(rv, Err(rv));
rv = aStatement->BindInt64ByName(NS_LITERAL_CSTRING("date_added"), RoundedPRNow());
NS_ENSURE_SUCCESS(rv, rv);
NS_ENSURE_SUCCESS(rv, Err(rv));
}
rv = aStatement->BindInt64ByName(NS_LITERAL_CSTRING("fk"), fkId);
NS_ENSURE_SUCCESS(rv, rv);
NS_ENSURE_SUCCESS(rv, Err(rv));
rv = aStatement->BindInt64ByName(NS_LITERAL_CSTRING("name_id"), nameID);
NS_ENSURE_SUCCESS(rv, rv);
NS_ENSURE_SUCCESS(rv, Err(rv));
rv = aStatement->BindInt32ByName(NS_LITERAL_CSTRING("flags"), aFlags);
NS_ENSURE_SUCCESS(rv, rv);
NS_ENSURE_SUCCESS(rv, Err(rv));
rv = aStatement->BindInt32ByName(NS_LITERAL_CSTRING("expiration"), aExpiration);
NS_ENSURE_SUCCESS(rv, rv);
NS_ENSURE_SUCCESS(rv, Err(rv));
rv = aStatement->BindInt32ByName(NS_LITERAL_CSTRING("type"), aType);
NS_ENSURE_SUCCESS(rv, rv);
NS_ENSURE_SUCCESS(rv, Err(rv));
rv = aStatement->BindInt64ByName(NS_LITERAL_CSTRING("last_modified"), RoundedPRNow());
NS_ENSURE_SUCCESS(rv, rv);
NS_ENSURE_SUCCESS(rv, Err(rv));
// On success, leave the statement open, the caller will set the value
// and execute the statement.
setAnnoScoper.Abandon();
return NS_OK;
return maybeBookmark;
}
////////////////////////////////////////////////////////////////////////////////

73
toolkit/components/places/nsAnnotationService.h
View File

@ -16,6 +16,8 @@
#include "Database.h"
#include "nsString.h"
#include "mozilla/Attributes.h"
#include "mozilla/Maybe.h"
#include "mozilla/Result.h"
namespace mozilla {
namespace places {
@ -111,42 +113,48 @@ protected:
const nsACString& aName,
nsCOMPtr<mozIStorageStatement>& aStatement);
nsresult StartSetAnnotation(nsIURI* aURI,
Result<Maybe<BookmarkData>, nsresult>
StartSetAnnotation(nsIURI* aURI,
int64_t aItemId,
const nsACString& aName,
int32_t aFlags,
uint16_t aExpiration,
uint16_t aType,
nsCOMPtr<mozIStorageStatement>& aStatement);
Result<Maybe<BookmarkData>, nsresult>
SetAnnotationStringInternal(nsIURI* aURI,
int64_t aItemId,
const nsACString& aName,
const nsAString& aValue,
int32_t aFlags,
uint16_t aExpiration,
uint16_t aType,
nsCOMPtr<mozIStorageStatement>& aStatement);
uint16_t aExpiration);
Result<Maybe<BookmarkData>, nsresult>
SetAnnotationInt32Internal(nsIURI* aURI,
int64_t aItemId,
const nsACString& aName,
int32_t aValue,
int32_t aFlags,
uint16_t aExpiration);
Result<Maybe<BookmarkData>, nsresult>
SetAnnotationInt64Internal(nsIURI* aURI,
int64_t aItemId,
const nsACString& aName,
int64_t aValue,
int32_t aFlags,
uint16_t aExpiration);
Result<Maybe<BookmarkData>, nsresult>
SetAnnotationDoubleInternal(nsIURI* aURI,
int64_t aItemId,
const nsACString& aName,
double aValue,
int32_t aFlags,
uint16_t aExpiration);
nsresult SetAnnotationStringInternal(nsIURI* aURI,
int64_t aItemId,
const nsACString& aName,
const nsAString& aValue,
int32_t aFlags,
uint16_t aExpiration);
nsresult SetAnnotationInt32Internal(nsIURI* aURI,
int64_t aItemId,
const nsACString& aName,
int32_t aValue,
int32_t aFlags,
uint16_t aExpiration);
nsresult SetAnnotationInt64Internal(nsIURI* aURI,
int64_t aItemId,
const nsACString& aName,
int64_t aValue,
int32_t aFlags,
uint16_t aExpiration);
nsresult SetAnnotationDoubleInternal(nsIURI* aURI,
int64_t aItemId,
const nsACString& aName,
double aValue,
int32_t aFlags,
uint16_t aExpiration);
nsresult RemoveAnnotationInternal(nsIURI* aURI,
int64_t aItemId,
const nsACString& aName);
Result<Maybe<BookmarkData>, nsresult>
RemoveAnnotationInternal(nsIURI* aURI,
int64_t aItemId,
const nsACString& aName);
public:
nsresult GetPagesWithAnnotationCOMArray(const nsACString& aName,
@ -156,6 +164,7 @@ public:
nsresult GetAnnotationNamesTArray(nsIURI* aURI,
int64_t aItemId,
nsTArray<nsCString>* _result);
nsresult RemoveItemAnnotationsWithoutNotifying(int64_t aItemId);
};
#endif /* nsAnnotationService_h___ */

9
toolkit/components/places/nsIAnnotationService.idl
View File

@ -112,7 +112,8 @@ interface nsIAnnotationService : nsISupports
*
* For item annotations, aSource should be a change source constant from
* nsINavBookmarksService::SOURCE_*, and defaults to SOURCE_DEFAULT if
* omitted.
* omitted. Setting an item annotation also notifies
* `nsINavBookmarkObserver::onItemChanged` for the affected item.
*
* The annotation "favicon" is special. Favicons are stored in the favicon
* service, but are special cased in the protocol handler so they look like
@ -340,6 +341,9 @@ interface nsIAnnotationService : nsISupports
/**
* Removes a specific annotation. Succeeds even if the annotation is
* not found.
*
* Removing an item annotation also notifies
* `nsINavBookmarkObserver::onItemChanged` for the affected item.
*/
void removePageAnnotation(in nsIURI aURI,
in AUTF8String aName);
@ -351,6 +355,9 @@ interface nsIAnnotationService : nsISupports
* Removes all annotations for the given page/item.
* We may want some other similar functions to get annotations with given
* flags (once we have flags defined).
*
* Unlike the other item methods, `removeItemAnnotations` does *not* notify
* `nsINavBookmarkObserver::onItemChanged` for the affected item.
*/
void removePageAnnotations(in nsIURI aURI);
void removeItemAnnotations(in long long aItemId,

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

@ -184,7 +184,6 @@ nsNavBookmarks::~nsNavBookmarks()
NS_IMPL_ISUPPORTS(nsNavBookmarks
, nsINavBookmarksService
, nsINavHistoryObserver
, nsIAnnotationObserver
, nsIObserver
, nsISupportsWeakReference
)
@ -209,17 +208,11 @@ nsNavBookmarks::Init()
nsCOMPtr<nsIObserverService> os = mozilla::services::GetObserverService();
if (os) {
(void)os->AddObserver(this, TOPIC_PLACES_SHUTDOWN, true);
(void)os->AddObserver(this, TOPIC_PLACES_CONNECTION_CLOSED, true);
}
mCanNotify = true;
// Observe annotations.
nsAnnotationService* annosvc = nsAnnotationService::GetAnnotationService();
NS_ENSURE_TRUE(annosvc, NS_ERROR_OUT_OF_MEMORY);
annosvc->AddObserver(this);
// Allows us to notify on title changes. MUST BE LAST so it is impossible
// to fail after this call, or the history service will have a reference to
// us and we won't go away.
@ -687,13 +680,15 @@ nsNavBookmarks::RemoveItem(int64_t aItemId, uint16_t aSource)
mozStorageTransaction transaction(mDB->MainConn(), false);
// First, if not a tag, remove item annotations.
// 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.
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->RemoveItemAnnotations(bookmark.id, aSource);
rv = annosvc->RemoveItemAnnotationsWithoutNotifying(bookmark.id);
NS_ENSURE_SUCCESS(rv, rv);
}
@ -3092,23 +3087,28 @@ nsNavBookmarks::NotifyItemChanged(const ItemChangeData& aData)
{
// A guid must always be defined.
MOZ_ASSERT(!aData.bookmark.guid.IsEmpty());
PRTime lastModified = aData.bookmark.lastModified;
if (aData.updateLastModified) {
lastModified = RoundedPRNow();
MOZ_ALWAYS_SUCCEEDS(SetItemDateInternal(
LAST_MODIFIED, DetermineSyncChangeDelta(aData.source),
aData.bookmark.id, lastModified));
}
NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
nsINavBookmarkObserver,
OnItemChanged(aData.bookmark.id,
aData.property,
aData.isAnnotation,
aData.newValue,
aData.bookmark.lastModified,
lastModified,
aData.bookmark.type,
aData.bookmark.parentId,
aData.bookmark.guid,
aData.bookmark.parentGuid,
aData.oldValue,
// We specify the default source here because
// this method is only called for history
// visits, and we don't track sources in
// history.
SOURCE_DEFAULT));
aData.source));
}
////////////////////////////////////////////////////////////////////////////////
@ -3120,14 +3120,7 @@ nsNavBookmarks::Observe(nsISupports *aSubject, const char *aTopic,
{
NS_ASSERTION(NS_IsMainThread(), "This can only be called on the main thread");
if (strcmp(aTopic, TOPIC_PLACES_SHUTDOWN) == 0) {
// Stop Observing annotations.
nsAnnotationService* annosvc = nsAnnotationService::GetAnnotationService();
if (annosvc) {
annosvc->RemoveObserver(this);
}
}
else if (strcmp(aTopic, TOPIC_PLACES_CONNECTION_CLOSED) == 0) {
if (strcmp(aTopic, TOPIC_PLACES_CONNECTION_CLOSED) == 0) {
// Don't even try to notify observers from this point on, the category
// cache would init services that could try to use our APIs.
mCanNotify = false;
@ -3306,63 +3299,3 @@ nsNavBookmarks::OnDeleteVisits(nsIURI* aURI, PRTime aVisitTime,
}
return NS_OK;
}
// nsIAnnotationObserver
NS_IMETHODIMP
nsNavBookmarks::OnPageAnnotationSet(nsIURI* aPage, const nsACString& aName)
{
return NS_OK;
}
NS_IMETHODIMP
nsNavBookmarks::OnItemAnnotationSet(int64_t aItemId, const nsACString& aName,
uint16_t aSource, bool aDontUpdateLastModified)
{
BookmarkData bookmark;
nsresult rv = FetchItemInfo(aItemId, bookmark);
NS_ENSURE_SUCCESS(rv, rv);
if (!aDontUpdateLastModified) {
bookmark.lastModified = RoundedPRNow();
rv = SetItemDateInternal(LAST_MODIFIED, DetermineSyncChangeDelta(aSource),
bookmark.id, bookmark.lastModified);
NS_ENSURE_SUCCESS(rv, rv);
}
NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
nsINavBookmarkObserver,
OnItemChanged(bookmark.id,
aName,
true,
EmptyCString(),
bookmark.lastModified,
bookmark.type,
bookmark.parentId,
bookmark.guid,
bookmark.parentGuid,
EmptyCString(),
aSource));
return NS_OK;
}
NS_IMETHODIMP
nsNavBookmarks::OnPageAnnotationRemoved(nsIURI* aPage, const nsACString& aName)
{
return NS_OK;
}
NS_IMETHODIMP
nsNavBookmarks::OnItemAnnotationRemoved(int64_t aItemId, const nsACString& aName,
uint16_t aSource)
{
// As of now this is doing the same as OnItemAnnotationSet, so just forward
// the call.
nsresult rv = OnItemAnnotationSet(aItemId, aName, aSource, false);
NS_ENSURE_SUCCESS(rv, rv);
return NS_OK;
}

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

@ -7,7 +7,6 @@
#define nsNavBookmarks_h_
#include "nsINavBookmarksService.h"
#include "nsIAnnotationService.h"
#include "nsITransaction.h"
#include "nsNavHistory.h"
#include "nsToolkitCompsCID.h"
@ -28,18 +27,18 @@ namespace places {
};
struct BookmarkData {
int64_t id;
int64_t id = -1;
nsCString url;
nsCString title;
int32_t position;
int64_t placeId;
int64_t parentId;
int64_t grandParentId;
int32_t type;
int32_t syncStatus;
int32_t position = -1;
int64_t placeId = -1;
int64_t parentId = -1;
int64_t grandParentId = -1;
int32_t type = 0;
int32_t syncStatus = nsINavBookmarksService::SYNC_STATUS_UNKNOWN;
nsCString serviceCID;
PRTime dateAdded;
PRTime lastModified;
PRTime dateAdded = 0;
PRTime lastModified = 0;
nsCString guid;
nsCString parentGuid;
};
@ -53,8 +52,10 @@ namespace places {
struct ItemChangeData {
BookmarkData bookmark;
bool isAnnotation = false;
bool updateLastModified = false;
uint16_t source = nsINavBookmarksService::SOURCE_DEFAULT;
nsCString property;
bool isAnnotation;
nsCString newValue;
nsCString oldValue;
};
@ -77,7 +78,6 @@ namespace places {
class nsNavBookmarks final : public nsINavBookmarksService
, public nsINavHistoryObserver
, public nsIAnnotationObserver
, public nsIObserver
, public nsSupportsWeakReference
{
@ -85,7 +85,6 @@ public:
NS_DECL_ISUPPORTS
NS_DECL_NSINAVBOOKMARKSSERVICE
NS_DECL_NSINAVHISTORYOBSERVER
NS_DECL_NSIANNOTATIONOBSERVER
NS_DECL_NSIOBSERVER
nsNavBookmarks();

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

@ -231,8 +231,6 @@ add_task(async function remove_bookmark() {
let observer = expectNotifications();
await PlacesUtils.bookmarks.remove(bm.guid);
// TODO (Bug 653910): onItemAnnotationRemoved notified even if there were no
// annotations.
observer.check([ { name: "onItemRemoved",
arguments: [ itemId, parentId, bm.index, bm.type, bm.url,
bm.guid, bm.parentGuid,
@ -254,8 +252,6 @@ add_task(async function remove_multiple_bookmarks() {
let observer = expectNotifications();
await PlacesUtils.bookmarks.remove([bm1, bm2]);
// TODO (Bug 653910): onItemAnnotationRemoved notified even if there were no
// annotations.
observer.check([ { name: "onItemRemoved",
arguments: [ itemId1, parentId1, bm1.index, bm1.type, bm1.url,
bm1.guid, bm1.parentGuid,

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

@ -211,7 +211,7 @@ add_task(async function onItemChanged_title_bookmark() {
"onItemChanged"
]),
gBookmarksObserver.setup([
{ name: "onItemChanged", // This is an unfortunate effect of bug 653910.
{ name: "onItemChanged",
args: [
{ name: "itemId", check: v => typeof(v) == "number" && v > 0 },
{ name: "property", check: v => v === "title" },
@ -393,23 +393,9 @@ add_task(async function onItemRemoved_bookmark() {
let uri = Services.io.newURI(url);
let promise = Promise.all([
gBookmarkSkipObserver.setup([
"onItemChanged", "onItemRemoved"
"onItemRemoved"
]),
gBookmarksObserver.setup([
{ name: "onItemChanged", // This is an unfortunate effect of bug 653910.
args: [
{ name: "itemId", check: v => typeof(v) == "number" && v > 0 },
{ name: "property", check: v => v === "" },
{ name: "isAnno", check: v => v === true },
{ name: "newValue", check: v => v === "" },
{ name: "lastModified", check: v => typeof(v) == "number" && v > 0 },
{ name: "itemType", check: v => v === PlacesUtils.bookmarks.TYPE_BOOKMARK },
{ name: "parentId", check: v => v === PlacesUtils.unfiledBookmarksFolderId },
{ name: "guid", check: v => typeof(v) == "string" && GUID_RE.test(v) },
{ name: "parentGuid", check: v => typeof(v) == "string" && GUID_RE.test(v) },
{ name: "oldValue", check: v => typeof(v) == "string" },
{ name: "source", check: v => Object.values(PlacesUtils.bookmarks.SOURCES).includes(v) },
] },
{ name: "onItemRemoved",
args: [
{ name: "itemId", check: v => typeof(v) == "number" && v > 0 },
@ -430,23 +416,9 @@ add_task(async function onItemRemoved_separator() {
let id = PlacesUtils.bookmarks.getIdForItemAt(PlacesUtils.unfiledBookmarksFolderId, 0);
let promise = Promise.all([
gBookmarkSkipObserver.setup([
"onItemChanged", "onItemRemoved"
"onItemRemoved"
]),
gBookmarksObserver.setup([
{ name: "onItemChanged", // This is an unfortunate effect of bug 653910.
args: [
{ name: "itemId", check: v => typeof(v) == "number" && v > 0 },
{ name: "property", check: v => v === "" },
{ name: "isAnno", check: v => v === true },
{ name: "newValue", check: v => v === "" },
{ name: "lastModified", check: v => typeof(v) == "number" && v > 0 },
{ name: "itemType", check: v => v === PlacesUtils.bookmarks.TYPE_SEPARATOR },
{ name: "parentId", check: v => typeof(v) == "number" && v > 0 },
{ name: "guid", check: v => typeof(v) == "string" && GUID_RE.test(v) },
{ name: "parentGuid", check: v => typeof(v) == "string" && GUID_RE.test(v) },
{ name: "oldValue", check: v => typeof(v) == "string" },
{ name: "source", check: v => Object.values(PlacesUtils.bookmarks.SOURCES).includes(v) },
] },
{ name: "onItemRemoved",
args: [
{ name: "itemId", check: v => typeof(v) == "number" && v > 0 },
@ -467,23 +439,9 @@ add_task(async function onItemRemoved_folder() {
let id = PlacesUtils.bookmarks.getIdForItemAt(PlacesUtils.unfiledBookmarksFolderId, 0);
let promise = Promise.all([
gBookmarkSkipObserver.setup([
"onItemChanged", "onItemRemoved"
"onItemRemoved"
]),
gBookmarksObserver.setup([
{ name: "onItemChanged", // This is an unfortunate effect of bug 653910.
args: [
{ name: "itemId", check: v => typeof(v) == "number" && v > 0 },
{ name: "property", check: v => v === "" },
{ name: "isAnno", check: v => v === true },
{ name: "newValue", check: v => v === "" },
{ name: "lastModified", check: v => typeof(v) == "number" && v > 0 },
{ name: "itemType", check: v => v === PlacesUtils.bookmarks.TYPE_FOLDER },
{ name: "parentId", check: v => typeof(v) == "number" && v > 0 },
{ name: "guid", check: v => typeof(v) == "string" && GUID_RE.test(v) },
{ name: "parentGuid", check: v => typeof(v) == "string" && GUID_RE.test(v) },
{ name: "oldValue", check: v => typeof(v) == "string" },
{ name: "source", check: v => Object.values(PlacesUtils.bookmarks.SOURCES).includes(v) },
] },
{ name: "onItemRemoved",
args: [
{ name: "itemId", check: v => typeof(v) == "number" && v > 0 },
@ -507,7 +465,7 @@ add_task(async function onItemRemoved_folder_recursive() {
let promise = Promise.all([
gBookmarkSkipObserver.setup([
"onItemAdded", "onItemAdded", "onItemAdded", "onItemAdded",
"onItemChanged", "onItemRemoved"
"onItemRemoved"
]),
gBookmarksObserver.setup([
{ name: "onItemAdded",
@ -562,20 +520,6 @@ add_task(async function onItemRemoved_folder_recursive() {
{ name: "parentGuid", check: v => typeof(v) == "string" && GUID_RE.test(v) },
{ name: "source", check: v => Object.values(PlacesUtils.bookmarks.SOURCES).includes(v) },
] },
{ name: "onItemChanged", // This is an unfortunate effect of bug 653910.
args: [
{ name: "itemId", check: v => typeof(v) == "number" && v > 0 },
{ name: "property", check: v => v === "" },
{ name: "isAnno", check: v => v === true },
{ name: "newValue", check: v => v === "" },
{ name: "lastModified", check: v => typeof(v) == "number" && v > 0 },
{ name: "itemType", check: v => v === PlacesUtils.bookmarks.TYPE_FOLDER },
{ name: "parentId", check: v => typeof(v) == "number" && v > 0 },
{ name: "guid", check: v => typeof(v) == "string" && GUID_RE.test(v) },
{ name: "parentGuid", check: v => typeof(v) == "string" && GUID_RE.test(v) },
{ name: "oldValue", check: v => typeof(v) == "string" },
{ name: "source", check: v => Object.values(PlacesUtils.bookmarks.SOURCES).includes(v) },
] },
{ name: "onItemRemoved",
args: [
{ name: "itemId", check: v => typeof(v) == "number" && v > 0 },

5
toolkit/components/places/tests/unit/test_null_interfaces.js
View File

@ -14,9 +14,8 @@ var testServices = [
["queryStringToQueries", "removePagesByTimeframe", "removePagesFromHost", "getObservers"]
],
["browser/nav-bookmarks-service;1",
["nsINavBookmarksService", "nsINavHistoryObserver", "nsIAnnotationObserver"],
["createFolder", "getObservers", "onFrecencyChanged", "onTitleChanged",
"onPageAnnotationSet", "onPageAnnotationRemoved", "onDeleteURI"]
["nsINavBookmarksService", "nsINavHistoryObserver"],
["createFolder", "getObservers", "onFrecencyChanged", "onTitleChanged", "onDeleteURI"]
],
["browser/livemark-service;2", ["mozIAsyncLivemarks"], ["reloadLivemarks"]],
["browser/annotation-service;1", ["nsIAnnotationService"], ["getObservers"]],