From 3706e25e63111db2b6a87f1c92ef3390713f96b5 Mon Sep 17 00:00:00 2001 From: Paolo Amadini Date: Thu, 12 Apr 2012 12:27:36 +0200 Subject: [PATCH] Bug 728174 - Replace old synchronous favicons calls in the bookmarks HTML import. r=mak --HG-- extra : rebase_source : ddb3109fa13f41bdfb2f265f7850453905145a07 --- ...41-import-export-corrupt-bookmarks-html.js | 31 ++++--- .../places/tests/unit/test_bookmarks_html.js | 90 +++++++++++++++++-- .../components/places/BookmarkHTMLUtils.jsm | 16 ++-- .../components/places/nsPlacesExportService.h | 0 .../components/places/tests/head_common.js | 20 +++++ 5 files changed, 133 insertions(+), 24 deletions(-) mode change 100644 => 100755 toolkit/components/places/nsPlacesExportService.h diff --git 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 index 6e5c82d7100..c526b5f767a 100644 --- 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 @@ -62,7 +62,7 @@ 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(); @@ -84,8 +84,7 @@ function after_import(success) { // 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"), @@ -112,22 +111,23 @@ function after_import(success) { // 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); @@ -226,7 +226,14 @@ function database_check() { 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(); + }); } diff --git a/browser/components/places/tests/unit/test_bookmarks_html.js b/browser/components/places/tests/unit/test_bookmarks_html.js index aa4772c7b73..7651f27fd13 100644 --- a/browser/components/places/tests/unit/test_bookmarks_html.js +++ b/browser/components/places/tests/unit/test_bookmarks_html.js @@ -183,11 +183,14 @@ 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) { @@ -236,6 +239,83 @@ add_test(function test_emptytitle_export() } 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 diff --git a/toolkit/components/places/BookmarkHTMLUtils.jsm b/toolkit/components/places/BookmarkHTMLUtils.jsm index e2cd4d0116d..25757445d8b 100644 --- a/toolkit/components/places/BookmarkHTMLUtils.jsm +++ b/toolkit/components/places/BookmarkHTMLUtils.jsm @@ -621,7 +621,8 @@ BookmarkImporter.prototype = { // worry about data if (aIconURI) { if (aIconURI.scheme == "chrome") { - PlacesUtils.favicons.setFaviconUrlForPage(aPageURI, aIconURI); + PlacesUtils.favicons.setAndFetchFaviconForPage(aPageURI, aIconURI, + false); return; } } @@ -636,7 +637,9 @@ BookmarkImporter.prototype = { 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 + "-" @@ -645,12 +648,11 @@ BookmarkImporter.prototype = { 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); }, /** diff --git a/toolkit/components/places/nsPlacesExportService.h b/toolkit/components/places/nsPlacesExportService.h old mode 100644 new mode 100755 diff --git a/toolkit/components/places/tests/head_common.js b/toolkit/components/places/tests/head_common.js index 271cf1e7d87..33b0d76fe2c 100644 --- a/toolkit/components/places/tests/head_common.js +++ b/toolkit/components/places/tests/head_common.js @@ -200,6 +200,26 @@ function readFileOfLength(aFileName, aExpectedLength) { } +/** + * 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. *