Skip to content
This repository was archived by the owner on Jul 9, 2025. It is now read-only.

Commit 631c81f

Browse files
author
Daisuke Akatsuka
committed
Bug 1915774: Remove setFaviconForPage() in head_common r=places-reviewers,mak
Differential Revision: https://phabricator.services.mozilla.com/D241887
1 parent f4be40f commit 631c81f

File tree

7 files changed

+71
-42
lines changed

7 files changed

+71
-42
lines changed

toolkit/components/places/tests/PlacesTestUtils.sys.mjs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -183,11 +183,13 @@ export var PlacesTestUtils = Object.freeze({
183183
/**
184184
* Get favicon data for given URL from database.
185185
*
186-
* @param {nsIURI} faviconURI
187-
* nsIURI for the favicon
186+
* @param {string or nsIURI} faviconURI
187+
* uri for the favicon
188188
* @return {nsIURI} data URL
189189
*/
190190
async getFaviconDataURLFromDB(faviconURI) {
191+
faviconURI = lazy.PlacesUtils.toURI(faviconURI);
192+
191193
const db = await lazy.PlacesUtils.promiseDBConnection();
192194
const rows = await db.executeCached(
193195
`SELECT data, width
@@ -218,7 +220,7 @@ export var PlacesTestUtils = Object.freeze({
218220
/**
219221
* Get favicon data for given URL from network.
220222
*
221-
* @param {nsIURI} faviconURI
223+
* @param {string or nsIURI} faviconURI
222224
* nsIURI for the favicon.
223225
* @param {nsIPrincipal} [optional] loadingPrincipal
224226
* The principal to load from network. If no, use system principal.
@@ -232,6 +234,7 @@ export var PlacesTestUtils = Object.freeze({
232234
faviconURI,
233235
loadingPrincipal = Services.scriptSecurityManager.getSystemPrincipal()
234236
) {
237+
faviconURI = lazy.PlacesUtils.toURI(faviconURI);
235238
if (faviconURI.schemeIs("data")) {
236239
return faviconURI;
237240
}

toolkit/components/places/tests/favicons/test_copyFavicons.js

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,11 @@ add_task(async function test_copyFavicons_noop() {
7272
);
7373

7474
info("Unknown dest uri, source has icon");
75-
await setFaviconForPage(TEST_URI1, SMALLPNG_DATA_URI);
75+
await PlacesTestUtils.setFaviconForPage(
76+
TEST_URI1,
77+
SMALLPNG_DATA_URI,
78+
SMALLPNG_DATA_URI
79+
);
7680
Assert.equal(
7781
await copyFavicons(TEST_URI1, TEST_URI2, false),
7882
null,
@@ -94,8 +98,16 @@ add_task(async function test_copyFavicons_noop() {
9498
add_task(async function test_copyFavicons() {
9599
info("Normal copy across 2 pages");
96100
await PlacesTestUtils.addVisits(TEST_URI1);
97-
await setFaviconForPage(TEST_URI1, SMALLPNG_DATA_URI);
98-
await setFaviconForPage(TEST_URI1, SMALLSVG_DATA_URI);
101+
await PlacesTestUtils.setFaviconForPage(
102+
TEST_URI1,
103+
SMALLPNG_DATA_URI,
104+
SMALLPNG_DATA_URI
105+
);
106+
await PlacesTestUtils.setFaviconForPage(
107+
TEST_URI1,
108+
SMALLSVG_DATA_URI,
109+
SMALLSVG_DATA_URI
110+
);
99111
await PlacesTestUtils.addVisits(TEST_URI2);
100112
let promiseChange = promisePageChanged(TEST_URI2.spec);
101113
Assert.equal(
@@ -145,10 +157,22 @@ add_task(async function test_copyFavicons() {
145157
add_task(async function test_copyFavicons_overlap() {
146158
info("Copy to a page that has one of the favicons already");
147159
await PlacesTestUtils.addVisits(TEST_URI1);
148-
await setFaviconForPage(TEST_URI1, SMALLPNG_DATA_URI);
149-
await setFaviconForPage(TEST_URI1, SMALLSVG_DATA_URI);
160+
await PlacesTestUtils.setFaviconForPage(
161+
TEST_URI1,
162+
SMALLPNG_DATA_URI,
163+
SMALLPNG_DATA_URI
164+
);
165+
await PlacesTestUtils.setFaviconForPage(
166+
TEST_URI1,
167+
SMALLSVG_DATA_URI,
168+
SMALLSVG_DATA_URI
169+
);
150170
await PlacesTestUtils.addVisits(TEST_URI2);
151-
await setFaviconForPage(TEST_URI2, SMALLPNG_DATA_URI);
171+
await PlacesTestUtils.setFaviconForPage(
172+
TEST_URI2,
173+
SMALLPNG_DATA_URI,
174+
SMALLPNG_DATA_URI
175+
);
152176
let promiseChange = promisePageChanged(TEST_URI2.spec);
153177
Assert.equal(
154178
(await copyFavicons(TEST_URI1, TEST_URI2, false)).spec,
@@ -170,7 +194,11 @@ add_task(async function test_copyFavicons_overlap() {
170194

171195
add_task(async function test_copyFavicons_local_uri() {
172196
await PlacesTestUtils.addVisits(TEST_URI1);
173-
await setFaviconForPage(TEST_URI1, SMALLPNG_DATA_URI);
197+
await PlacesTestUtils.setFaviconForPage(
198+
TEST_URI1,
199+
SMALLPNG_DATA_URI,
200+
SMALLPNG_DATA_URI
201+
);
174202

175203
Assert.throws(
176204
() => PlacesUtils.favicons.copyFavicons(TEST_URI1, TEST_LOCAL_URI, false),

toolkit/components/places/tests/favicons/test_expireAllFavicons.js

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,11 @@ add_task(async function test_expireAllFavicons() {
1515
});
1616

1717
// Set a favicon for our test page.
18-
await setFaviconForPage(TEST_PAGE_URI, SMALLPNG_DATA_URI);
18+
await PlacesTestUtils.setFaviconForPage(
19+
TEST_PAGE_URI,
20+
SMALLPNG_DATA_URI,
21+
SMALLPNG_DATA_URI
22+
);
1923

2024
// Add a page with a bookmark.
2125
await PlacesUtils.bookmarks.insert({
@@ -25,7 +29,11 @@ add_task(async function test_expireAllFavicons() {
2529
});
2630

2731
// Set a favicon for our bookmark.
28-
await setFaviconForPage(BOOKMARKED_PAGE_URI, SMALLPNG_DATA_URI);
32+
await PlacesTestUtils.setFaviconForPage(
33+
BOOKMARKED_PAGE_URI,
34+
SMALLPNG_DATA_URI,
35+
SMALLPNG_DATA_URI
36+
);
2937

3038
// Start expiration only after data has been saved in the database.
3139
let promise = promiseTopicObserved(PlacesUtils.TOPIC_FAVICONS_EXPIRED);

toolkit/components/places/tests/favicons/test_favicons_protocols_ref.js

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,16 @@ add_task(async function () {
6767

6868
// Add the icon also for the page with ref.
6969
await PlacesTestUtils.addVisits(PAGE_URL + "#other§=12");
70-
await setFaviconForPage(PAGE_URL + "#other§=12", ICON16_URL, false);
71-
await setFaviconForPage(PAGE_URL + "#other§=12", ICON32_URL, false);
70+
await PlacesTestUtils.setFaviconForPage(
71+
PAGE_URL + "#other§=12",
72+
ICON16_URL,
73+
await PlacesTestUtils.getFaviconDataURLFromDB(ICON16_URL)
74+
);
75+
await PlacesTestUtils.setFaviconForPage(
76+
PAGE_URL + "#other§=12",
77+
ICON32_URL,
78+
await PlacesTestUtils.getFaviconDataURLFromDB(ICON32_URL)
79+
);
7280
await compareFavicons(
7381
PlacesUtils.urlWithSizeRef(win, PAGE_ICON_URL + "#other§=12", 16),
7482
PlacesUtils.favicons.getFaviconLinkForIcon(Services.io.newURI(ICON16_URL)),

toolkit/components/places/tests/favicons/test_page-icon_protocol.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,11 @@ add_task(async function page_with_ref() {
157157
"http://places.test.ref/#",
158158
]) {
159159
await PlacesTestUtils.addVisits(url);
160-
await setFaviconForPage(url, ICON_URI, false);
160+
await PlacesTestUtils.setFaviconForPage(
161+
url,
162+
ICON_URI,
163+
await PlacesTestUtils.getFaviconDataURLFromDB(ICON_URI)
164+
);
161165
let { data, contentType } = await fetchIconForSpec("page-icon:" + url);
162166
Assert.equal(contentType, gFavicon.contentType);
163167
Assert.deepEqual(data, gFavicon.data, "Got the favicon data");

toolkit/components/places/tests/head_common.js

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -734,32 +734,6 @@ function sortBy(array, prop) {
734734
return array.sort((a, b) => compareAscending(a[prop], b[prop]));
735735
}
736736

737-
/**
738-
* Asynchronously set the favicon associated with a page.
739-
* @param page
740-
* The page's URL
741-
* @param icon
742-
* The URL of the favicon to be set.
743-
* @param [optional] forceReload
744-
* Whether to enforce reloading the icon.
745-
*/
746-
async function setFaviconForPage(page, icon, forceReload = true) {
747-
let pageURI =
748-
page instanceof Ci.nsIURI ? page : NetUtil.newURI(new URL(page).href);
749-
let iconURI =
750-
icon instanceof Ci.nsIURI ? icon : NetUtil.newURI(new URL(icon).href);
751-
752-
let dataURL;
753-
if (!forceReload) {
754-
dataURL = await PlacesTestUtils.getFaviconDataURLFromDB(iconURI);
755-
}
756-
if (!dataURL) {
757-
dataURL = await PlacesTestUtils.getFaviconDataURLFromNetwork(iconURI);
758-
}
759-
760-
await PlacesUtils.favicons.setFaviconForPage(pageURI, iconURI, dataURL);
761-
}
762-
763737
function getFaviconUrlForPage(page, width = 0) {
764738
let pageURI =
765739
page instanceof Ci.nsIURI ? page : NetUtil.newURI(new URL(page).href);

toolkit/components/places/tests/unit/test_promiseBookmarksTree.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,11 @@ add_task(async function () {
253253
url: urlWithCharsetAndFavicon,
254254
annotations: new Map([[PlacesUtils.CHARSET_ANNO, "UTF-16"]]),
255255
});
256-
await setFaviconForPage(urlWithCharsetAndFavicon, SMALLPNG_DATA_URI);
256+
await PlacesTestUtils.setFaviconForPage(
257+
urlWithCharsetAndFavicon,
258+
SMALLPNG_DATA_URI,
259+
SMALLPNG_DATA_URI
260+
);
257261
// Test the default places root without specifying it.
258262
await test_promiseBookmarksTreeAgainstResult();
259263

0 commit comments

Comments
 (0)