Bug 1586453 - Don't try to wait for favicons in policy r=Standard8

Differential Revision: https://phabricator.services.mozilla.com/D48240

--HG--
extra : moz-landing-system : lando

browser/components/enterprisepolicies/helpers/BookmarksPolicies.jsm

@@ -206,13 +206,11 @@ async function insertBookmark(bookmark) {
   });
 
   if (bookmark.Favicon) {
-    await setFaviconForBookmark(bookmark).catch(() =>
-      log.error(`Error setting favicon for ${bookmark.Title}`)
-    );
+    setFaviconForBookmark(bookmark);
   }
 }
 
-async function setFaviconForBookmark(bookmark) {
+function setFaviconForBookmark(bookmark) {
   let faviconURI;
   let nullPrincipal = Services.scriptSecurityManager.createNullPrincipal({});
 
@@ -238,19 +236,17 @@ async function setFaviconForBookmark(bookmark) {
 
     default:
       log.error(`Bad URL given for favicon on bookmark "${bookmark.Title}"`);
-      return Promise.resolve();
+      return;
   }
 
-  return new Promise(resolve => {
-    PlacesUtils.favicons.setAndFetchFaviconForPage(
-      Services.io.newURI(bookmark.URL.href),
-      faviconURI,
-      false /* forceReload */,
-      PlacesUtils.favicons.FAVICON_LOAD_NON_PRIVATE,
-      resolve,
-      nullPrincipal
-    );
-  });
+  PlacesUtils.favicons.setAndFetchFaviconForPage(
+    Services.io.newURI(bookmark.URL.href),
+    faviconURI,
+    false /* forceReload */,
+    PlacesUtils.favicons.FAVICON_LOAD_NON_PRIVATE,
+    null,
+    nullPrincipal
+  );
 }
 
 // Cache of folder names to guids to be used by the getParentGuid

browser/components/enterprisepolicies/tests/browser/404.sjs

+function handleRequest(request, response) {
+  response.setStatusLine(request.httpVersion, 404, "Not Found");
+}

browser/components/enterprisepolicies/tests/browser/browser.ini

@@ -10,6 +10,7 @@ support-files =
   ../../../../../toolkit/components/antitracking/test/browser/page.html
   ../../../../../toolkit/components/antitracking/test/browser/subResources.sjs
   extensionsettings.html
+  404.sjs
 
 [browser_policies_getActivePolicies.js]
 skip-if = os != 'mac'

browser/components/enterprisepolicies/tests/browser/browser_policy_bookmarks.js

@@ -16,6 +16,11 @@ const { BookmarksPolicies } = ChromeUtils.import(
 
 let CURRENT_POLICY;
 
+const basePath = getRootDirectory(gTestPath).replace(
+  "chrome://mochitests/content",
+  "http://mochi.test:8888"
+);
+
 const BASE_POLICY = {
   policies: {
     DisplayBookmarksToolbar: true,
@@ -28,21 +33,26 @@ const BASE_POLICY = {
       {
         Title: "Bookmark 2",
         URL: "https://bookmark2.example.com/",
-        Folder: "Folder 1",
+        Favicon: `${basePath}/404.sjs`,
       },
       {
         Title: "Bookmark 3",
         URL: "https://bookmark3.example.com/",
-        Placement: "menu",
+        Folder: "Folder 1",
       },
       {
         Title: "Bookmark 4",
         URL: "https://bookmark4.example.com/",
-        Folder: "Folder 1",
+        Placement: "menu",
       },
       {
         Title: "Bookmark 5",
         URL: "https://bookmark5.example.com/",
+        Folder: "Folder 1",
+      },
+      {
+        Title: "Bookmark 6",
+        URL: "https://bookmark6.example.com/",
         Placement: "menu",
         Folder: "Folder 2",
       },
@@ -199,7 +209,7 @@ add_task(async function test_initial_bookmarks() {
 
   await Promise.all([
     promiseAllChangesMade({
-      itemsToAdd: 7, // 5 bookmarks + 2 folders
+      itemsToAdd: 8, // 6 bookmarks + 2 folders
       itemsToRemove: 0,
     }),
     setupPolicyEngineWithJson(CURRENT_POLICY),
@@ -240,7 +250,7 @@ add_task(async function test_remove_Bookmark_2() {
   //
   // Remove the 2nd bookmark. It is inside "Folder 1", but that
   // folder won't be empty, so it must not be removed.
-  CURRENT_POLICY.policies.Bookmarks.splice(2, 1);
+  CURRENT_POLICY.policies.Bookmarks.splice(3, 1);
 
   await Promise.all([
     promiseAllChangesMade({
@@ -298,7 +308,7 @@ add_task(async function test_empty_all_bookmarks() {
   await Promise.all([
     promiseAllChangesMade({
       itemsToAdd: 0,
-      itemsToRemove: 7, // 5 bookmarks and 2 folders
+      itemsToRemove: 8, // 6 bookmarks and 2 folders
     }),
     setupPolicyEngineWithJson(CURRENT_POLICY),
   ]);