Bug 728174 - Replace old synchronous favicons calls in the bookmarks HTML import. r=mak
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Thu, 12 Apr 2012 12:27:36 +0200
changeset 94844 27616988a4a1489faac211601cd6494b699b0a1b
parent 94843 308e3b7254b397b8837131ac5c6782f85d7a6636
child 94845 6a4d396a3c438d0279544a1e21a7cb7e1535678c
push id886
push userlsblakk@mozilla.com
push dateMon, 04 Jun 2012 19:57:52 +0000
treeherdermozilla-beta@bbd8d5efd6d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs728174
milestone14.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 728174 - Replace old synchronous favicons calls in the bookmarks HTML import. r=mak
browser/components/places/tests/unit/test_457441-import-export-corrupt-bookmarks-html.js
browser/components/places/tests/unit/test_bookmarks_html.js
toolkit/components/places/BookmarkHTMLUtils.jsm
toolkit/components/places/nsPlacesExportService.h
toolkit/components/places/tests/head_common.js
--- a/browser/components/places/tests/unit/test_457441-import-export-corrupt-bookmarks-html.js
+++ b/browser/components/places/tests/unit/test_457441-import-export-corrupt-bookmarks-html.js
@@ -57,17 +57,17 @@ var ies = Cc["@mozilla.org/browser/place
           getService(Ci.nsIPlacesImportExportService);
 Cu.import("resource://gre/modules/BookmarkHTMLUtils.jsm");
 
 const DESCRIPTION_ANNO = "bookmarkProperties/description";
 const LOAD_IN_SIDEBAR_ANNO = "bookmarkProperties/loadInSidebar";
 const POST_DATA_ANNO = "bookmarkProperties/POSTData";
 
 const TEST_FAVICON_PAGE_URL = "http://en-US.www.mozilla.com/en-US/firefox/central/";
-const TEST_FAVICON_DATA_URL = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAYAAAAf8/9hAAAABGdBTUEAAK/INwWK6QAAABl0RVh0U29mdHdhcmUAQWRvYmUgSW1hZ2VSZWFkeXHJZTwAAAHWSURBVHjaYvz//z8DJQAggJiQOe/fv2fv7Oz8rays/N+VkfG/iYnJfyD/1+rVq7ffu3dPFpsBAAHEAHIBCJ85c8bN2Nj4vwsDw/8zQLwKiO8CcRoQu0DxqlWrdsHUwzBAAIGJmTNnPgYa9j8UqhFElwPxf2MIDeIrKSn9FwSJoRkAEEAM0DD4DzMAyPi/G+QKY4hh5WAXGf8PDQ0FGwJ22d27CjADAAIIrLmjo+MXA9R2kAHvGBA2wwx6B8W7od6CeQcggKCmCEL8bgwxYCbUIGTDVkHDBia+CuotgACCueD3TDQN75D4xmAvCoK9ARMHBzAw0AECiBHkAlC0Mdy7x9ABNA3obAZXIAa6iKEcGlMVQHwWyjYuL2d4v2cPg8vZswx7gHyAAAK7AOif7SAbOqCmn4Ha3AHFsIDtgPq/vLz8P4MSkJ2W9h8ggBjevXvHDo4FQUQg/kdypqCg4H8lUIACnQ/SOBMYI8bAsAJFPcj1AAEEjwVQqLpAbXmH5BJjqI0gi9DTAAgDBBCcAVLkgmQ7yKCZxpCQxqUZhAECCJ4XgMl493ug21ZD+aDAXH0WLM4A9MZPXJkJIIAwTAR5pQMalaCABQUULttBGCCAGCnNzgABBgAMJ5THwGvJLAAAAABJRU5ErkJggg==";
+const TEST_FAVICON_DATA_SIZE = 580;
 
 function run_test() {
   do_test_pending();
 
   // avoid creating the places smart folder during tests
   ps.setIntPref("browser.places.smartBookmarksVersion", -1);
 
   // import bookmarks from corrupt file
@@ -79,18 +79,17 @@ function run_test() {
 
 function after_import(success) {
   if (!success) {
     do_throw("Couldn't import corrupt bookmarks file.");
   }
 
   // Check that every bookmark is correct
   // Corrupt bookmarks should not have been imported
-  database_check();
-  waitForAsyncUpdates(function() {
+  database_check(function () {
     // Create corruption in database
     var corruptItemId = bs.insertBookmark(bs.toolbarFolder,
                                           uri("http://test.mozilla.org"),
                                           bs.DEFAULT_INDEX, "We love belugas");
     var stmt = dbConn.createStatement("UPDATE moz_bookmarks SET fk = NULL WHERE id = :itemId");
     stmt.params.itemId = corruptItemId;
     stmt.execute();
     stmt.finalize();
@@ -107,32 +106,33 @@ function after_import(success) {
       ies.exportHTMLToFile(bookmarksFile);
     } catch(ex) { do_throw("couldn't export to bookmarks.exported.html: " + ex); }
 
     // Clear all bookmarks
     remove_all_bookmarks();
 
     // Import bookmarks
     try {
-      BookmarkHTMLUtils.importFromFile(bookmarksFile, true, before_database_check);
+    BookmarkHTMLUtils.importFromFile(bookmarksFile, true, before_database_check);
     } catch(ex) { do_throw("couldn't import the exported file: " + ex); }
   });
 }
 
 function before_database_check(success) {
-    // Check that every bookmark is correct
-    database_check();
-
-    waitForAsyncUpdates(do_test_finished);
+  // Check that every bookmark is correct
+  database_check(do_test_finished);
 }
 
 /*
  * Check for imported bookmarks correctness
+ *
+ * @param aCallback
+ *        Called when the checks are finished.
  */
-function database_check() {
+function database_check(aCallback) {
   // BOOKMARKS MENU
   var query = hs.getNewQuery();
   query.setFolders([bs.bookmarksMenuFolder], 1);
   var options = hs.getNewQueryOptions();
   options.queryType = Ci.nsINavHistoryQueryOptions.QUERY_TYPE_BOOKMARKS;
   var result = hs.executeQuery(query, options);
   var rootNode = result.root;
   rootNode.containerOpen = true;
@@ -221,12 +221,19 @@ function database_check() {
   query.setFolders([bs.unfiledBookmarksFolder], 1);
   result = hs.executeQuery(query, hs.getNewQueryOptions());
   var unfiledBookmarks = result.root;
   unfiledBookmarks.containerOpen = true;
   do_check_eq(unfiledBookmarks.childCount, 1);
   unfiledBookmarks.containerOpen = false;
 
   // favicons
-  var faviconURI = icos.getFaviconForPage(uri(TEST_FAVICON_PAGE_URL));
-  var dataURL = icos.getFaviconDataAsDataURL(faviconURI);
-  do_check_eq(TEST_FAVICON_DATA_URL, dataURL);
+  icos.getFaviconDataForPage(uri(TEST_FAVICON_PAGE_URL),
+    function DC_onComplete(aURI, aDataLen, aData, aMimeType) {
+      // aURI should never be null when aDataLen > 0.
+      do_check_neq(aURI, null);
+      // Favicon data is stored in the bookmarks file as a "data:" URI.  For
+      // simplicity, instead of converting the data we receive to a "data:" URI
+      // and comparing it, we just check the data size.
+      do_check_eq(TEST_FAVICON_DATA_SIZE, aDataLen);
+      aCallback();
+    });
 }
--- a/browser/components/places/tests/unit/test_bookmarks_html.js
+++ b/browser/components/places/tests/unit/test_bookmarks_html.js
@@ -178,21 +178,24 @@ add_test(function test_import_new()
     });
   } catch(ex) { do_throw("couldn't import the exported file: " + ex); }
 });
 
 add_test(function test_emptytitle_export()
 {
   // Test exporting and importing with an empty-titled bookmark.
   // 1. import bookmarks
-  // 1. create an empty-titled bookmark.
-  // 2. export to bookmarks.exported.html
-  // 3. empty bookmarks db
-  // 4. import bookmarks.exported.html
-  // 5. run the test-suite
+  // 2. create an empty-titled bookmark.
+  // 3. export to bookmarks.exported.html
+  // 4. empty bookmarks db
+  // 5. import bookmarks.exported.html
+  // 6. run the test-suite
+  // 7. remove the empty-titled bookmark
+  // 8. export to bookmarks.exported.html
+  // 9. empty bookmarks db and continue
 
   try {
     BookmarkHTMLUtils.importFromFile(gBookmarksFileNew, true, function(success) {
       if (success) {
         const NOTITLE_URL = "http://notitle.mozilla.org/";
         let id = PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
                                                       NetUtil.newURI(NOTITLE_URL),
                                                       PlacesUtils.bookmarks.DEFAULT_INDEX,
@@ -231,16 +234,93 @@ add_test(function test_emptytitle_export
         } catch(ex) { do_throw("couldn't import the exported file: " + ex); }
       } else {
         do_throw("couldn't import the exported file.");
       }
     });
   } catch(ex) { do_throw("couldn't import the exported file: " + ex); }
 });
 
+add_test(function test_import_chromefavicon()
+{
+  // Test exporting and importing with a bookmark pointing to a chrome favicon.
+  // 1. import bookmarks
+  // 2. create a bookmark pointing to a chrome favicon.
+  // 3. export to bookmarks.exported.html
+  // 4. empty bookmarks db
+  // 5. import bookmarks.exported.html
+  // 6. run the test-suite
+  // 7. remove the bookmark pointing to a chrome favicon.
+  // 8. export to bookmarks.exported.html
+  // 9. empty bookmarks db and continue
+
+  const PAGE_URI = NetUtil.newURI("http://example.com/chromefavicon_page");
+  const CHROME_FAVICON_URI = NetUtil.newURI("chrome://global/skin/icons/information-16.png");
+  const CHROME_FAVICON_URI_2 = NetUtil.newURI("chrome://global/skin/icons/error-16.png");
+
+  try {
+    BookmarkHTMLUtils.importFromFile(gBookmarksFileNew, true, function(success) {
+      if (!success) {
+        do_throw("couldn't import the exported file.");
+      }
+      let id = PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
+                                                    PAGE_URI,
+                                                    PlacesUtils.bookmarks.DEFAULT_INDEX,
+                                                    "Test");
+
+      PlacesUtils.favicons.setAndFetchFaviconForPage(
+        PAGE_URI, CHROME_FAVICON_URI, true, function () {
+          PlacesUtils.favicons.getFaviconDataForPage(
+            PAGE_URI, function (aURI, aDataLen, aData, aMimeType) {
+              let base64Icon = "data:image/png;base64," +
+                  base64EncodeString(String.fromCharCode.apply(String, aData));
+
+              test_bookmarks.unfiled.push(
+                { title: "Test", url: PAGE_URI.spec, icon: base64Icon });
+
+              try {
+                exporter.exportHTMLToFile(gBookmarksFileNew);
+              } catch(ex) { do_throw("couldn't export to file: " + ex); }
+
+              // Change the favicon to check it's really imported again later.
+              PlacesUtils.favicons.setAndFetchFaviconForPage(
+                PAGE_URI, CHROME_FAVICON_URI_2, true, function () {
+
+                  remove_all_bookmarks();
+
+                  try {
+                    BookmarkHTMLUtils.importFromFile(gBookmarksFileNew, true, function(success) {
+                     if (!success) {
+                        do_throw("couldn't import the exported file.");
+                      }
+                      waitForAsyncUpdates(function () {
+                        testImportedBookmarks();
+
+                        // Cleanup.
+                        test_bookmarks.unfiled.pop();
+                        PlacesUtils.bookmarks.removeItem(id);
+
+                        try {
+                          exporter.exportHTMLToFile(gBookmarksFileNew);
+                        } catch(ex) { do_throw("couldn't export to file: " + ex); }
+
+                        waitForAsyncUpdates(function () {
+                          remove_all_bookmarks();
+                          run_next_test();
+                        });
+                      });
+                    });
+                  } catch(ex) { do_throw("couldn't import the exported file: " + ex); }
+                });
+            });
+        });
+    });
+  } catch(ex) { do_throw("couldn't import the exported file: " + ex); }
+});
+
 add_test(function test_import_ontop()
 {
   // Test importing the exported bookmarks.html file *on top of* the existing
   // bookmarks.
   // 1. empty bookmarks db
   // 2. import the exported bookmarks file
   // 3. export to file
   // 3. import the exported bookmarks file
--- a/toolkit/components/places/BookmarkHTMLUtils.jsm
+++ b/toolkit/components/places/BookmarkHTMLUtils.jsm
@@ -616,46 +616,48 @@ BookmarkImporter.prototype = {
    * the user visits the page. Our made up one should get expired when the
    * page no longer references it.
    */
   _setFaviconForURI: function setFaviconForURI(aPageURI, aIconURI, aData) {
     // if the input favicon URI is a chrome: URI, then we just save it and don't
     // worry about data
     if (aIconURI) {
       if (aIconURI.scheme == "chrome") {
-        PlacesUtils.favicons.setFaviconUrlForPage(aPageURI, aIconURI);
+        PlacesUtils.favicons.setAndFetchFaviconForPage(aPageURI, aIconURI,
+                                                       false);
         return;
       }
     }
 
     // some bookmarks have placeholder URIs that contain just "data:"
     // ignore these
     if (aData.length <= 5) {
       return;
     }
 
     let faviconURI;
     if (aIconURI) {
       faviconURI = aIconURI;
     } else {
-      // make up favicon URL
+      // Make up a favicon URI for this page.  Later, we'll make sure that this
+      // favicon URI is always associated with local favicon data, so that we
+      // don't load this URI from the network.
       let faviconSpec = "http://www.mozilla.org/2005/made-up-favicon/"
                       + serialNumber
                       + "-"
                       + new Date().getTime();
       faviconURI = NetUtil.newURI(faviconSpec);
       serialNumber++;
     }
 
-    // save the favicon data
     // This could fail if the favicon is bigger than defined limit, in such a
-    // case data will not be saved to the db but we will still continue.
-    PlacesUtils.favicons.setFaviconDataFromDataURL(faviconURI, aData, 0);
-
-    PlacesUtils.favicons.setFaviconUrlForPage(aPageURI, faviconURI);
+    // case neither the favicon URI nor the favicon data will be saved.  If the
+    // bookmark is visited again later, the URI and data will be fetched.
+    PlacesUtils.favicons.replaceFaviconDataFromDataURL(faviconURI, aData);
+    PlacesUtils.favicons.setAndFetchFaviconForPage(aPageURI, faviconURI, false);
   },
 
   /**
    * Converts a string date in seconds to an int date in microseconds
    */
   _convertImportedDateToInternalDate: function convertImportedDateToInternalDate(aDate) {
     if (aDate && !isNaN(aDate)) {
       return parseInt(aDate) * 1000000; // in bookmarks.html this value is in seconds, not microseconds
old mode 100644
new mode 100755
--- a/toolkit/components/places/tests/head_common.js
+++ b/toolkit/components/places/tests/head_common.js
@@ -196,16 +196,36 @@ function readFileData(aFile) {
 function readFileOfLength(aFileName, aExpectedLength) {
   let data = readFileData(do_get_file(aFileName));
   do_check_eq(data.length, aExpectedLength);
   return data;
 }
 
 
 /**
+ * Returns the base64-encoded version of the given string.  This function is
+ * similar to window.btoa, but is available to xpcshell tests also.
+ *
+ * @param aString
+ *        Each character in this string corresponds to a byte, and must be a
+ *        code point in the range 0-255.
+ *
+ * @return The base64-encoded string.
+ */
+function base64EncodeString(aString) {
+  var stream = Cc["@mozilla.org/io/string-input-stream;1"]
+               .createInstance(Ci.nsIStringInputStream);
+  stream.setData(aString, aString.length);
+  var encoder = Cc["@mozilla.org/scriptablebase64encoder;1"]
+                .createInstance(Ci.nsIScriptableBase64Encoder);
+  return encoder.encodeToString(stream, aString.length);
+}
+
+
+/**
  * Compares two arrays, and returns true if they are equal.
  *
  * @param aArray1
  *        First array to compare.
  * @param aArray2
  *        Second array to compare.
  */
 function compareArrays(aArray1, aArray2) {