Bug 1337858 - Replace CopyFavicon in the Docshell with a proper favicons API. r=standard8
authorMarco Bonardo <mbonardo@mozilla.com>
Thu, 18 May 2017 16:48:23 +0200
changeset 412809 d48d0c5b01913ba8535a37750e7fba0c1753a43d
parent 412773 19acfff499d3ee44cb09ed76b43646c146d06f58
child 412810 fe9f05834bbab6d5642a04e482a9cbf71bcaf688
push id1490
push usermtabara@mozilla.com
push dateMon, 31 Jul 2017 14:08:16 +0000
treeherdermozilla-release@70e32e6bf15e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersstandard8
bugs1337858
milestone55.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 1337858 - Replace CopyFavicon in the Docshell with a proper favicons API. r=standard8 MozReview-Commit-ID: 8wjSbj0FTwE
browser/base/content/browser.js
docshell/base/nsDocShell.cpp
toolkit/components/places/FaviconHelpers.cpp
toolkit/components/places/FaviconHelpers.h
toolkit/components/places/mozIAsyncFavicons.idl
toolkit/components/places/nsFaviconService.cpp
toolkit/components/places/tests/favicons/test_copyFavicons.js
toolkit/components/places/tests/favicons/xpcshell.ini
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -4023,31 +4023,26 @@ function FillHistoryMenu(aParent) {
 
     for (let j = end - 1; j >= start; j--) {
       let entry = sessionHistory.entries[j];
       let uri = entry.url;
 
       let item = existingIndex < children.length ?
                    children[existingIndex] : document.createElement("menuitem");
 
-      let entryURI = Services.io.newURI(entry.url, entry.charset);
       item.setAttribute("uri", uri);
       item.setAttribute("label", entry.title || uri);
       item.setAttribute("index", j);
 
       // Cache this so that gotoHistoryIndex doesn't need the original index
       item.setAttribute("historyindex", j - index);
 
       if (j != index) {
-        PlacesUtils.favicons.getFaviconURLForPage(entryURI, function(aURI) {
-          if (aURI) {
-            let iconURL = PlacesUtils.favicons.getFaviconLinkForIcon(aURI).spec;
-            item.style.listStyleImage = "url(" + iconURL + ")";
-          }
-        });
+        item.setAttribute("image",
+                          PlacesUtils.urlWithSizeRef(window, "page-icon:" + uri, 16));
       }
 
       if (j < index) {
         item.className = "unified-nav-back menuitem-iconic menuitem-with-favicon";
         item.setAttribute("tooltiptext", tooltipBack);
       } else if (j == index) {
         item.setAttribute("type", "radio");
         item.setAttribute("checked", "true");
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -9584,71 +9584,16 @@ nsDocShell::CheckLoadingPermissions()
 
   return NS_ERROR_DOM_PROP_ACCESS_DENIED;
 }
 
 //*****************************************************************************
 // nsDocShell: Site Loading
 //*****************************************************************************
 
-namespace {
-
-#ifdef MOZ_PLACES
-// Callback used by CopyFavicon to inform the favicon service that one URI
-// (mNewURI) has the same favicon URI (OnComplete's aFaviconURI) as another.
-class nsCopyFaviconCallback final : public nsIFaviconDataCallback
-{
-public:
-  NS_DECL_ISUPPORTS
-
-  nsCopyFaviconCallback(mozIAsyncFavicons* aSvc,
-                        nsIURI* aNewURI,
-                        nsIPrincipal* aLoadingPrincipal,
-                        bool aInPrivateBrowsing)
-    : mSvc(aSvc)
-    , mNewURI(aNewURI)
-    , mLoadingPrincipal(aLoadingPrincipal)
-    , mInPrivateBrowsing(aInPrivateBrowsing)
-  {
-  }
-
-  NS_IMETHOD
-  OnComplete(nsIURI* aFaviconURI, uint32_t aDataLen,
-             const uint8_t* aData, const nsACString& aMimeType, uint16_t aWidth) override
-  {
-    // Continue only if there is an associated favicon.
-    if (!aFaviconURI) {
-      return NS_OK;
-    }
-
-    MOZ_ASSERT(aDataLen == 0,
-               "We weren't expecting the callback to deliver data.");
-
-    nsCOMPtr<mozIPlacesPendingOperation> po;
-    return mSvc->SetAndFetchFaviconForPage(
-      mNewURI, aFaviconURI, false,
-      mInPrivateBrowsing ? nsIFaviconService::FAVICON_LOAD_PRIVATE :
-                           nsIFaviconService::FAVICON_LOAD_NON_PRIVATE,
-      nullptr, mLoadingPrincipal, getter_AddRefs(po));
-  }
-
-private:
-  ~nsCopyFaviconCallback() {}
-
-  nsCOMPtr<mozIAsyncFavicons> mSvc;
-  nsCOMPtr<nsIURI> mNewURI;
-  nsCOMPtr<nsIPrincipal> mLoadingPrincipal;
-  bool mInPrivateBrowsing;
-};
-
-NS_IMPL_ISUPPORTS(nsCopyFaviconCallback, nsIFaviconDataCallback)
-#endif
-
-} // namespace
-
 void
 nsDocShell::CopyFavicon(nsIURI* aOldURI,
                         nsIURI* aNewURI,
                         nsIPrincipal* aLoadingPrincipal,
                         bool aInPrivateBrowsing)
 {
   if (XRE_IsContentProcess()) {
     dom::ContentChild* contentChild = dom::ContentChild::GetSingleton();
@@ -9662,21 +9607,19 @@ nsDocShell::CopyFavicon(nsIURI* aOldURI,
     }
     return;
   }
 
 #ifdef MOZ_PLACES
   nsCOMPtr<mozIAsyncFavicons> favSvc =
     do_GetService("@mozilla.org/browser/favicon-service;1");
   if (favSvc) {
-    nsCOMPtr<nsIFaviconDataCallback> callback =
-      new nsCopyFaviconCallback(favSvc, aNewURI,
-                                aLoadingPrincipal,
-                                aInPrivateBrowsing);
-    favSvc->GetFaviconURLForPage(aOldURI, callback, 0);
+    favSvc->CopyFavicons(aOldURI, aNewURI,
+      aInPrivateBrowsing ? nsIFaviconService::FAVICON_LOAD_PRIVATE
+                         : nsIFaviconService::FAVICON_LOAD_NON_PRIVATE, nullptr);
   }
 #endif
 }
 
 class InternalLoadEvent : public Runnable
 {
 public:
   InternalLoadEvent(nsDocShell* aDocShell, nsIURI* aURI,
--- a/toolkit/components/places/FaviconHelpers.cpp
+++ b/toolkit/components/places/FaviconHelpers.cpp
@@ -872,18 +872,16 @@ AsyncAssociateIconToPage::Run()
       NS_ENSURE_STATE(stmt);
       mozStorageStatementScoper scoper(stmt);
       rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("page_id"), mPage.id);
       NS_ENSURE_SUCCESS(rv, rv);
       rv = stmt->Execute();
       NS_ENSURE_SUCCESS(rv, rv);
     } else {
       // We need to create the page entry.
-      // By default, we use the place id for the insertion. While we can't
-      // guarantee 1:1 mapping, in general it should do.
       nsCOMPtr<mozIStorageStatement> stmt;
       stmt = DB->GetStatement(
         "INSERT OR IGNORE INTO moz_pages_w_icons (page_url, page_url_hash) "
         "VALUES (:page_url, hash(:page_url)) "
       );
       NS_ENSURE_STATE(stmt);
       mozStorageStatementScoper scoper(stmt);
       rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("page_url"), mPage.spec);
@@ -1371,10 +1369,104 @@ FetchAndConvertUnsupportedPayloads::Stor
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = stmt->Execute();
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
 }
 
+////////////////////////////////////////////////////////////////////////////////
+//// AsyncCopyFavicons
+
+AsyncCopyFavicons::AsyncCopyFavicons(
+  PageData& aFromPage
+, PageData& aToPage
+, nsIFaviconDataCallback* aCallback
+) : mFromPage(aFromPage)
+  , mToPage(aToPage)
+  , mCallback(new nsMainThreadPtrHolder<nsIFaviconDataCallback>(aCallback))
+{
+  MOZ_ASSERT(NS_IsMainThread());
+}
+
+NS_IMETHODIMP
+AsyncCopyFavicons::Run()
+{
+  MOZ_ASSERT(!NS_IsMainThread());
+
+  IconData icon;
+
+  // Ensure we'll callback and dispatch notifications to the main-thread.
+  auto cleanup = MakeScopeExit([&] () {
+    // If we bailed out early, just return a null icon uri, since we didn't
+    // copy anything.
+    if (!(icon.status & ICON_STATUS_ASSOCIATED)) {
+      icon.spec.Truncate();
+    }
+    nsCOMPtr<nsIRunnable> event = new NotifyIconObservers(icon, mToPage, mCallback);
+    NS_DispatchToMainThread(event);
+  });
+
+  RefPtr<Database> DB = Database::GetDatabase();
+  NS_ENSURE_STATE(DB);
+
+  nsresult rv = FetchPageInfo(DB, mToPage);
+  if (rv == NS_ERROR_NOT_AVAILABLE || !mToPage.placeId) {
+    // We have never seen this page, or we can't add this page to history and
+    // and it's not a bookmark. We won't add the page.
+    return NS_OK;
+  }
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  // Get just one icon, to check whether the page has any, and to notify later.
+  rv = FetchIconPerSpec(DB, mFromPage.spec, EmptyCString(), icon, UINT16_MAX);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  if (icon.spec.IsEmpty()) {
+    // There's nothing to copy.
+    return NS_OK;
+  }
+
+  // Insert an entry in moz_pages_w_icons if needed.
+  if (!mToPage.id) {
+    // We need to create the page entry.
+    nsCOMPtr<mozIStorageStatement> stmt;
+    stmt = DB->GetStatement(
+      "INSERT OR IGNORE INTO moz_pages_w_icons (page_url, page_url_hash) "
+      "VALUES (:page_url, hash(:page_url)) "
+    );
+    NS_ENSURE_STATE(stmt);
+    mozStorageStatementScoper scoper(stmt);
+    rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("page_url"), mToPage.spec);
+    NS_ENSURE_SUCCESS(rv, rv);
+    rv = stmt->Execute();
+    NS_ENSURE_SUCCESS(rv, rv);
+    // Required to to fetch the id and the guid.
+    rv = FetchPageInfo(DB, mToPage);
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+
+  // Create the relations.
+  nsCOMPtr<mozIStorageStatement> stmt = DB->GetStatement(
+    "INSERT OR IGNORE INTO moz_icons_to_pages (page_id, icon_id) "
+      "SELECT :id, icon_id "
+      "FROM moz_icons_to_pages "
+      "WHERE page_id = (SELECT id FROM moz_pages_w_icons WHERE page_url_hash = hash(:url) AND page_url = :url) "
+  );
+  NS_ENSURE_STATE(stmt);
+  mozStorageStatementScoper scoper(stmt);
+  rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("id"), mToPage.id);
+  NS_ENSURE_SUCCESS(rv, rv);
+  rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("url"), mFromPage.spec);
+  NS_ENSURE_SUCCESS(rv, rv);
+  rv = stmt->Execute();
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  // Setting this will make us send pageChanged notifications.
+  // The scope exit will take care of the callback and notifications.
+  icon.status |= ICON_STATUS_ASSOCIATED;
+
+  return NS_OK;
+}
+
 } // namespace places
 } // namespace mozilla
--- a/toolkit/components/places/FaviconHelpers.h
+++ b/toolkit/components/places/FaviconHelpers.h
@@ -349,10 +349,40 @@ public:
 private:
   nsresult ConvertPayload(int64_t aId, const nsACString& aMimeType,
                           nsCString& aPayload, int32_t* aWidth);
   nsresult StorePayload(int64_t aId, int32_t aWidth, const nsCString& aPayload);
 
   nsCOMPtr<mozIStorageConnection> mDB;
 };
 
+/**
+ * Copies Favicons from one page to another one.
+ */
+class AsyncCopyFavicons final : public Runnable
+{
+public:
+  NS_DECL_NSIRUNNABLE
+
+  /**
+   * Constructor.
+   *
+   * @param aFromPage
+   *        The originating page.
+   * @param aToPage
+   *        The destination page.
+   * @param aFaviconLoadPrivate
+   *        Whether this favicon load is in private browsing.
+   * @param aCallback
+   *        An optional callback to invoke when done.
+   */
+  AsyncCopyFavicons(PageData& aFromPage,
+                    PageData& aToPage,
+                    nsIFaviconDataCallback* aCallback);
+
+private:
+  PageData mFromPage;
+  PageData mToPage;
+  nsMainThreadPtrHandle<nsIFaviconDataCallback> mCallback;
+};
+
 } // namespace places
 } // namespace mozilla
--- a/toolkit/components/places/mozIAsyncFavicons.idl
+++ b/toolkit/components/places/mozIAsyncFavicons.idl
@@ -42,20 +42,20 @@ interface mozIAsyncFavicons : nsISupport
    * @param aForceReload
    *        If aForceReload is false, we try to reload the favicon only if we
    *        don't have it or it has expired from the cache.  Setting
    *        aForceReload to true causes us to reload the favicon even if we
    *        have a usable copy.
    * @param aFaviconLoadType
    *        Set to FAVICON_LOAD_PRIVATE if the favicon is loaded from a private
    *        browsing window.  Set to FAVICON_LOAD_NON_PRIVATE otherwise.
-   * @param aCallback
+   * @param [optional] aCallback
    *        Once we're done setting and/or fetching the favicon, we invoke this
    *        callback.
-   * @param aLoadingPrincipal
+   * @param [optional] aLoadingPrincipal
    *        Principal of the page whose favicon is being set. If this argument
    *        is omitted, the loadingPrincipal defaults to the nullPrincipal.
    *
    * @see nsIFaviconDataCallback in nsIFaviconService.idl.
    */
   mozIPlacesPendingOperation setAndFetchFaviconForPage(
     in nsIURI aPageURI,
     in nsIURI aFaviconURI,
@@ -93,17 +93,17 @@ interface mozIAsyncFavicons : nsISupport
    * @param aData
    *        Binary contents of the favicon to save
    * @param aDataLength
    *        Length of binary data
    * @param aMimeType
    *        MIME type of the data to store.  This is important so that we know
    *        what to report when the favicon is used.  You should always set this
    *        param unless you are clearing an icon.
-   * @param aExpiration
+   * @param [optional] aExpiration
    *        Time in microseconds since the epoch when this favicon expires.
    *        Until this time, we won't try to load it again.
    * @throws NS_ERROR_FAILURE
    *         Thrown if the favicon is overbloated and won't be saved to the db.
    */
   void replaceFaviconData(in nsIURI aFaviconURI,
                           [const,array,size_is(aDataLen)] in octet aData,
                           in unsigned long aDataLen,
@@ -116,20 +116,20 @@ interface mozIAsyncFavicons : nsISupport
    *
    * @see replaceFaviconData
    *
    * @param aFaviconURI
    *        URI of the favicon whose data is being set.
    * @param aDataURL
    *        string containing a data URL that represents the contents of
    *        the favicon to save
-   * @param aExpiration
+   * @param [optional] aExpiration
    *        Time in microseconds since the epoch when this favicon expires.
    *        Until this time, we won't try to load it again.
-   * @param aLoadingPrincipal
+   * @param [optional] aLoadingPrincipal
    *        Principal of the page whose favicon is being set. If this argument
    *        is omitted, the loadingPrincipal defaults to the nullPrincipal.
    * @throws NS_ERROR_FAILURE
    *         Thrown if the favicon is overbloated and won't be saved to the db.
    */
   void replaceFaviconDataFromDataURL(in nsIURI aFaviconURI,
                                      in AString aDataURL,
                                      [optional] in PRTime aExpiration,
@@ -142,17 +142,17 @@ interface mozIAsyncFavicons : nsISupport
    *        URI of the page whose favicon URI we're looking up.
    * @param aCallback
    *        This callback is always invoked to notify the result of the lookup.
    *        The aURI parameter will be the favicon URI, or null when no favicon
    *        is associated with the page or an error occurred while fetching it.
    *        aDataLen will be always 0, aData will be an empty array, and
    *        aMimeType will be an empty string, regardless of whether a favicon
    *        was found.
-   * @param aPreferredWidth
+   * @param [optional] aPreferredWidth
    *        The preferred icon width, 0 for the biggest available.
    *
    * @note If a favicon specific to this page cannot be found, this will try to
    *       fallback to the /favicon.ico for the root domain.
    *
    * @see nsIFaviconDataCallback in nsIFaviconService.idl.
    */
   void getFaviconURLForPage(in nsIURI aPageURI,
@@ -169,19 +169,39 @@ interface mozIAsyncFavicons : nsISupport
    * @param aCallback
    *        This callback is always invoked to notify the result of the lookup.  The aURI
    *        parameter will be the favicon URI, or null when no favicon is
    *        associated with the page or an error occurred while fetching it.  If
    *        aURI is not null, the other parameters may contain the favicon data.
    *        However, if no favicon data is currently associated with the favicon
    *        URI, aDataLen will be 0, aData will be an empty array, and aMimeType
    *        will be an empty string.
-   * @param aPreferredWidth
+   * @param [optional] aPreferredWidth
    *        The preferred icon width, 0 for the biggest available.
    * @note If a favicon specific to this page cannot be found, this will try to
    *       fallback to the /favicon.ico for the root domain.
    *
    * @see nsIFaviconDataCallback in nsIFaviconService.idl.
    */
   void getFaviconDataForPage(in nsIURI aPageURI,
                              in nsIFaviconDataCallback aCallback,
                              [optional] in unsigned short aPreferredWidth);
+
+  /**
+   * Copies cached favicons from a page to another one.
+   *
+   * @param aFromPageURI
+   *        URI of the originating page.
+   * @param aToPageURI
+   *        URI of the destination page.
+   * @param aFaviconLoadType
+   *        Set to FAVICON_LOAD_PRIVATE if the copy is started from a private
+   *        browsing window.  Set to FAVICON_LOAD_NON_PRIVATE otherwise.
+   * @param [optional] aCallback
+   *        Once we're done copying the favicon, we invoke this callback.
+   *        If a copy has been done, the callback will report one of the
+   *        favicons uri as aFaviconURI, otherwise all the params will be null.
+   */
+  void copyFavicons(in nsIURI aFromPageURI,
+                    in nsIURI aToPageURI,
+                    in unsigned long aFaviconLoadType,
+                    [optional] in nsIFaviconDataCallback aCallback);
 };
--- a/toolkit/components/places/nsFaviconService.cpp
+++ b/toolkit/components/places/nsFaviconService.cpp
@@ -630,16 +630,55 @@ nsFaviconService::GetFaviconDataForPage(
     new AsyncGetFaviconDataForPage(pageSpec, pageHost, aPreferredWidth, aCallback);
   RefPtr<Database> DB = Database::GetDatabase();
   NS_ENSURE_STATE(DB);
   DB->DispatchToAsyncThread(event);
 
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsFaviconService::CopyFavicons(nsIURI* aFromPageURI,
+                               nsIURI* aToPageURI,
+                               uint32_t aFaviconLoadType,
+                               nsIFaviconDataCallback* aCallback)
+{
+  MOZ_ASSERT(NS_IsMainThread());
+  NS_ENSURE_ARG(aFromPageURI);
+  NS_ENSURE_ARG(aToPageURI);
+  NS_ENSURE_TRUE(aFaviconLoadType >= nsIFaviconService::FAVICON_LOAD_PRIVATE &&
+                 aFaviconLoadType <= nsIFaviconService::FAVICON_LOAD_NON_PRIVATE,
+                 NS_ERROR_INVALID_ARG);
+
+  PageData fromPage;
+  nsresult rv = aFromPageURI->GetSpec(fromPage.spec);
+  NS_ENSURE_SUCCESS(rv, rv);
+  PageData toPage;
+  rv = aToPageURI->GetSpec(toPage.spec);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  bool canAddToHistory;
+  nsNavHistory* navHistory = nsNavHistory::GetHistoryService();
+  NS_ENSURE_TRUE(navHistory, NS_ERROR_OUT_OF_MEMORY);
+  rv = navHistory->CanAddURI(aToPageURI, &canAddToHistory);
+  NS_ENSURE_SUCCESS(rv, rv);
+  toPage.canAddToHistory = !!canAddToHistory &&
+                           aFaviconLoadType != nsIFaviconService::FAVICON_LOAD_PRIVATE;
+
+  RefPtr<AsyncCopyFavicons> event = new AsyncCopyFavicons(fromPage, toPage, aCallback);
+
+  // Get the target thread and start the work.
+  // DB will be updated and observers notified when done.
+  RefPtr<Database> DB = Database::GetDatabase();
+  NS_ENSURE_STATE(DB);
+  DB->DispatchToAsyncThread(event);
+
+  return NS_OK;
+}
+
 nsresult
 nsFaviconService::GetFaviconLinkForIcon(nsIURI* aFaviconURI,
                                         nsIURI** aOutputURI)
 {
   NS_ENSURE_ARG(aFaviconURI);
   NS_ENSURE_ARG_POINTER(aOutputURI);
 
   nsAutoCString spec;
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/favicons/test_copyFavicons.js
@@ -0,0 +1,123 @@
+const TEST_URI1 = Services.io.newURI("http://mozilla.com/");
+const TEST_URI2 = Services.io.newURI("http://places.com/");
+const TEST_URI3 = Services.io.newURI("http://bookmarked.com/");
+const LOAD_NON_PRIVATE = PlacesUtils.favicons.FAVICON_LOAD_NON_PRIVATE;
+const LOAD_PRIVATE = PlacesUtils.favicons.FAVICON_LOAD_PRIVATE;
+
+function copyFavicons(source, dest, inPrivate) {
+  return new Promise(resolve => {
+     PlacesUtils.favicons.copyFavicons(source, dest, inPrivate ? LOAD_PRIVATE
+                                                               : LOAD_NON_PRIVATE,
+                                       resolve);
+  });
+}
+
+function promisePageChanged() {
+  return new Promise(resolve => {
+    let observer = new NavHistoryObserver();
+    observer.onPageChanged = (uri, attribute, newValue, guid) => {
+      do_print("onPageChanged for attribute " + attribute + " and uri " + uri.spec);
+      if (attribute == Ci.nsINavHistoryObserver.ATTRIBUTE_FAVICON) {
+        PlacesUtils.history.removeObserver(observer);
+        Assert.ok(newValue, "newValue should be a valid value");
+        Assert.ok(guid, "Guid should be a valid value");
+        resolve(uri);
+      }
+    };
+    PlacesUtils.history.addObserver(observer, false);
+  });
+}
+
+add_task(async function test_copyFavicons_inputcheck() {
+  Assert.throws(() => PlacesUtils.favicons.copyFavicons(null, TEST_URI2, LOAD_PRIVATE),
+                /NS_ERROR_ILLEGAL_VALUE/);
+  Assert.throws(() => PlacesUtils.favicons.copyFavicons(TEST_URI1, null, LOAD_PRIVATE),
+                /NS_ERROR_ILLEGAL_VALUE/);
+  Assert.throws(() => PlacesUtils.favicons.copyFavicons(TEST_URI1, TEST_URI2, 3),
+                /NS_ERROR_ILLEGAL_VALUE/);
+  Assert.throws(() => PlacesUtils.favicons.copyFavicons(TEST_URI1, TEST_URI2, -1),
+                /NS_ERROR_ILLEGAL_VALUE/);
+  Assert.throws(() => PlacesUtils.favicons.copyFavicons(TEST_URI1, TEST_URI2, null),
+                /NS_ERROR_ILLEGAL_VALUE/);
+});
+
+add_task(async function test_copyFavicons_noop() {
+  do_print("Unknown uris");
+  Assert.equal(await copyFavicons(TEST_URI1, TEST_URI2, false),
+               null, "Icon should not have been copied");
+
+  do_print("Unknown dest uri");
+  await PlacesTestUtils.addVisits(TEST_URI1);
+  Assert.equal(await copyFavicons(TEST_URI1, TEST_URI2, false),
+               null, "Icon should not have been copied");
+
+  do_print("Unknown dest uri");
+  await PlacesTestUtils.addVisits(TEST_URI1);
+  Assert.equal(await copyFavicons(TEST_URI1, TEST_URI2, false),
+               null, "Icon should not have been copied");
+
+  do_print("Unknown dest uri, source has icon");
+  await setFaviconForPage(TEST_URI1, SMALLPNG_DATA_URI);
+  Assert.equal(await copyFavicons(TEST_URI1, TEST_URI2, false),
+               null, "Icon should not have been copied");
+
+  do_print("Known uris, source has icon, private");
+  await PlacesTestUtils.addVisits(TEST_URI2);
+  Assert.equal(await copyFavicons(TEST_URI1, TEST_URI2, true),
+               null, "Icon should not have been copied");
+
+  PlacesUtils.favicons.expireAllFavicons();
+  await PlacesTestUtils.clearHistory();
+});
+
+add_task(async function test_copyFavicons() {
+  do_print("Normal copy across 2 pages");
+  await PlacesTestUtils.addVisits(TEST_URI1);
+  await setFaviconForPage(TEST_URI1, SMALLPNG_DATA_URI);
+  await setFaviconForPage(TEST_URI1, SMALLSVG_DATA_URI);
+  await PlacesTestUtils.addVisits(TEST_URI2);
+  let promiseChange = promisePageChanged();
+  Assert.equal((await copyFavicons(TEST_URI1, TEST_URI2, false)).spec,
+               SMALLSVG_DATA_URI.spec, "Icon should have been copied");
+  Assert.equal((await promiseChange).spec,
+               TEST_URI2.spec, "Notification should have fired");
+  Assert.equal(await getFaviconUrlForPage(TEST_URI2, 1),
+               SMALLPNG_DATA_URI.spec, "Small icon found");
+  Assert.equal(await getFaviconUrlForPage(TEST_URI2),
+               SMALLSVG_DATA_URI.spec, "Large icon found");
+
+  do_print("Private copy to a bookmarked page");
+  await PlacesUtils.bookmarks.insert({
+    url: TEST_URI3, parentGuid: PlacesUtils.bookmarks.unfiledGuid
+  });
+  promiseChange = promisePageChanged();
+  Assert.equal((await copyFavicons(TEST_URI1, TEST_URI3, true)).spec,
+               SMALLSVG_DATA_URI.spec, "Icon should have been copied");
+  Assert.equal((await promiseChange).spec,
+               TEST_URI3.spec, "Notification should have fired");
+  Assert.equal(await getFaviconUrlForPage(TEST_URI3, 1),
+               SMALLPNG_DATA_URI.spec, "Small icon found");
+  Assert.equal(await getFaviconUrlForPage(TEST_URI3),
+               SMALLSVG_DATA_URI.spec, "Large icon found");
+
+  PlacesUtils.favicons.expireAllFavicons();
+  await PlacesTestUtils.clearHistory();
+});
+
+add_task(async function test_copyFavicons_overlap() {
+  do_print("Copy to a page that has one of the favicons already");
+  await PlacesTestUtils.addVisits(TEST_URI1);
+  await setFaviconForPage(TEST_URI1, SMALLPNG_DATA_URI);
+  await setFaviconForPage(TEST_URI1, SMALLSVG_DATA_URI);
+  await PlacesTestUtils.addVisits(TEST_URI2);
+  await setFaviconForPage(TEST_URI2, SMALLPNG_DATA_URI);
+  let promiseChange = promisePageChanged();
+  Assert.equal((await copyFavicons(TEST_URI1, TEST_URI2, false)).spec,
+               SMALLSVG_DATA_URI.spec, "Icon should have been copied");
+  Assert.equal((await promiseChange).spec,
+               TEST_URI2.spec, "Notification should have fired");
+  Assert.equal(await getFaviconUrlForPage(TEST_URI2, 1),
+               SMALLPNG_DATA_URI.spec, "Small icon found");
+  Assert.equal(await getFaviconUrlForPage(TEST_URI2),
+               SMALLSVG_DATA_URI.spec, "Large icon found");
+});
--- a/toolkit/components/places/tests/favicons/xpcshell.ini
+++ b/toolkit/components/places/tests/favicons/xpcshell.ini
@@ -19,16 +19,17 @@ support-files =
   favicon-multi-frame32.png
   favicon-multi-frame64.png
   favicon-normal16.png
   favicon-normal32.png
   favicon-scale160x3.jpg
   favicon-scale3x160.jpg
   noise.png
 
+[test_copyFavicons.js]
 [test_expireAllFavicons.js]
 [test_expire_migrated_icons.js]
 [test_expire_on_new_icons.js]
 [test_favicons_conversions.js]
 [test_favicons_protocols_ref.js]
 [test_getFaviconDataForPage.js]
 [test_getFaviconURLForPage.js]
 [test_heavy_favicon.js]