Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-11-26 06:11:37 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1462135 - Replace PlacesUtils.getCharsetForURI with PU.history.fetch with an option to fetch annotations. r=mak

Browse Source 
This also avoids us doing main thread sync io.

MozReview-Commit-ID: KR0p7eeu1sj

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

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Mark Banner 2018-07-24 16:41:22 +00:00
parent 15993592c1
commit 8de4344378
13 changed files with 139 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
browser/components/places/tests/browser/browser_bookmarkProperties_addKeywordForThisSearch.js
View File

@ -42,8 +42,8 @@ add_task(async function() {
Assert.equal(entry.postData, "accenti%3D%E0%E8%EC%F2%F9&search%3D%25s", "POST data is correct");
info("Check the charset has been saved");
let charset = await PlacesUtils.getCharsetForURI(NetUtil.newURI(TEST_URL));
Assert.equal(charset, "windows-1252", "charset is correct");
let pageInfo = await PlacesUtils.history.fetch(TEST_URL, {includeAnnotations: true});
Assert.equal(pageInfo.annotations.get(PlacesUtils.CHARSET_ANNO), "windows-1252", "charset is correct");
// Now check getShortcutOrURI.
let data = await getShortcutOrURIAndPostData("kw test");

24
toolkit/components/places/History.jsm
View File

@ -37,6 +37,8 @@
* passed as argument to a function of this API will be ignored.
* - visits: (Array<VisitInfo>)
* All the visits for this page, if any.
* - annotations: (Map)
* A map containing key/value pairs of the annotations for this page, if any.
*
* See the documentation of individual methods to find out which properties
* are required for `PageInfo` arguments or returned for `PageInfo` results.
@ -123,6 +125,8 @@ var History = Object.freeze({
* By default, `visits` is undefined inside the returned `PageInfo`.
* - `includeMeta` (boolean) set this to true to fetch page meta fields,
* i.e. `description` and `preview_image_url`.
* - `includeAnnotations` (boolean) set this to true to fetch any
* annotations that are associated with the page.
*
* @return (Promise)
* A promise resolved once the operation is complete.
@ -156,6 +160,11 @@ var History = Object.freeze({
throw new TypeError("includeMeta should be a boolean if exists");
}
let hasIncludeAnnotations = "includeAnnotations" in options;
if (hasIncludeAnnotations && typeof options.includeAnnotations !== "boolean") {
throw new TypeError("includeAnnotations should be a boolean if exists");
}
return PlacesUtils.promiseDBConnection()
.then(db => fetch(db, guidOrURI, options));
},
@ -1001,6 +1010,7 @@ var fetch = async function(db, guidOrURL, options) {
${whereClauseFragment}
${visitOrderFragment}`;
let pageInfo = null;
let placeId = null;
await db.executeCached(
query,
params,
@ -1013,6 +1023,7 @@ var fetch = async function(db, guidOrURL, options) {
frecency: row.getResultByName("frecency"),
title: row.getResultByName("title") || ""
};
placeId = row.getResultByName("id");
}
if (options.includeMeta) {
pageInfo.description = row.getResultByName("description") || "";
@ -1031,6 +1042,19 @@ var fetch = async function(db, guidOrURL, options) {
pageInfo.visits.push({ date, transition });
}
});
// Only try to get annotations if requested, and if there's an actual page found.
if (pageInfo && options.includeAnnotations) {
let rows = await db.executeCached(`
SELECT n.name, a.content FROM moz_anno_attributes n
JOIN moz_annos a ON n.id = a.anno_attribute_id
WHERE a.place_id = :placeId
`, {placeId});
pageInfo.annotations = new Map(rows.map(
row => [row.getResultByName("name"), row.getResultByName("content")]
));
}
return pageInfo;
};

20
toolkit/components/places/PlacesUtils.jsm
View File

@ -1482,26 +1482,6 @@ var PlacesUtils = {
});
},
/**
* Gets the last saved character-set for a URI.
*
* @param aURI nsIURI
* @return {Promise}
* @resolve a character-set or null.
*/
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);
});
});
},
/**
* Gets favicon data for a given page url.
*

53
toolkit/components/places/tests/history/test_fetch.js
View File

@ -106,6 +106,55 @@ add_task(async function test_fetch_page_meta_info() {
Assert.ok(!("previewImageURL" in pageInfo), "fetch should not return a previewImageURL if includeMeta is false");
});
add_task(async function test_fetch_annotations() {
await PlacesUtils.history.clear();
const TEST_URI = "http://mozilla.com/test_fetch_page_meta_info";
await PlacesTestUtils.addVisits(TEST_URI);
Assert.ok(page_in_database(TEST_URI));
let includeAnnotations = true;
let pageInfo = await PlacesUtils.history.fetch(TEST_URI, {includeAnnotations});
Assert.equal(pageInfo.annotations.size, 0,
"fetch should return an empty annotation map");
PlacesUtils.annotations.setPageAnnotation(
Services.io.newURI(TEST_URI),
"test/annotation",
"testContent",
0,
PlacesUtils.annotations.EXPIRE_NEVER
);
pageInfo = await PlacesUtils.history.fetch(TEST_URI, {includeAnnotations});
Assert.equal(pageInfo.annotations.size, 1,
"fetch should have only one annotation");
Assert.equal(pageInfo.annotations.get("test/annotation"), "testContent",
"fetch should return the expected annotation");
PlacesUtils.annotations.setPageAnnotation(
Services.io.newURI(TEST_URI),
"test/annotation2",
123,
0,
PlacesUtils.annotations.EXPIRE_NEVER
);
pageInfo = await PlacesUtils.history.fetch(TEST_URI, {includeAnnotations});
Assert.equal(pageInfo.annotations.size, 2,
"fetch should have returned two annotations");
Assert.equal(pageInfo.annotations.get("test/annotation"), "testContent",
"fetch should still have the first annotation");
Assert.equal(pageInfo.annotations.get("test/annotation2"), 123,
"fetch should have the second annotation");
includeAnnotations = false;
pageInfo = await PlacesUtils.history.fetch(TEST_URI, {includeAnnotations});
Assert.ok(!("annotations" in pageInfo),
"fetch should not return annotations if includeAnnotations is false");
});
add_task(async function test_fetch_nonexistent() {
await PlacesUtils.history.clear();
await PlacesUtils.bookmarks.eraseEverything();
@ -140,4 +189,8 @@ add_task(async function test_error_cases() {
() => PlacesUtils.history.fetch("http://valid.uri.come", {includeMeta: "not a boolean"}),
/TypeError: includeMeta should be a/
);
Assert.throws(
() => PlacesUtils.history.fetch("http://valid.url.com", {includeAnnotations: "not a boolean"}),
/TypeError: includeAnnotations should be a/
);
});

12
toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js
View File

@ -109,7 +109,8 @@ AutoCompleteInput.prototype = {
*
* @param {Object} match The expected match for the result, in the following form:
* {
* uri: {nsIURI} The expected uri.
* uri: {String|nsIURI} The expected uri. Note: nsIURI should be considered
* deprecated.
* title: {String} The title of the entry.
* tags: {String} The tags for the entry.
* style: {Array} The style of the entry.
@ -120,6 +121,9 @@ AutoCompleteInput.prototype = {
*/
async function _check_autocomplete_matches(match, result) {
let { uri, tags, style } = match;
if (uri instanceof Ci.nsIURI) {
uri = uri.spec;
}
let title = match.comment || match.title;
if (tags)
@ -130,7 +134,7 @@ async function _check_autocomplete_matches(match, result) {
style = ["favicon"];
let actual = { value: result.value, comment: result.comment };
let expected = { value: match.value || uri.spec, comment: title };
let expected = { value: match.value || uri, comment: title };
info(`Checking match: ` +
`actual=${JSON.stringify(actual)} ... ` +
`expected=${JSON.stringify(expected)}`);
@ -141,7 +145,7 @@ async function _check_autocomplete_matches(match, result) {
let actualStyle = result.style.split(/\s+/).sort();
if (style)
Assert.equal(actualStyle.toString(), style.toString(), "Match should have expected style");
if (uri && uri.spec.startsWith("moz-action:")) {
if (uri && uri.startsWith("moz-action:")) {
Assert.ok(actualStyle.includes("action"), "moz-action results should always have 'action' in their style");
}
@ -298,7 +302,7 @@ var addBookmark = async function(aBookmarkObj) {
if (aBookmarkObj.keyword) {
await PlacesUtils.keywords.insert({ keyword: aBookmarkObj.keyword,
url: aBookmarkObj.uri.spec,
url: aBookmarkObj.uri.spec ? aBookmarkObj.uri.spec : aBookmarkObj.uri,
postData: aBookmarkObj.postData
});
}

28
toolkit/components/places/tests/unifiedcomplete/test_keyword_search_actions.js
View File

@ -13,14 +13,16 @@
*/
add_task(async function test_keyword_search() {
let uri1 = NetUtil.newURI("http://abc/?search=%s");
let uri2 = NetUtil.newURI("http://abc/?search=ThisPageIsInHistory");
let uri3 = NetUtil.newURI("http://abc/?search=%s&raw=%S");
let uri4 = NetUtil.newURI("http://abc/?search=%s&raw=%S&mozcharset=ISO-8859-1");
let uri5 = NetUtil.newURI("http://def/?search=%s");
let uri1 = "http://abc/?search=%s";
let uri2 = "http://abc/?search=ThisPageIsInHistory";
let uri3 = "http://abc/?search=%s&raw=%S";
let uri4 = "http://abc/?search=%s&raw=%S&mozcharset=ISO-8859-1";
let uri5 = "http://def/?search=%s";
let uri6 = "http://ghi/?search=%s&raw=%S";
await PlacesTestUtils.addVisits([{ uri: uri1 },
{ uri: uri2 },
{ uri: uri3 }]);
{ uri: uri3 },
{ uri: uri6 }]);
await addBookmark({ uri: uri1, title: "Keyword", keyword: "key"});
await addBookmark({ uri: uri1, title: "Post", keyword: "post", postData: "post_search=%s"});
await addBookmark({ uri: uri3, title: "Encoded", keyword: "encoded"});
@ -28,6 +30,10 @@ add_task(async function test_keyword_search() {
await addBookmark({ uri: uri2, title: "Noparam", keyword: "noparam"});
await addBookmark({ uri: uri2, title: "Noparam-Post", keyword: "post_noparam", postData: "noparam=1"});
await addBookmark({ uri: uri5, title: "Keyword", keyword: "key2"});
await addBookmark({ uri: uri6, title: "Charset-history", keyword: "charset_history"});
PlacesUtils.annotations.setPageAnnotation(Services.io.newURI(uri6),
PlacesUtils.CHARSET_ANNO, "ISO-8859-1", 0, PlacesUtils.annotations.EXPIRE_NEVER);
info("Plain keyword query");
await check_autocomplete({
@ -40,7 +46,7 @@ add_task(async function test_keyword_search() {
info("Plain keyword UC");
await check_autocomplete({
search: "key TERM",
matches: [ { uri: NetUtil.newURI("http://abc/?search=TERM"),
matches: [ { uri: "http://abc/?search=TERM",
title: "abc", style: ["keyword", "heuristic"] } ]
});
@ -150,6 +156,14 @@ add_task(async function test_keyword_search() {
title: "abc", style: [ "action", "keyword", "heuristic" ] } ]
});
info("escaping with ISO-8859-1 charset annotated in history");
await check_autocomplete({
search: "charset_history foé",
searchParam: "enable-actions",
matches: [ { uri: makeActionURI("keyword", {url: "http://ghi/?search=fo%E9&raw=foé", input: "charset_history foé" }),
title: "ghi", style: [ "action", "keyword", "heuristic" ] } ]
});
info("Bug 359809: escaping +, / and @ with default UTF-8 charset");
await check_autocomplete({
search: "encoded +/@",

26
toolkit/components/places/tests/unit/test_317472.js
View File

@ -5,7 +5,7 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
const charset = "UTF-8";
const CHARSET_ANNO = "URIProperties/characterSet";
const CHARSET_ANNO = PlacesUtils.CHARSET_ANNO;
const TEST_URI = uri("http://foo.com");
const TEST_BOOKMARKED_URI = uri("http://bar.com");
@ -35,16 +35,18 @@ add_task(async function test_execute() {
// check that we have created a page annotation
Assert.equal(PlacesUtils.annotations.getPageAnnotation(TEST_URI, CHARSET_ANNO), charset);
// get charset from not-bookmarked page
Assert.equal((await PlacesUtils.getCharsetForURI(TEST_URI)), charset);
let pageInfo = await PlacesUtils.history.fetch(TEST_URI, {includeAnnotations: true});
Assert.equal(pageInfo.annotations.get(PlacesUtils.CHARSET_ANNO), charset,
"Should return correct charset for a not-bookmarked page");
// get charset from bookmarked page
Assert.equal((await PlacesUtils.getCharsetForURI(TEST_BOOKMARKED_URI)), charset);
pageInfo = await PlacesUtils.history.fetch(TEST_BOOKMARKED_URI, {includeAnnotations: true});
Assert.equal(pageInfo.annotations.get(PlacesUtils.CHARSET_ANNO), charset,
"Should return correct charset for a bookmarked page");
await PlacesUtils.history.clear();
// ensure that charset has gone for not-bookmarked page
Assert.notEqual((await PlacesUtils.getCharsetForURI(TEST_URI)), charset);
pageInfo = await PlacesUtils.history.fetch(TEST_URI, {includeAnnotations: true});
Assert.ok(!pageInfo, "Should not return pageInfo for a page after history cleared");
// check that page annotation has been removed
try {
@ -52,10 +54,12 @@ add_task(async function test_execute() {
do_throw("Charset page annotation has not been removed correctly");
} catch (e) {}
// ensure that charset still exists for bookmarked page
Assert.equal((await PlacesUtils.getCharsetForURI(TEST_BOOKMARKED_URI)), charset);
pageInfo = await PlacesUtils.history.fetch(TEST_BOOKMARKED_URI, {includeAnnotations: true});
Assert.equal(pageInfo.annotations.get(PlacesUtils.CHARSET_ANNO), charset,
"Charset should still be set for a bookmarked page after history clear");
// remove charset from bookmark and check that has gone
await PlacesUtils.setCharsetForURI(TEST_BOOKMARKED_URI, "");
Assert.notEqual((await PlacesUtils.getCharsetForURI(TEST_BOOKMARKED_URI)), charset);
pageInfo = await PlacesUtils.history.fetch(TEST_BOOKMARKED_URI, {includeAnnotations: true});
Assert.notEqual(pageInfo.annotations.get(PlacesUtils.CHARSET_ANNO), charset,
"Should not have a charset after it has been removed from the page");
});

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

@ -104,8 +104,9 @@ async function testMenuBookmarks() {
Assert.equal("test", entry.keyword);
Assert.equal("hidden1%3Dbar&text1%3D%25s", entry.postData);
Assert.equal("ISO-8859-1",
(await PlacesUtils.getCharsetForURI(NetUtil.newURI(bookmarkNode.uri))));
let pageInfo = await PlacesUtils.history.fetch(bookmarkNode.uri, {includeAnnotations: true});
Assert.equal(pageInfo.annotations.get(PlacesUtils.CHARSET_ANNO), "ISO-8859-1",
"Should have the correct charset");
folderNode.containerOpen = false;
root.containerOpen = false;

4
toolkit/components/places/tests/unit/test_bookmarks_html.js
View File

@ -314,8 +314,8 @@ function checkItem(aExpected, aNode) {
break;
}
case "charset":
let testURI = NetUtil.newURI(aNode.uri);
Assert.equal((await PlacesUtils.getCharsetForURI(testURI)), aExpected.charset);
let pageInfo = await PlacesUtils.history.fetch(aNode.uri, {includeAnnotations: true});
Assert.equal(pageInfo.annotations.get(PlacesUtils.CHARSET_ANNO), aExpected.charset);
break;
case "feedUrl":
let livemark = await PlacesUtils.livemarks.getLivemark({ id });

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

@ -77,8 +77,9 @@ var database_check = async function() {
Assert.equal(bookmarkNode.dateAdded, 1177375336000000);
Assert.equal(bookmarkNode.lastModified, 1177375423000000);
Assert.equal((await PlacesUtils.getCharsetForURI(NetUtil.newURI(bookmarkNode.uri))),
"ISO-8859-1");
let pageInfo = await PlacesUtils.history.fetch(bookmarkNode.uri, {includeAnnotations: true});
Assert.equal(pageInfo.annotations.get(PlacesUtils.CHARSET_ANNO), "ISO-8859-1",
"Should have the correct charset");
// clean up
folderNode.containerOpen = false;

4
toolkit/components/places/tests/unit/test_bookmarks_json.js
View File

@ -213,8 +213,8 @@ async function checkItem(aExpected, aNode) {
break;
}
case "charset":
let testURI = Services.io.newURI(aNode.uri);
Assert.equal((await PlacesUtils.getCharsetForURI(testURI)), aExpected.charset);
let pageInfo = await PlacesUtils.history.fetch(aNode.uri, {includeAnnotations: true});
Assert.equal(pageInfo.annotations.get(PlacesUtils.CHARSET_ANNO), aExpected.charset);
break;
case "feedUrl":
let livemark = await PlacesUtils.livemarks.getLivemark({ id });

4
toolkit/components/places/tests/unit/test_promiseBookmarksTree.js
View File

@ -124,8 +124,8 @@ async function compareToNode(aItem, aNode, aIsRootItem, aExcludedGuids = []) {
check_unset(...FOLDER_ONLY_PROPS);
let itemURI = uri(aNode.uri);
let expectedCharset = await PlacesUtils.getCharsetForURI(itemURI);
let pageInfo = await PlacesUtils.history.fetch(aNode.uri, {includeAnnotations: true});
let expectedCharset = pageInfo.annotations.get(PlacesUtils.CHARSET_ANNO);
if (expectedCharset)
Assert.equal(aItem.charset, expectedCharset);
else

5
toolkit/modules/BrowserUtils.jsm
View File

@ -606,7 +606,10 @@ var BrowserUtils = {
// Try to fetch a charset from History.
try {
// Will return an empty string if character-set is not found.
charset = await PlacesUtils.getCharsetForURI(this.makeURI(url));
let pageInfo = await PlacesUtils.history.fetch(url, {includeAnnotations: true});
if (pageInfo && pageInfo.annotations.has(PlacesUtils.CHARSET_ANNO)) {
charset = pageInfo.annotations.get(PlacesUtils.CHARSET_ANNO);
}
} catch (ex) {
// makeURI() throws if url is invalid.
Cu.reportError(ex);