Bug 1462135 - Replace PlacesUtils.getCharsetForURI with PU.history.fetch with an option to fetch annotations. r=mak
authorMark Banner <standard8@mozilla.com>
Tue, 24 Jul 2018 16:41:22 +0000
changeset 428041 da4ff0f7bff51632758a9081d4ad3b6fb1be0823
parent 428040 70d57bb33a7fe978d5f1ff120fb066059fe3c341
child 428042 194afde62e6959c718bf9138135b559914d3442d
push id66784
push usermbanner@mozilla.com
push dateTue, 24 Jul 2018 16:43:59 +0000
treeherderautoland@da4ff0f7bff5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1462135
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1462135 - Replace PlacesUtils.getCharsetForURI with PU.history.fetch with an option to fetch annotations. r=mak This also avoids us doing main thread sync io. MozReview-Commit-ID: KR0p7eeu1sj Differential Revision: https://phabricator.services.mozilla.com/D2117
browser/components/places/tests/browser/browser_bookmarkProperties_addKeywordForThisSearch.js
toolkit/components/places/History.jsm
toolkit/components/places/PlacesUtils.jsm
toolkit/components/places/tests/history/test_fetch.js
toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js
toolkit/components/places/tests/unifiedcomplete/test_keyword_search_actions.js
toolkit/components/places/tests/unit/test_317472.js
toolkit/components/places/tests/unit/test_384370.js
toolkit/components/places/tests/unit/test_bookmarks_html.js
toolkit/components/places/tests/unit/test_bookmarks_html_corrupt.js
toolkit/components/places/tests/unit/test_bookmarks_json.js
toolkit/components/places/tests/unit/test_promiseBookmarksTree.js
toolkit/modules/BrowserUtils.jsm
--- a/browser/components/places/tests/browser/browser_bookmarkProperties_addKeywordForThisSearch.js
+++ b/browser/components/places/tests/browser/browser_bookmarkProperties_addKeywordForThisSearch.js
@@ -37,18 +37,18 @@ add_task(async function() {
         entry = await PlacesUtils.keywords.fetch("kw");
         return !!entry;
       }, "Unable to find the expected keyword");
       Assert.equal(entry.keyword, "kw", "keyword is correct");
       Assert.equal(entry.url.href, TEST_URL, "URL is correct");
       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");
       Assert.equal(getPostDataString(data.postData), "accenti=\u00E0\u00E8\u00EC\u00F2\u00F9&search=test", "getShortcutOrURI POST data is correct");
       Assert.equal(data.url, TEST_URL, "getShortcutOrURI URL is correct");
     }, closeHandler);
   });
 });
--- a/toolkit/components/places/History.jsm
+++ b/toolkit/components/places/History.jsm
@@ -32,16 +32,18 @@
  * - frecency: (number)
  *     The frecency of the page, if any.
  *     See https://developer.mozilla.org/en-US/docs/Mozilla/Tech/Places/Frecency_algorithm
  *     Note that this property may not be used to change the actualy frecency
  *     score of a page, only to retrieve it. In other words, any `frecency` field
  *     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.
  *
  * A `VisitInfo` object is any object that contains A SUBSET of the following
  * properties:
  * - date: (Date)
  *     The time the visit occurred.
@@ -118,16 +120,18 @@ var History = Object.freeze({
    *      Either the full URI of the page or the GUID of the page.
    * @param [optional] options (object)
    *      An optional object whose properties describe options:
    *        - `includeVisits` (boolean) set this to true if `visits` in the
    *           PageInfo needs to contain VisitInfo in a reverse chronological order.
    *           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.
    * @resolves (PageInfo | null) If the page could be found, the information
    *      on that page.
    * @note the VisitInfo objects returned while fetching visits do not
    *       contain the property `referrer`.
    *       TODO: Add `referrer` to VisitInfo. See Bug #1365913.
@@ -151,16 +155,21 @@ var History = Object.freeze({
       throw new TypeError("includeVisits should be a boolean if exists");
     }
 
     let hasIncludeMeta = "includeMeta" in options;
     if (hasIncludeMeta && typeof options.includeMeta !== "boolean") {
       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));
   },
 
   /**
    * Adds a number of visits for a single page.
    *
    * Any change may be observed through nsINavHistoryObserver
@@ -996,28 +1005,30 @@ var fetch = async function(db, guidOrURL
   }
 
   let query = `SELECT h.id, guid, url, title, frecency
                ${pageMetaSelectionFragment} ${visitSelectionFragment}
                FROM moz_places h ${joinFragment}
                ${whereClauseFragment}
                ${visitOrderFragment}`;
   let pageInfo = null;
+  let placeId = null;
   await db.executeCached(
     query,
     params,
     row => {
       if (pageInfo === null) {
         // This means we're on the first row, so we need to get all the page info.
         pageInfo = {
           guid: row.getResultByName("guid"),
           url: new URL(row.getResultByName("url")),
           frecency: row.getResultByName("frecency"),
           title: row.getResultByName("title") || ""
         };
+        placeId = row.getResultByName("id");
       }
       if (options.includeMeta) {
         pageInfo.description = row.getResultByName("description") || "";
         let previewImageURL = row.getResultByName("preview_image_url");
         pageInfo.previewImageURL = previewImageURL ? new URL(previewImageURL) : null;
       }
       if (options.includeVisits) {
         // On every row (not just the first), we need to collect visit data.
@@ -1026,16 +1037,29 @@ var fetch = async function(db, guidOrURL
         }
         let date = PlacesUtils.toDate(row.getResultByName("visit_date"));
         let transition = row.getResultByName("visit_type");
 
         // TODO: Bug #1365913 add referrer URL to the `VisitInfo` data as well.
         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;
 };
 
 // Inner implementation of History.removeVisitsByFilter.
 var removeVisitsByFilter = async function(db, filter, onResult = null) {
   // 1. Determine visits that took place during the interval.  Note
   // that the database uses microseconds, while JS uses milliseconds,
   // so we need to *1000 one way and /1000 the other way.
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -1478,36 +1478,16 @@ var PlacesUtils = {
             aURI, PlacesUtils.CHARSET_ANNO);
         }
         resolve();
       });
     });
   },
 
   /**
-   * 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.
    *
    * @param aPageUrl url of the page to look favicon for.
    * @resolves to an object representing a favicon entry, having the following
    *           properties: { uri, dataLen, data, mimeType }
    * @rejects JavaScript exception if the given url has no associated favicon.
    */
   promiseFaviconData(aPageUrl) {
--- a/toolkit/components/places/tests/history/test_fetch.js
+++ b/toolkit/components/places/tests/history/test_fetch.js
@@ -101,16 +101,65 @@ add_task(async function test_fetch_page_
   Assert.equal(description, pageInfo.description, "fetch should return a description");
 
   includeMeta = false;
   pageInfo = await PlacesUtils.history.fetch(TEST_URI, {includeMeta});
   Assert.ok(!("description" in pageInfo), "fetch should not return a description if includeMeta is false");
   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();
 
   let uri = NetUtil.newURI("http://doesntexist.in.db");
   let pageInfo = await PlacesUtils.history.fetch(uri);
   Assert.equal(pageInfo, null);
 });
@@ -135,9 +184,13 @@ add_task(async function test_error_cases
   Assert.throws(
     () => PlacesUtils.history.fetch("http://valid.uri.come", {includeVisits: "not a boolean"}),
       /TypeError: includeVisits should be a/
   );
   Assert.throws(
     () => 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/
+  );
 });
--- a/toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js
+++ b/toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js
@@ -104,49 +104,53 @@ AutoCompleteInput.prototype = {
 };
 
 /**
  * A helper for check_autocomplete to check a specific match against data from
  * the controller.
  *
  * @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.
  * }
  * @param {Object} result The result to compare the result against with the same
  *                        properties as the match param.
  * @returns {boolean} Returns true if the result matches.
  */
 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)
     title += " \u2013 " + tags.sort().join(", ");
   if (style)
     style = style.sort();
   else
     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)}`);
   if (actual.value != expected.value || actual.comment != expected.comment) {
     return false;
   }
 
   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");
   }
 
   if (match.icon) {
     await compareFavicons(result.image, match.icon, "Match should have the expected icon");
   }
 
   return true;
@@ -293,17 +297,17 @@ var addBookmark = async function(aBookma
   await PlacesUtils.bookmarks.insert({
     parentGuid: PlacesUtils.bookmarks.unfiledGuid,
     title: aBookmarkObj.title || "A bookmark",
     url: aBookmarkObj.uri
   });
 
   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
                                       });
   }
 
   if (aBookmarkObj.tags) {
     PlacesUtils.tagging.tagURI(aBookmarkObj.uri, aBookmarkObj.tags);
   }
 };
--- a/toolkit/components/places/tests/unifiedcomplete/test_keyword_search_actions.js
+++ b/toolkit/components/places/tests/unifiedcomplete/test_keyword_search_actions.js
@@ -8,44 +8,50 @@
  * to %2B, non-ascii become escaped, and pages in history that match the
  * keyword uses the page's title.
  *
  * Also test for bug 249468 by making sure multiple keyword bookmarks with the
  * same keyword appear in the list.
  */
 
 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"});
   await addBookmark({ uri: uri4, title: "Charset", keyword: "charset"});
   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({
     search: "key term",
     searchParam: "enable-actions",
     matches: [ { uri: makeActionURI("keyword", {url: "http://abc/?search=term", input: "key term"}),
                  title: "abc", style: [ "action", "keyword", "heuristic" ] } ]
   });
 
   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"] } ]
   });
 
   info("Multi-word keyword query");
   await check_autocomplete({
     search: "key multi word",
     searchParam: "enable-actions",
     matches: [ { uri: makeActionURI("keyword", {url: "http://abc/?search=multi%20word", input: "key multi word"}),
@@ -145,16 +151,24 @@ add_task(async function test_keyword_sea
   info("escaping with forced ISO-8859-1 charset");
   await check_autocomplete({
     search: "charset foé",
     searchParam: "enable-actions",
     matches: [ { uri: makeActionURI("keyword", {url: "http://abc/?search=fo%E9&raw=foé", input: "charset foé" }),
                  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 +/@",
     searchParam: "enable-actions",
     matches: [ { uri: makeActionURI("keyword", {url: "http://abc/?search=%2B%2F%40&raw=+/@", input: "encoded +/@" }),
                  title: "abc", style: [ "action", "keyword", "heuristic" ] } ]
   });
 
--- a/toolkit/components/places/tests/unit/test_317472.js
+++ b/toolkit/components/places/tests/unit/test_317472.js
@@ -1,16 +1,16 @@
 /* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim:set ts=2 sw=2 sts=2 et: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * 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");
 
 add_task(async function test_execute() {
   // add pages to history
   await PlacesTestUtils.addVisits(TEST_URI);
   await PlacesTestUtils.addVisits(TEST_BOOKMARKED_URI);
@@ -30,32 +30,36 @@ add_task(async function test_execute() {
   // set charset on not-bookmarked page
   await PlacesUtils.setCharsetForURI(TEST_URI, charset);
   // set charset on bookmarked page
   await PlacesUtils.setCharsetForURI(TEST_BOOKMARKED_URI, charset);
 
   // 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 {
     PlacesUtils.annotations.getPageAnnotation(TEST_URI, CHARSET_ANNO);
     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");
 });
--- a/toolkit/components/places/tests/unit/test_384370.js
+++ b/toolkit/components/places/tests/unit/test_384370.js
@@ -99,18 +99,19 @@ async function testMenuBookmarks() {
   Assert.equal("http://test/post", bookmarkNode.uri);
   Assert.equal("test post keyword", bookmarkNode.title);
   Assert.equal(bookmarkNode.dateAdded, 1177375336000000);
 
   let entry = await PlacesUtils.keywords.fetch({ url: bookmarkNode.uri });
   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;
 }
 
 async function testToolbarBookmarks() {
   let root = PlacesUtils.getFolderContents(PlacesUtils.bookmarks.toolbarGuid).root;
 
--- a/toolkit/components/places/tests/unit/test_bookmarks_html.js
+++ b/toolkit/components/places/tests/unit/test_bookmarks_html.js
@@ -309,18 +309,18 @@ function checkItem(aExpected, aNode) {
           break;
         }
         case "postData": {
           let entry = await PlacesUtils.keywords.fetch({ url: aNode.uri });
           Assert.equal(entry.postData, aExpected.postData);
           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 });
           if (aExpected.url) {
             Assert.equal(livemark.siteURI.spec, aExpected.url);
           }
           Assert.equal(livemark.feedURI.spec, aExpected.feedUrl);
           break;
--- a/toolkit/components/places/tests/unit/test_bookmarks_html_corrupt.js
+++ b/toolkit/components/places/tests/unit/test_bookmarks_html_corrupt.js
@@ -72,18 +72,19 @@ var database_check = async function() {
 
   let entry = await PlacesUtils.keywords.fetch({ url: bookmarkNode.uri });
   Assert.equal("test", entry.keyword);
   Assert.equal("hidden1%3Dbar&text1%3D%25s", entry.postData);
 
   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;
   root.containerOpen = false;
 
   // BOOKMARKS TOOLBAR
   root = PlacesUtils.getFolderContents(PlacesUtils.bookmarks.toolbarGuid).root;
   Assert.equal(root.childCount, 3);
--- a/toolkit/components/places/tests/unit/test_bookmarks_json.js
+++ b/toolkit/components/places/tests/unit/test_bookmarks_json.js
@@ -208,18 +208,18 @@ async function checkItem(aExpected, aNod
         Assert.equal(bookmark.guid, aExpected.guid);
         break;
       case "postData": {
         let entry = await PlacesUtils.keywords.fetch({ url: aNode.uri });
         Assert.equal(entry.postData, aExpected.postData);
         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 });
         Assert.equal(livemark.siteURI.spec, aExpected.url);
         Assert.equal(livemark.feedURI.spec, aExpected.feedUrl);
         break;
       case "children":
         let folder = aNode.QueryInterface(Ci.nsINavHistoryContainerResultNode);
--- a/toolkit/components/places/tests/unit/test_promiseBookmarksTree.js
+++ b/toolkit/components/places/tests/unit/test_promiseBookmarksTree.js
@@ -119,18 +119,18 @@ async function compareToNode(aItem, aNod
           todo_check_true(false);
         }
       } else {
         check_unset(aItem.iconuri);
       }
 
       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
         check_unset("charset");
 
       let entry = await PlacesUtils.keywords.fetch({ url: aNode.uri });
       if (entry)
         Assert.equal(aItem.keyword, entry.keyword);
--- a/toolkit/modules/BrowserUtils.jsm
+++ b/toolkit/modules/BrowserUtils.jsm
@@ -601,17 +601,20 @@ var BrowserUtils = {
     const re = /^(.*)\&mozcharset=([a-zA-Z][_\-a-zA-Z0-9]+)\s*$/;
     let matches = url.match(re);
     if (matches) {
       [, url, charset] = matches;
     } else {
       // 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);
       }
     }
 
     // encodeURIComponent produces UTF-8, and cannot be used for other charsets.
     // escape() works in those cases, but it doesn't uri-encode +, @, and /.