Bug 1552815: Use setFaviconForPage() instead of replaceFaviconData() r=mak,migration-reviewers,mconley

Depends on D192536

Differential Revision: https://phabricator.services.mozilla.com/D192537
This commit is contained in:
Daisuke Akatsuka 2024-05-01 20:40:50 +00:00
parent 22fab2b70a
commit daa9cee106
17 changed files with 190 additions and 108 deletions

View File

@ -785,7 +785,8 @@ async function GetBookmarksResource(aProfileFolder, aBrowserKey) {
}
// Import Bookmark Favicons
MigrationUtils.insertManyFavicons(favicons);
MigrationUtils.insertManyFavicons(favicons).catch(console.error);
if (gotErrors) {
throw new Error("The migration included errors.");
}

View File

@ -381,7 +381,7 @@ Bookmarks.prototype = {
}
await MigrationUtils.insertManyBookmarksWrapper(bookmarks, aDestFolderGuid);
MigrationUtils.insertManyFavicons(favicons);
MigrationUtils.insertManyFavicons(favicons).catch(console.error);
},
/**

View File

@ -870,36 +870,53 @@ class MigrationUtils {
* Iterates through the favicons, sniffs for a mime type,
* and uses the mime type to properly import the favicon.
*
* Note: You may not want to await on the returned promise, especially if by
* doing so there's risk of interrupting the migration of more critical
* data (e.g. bookmarks).
*
* @param {object[]} favicons
* An array of Objects with these properties:
* {Uint8Array} faviconData: The binary data of a favicon
* {nsIURI} uri: The URI of the associated page
*/
insertManyFavicons(favicons) {
async insertManyFavicons(favicons) {
let sniffer = Cc["@mozilla.org/image/loader;1"].createInstance(
Ci.nsIContentSniffer
);
for (let faviconDataItem of favicons) {
let mimeType = sniffer.getMIMETypeFromContent(
null,
faviconDataItem.faviconData,
faviconDataItem.faviconData.length
);
let dataURL;
try {
// getMIMETypeFromContent throws error if could not get the mime type
// from the data.
let mimeType = sniffer.getMIMETypeFromContent(
null,
faviconDataItem.faviconData,
faviconDataItem.faviconData.length
);
dataURL = await new Promise((resolve, reject) => {
let buffer = new Uint8ClampedArray(faviconDataItem.faviconData);
let blob = new Blob([buffer], { type: mimeType });
let reader = new FileReader();
reader.addEventListener("load", () => resolve(reader.result));
reader.addEventListener("error", reject);
reader.readAsDataURL(blob);
});
} catch (e) {
// Even if error happens for favicon, continue the process.
console.warn(e);
continue;
}
let fakeFaviconURI = Services.io.newURI(
"fake-favicon-uri:" + faviconDataItem.uri.spec
);
lazy.PlacesUtils.favicons.replaceFaviconData(
fakeFaviconURI,
faviconDataItem.faviconData,
mimeType
);
lazy.PlacesUtils.favicons.setAndFetchFaviconForPage(
lazy.PlacesUtils.favicons.setFaviconForPage(
faviconDataItem.uri,
fakeFaviconURI,
true,
lazy.PlacesUtils.favicons.FAVICON_LOAD_NON_PRIVATE,
null,
Services.scriptSecurityManager.getSystemPrincipal()
Services.io.newURI(dataURL)
);
}
}

View File

@ -98,7 +98,7 @@ Bookmarks.prototype = {
let rows = await MigrationUtils.getRowsFromDBWithoutLocks(
dbPath,
"Safari favicons",
`SELECT I.uuid, I.url AS favicon_url, P.url
`SELECT I.uuid, I.url AS favicon_url, P.url
FROM icon_info I
INNER JOIN page_url P ON I.uuid = P.uuid;`
);
@ -253,7 +253,7 @@ Bookmarks.prototype = {
parentGuid
);
MigrationUtils.insertManyFavicons(favicons);
MigrationUtils.insertManyFavicons(favicons).catch(console.error);
},
/**

View File

@ -117,6 +117,34 @@ async function assertFavicons(pageURIs) {
}
}
/**
* Check the image data for favicon of given page uri.
*
* @param {string} pageURI
* The page URI to which the favicon belongs.
* @param {Array} expectedImageData
* Expected image data of the favicon.
* @param {string} expectedMimeType
* Expected mime type of the favicon.
*/
async function assertFavicon(pageURI, expectedImageData, expectedMimeType) {
let result = await new Promise(resolve => {
PlacesUtils.favicons.getFaviconDataForPage(
Services.io.newURI(pageURI),
(faviconURI, dataLen, imageData, mimeType) => {
resolve({ faviconURI, dataLen, imageData, mimeType });
}
);
});
Assert.ok(!!result, `Got favicon for ${pageURI}`);
Assert.equal(
result.imageData.join(","),
expectedImageData.join(","),
"Image data is correct"
);
Assert.equal(result.mimeType, expectedMimeType, "Mime type is correct");
}
/**
* Replaces a directory service entry with a given nsIFile.
*

View File

@ -71,11 +71,13 @@ async function testBookmarks(migratorKey, subDirs) {
).path;
await IOUtils.copy(sourcePath, target.path);
// Get page url for each favicon
let faviconURIs = await MigrationUtils.getRowsFromDBWithoutLocks(
// Get page url and the image data for each favicon
let favicons = await MigrationUtils.getRowsFromDBWithoutLocks(
sourcePath,
"Chrome Bookmark Favicons",
`select page_url from icon_mapping`
`SELECT page_url, image_data FROM icon_mapping
INNER JOIN favicon_bitmaps ON (favicon_bitmaps.icon_id = icon_mapping.icon_id)
`
);
target.append("Bookmarks");
@ -171,10 +173,14 @@ async function testBookmarks(migratorKey, subDirs) {
"Telemetry reporting correct."
);
Assert.ok(observerNotified, "The observer should be notified upon migration");
let pageUrls = Array.from(faviconURIs, f =>
Services.io.newURI(f.getResultByName("page_url"))
);
await assertFavicons(pageUrls);
for (const favicon of favicons) {
await assertFavicon(
favicon.getResultByName("page_url"),
favicon.getResultByName("image_data"),
"image/png"
);
}
}
add_task(async function test_Chrome() {

View File

@ -27,6 +27,7 @@ let uniqueFaviconId = 0;
* Expected MIME type of the icon, for example "image/png".
* @param aExpectedData
* Expected icon data, expressed as an array of byte values.
* If set null, skip the test for the favicon data.
* @param aCallback
* This function is called after the check finished.
*/
@ -40,7 +41,9 @@ function checkFaviconDataForPage(
aPageURI,
async function (aURI, aDataLen, aData, aMimeType) {
Assert.equal(aExpectedMimeType, aMimeType);
Assert.ok(compareArrays(aExpectedData, aData));
if (aExpectedData) {
Assert.ok(compareArrays(aExpectedData, aData));
}
await check_guid_for_uri(aPageURI);
aCallback();
}

View File

@ -28,23 +28,23 @@ add_task(async function test_expire_associated() {
];
for (let icon of favicons) {
let data = readFileData(do_get_file(icon.name));
PlacesUtils.favicons.replaceFaviconData(
NetUtil.newURI(TEST_URL + icon.name),
data,
let dataURL = await readFileDataAsDataURL(
do_get_file(icon.name),
icon.mimeType
);
await setFaviconForPage(TEST_URL, TEST_URL + icon.name);
await PlacesTestUtils.setFaviconForPage(
TEST_URL,
TEST_URL + icon.name,
dataURL
);
if (icon.expired) {
await expireIconRelationsForPage(TEST_URL);
// Add the same icon to another page.
PlacesUtils.favicons.replaceFaviconData(
NetUtil.newURI(TEST_URL + icon.name),
data,
icon.mimeType,
icon.expire
await PlacesTestUtils.setFaviconForPage(
TEST_URL2,
TEST_URL + icon.name,
dataURL
);
await setFaviconForPage(TEST_URL2, TEST_URL + icon.name);
}
}

View File

@ -43,29 +43,24 @@ async function checkFaviconDataConversion(
});
let faviconURI = NetUtil.newURI("http://places.test/icon/" + aFileName);
let fileData = readFileOfLength(aFileName, aFileLength);
let fileDataURL = await fileDataToDataURL(fileData, aFileMimeType);
await PlacesTestUtils.setFaviconForPage(
pageURI.spec,
faviconURI.spec,
fileDataURL
);
PlacesUtils.favicons.replaceFaviconData(faviconURI, fileData, aFileMimeType);
await new Promise(resolve => {
PlacesUtils.favicons.setAndFetchFaviconForPage(
pageURI,
faviconURI,
true,
PlacesUtils.favicons.FAVICON_LOAD_NON_PRIVATE,
(aURI, aDataLen, aData, aMimeType) => {
if (!aExpectConversion) {
Assert.ok(compareArrays(aData, fileData));
Assert.equal(aMimeType, aFileMimeType);
} else {
if (!aVaryOnWindows || !isWindows) {
let expectedFile = do_get_file("expected-" + aFileName + ".png");
Assert.ok(compareArrays(aData, readFileData(expectedFile)));
}
Assert.equal(aMimeType, "image/png");
}
resolve();
},
Services.scriptSecurityManager.getSystemPrincipal()
);
if (!aExpectConversion) {
checkFaviconDataForPage(pageURI, aFileMimeType, fileData, resolve);
} else if (!aVaryOnWindows || !isWindows) {
let expectedFile = do_get_file("expected-" + aFileName + ".png");
let expectedData = readFileData(expectedFile);
checkFaviconDataForPage(pageURI, "image/png", expectedData, resolve);
} else {
// Not check the favicon data.
checkFaviconDataForPage(pageURI, "image/png", null, resolve);
}
});
}

View File

@ -9,21 +9,16 @@ const ICON32_URL = "http://places.test/favicon-normal32.png";
add_task(async function () {
await PlacesTestUtils.addVisits(PAGE_URL);
// Add 2 differently sized favicons for this page.
let data = readFileData(do_get_file("favicon-normal16.png"));
PlacesUtils.favicons.replaceFaviconData(
Services.io.newURI(ICON16_URL),
data,
let dataURL16 = await readFileDataAsDataURL(
do_get_file("favicon-normal16.png"),
"image/png"
);
await setFaviconForPage(PAGE_URL, ICON16_URL);
data = readFileData(do_get_file("favicon-normal32.png"));
PlacesUtils.favicons.replaceFaviconData(
Services.io.newURI(ICON32_URL),
data,
await PlacesTestUtils.setFaviconForPage(PAGE_URL, ICON16_URL, dataURL16);
let dataURL32 = await readFileDataAsDataURL(
do_get_file("favicon-normal32.png"),
"image/png"
);
await setFaviconForPage(PAGE_URL, ICON32_URL);
await PlacesTestUtils.setFaviconForPage(PAGE_URL, ICON32_URL, dataURL32);
const PAGE_ICON_URL = "page-icon:" + PAGE_URL;

View File

@ -60,12 +60,8 @@ add_task(async function test_fallback() {
info("Set icon for the root");
await PlacesTestUtils.addVisits(ROOT_URL);
let data = readFileData(do_get_file("favicon-normal16.png"));
PlacesUtils.favicons.replaceFaviconData(
NetUtil.newURI(ROOT_ICON_URL),
data,
"image/png"
);
await setFaviconForPage(ROOT_URL, ROOT_ICON_URL);
let dataURL = await fileDataToDataURL(data, "image/png");
await PlacesTestUtils.setFaviconForPage(ROOT_URL, ROOT_ICON_URL, dataURL);
info("check fallback icons");
await new Promise(resolve => {
@ -96,12 +92,8 @@ add_task(async function test_fallback() {
info("Now add a proper icon for the page");
await PlacesTestUtils.addVisits(SUBPAGE_URL);
let data32 = readFileData(do_get_file("favicon-normal32.png"));
PlacesUtils.favicons.replaceFaviconData(
NetUtil.newURI(ICON32_URL),
data32,
"image/png"
);
await setFaviconForPage(SUBPAGE_URL, ICON32_URL);
let dataURL32 = await fileDataToDataURL(data32, "image/png");
await PlacesTestUtils.setFaviconForPage(SUBPAGE_URL, ICON32_URL, dataURL32);
info("check no fallback icons");
await new Promise(resolve => {

View File

@ -57,13 +57,11 @@ add_task(async function test_fallback() {
info("Set icon for the root");
await PlacesTestUtils.addVisits(ROOT_URL);
let data = readFileData(do_get_file("favicon-normal16.png"));
PlacesUtils.favicons.replaceFaviconData(
NetUtil.newURI(ROOT_ICON_URL),
data,
let dataURL = await readFileDataAsDataURL(
do_get_file("favicon-normal16.png"),
"image/png"
);
await setFaviconForPage(ROOT_URL, ROOT_ICON_URL);
await PlacesTestUtils.setFaviconForPage(ROOT_URL, ROOT_ICON_URL, dataURL);
info("check fallback icons");
Assert.equal(
@ -79,13 +77,11 @@ add_task(async function test_fallback() {
info("Now add a proper icon for the page");
await PlacesTestUtils.addVisits(SUBPAGE_URL);
let data32 = readFileData(do_get_file("favicon-normal32.png"));
PlacesUtils.favicons.replaceFaviconData(
NetUtil.newURI(ICON32_URL),
data32,
let dataURL32 = await readFileDataAsDataURL(
do_get_file("favicon-normal32.png"),
"image/png"
);
await setFaviconForPage(SUBPAGE_URL, ICON32_URL);
await PlacesTestUtils.setFaviconForPage(SUBPAGE_URL, ICON32_URL, dataURL32);
info("check no fallback icons");
Assert.equal(

View File

@ -26,8 +26,8 @@ add_task(async function () {
let pageURI = uri("http://foo.bar/");
await PlacesTestUtils.addVisits(pageURI);
PlacesUtils.favicons.replaceFaviconData(icon.uri, icon.data, icon.mimetype);
await setFaviconForPage(pageURI, icon.uri);
let dataURI = await fileDataToDataURL(icon.data, icon.mimetype);
await PlacesTestUtils.setFaviconForPage(pageURI.spec, icon.uri.spec, dataURI);
Assert.equal(
await getFaviconUrlForPage(pageURI),
icon.uri.spec,

View File

@ -16,10 +16,9 @@ add_task(async function () {
let url = "http://foo.bar/";
await PlacesTestUtils.addVisits(url);
for (let i = 0; i < 10; ++i) {
let iconUri = NetUtil.newURI("http://mozilla.org/" + i);
let data = readFileData(icon.file);
PlacesUtils.favicons.replaceFaviconData(iconUri, data, icon.mimetype);
await setFaviconForPage(url, iconUri);
let iconUri = "http://mozilla.org/" + i;
let dataURL = await readFileDataAsDataURL(icon.file, icon.mimetype);
await PlacesTestUtils.setFaviconForPage(url, iconUri, dataURL);
}
let promise = TestUtils.topicObserved("places-favicons-expired");

View File

@ -14,9 +14,15 @@ add_task(async function () {
let faviconURI = NetUtil.newURI("http://places.test/icon/favicon-multi.ico");
// Fake window.
let win = { devicePixelRatio: 1.0 };
let icoData = readFileData(do_get_file("favicon-multi.ico"));
PlacesUtils.favicons.replaceFaviconData(faviconURI, icoData, "image/x-icon");
await setFaviconForPage(pageURI, faviconURI);
let icoDataURL = await readFileDataAsDataURL(
do_get_file("favicon-multi.ico"),
"image/x-icon"
);
await PlacesTestUtils.setFaviconForPage(
pageURI.spec,
faviconURI.spec,
icoDataURL
);
for (let size of [16, 32, 64]) {
let file = do_get_file(`favicon-multi-frame${size}.png`);

View File

@ -158,16 +158,25 @@ add_task(async function test_different_host() {
add_task(async function test_same_size() {
// Add two icons with the same size, one is a root icon. Check that the
// non-root icon is preferred when a smaller size is requested.
let data = readFileData(do_get_file("favicon-normal32.png"));
let dataURL = await readFileDataAsDataURL(
do_get_file("favicon-normal32.png"),
"image/png"
);
let pageURI = NetUtil.newURI("http://new_places.test/page/");
await PlacesTestUtils.addVisits(pageURI);
let faviconURI = NetUtil.newURI("http://new_places.test/favicon.ico");
PlacesUtils.favicons.replaceFaviconData(faviconURI, data, "image/png");
await setFaviconForPage(pageURI, faviconURI);
await PlacesTestUtils.setFaviconForPage(
pageURI.spec,
faviconURI.spec,
dataURL
);
faviconURI = NetUtil.newURI("http://new_places.test/another_icon.ico");
PlacesUtils.favicons.replaceFaviconData(faviconURI, data, "image/png");
await setFaviconForPage(pageURI, faviconURI);
await PlacesTestUtils.setFaviconForPage(
pageURI.spec,
faviconURI.spec,
dataURL
);
Assert.equal(
await getFaviconUrlForPage(pageURI, 20),

View File

@ -169,7 +169,6 @@ function readFileData(aFile) {
}
return bytes;
}
/**
* Reads the data from the named file, verifying the expected file length.
*
@ -186,6 +185,42 @@ function readFileOfLength(aFileName, aExpectedLength) {
return data;
}
/**
* Reads the data from the specified nsIFile, then returns it as data URL.
*
* @param file
* The nsIFile to read from.
* @param mimeType
* The mime type of the file content.
* @return Promise that retunes data URL.
*/
async function readFileDataAsDataURL(file, mimeType) {
const data = readFileData(file);
return fileDataToDataURL(data, mimeType);
}
/**
* Converts the given data to the data URL.
*
* @param data
* The file data.
* @param mimeType
* The mime type of the file content.
* @return Promise that retunes data URL.
*/
async function fileDataToDataURL(data, mimeType) {
const dataURL = await new Promise(resolve => {
const buffer = new Uint8ClampedArray(data);
const blob = new Blob([buffer], { type: mimeType });
const reader = new FileReader();
reader.onload = e => {
resolve(e.target.result);
};
reader.readAsDataURL(blob);
});
return dataURL;
}
/**
* Returns the base64-encoded version of the given string. This function is
* similar to window.btoa, but is available to xpcshell tests also.