Bug 1434607 - Remove synchronous Bookmarks::setKeywordForBookmark. r=standard8

MozReview-Commit-ID: 3thDN9FIDgm

--HG--
extra : rebase_source : 71c53c5e779bf1e121a5efff3d387f2c0ff2abf3
This commit is contained in:
Marco Bonardo 2018-02-01 11:02:41 +01:00
Родитель ba2d560b34
Коммит 42db636d46
5 изменённых файлов: 53 добавлений и 343 удалений

Просмотреть файл

@ -34,12 +34,10 @@ const NEWLINE = AppConstants.platform == "macosx" ? "\n" : "\r\n";
const TIMERS_RESOLUTION_SKEW_MS = 16;
function QI_node(aNode, aIID) {
var result = null;
try {
result = aNode.QueryInterface(aIID);
} catch (e) {
}
return result;
return aNode.QueryInterface(aIID);
} catch (ex) {}
return null;
}
function asContainer(aNode) {
return QI_node(aNode, Ci.nsINavHistoryContainerResultNode);
@ -79,8 +77,10 @@ async function notifyKeywordChange(url, keyword, source) {
let bookmarks = [];
await PlacesUtils.bookmarks.fetch({ url }, b => bookmarks.push(b));
for (let bookmark of bookmarks) {
bookmark.id = await PlacesUtils.promiseItemId(bookmark.guid);
bookmark.parentId = await PlacesUtils.promiseItemId(bookmark.parentGuid);
let ids = await PlacesUtils.promiseManyItemIds([bookmark.guid,
bookmark.parentGuid]);
bookmark.id = ids.get(bookmark.guid);
bookmark.parentId = ids.get(bookmark.parentGuid);
}
let observers = PlacesUtils.bookmarks.getObservers();
for (let bookmark of bookmarks) {
@ -193,6 +193,22 @@ const DB_URL_LENGTH_MAX = 65536;
const DB_TITLE_LENGTH_MAX = 4096;
const DB_DESCRIPTION_LENGTH_MAX = 256;
/**
* Executes a boolean validate function, throwing if it returns false.
*
* @param boolValidateFn
* A boolean validate function.
* @return the input value.
* @throws if input doesn't pass the validate function.
*/
function simpleValidateFunc(boolValidateFn) {
return (v, input) => {
if (!boolValidateFn(v, input))
throw new Error("Invalid value");
return v;
};
}
/**
* List of bookmark object validators, one per each known property.
* Validators must throw if the property value is invalid and return a fixed up
@ -678,9 +694,6 @@ this.PlacesUtils = {
}
},
onPageAnnotationSet() {},
onPageAnnotationRemoved() {},
/**
* Determines whether or not a ResultNode is a host container.
* @param aNode
@ -956,7 +969,7 @@ this.PlacesUtils = {
try {
titleString = Services.io.newURI(uriString).QueryInterface(Ci.nsIURL)
.fileName;
} catch (e) {}
} catch (ex) {}
}
// note: Services.io.newURI() will throw if uriString is not a valid URI
if (Services.io.newURI(uriString)) {
@ -1419,7 +1432,6 @@ this.PlacesUtils = {
*/
setCharsetForURI: function PU_setCharsetForURI(aURI, aCharset) {
return new Promise(resolve => {
// Delaying to catch issues with asynchronous behavior while waiting
// to implement asynchronous annotations in bug 699844.
Services.tm.dispatchToMainThread(function() {
@ -1433,7 +1445,6 @@ this.PlacesUtils = {
}
resolve();
});
});
},
@ -1446,18 +1457,14 @@ this.PlacesUtils = {
*/
getCharsetForURI: function PU_getCharsetForURI(aURI) {
return new Promise(resolve => {
Services.tm.dispatchToMainThread(function() {
let charset = null;
try {
charset = PlacesUtils.annotations.getPageAnnotation(aURI,
PlacesUtils.CHARSET_ANNO);
} catch (ex) { }
resolve(charset);
});
});
},
@ -1472,12 +1479,9 @@ this.PlacesUtils = {
promiseFaviconData(aPageUrl) {
return new Promise((resolve, reject) => {
PlacesUtils.favicons.getFaviconDataForPage(NetUtil.newURI(aPageUrl),
function(aURI, aDataLen, aData, aMimeType) {
if (aURI) {
resolve({ uri: aURI,
dataLen: aDataLen,
data: aData,
mimeType: aMimeType });
function(uri, dataLen, data, mimeType) {
if (uri) {
resolve({ uri, dataLen, data, mimeType });
} else {
reject();
}
@ -1677,8 +1681,8 @@ this.PlacesUtils = {
if (aRow.getResultByName("has_annos")) {
try {
item.annos = PlacesUtils.getAnnotationsForItem(itemId);
} catch (e) {
Cu.reportError("Unexpected error while reading annotations " + e);
} catch (ex) {
Cu.reportError("Unexpected error while reading annotations " + ex);
}
}
@ -1756,7 +1760,6 @@ this.PlacesUtils = {
LEFT JOIN moz_places h ON h.id = d.fk
ORDER BY d.level, d.parent, d.position`;
if (!aItemGuid)
aItemGuid = this.bookmarks.rootGuid;
@ -2006,8 +2009,13 @@ PlacesUtils.keywords = {
if (onResult && typeof onResult != "function")
throw new Error("onResult callback must be a valid function");
if (hasUrl)
keywordOrEntry.url = new URL(keywordOrEntry.url);
if (hasUrl) {
try {
keywordOrEntry.url = BOOKMARK_VALIDATORS.url(keywordOrEntry.url);
} catch (ex) {
throw new Error(keywordOrEntry.url + " is not a valid URL");
}
}
if (hasKeyword)
keywordOrEntry.keyword = keywordOrEntry.keyword.trim().toLowerCase();
@ -2052,7 +2060,7 @@ PlacesUtils.keywords = {
* An object describing the keyword to insert, in the form:
* {
* keyword: non-empty string,
* URL: URL or href to associate to the keyword,
* url: URL or href to associate to the keyword,
* postData: optional POST data to associate to the keyword
* source: The change source, forwarded to all bookmark observers.
* Defaults to nsINavBookmarksService::SOURCE_DEFAULT.
@ -2081,7 +2089,11 @@ PlacesUtils.keywords = {
keyword = keyword.trim().toLowerCase();
let postData = keywordEntry.postData || "";
// This also checks href for validity
url = new URL(url);
try {
url = BOOKMARK_VALIDATORS.url(url);
} catch (ex) {
throw new Error(url + " is not a valid URL");
}
return PlacesUtils.withConnectionWrapper("PlacesUtils.keywords.insert", async db => {
let cache = await promiseKeywordsCache();
@ -2566,19 +2578,3 @@ var GuidHelper = {
}
}
};
/**
* Executes a boolean validate function, throwing if it returns false.
*
* @param boolValidateFn
* A boolean validate function.
* @return the input value.
* @throws if input doesn't pass the validate function.
*/
function simpleValidateFunc(boolValidateFn) {
return (v, input) => {
if (!boolValidateFn(v, input))
throw new Error("Invalid value");
return v;
};
}

Просмотреть файл

@ -536,18 +536,6 @@ interface nsINavBookmarksService : nsISupports
*/
long long getFolderIdForItem(in long long aItemId);
/**
* Associates the given keyword with the given bookmark.
*
* Use an empty keyword to clear the keyword associated with the URI.
* In both of these cases, succeeds but does nothing if the URL/keyword is not found.
*
* @deprecated Use PlacesUtils.keywords.insert() API instead.
*/
void setKeywordForBookmark(in long long aItemId,
in AString aKeyword,
[optional] in unsigned short aSource);
/**
* Retrieves the keyword for the given bookmark. Will be void string
* (null in JS) if no such keyword is found.
@ -555,6 +543,7 @@ interface nsINavBookmarksService : nsISupports
* @deprecated Use PlacesUtils.keywords.fetch() API instead.
*/
AString getKeywordForBookmark(in long long aItemId);
/**
* Adds a bookmark observer. If ownsWeak is false, the bookmark service will
* keep an owning reference to the observer. If ownsWeak is true, then

Просмотреть файл

@ -2249,265 +2249,6 @@ nsNavBookmarks::GetBookmarksForURI(nsIURI* aURI,
NS_IMETHODIMP
nsNavBookmarks::SetKeywordForBookmark(int64_t aBookmarkId,
const nsAString& aUserCasedKeyword,
uint16_t aSource)
{
NS_ENSURE_ARG_MIN(aBookmarkId, 1);
// This also ensures the bookmark is valid.
BookmarkData bookmark;
nsresult rv = FetchItemInfo(aBookmarkId, bookmark);
NS_ENSURE_SUCCESS(rv, rv);
nsCOMPtr<nsIURI> uri;
rv = NS_NewURI(getter_AddRefs(uri), bookmark.url);
NS_ENSURE_SUCCESS(rv, rv);
// Shortcuts are always lowercased internally.
nsAutoString keyword(aUserCasedKeyword);
ToLowerCase(keyword);
// The same URI can be associated to more than one keyword, provided the post
// data differs. Check if there are already keywords associated to this uri.
nsTArray<nsString> oldKeywords;
{
nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(
"SELECT keyword FROM moz_keywords WHERE place_id = :place_id"
);
NS_ENSURE_STATE(stmt);
mozStorageStatementScoper scoper(stmt);
rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("place_id"), bookmark.placeId);
NS_ENSURE_SUCCESS(rv, rv);
bool hasMore;
while (NS_SUCCEEDED(stmt->ExecuteStep(&hasMore)) && hasMore) {
nsString oldKeyword;
rv = stmt->GetString(0, oldKeyword);
NS_ENSURE_SUCCESS(rv, rv);
oldKeywords.AppendElement(oldKeyword);
}
}
// Trying to remove a non-existent keyword is a no-op.
if (keyword.IsEmpty() && oldKeywords.Length() == 0) {
return NS_OK;
}
int64_t syncChangeDelta = DetermineSyncChangeDelta(aSource);
mozStorageTransaction transaction(mDB->MainConn(), false);
// Remove the existing keywords.
// Note this is wrong because in the insert-new-keyword case we should only
// remove those having the same POST data, but this API doesn't allow that.
// And in any case, this API is going away.
for (uint32_t i = 0; i < oldKeywords.Length(); ++i) {
nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(
"DELETE FROM moz_keywords WHERE keyword = :old_keyword"
);
NS_ENSURE_STATE(stmt);
mozStorageStatementScoper scoper(stmt);
rv = stmt->BindStringByName(NS_LITERAL_CSTRING("old_keyword"),
oldKeywords[i]);
NS_ENSURE_SUCCESS(rv, rv);
rv = stmt->Execute();
NS_ENSURE_SUCCESS(rv, rv);
}
if (keyword.IsEmpty()) {
// We are removing the existing keywords.
for (uint32_t i = 0; i < oldKeywords.Length(); ++i) {
nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(
"DELETE FROM moz_keywords WHERE keyword = :old_keyword"
);
NS_ENSURE_STATE(stmt);
mozStorageStatementScoper scoper(stmt);
rv = stmt->BindStringByName(NS_LITERAL_CSTRING("old_keyword"),
oldKeywords[i]);
NS_ENSURE_SUCCESS(rv, rv);
rv = stmt->Execute();
NS_ENSURE_SUCCESS(rv, rv);
}
nsTArray<BookmarkData> bookmarks;
rv = GetBookmarksForURI(uri, bookmarks);
NS_ENSURE_SUCCESS(rv, rv);
if (syncChangeDelta && !bookmarks.IsEmpty()) {
// Build a query to update all bookmarks in a single statement.
nsAutoCString changedIds;
changedIds.AppendInt(bookmarks[0].id);
for (uint32_t i = 1; i < bookmarks.Length(); ++i) {
changedIds.Append(',');
changedIds.AppendInt(bookmarks[i].id);
}
// Update the sync change counter for all bookmarks with the removed
// keyword.
nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(
NS_LITERAL_CSTRING(
"UPDATE moz_bookmarks SET "
"syncChangeCounter = syncChangeCounter + :delta "
"WHERE id IN (") + changedIds + NS_LITERAL_CSTRING(")")
);
NS_ENSURE_STATE(stmt);
mozStorageStatementScoper scoper(stmt);
rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("delta"),
syncChangeDelta);
NS_ENSURE_SUCCESS(rv, rv);
rv = stmt->Execute();
NS_ENSURE_SUCCESS(rv, rv);
}
rv = transaction.Commit();
NS_ENSURE_SUCCESS(rv, rv);
for (uint32_t i = 0; i < bookmarks.Length(); ++i) {
NOTIFY_OBSERVERS(mCanNotify, mObservers, nsINavBookmarkObserver,
OnItemChanged(bookmarks[i].id,
NS_LITERAL_CSTRING("keyword"),
false,
EmptyCString(),
bookmarks[i].lastModified,
TYPE_BOOKMARK,
bookmarks[i].parentId,
bookmarks[i].guid,
bookmarks[i].parentGuid,
// Abusing oldVal to pass out the url.
bookmark.url,
aSource));
}
return NS_OK;
}
// A keyword can only be associated to a single URI. Check if the requested
// keyword was already associated, in such a case we will need to notify about
// the change.
nsAutoCString oldSpec;
nsCOMPtr<nsIURI> oldUri;
{
nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(
"SELECT url "
"FROM moz_keywords "
"JOIN moz_places h ON h.id = place_id "
"WHERE keyword = :keyword"
);
NS_ENSURE_STATE(stmt);
mozStorageStatementScoper scoper(stmt);
rv = stmt->BindStringByName(NS_LITERAL_CSTRING("keyword"), keyword);
NS_ENSURE_SUCCESS(rv, rv);
bool hasMore;
if (NS_SUCCEEDED(stmt->ExecuteStep(&hasMore)) && hasMore) {
rv = stmt->GetUTF8String(0, oldSpec);
NS_ENSURE_SUCCESS(rv, rv);
rv = NS_NewURI(getter_AddRefs(oldUri), oldSpec);
NS_ENSURE_SUCCESS(rv, rv);
}
}
// If another uri is using the new keyword, we must update the keyword entry.
// Note we cannot use INSERT OR REPLACE cause it wouldn't invoke the delete
// trigger.
nsCOMPtr<mozIStorageStatement> stmt;
if (oldUri) {
// In both cases, notify about the change.
nsTArray<BookmarkData> bookmarks;
rv = GetBookmarksForURI(oldUri, bookmarks);
NS_ENSURE_SUCCESS(rv, rv);
for (uint32_t i = 0; i < bookmarks.Length(); ++i) {
NOTIFY_OBSERVERS(mCanNotify, mObservers, nsINavBookmarkObserver,
OnItemChanged(bookmarks[i].id,
NS_LITERAL_CSTRING("keyword"),
false,
EmptyCString(),
bookmarks[i].lastModified,
TYPE_BOOKMARK,
bookmarks[i].parentId,
bookmarks[i].guid,
bookmarks[i].parentGuid,
// Abusing oldVal to pass out the url.
oldSpec,
aSource));
}
stmt = mDB->GetStatement(
"UPDATE moz_keywords SET place_id = :place_id WHERE keyword = :keyword"
);
NS_ENSURE_STATE(stmt);
}
else {
stmt = mDB->GetStatement(
"INSERT INTO moz_keywords (keyword, place_id, post_data) "
"VALUES (:keyword, :place_id, '')"
);
}
NS_ENSURE_STATE(stmt);
mozStorageStatementScoper scoper(stmt);
rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("place_id"), bookmark.placeId);
NS_ENSURE_SUCCESS(rv, rv);
rv = stmt->BindStringByName(NS_LITERAL_CSTRING("keyword"), keyword);
NS_ENSURE_SUCCESS(rv, rv);
rv = stmt->Execute();
NS_ENSURE_SUCCESS(rv, rv);
nsTArray<BookmarkData> bookmarks;
rv = GetBookmarksForURI(uri, bookmarks);
NS_ENSURE_SUCCESS(rv, rv);
if (syncChangeDelta && !bookmarks.IsEmpty()) {
// Build a query to update all bookmarks in a single statement.
nsAutoCString changedIds;
changedIds.AppendInt(bookmarks[0].id);
for (uint32_t i = 1; i < bookmarks.Length(); ++i) {
changedIds.Append(',');
changedIds.AppendInt(bookmarks[i].id);
}
// Update the sync change counter for all bookmarks with the new keyword.
nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(
NS_LITERAL_CSTRING(
"UPDATE moz_bookmarks SET "
"syncChangeCounter = syncChangeCounter + :delta "
"WHERE id IN (") + changedIds + NS_LITERAL_CSTRING(")")
);
NS_ENSURE_STATE(stmt);
mozStorageStatementScoper scoper(stmt);
rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("delta"), syncChangeDelta);
NS_ENSURE_SUCCESS(rv, rv);
rv = stmt->Execute();
NS_ENSURE_SUCCESS(rv, rv);
}
rv = transaction.Commit();
NS_ENSURE_SUCCESS(rv, rv);
// In both cases, notify about the change.
for (uint32_t i = 0; i < bookmarks.Length(); ++i) {
NOTIFY_OBSERVERS(mCanNotify, mObservers, nsINavBookmarkObserver,
OnItemChanged(bookmarks[i].id,
NS_LITERAL_CSTRING("keyword"),
false,
NS_ConvertUTF16toUTF8(keyword),
bookmarks[i].lastModified,
TYPE_BOOKMARK,
bookmarks[i].parentId,
bookmarks[i].guid,
bookmarks[i].parentGuid,
// Abusing oldVal to pass out the url.
bookmark.url,
aSource));
}
return NS_OK;
}
NS_IMETHODIMP
nsNavBookmarks::GetKeywordForBookmark(int64_t aBookmarkId, nsAString& aKeyword)
{

Просмотреть файл

@ -79,10 +79,6 @@ add_task(function test_invalid_input() {
/NS_ERROR_ILLEGAL_VALUE/);
Assert.throws(() => PlacesUtils.bookmarks.getKeywordForBookmark(0),
/NS_ERROR_ILLEGAL_VALUE/);
Assert.throws(() => PlacesUtils.bookmarks.setKeywordForBookmark(null, "k"),
/NS_ERROR_ILLEGAL_VALUE/);
Assert.throws(() => PlacesUtils.bookmarks.setKeywordForBookmark(0, "k"),
/NS_ERROR_ILLEGAL_VALUE/);
});
add_task(async function test_addBookmarkAndKeyword() {
@ -554,8 +550,6 @@ add_task(async function test_invalidation() {
ok(!(await PlacesUtils.keywords.fetch({ keyword: "tb" })),
"Should not return URL for removed bookmark keyword");
await PlacesTestUtils.promiseAsyncUpdates();
await check_orphans();
await PlacesUtils.bookmarks.eraseEverything();

Просмотреть файл

@ -117,13 +117,16 @@ class TestCases {
info(`Tagged bookmark ${guid}`);
await checkSyncFields(guid, { syncChangeCounter: 4 });
await this.setKeyword(guid, "keyword");
info(`Set keyword for bookmark ${guid}`);
await checkSyncFields(guid, { syncChangeCounter: 5 });
await this.removeKeyword(guid, "keyword");
info(`Removed keyword from bookmark ${guid}`);
await checkSyncFields(guid, { syncChangeCounter: 6 });
if ("setKeyword" in this) {
await this.setKeyword(guid, "keyword");
info(`Set keyword for bookmark ${guid}`);
await checkSyncFields(guid, { syncChangeCounter: 5 });
}
if ("removeKeyword" in this) {
await this.removeKeyword(guid, "keyword");
info(`Removed keyword from bookmark ${guid}`);
await checkSyncFields(guid, { syncChangeCounter: 6 });
}
}
async testSeparators() {
@ -284,19 +287,6 @@ class SyncTestCases extends TestCases {
PlacesUtils.bookmarks.setItemTitle(id, title);
}
async setKeyword(guid, keyword) {
let id = await PlacesUtils.promiseItemId(guid);
PlacesUtils.bookmarks.setKeywordForBookmark(id, keyword);
}
async removeKeyword(guid, keyword) {
let id = await PlacesUtils.promiseItemId(guid);
if (PlacesUtils.bookmarks.getKeywordForBookmark(id) != keyword) {
throw new Error(`Keyword ${keyword} not set for bookmark ${guid}`);
}
PlacesUtils.bookmarks.setKeywordForBookmark(id, "");
}
async tagURI(uri, tags) {
PlacesUtils.tagging.tagURI(uri, tags);
}