Bug 1433307 - Remove synchronous Bookmarks::getKeywordForBookmark. r=standard8
MozReview-Commit-ID: 1XR5N2WJ7P7
--- a/toolkit/components/places/nsINavBookmarksService.idl
+++ b/toolkit/components/places/nsINavBookmarksService.idl
@@ -532,24 +532,17 @@ interface nsINavBookmarksService : nsISu
/**
/**
* Get the parent folder's id for an item.
*/
long long getFolderIdForItem(in long long aItemId);
/**
- * Retrieves the keyword for the given bookmark. Will be void string
- * (null in JS) if no such keyword is found.
- *
- * @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
* aObserver must implement nsISupportsWeakReference, and the bookmark
* service will keep a weak reference to the observer.
*/
void addObserver(in nsINavBookmarkObserver observer,
[optional] in boolean ownsWeak);
--- a/toolkit/components/places/nsINavHistoryService.idl
+++ b/toolkit/components/places/nsINavHistoryService.idl
@@ -995,18 +995,16 @@ interface nsINavHistoryQueryOptions : ns
const unsigned short SORT_BY_TITLE_ASCENDING = 1;
const unsigned short SORT_BY_TITLE_DESCENDING = 2;
const unsigned short SORT_BY_DATE_ASCENDING = 3;
const unsigned short SORT_BY_DATE_DESCENDING = 4;
const unsigned short SORT_BY_URI_ASCENDING = 5;
const unsigned short SORT_BY_URI_DESCENDING = 6;
const unsigned short SORT_BY_VISITCOUNT_ASCENDING = 7;
const unsigned short SORT_BY_VISITCOUNT_DESCENDING = 8;
- const unsigned short SORT_BY_KEYWORD_ASCENDING = 9;
- const unsigned short SORT_BY_KEYWORD_DESCENDING = 10;
const unsigned short SORT_BY_DATEADDED_ASCENDING = 11;
const unsigned short SORT_BY_DATEADDED_DESCENDING = 12;
const unsigned short SORT_BY_LASTMODIFIED_ASCENDING = 13;
const unsigned short SORT_BY_LASTMODIFIED_DESCENDING = 14;
const unsigned short SORT_BY_TAGS_ASCENDING = 17;
const unsigned short SORT_BY_TAGS_DESCENDING = 18;
const unsigned short SORT_BY_ANNOTATION_ASCENDING = 19;
const unsigned short SORT_BY_ANNOTATION_DESCENDING = 20;
--- a/toolkit/components/places/nsNavBookmarks.cpp
+++ b/toolkit/components/places/nsNavBookmarks.cpp
@@ -2245,54 +2245,16 @@ nsNavBookmarks::GetBookmarksForURI(nsIUR
}
return NS_OK;
}
NS_IMETHODIMP
-nsNavBookmarks::GetKeywordForBookmark(int64_t aBookmarkId, nsAString& aKeyword)
-{
- NS_ENSURE_ARG_MIN(aBookmarkId, 1);
- aKeyword.Truncate(0);
-
- // We can have multiple keywords for the same uri, here we'll just return the
- // last created one.
- nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(NS_LITERAL_CSTRING(
- "SELECT k.keyword "
- "FROM moz_bookmarks b "
- "JOIN moz_keywords k ON k.place_id = b.fk "
- "WHERE b.id = :item_id "
- "ORDER BY k.ROWID DESC "
- "LIMIT 1"
- ));
- NS_ENSURE_STATE(stmt);
- mozStorageStatementScoper scoper(stmt);
-
- nsresult rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("item_id"),
- aBookmarkId);
- NS_ENSURE_SUCCESS(rv, rv);
-
- bool hasMore;
- if (NS_SUCCEEDED(stmt->ExecuteStep(&hasMore)) && hasMore) {
- nsAutoString keyword;
- rv = stmt->GetString(0, keyword);
- NS_ENSURE_SUCCESS(rv, rv);
- aKeyword = keyword;
- return NS_OK;
- }
-
- aKeyword.SetIsVoid(true);
-
- return NS_OK;
-}
-
-
-NS_IMETHODIMP
nsNavBookmarks::RunInBatchMode(nsINavHistoryBatchCallback* aCallback,
nsISupports* aUserData) {
AUTO_PROFILER_LABEL("nsNavBookmarks::RunInBatchMode", OTHER);
NS_ENSURE_ARG(aCallback);
mBatching = true;
--- a/toolkit/components/places/nsNavHistoryResult.cpp
+++ b/toolkit/components/places/nsNavHistoryResult.cpp
@@ -737,20 +737,16 @@ nsNavHistoryContainerResultNode::GetSort
case nsINavHistoryQueryOptions::SORT_BY_URI_ASCENDING:
return &SortComparison_URILess;
case nsINavHistoryQueryOptions::SORT_BY_URI_DESCENDING:
return &SortComparison_URIGreater;
case nsINavHistoryQueryOptions::SORT_BY_VISITCOUNT_ASCENDING:
return &SortComparison_VisitCountLess;
case nsINavHistoryQueryOptions::SORT_BY_VISITCOUNT_DESCENDING:
return &SortComparison_VisitCountGreater;
- case nsINavHistoryQueryOptions::SORT_BY_KEYWORD_ASCENDING:
- return &SortComparison_KeywordLess;
- case nsINavHistoryQueryOptions::SORT_BY_KEYWORD_DESCENDING:
- return &SortComparison_KeywordGreater;
case nsINavHistoryQueryOptions::SORT_BY_ANNOTATION_ASCENDING:
return &SortComparison_AnnotationLess;
case nsINavHistoryQueryOptions::SORT_BY_ANNOTATION_DESCENDING:
return &SortComparison_AnnotationGreater;
case nsINavHistoryQueryOptions::SORT_BY_DATEADDED_ASCENDING:
return &SortComparison_DateAddedLess;
case nsINavHistoryQueryOptions::SORT_BY_DATEADDED_DESCENDING:
return &SortComparison_DateAddedGreater;
@@ -1026,53 +1022,16 @@ int32_t nsNavHistoryContainerResultNode:
return value;
}
int32_t nsNavHistoryContainerResultNode::SortComparison_URIGreater(
nsNavHistoryResultNode* a, nsNavHistoryResultNode* b, void* closure)
{
return -SortComparison_URILess(a, b, closure);
}
-
-int32_t nsNavHistoryContainerResultNode::SortComparison_KeywordLess(
- nsNavHistoryResultNode* a, nsNavHistoryResultNode* b, void* closure)
-{
- int32_t value = 0;
- if (a->mItemId != -1 || b->mItemId != -1) {
- // compare the keywords
- nsAutoString keywordA, keywordB;
- nsNavBookmarks* bookmarks = nsNavBookmarks::GetBookmarksService();
- NS_ENSURE_TRUE(bookmarks, 0);
-
- nsresult rv;
- if (a->mItemId != -1) {
- rv = bookmarks->GetKeywordForBookmark(a->mItemId, keywordA);
- NS_ENSURE_SUCCESS(rv, 0);
- }
- if (b->mItemId != -1) {
- rv = bookmarks->GetKeywordForBookmark(b->mItemId, keywordB);
- NS_ENSURE_SUCCESS(rv, 0);
- }
-
- value = SortComparison_StringLess(keywordA, keywordB);
- }
-
- // Fall back to title sorting.
- if (value == 0)
- value = SortComparison_TitleLess(a, b, closure);
-
- return value;
-}
-
-int32_t nsNavHistoryContainerResultNode::SortComparison_KeywordGreater(
- nsNavHistoryResultNode* a, nsNavHistoryResultNode* b, void* closure)
-{
- return -SortComparison_KeywordLess(a, b, closure);
-}
-
int32_t nsNavHistoryContainerResultNode::SortComparison_AnnotationLess(
nsNavHistoryResultNode* a, nsNavHistoryResultNode* b, void* closure)
{
nsAutoCString annoName(static_cast<char*>(closure));
NS_ENSURE_TRUE(!annoName.IsEmpty(), 0);
bool a_itemAnno = false;
bool b_itemAnno = false;
--- a/toolkit/components/places/nsNavHistoryResult.h
+++ b/toolkit/components/places/nsNavHistoryResult.h
@@ -533,22 +533,16 @@ public:
nsNavHistoryResultNode* b,
void* closure);
static int32_t SortComparison_VisitCountLess(nsNavHistoryResultNode* a,
nsNavHistoryResultNode* b,
void* closure);
static int32_t SortComparison_VisitCountGreater(nsNavHistoryResultNode* a,
nsNavHistoryResultNode* b,
void* closure);
- static int32_t SortComparison_KeywordLess(nsNavHistoryResultNode* a,
- nsNavHistoryResultNode* b,
- void* closure);
- static int32_t SortComparison_KeywordGreater(nsNavHistoryResultNode* a,
- nsNavHistoryResultNode* b,
- void* closure);
static int32_t SortComparison_AnnotationLess(nsNavHistoryResultNode* a,
nsNavHistoryResultNode* b,
void* closure);
static int32_t SortComparison_AnnotationGreater(nsNavHistoryResultNode* a,
nsNavHistoryResultNode* b,
void* closure);
static int32_t SortComparison_DateAddedLess(nsNavHistoryResultNode* a,
nsNavHistoryResultNode* b,
--- a/toolkit/components/places/tests/bookmarks/test_keywords.js
+++ b/toolkit/components/places/tests/bookmarks/test_keywords.js
@@ -7,34 +7,27 @@ ChromeUtils.defineModuleGetter(this, "Pr
const URI1 = "http://test1.mozilla.org/";
const URI2 = "http://test2.mozilla.org/";
const URI3 = "http://test3.mozilla.org/";
async function check_keyword(aURI, aKeyword) {
if (aKeyword)
aKeyword = aKeyword.toLowerCase();
- let bms = [];
- await PlacesUtils.bookmarks.fetch({ url: aURI }, bm => bms.push(bm));
- for (let bm of bms) {
- let itemId = await PlacesUtils.promiseItemId(bm.guid);
- let keyword = PlacesUtils.bookmarks.getKeywordForBookmark(itemId);
- if (keyword && !aKeyword) {
- throw (`${aURI} should not have a keyword, found keyword "${keyword}"`);
- } else if (aKeyword && keyword == aKeyword) {
- Assert.equal(keyword, aKeyword);
- }
- }
-
if (aKeyword) {
let uri = await PlacesUtils.keywords.fetch(aKeyword);
Assert.equal(uri.url, aURI);
// Check case insensitivity.
uri = await PlacesUtils.keywords.fetch(aKeyword.toUpperCase());
Assert.equal(uri.url, aURI);
+ } else {
+ let entry = await PlacesUtils.keywords.fetch({ url: aURI });
+ if (entry) {
+ throw (`${aURI.spec} should not have a keyword`);
+ }
}
}
async function check_orphans() {
let db = await PlacesUtils.promiseDBConnection();
let rows = await db.executeCached(
`SELECT id FROM moz_keywords k
WHERE NOT EXISTS (SELECT 1 FROM moz_places WHERE id = k.place_id)
@@ -70,20 +63,16 @@ function expectNotifications() {
return target[name];
}
});
PlacesUtils.bookmarks.addObserver(observer);
return observer;
}
add_task(function test_invalid_input() {
- Assert.throws(() => PlacesUtils.bookmarks.getKeywordForBookmark(null),
- /NS_ERROR_ILLEGAL_VALUE/);
- Assert.throws(() => PlacesUtils.bookmarks.getKeywordForBookmark(0),
- /NS_ERROR_ILLEGAL_VALUE/);
});
add_task(async function test_addBookmarkAndKeyword() {
let timerPrecision = Preferences.get("privacy.reduceTimerPrecision");
Preferences.set("privacy.reduceTimerPrecision", false);
registerCleanupFunction(function() {
Preferences.set("privacy.reduceTimerPrecision", timerPrecision);
--- a/toolkit/components/places/tests/queries/test_sorting.js
+++ b/toolkit/components/places/tests/queries/test_sorting.js
@@ -13,33 +13,30 @@ tests.push({
info("Sorting test 1: SORT BY NONE");
this._unsortedData = [
{ isBookmark: true,
uri: "http://example.com/b",
parentGuid: PlacesUtils.bookmarks.toolbarGuid,
index: PlacesUtils.bookmarks.DEFAULT_INDEX,
title: "y",
- keyword: "b",
isInQuery: true },
{ isBookmark: true,
uri: "http://example.com/a",
parentGuid: PlacesUtils.bookmarks.toolbarGuid,
index: PlacesUtils.bookmarks.DEFAULT_INDEX,
title: "z",
- keyword: "a",
isInQuery: true },
{ isBookmark: true,
uri: "http://example.com/c",
parentGuid: PlacesUtils.bookmarks.toolbarGuid,
index: PlacesUtils.bookmarks.DEFAULT_INDEX,
title: "x",
- keyword: "c",
isInQuery: true },
];
this._sortedData = this._unsortedData;
// This function in head_queries.js creates our database with the above data
await task_populateDB(this._unsortedData);
},
@@ -437,113 +434,16 @@ tests.push({
check_reverse() {
this._sortingMode = Ci.nsINavHistoryQueryOptions.SORT_BY_VISITCOUNT_DESCENDING;
this._sortedData.reverse();
this.check();
}
});
tests.push({
- _sortingMode: Ci.nsINavHistoryQueryOptions.SORT_BY_KEYWORD_ASCENDING,
-
- async setup() {
- info("Sorting test 6: SORT BY KEYWORD");
-
- this._unsortedData = [
- { isBookmark: true,
- uri: "http://example.com/a",
- parentGuid: PlacesUtils.bookmarks.toolbarGuid,
- index: PlacesUtils.bookmarks.DEFAULT_INDEX,
- title: "z",
- keyword: "a",
- isInQuery: true },
-
- { isBookmark: true,
- uri: "http://example.com/c",
- parentGuid: PlacesUtils.bookmarks.toolbarGuid,
- index: PlacesUtils.bookmarks.DEFAULT_INDEX,
- title: "x",
- keyword: "c",
- isInQuery: true },
-
- { isBookmark: true,
- uri: "http://example.com/b1",
- parentGuid: PlacesUtils.bookmarks.toolbarGuid,
- index: PlacesUtils.bookmarks.DEFAULT_INDEX,
- title: "y9",
- keyword: "b",
- isInQuery: true },
-
- // without a keyword, should fall back to title
- { isBookmark: true,
- uri: "http://example.com/null2",
- parentGuid: PlacesUtils.bookmarks.toolbarGuid,
- index: PlacesUtils.bookmarks.DEFAULT_INDEX,
- title: "null8",
- keyword: null,
- isInQuery: true },
-
- // without a keyword, should fall back to title
- { isBookmark: true,
- uri: "http://example.com/null1",
- parentGuid: PlacesUtils.bookmarks.toolbarGuid,
- index: PlacesUtils.bookmarks.DEFAULT_INDEX,
- title: "null9",
- keyword: null,
- isInQuery: true },
-
- // if keywords are equal, should fall back to title
- { isBookmark: true,
- uri: "http://example.com/b1",
- parentGuid: PlacesUtils.bookmarks.toolbarGuid,
- index: PlacesUtils.bookmarks.DEFAULT_INDEX,
- title: "y8",
- keyword: "b",
- isInQuery: true },
- ];
-
- this._sortedData = [
- this._unsortedData[3],
- this._unsortedData[4],
- this._unsortedData[0],
- this._unsortedData[5],
- this._unsortedData[2],
- this._unsortedData[1],
- ];
-
- // This function in head_queries.js creates our database with the above data
- await task_populateDB(this._unsortedData);
- },
-
- check() {
- // Query
- var query = PlacesUtils.history.getNewQuery();
- query.setFolders([PlacesUtils.bookmarks.toolbarFolder], 1);
- query.onlyBookmarked = true;
-
- // query options
- var options = PlacesUtils.history.getNewQueryOptions();
- options.sortingMode = this._sortingMode;
-
- // Results - this gets the result set and opens it for reading and modification.
- var result = PlacesUtils.history.executeQuery(query, options);
- var root = result.root;
- root.containerOpen = true;
- compareArrayToResult(this._sortedData, root);
- root.containerOpen = false;
- },
-
- check_reverse() {
- this._sortingMode = Ci.nsINavHistoryQueryOptions.SORT_BY_KEYWORD_DESCENDING;
- this._sortedData.reverse();
- this.check();
- }
-});
-
-tests.push({
_sortingMode: Ci.nsINavHistoryQueryOptions.SORT_BY_DATEADDED_ASCENDING,
async setup() {
info("Sorting test 7: SORT BY DATEADDED");
var timeInMicroseconds = Date.now() * 1000;
this._unsortedData = [
{ isBookmark: true,
--- a/toolkit/components/places/tests/unit/test_result_sort.js
+++ b/toolkit/components/places/tests/unit/test_result_sort.js
@@ -91,24 +91,16 @@ add_task(async function test() {
});
checkOrder(guid3, guid1, guid2);
await PlacesUtils.bookmarks.update({
guid: guid1,
url: uri1,
});
checkOrder(guid1, guid3, guid2);
- // keyword sort
- info("Sort by keyword asc");
- result.sortingMode = NHQO.SORT_BY_KEYWORD_ASCENDING;
- checkOrder(guid3, guid2, guid1); // no keywords set - falling back to title sort
- await PlacesUtils.keywords.insert({ url: uri1, keyword: "a" });
- await PlacesUtils.keywords.insert({ url: uri2, keyword: "z" });
- checkOrder(guid3, guid1, guid2);
-
// XXXtodo: test history sortings (visit count, visit date)
// XXXtodo: test different item types once folderId and bookmarkId are merged.
// XXXtodo: test sortingAnnotation functionality with non-bookmark nodes
info("Sort by annotation desc");
let ids = await PlacesUtils.promiseManyItemIds([guid1, guid3]);
PlacesUtils.annotations.setItemAnnotation(ids.get(guid1), "testAnno", "a", 0, 0);
PlacesUtils.annotations.setItemAnnotation(ids.get(guid3), "testAnno", "b", 0, 0);