Bug 1942898 - Decouple UrlbarProviderTopSites.sys.mjs from Reducers.sys.mjs r=home-newtab-reviewers,urlbar-reviewers,mconley,jteow

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

browser/components/newtab/common/Reducers.sys.mjs

@@ -5,8 +5,11 @@
 import { actionTypes as at } from "resource://activity-stream/common/Actions.mjs";
 import { Dedupe } from "resource://activity-stream/common/Dedupe.sys.mjs";
 
-export const TOP_SITES_DEFAULT_ROWS = 1;
+export {
-export const TOP_SITES_MAX_SITES_PER_ROW = 8;
+  TOP_SITES_DEFAULT_ROWS,
+  TOP_SITES_MAX_SITES_PER_ROW,
+} from "resource:///modules/topsites/constants.mjs";
+
 const PREF_COLLECTION_DISMISSIBLE = "discoverystream.isCollectionDismissible";
 
 const dedupe = new Dedupe(site => site && site.url);
@@ -174,43 +177,6 @@ function App(prevState = INITIAL_STATE.App, action) {
   }
 }
 
-/**
- * insertPinned - Inserts pinned links in their specified slots
- *
- * @param {array} a list of links
- * @param {array} a list of pinned links
- * @return {array} resulting list of links with pinned links inserted
- */
-export function insertPinned(links, pinned) {
-  // Remove any pinned links
-  const pinnedUrls = pinned.map(link => link && link.url);
-  let newLinks = links.filter(link =>
-    link ? !pinnedUrls.includes(link.url) : false
-  );
-  newLinks = newLinks.map(link => {
-    if (link && link.isPinned) {
-      delete link.isPinned;
-      delete link.pinIndex;
-    }
-    return link;
-  });
-
-  // Then insert them in their specified location
-  pinned.forEach((val, index) => {
-    if (!val) {
-      return;
-    }
-    let link = Object.assign({}, val, { isPinned: true, pinIndex: index });
-    if (index > newLinks.length) {
-      newLinks[index] = link;
-    } else {
-      newLinks.splice(index, 0, link);
-    }
-  });
-
-  return newLinks;
-}
-
 function TopSites(prevState = INITIAL_STATE.TopSites, action) {
   let hasMatch;
   let newRows;

browser/components/newtab/data/content/activity-stream.bundle.js

@@ -6557,6 +6557,14 @@ class Dedupe {
   }
 }
 
+;// CONCATENATED MODULE: ../topsites/constants.mjs
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+const TOP_SITES_DEFAULT_ROWS = 1;
+const TOP_SITES_MAX_SITES_PER_ROW = 8;
+
 ;// CONCATENATED MODULE: ./common/Reducers.sys.mjs
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -6565,8 +6573,8 @@ class Dedupe {
 
 
 
-const TOP_SITES_DEFAULT_ROWS = 1;
+
-const TOP_SITES_MAX_SITES_PER_ROW = 8;
+
 const PREF_COLLECTION_DISMISSIBLE = "discoverystream.isCollectionDismissible";
 
 const dedupe = new Dedupe(site => site && site.url);
@@ -6734,43 +6742,6 @@ function App(prevState = INITIAL_STATE.App, action) {
   }
 }
 
-/**
- * insertPinned - Inserts pinned links in their specified slots
- *
- * @param {array} a list of links
- * @param {array} a list of pinned links
- * @return {array} resulting list of links with pinned links inserted
- */
-function insertPinned(links, pinned) {
-  // Remove any pinned links
-  const pinnedUrls = pinned.map(link => link && link.url);
-  let newLinks = links.filter(link =>
-    link ? !pinnedUrls.includes(link.url) : false
-  );
-  newLinks = newLinks.map(link => {
-    if (link && link.isPinned) {
-      delete link.isPinned;
-      delete link.pinIndex;
-    }
-    return link;
-  });
-
-  // Then insert them in their specified location
-  pinned.forEach((val, index) => {
-    if (!val) {
-      return;
-    }
-    let link = Object.assign({}, val, { isPinned: true, pinIndex: index });
-    if (index > newLinks.length) {
-      newLinks[index] = link;
-    } else {
-      newLinks.splice(index, 0, link);
-    }
-  });
-
-  return newLinks;
-}
-
 function TopSites(prevState = INITIAL_STATE.TopSites, action) {
   let hasMatch;
   let newRows;

browser/components/newtab/lib/HighlightsFeed.sys.mjs

@@ -8,7 +8,7 @@ import { shortURL } from "resource://activity-stream/lib/ShortURL.sys.mjs";
 import {
   TOP_SITES_DEFAULT_ROWS,
   TOP_SITES_MAX_SITES_PER_ROW,
-} from "resource://activity-stream/common/Reducers.sys.mjs";
+} from "resource:///modules/topsites/constants.mjs";
 import { Dedupe } from "resource://activity-stream/common/Dedupe.sys.mjs";
 
 const lazy = {};

browser/components/newtab/lib/TopSitesFeed.sys.mjs

@@ -7,10 +7,8 @@ import {
   actionTypes as at,
 } from "resource://activity-stream/common/Actions.mjs";
 import { TippyTopProvider } from "resource:///modules/topsites/TippyTopProvider.sys.mjs";
-import {
+import { insertPinned } from "resource:///modules/topsites/TopSites.sys.mjs";
-  insertPinned,
+import { TOP_SITES_MAX_SITES_PER_ROW } from "resource:///modules/topsites/constants.mjs";
-  TOP_SITES_MAX_SITES_PER_ROW,
-} from "resource://activity-stream/common/Reducers.sys.mjs";
 import { Dedupe } from "resource://activity-stream/common/Dedupe.sys.mjs";
 import {
   shortURL,

browser/components/newtab/test/unit/common/Reducers.test.js

@@ -1,4 +1,4 @@
-import { INITIAL_STATE, insertPinned, reducers } from "common/Reducers.sys.mjs";
+import { INITIAL_STATE, reducers } from "common/Reducers.sys.mjs";
 const {
   TopSites,
   App,
@@ -735,76 +735,6 @@ describe("Reducers", () => {
       assert.equal(oldRow, oldState[0].rows[1]);
     });
   });
-  describe("#insertPinned", () => {
-    let links;
-
-    beforeEach(() => {
-      links = new Array(12).fill(null).map((v, i) => ({ url: `site${i}.com` }));
-    });
-
-    it("should place pinned links where they belong", () => {
-      const pinned = [
-        { url: "http://github.com/mozilla/activity-stream", title: "moz/a-s" },
-        { url: "http://example.com", title: "example" },
-      ];
-      const result = insertPinned(links, pinned);
-      for (let index of [0, 1]) {
-        assert.equal(result[index].url, pinned[index].url);
-        assert.ok(result[index].isPinned);
-        assert.equal(result[index].pinIndex, index);
-      }
-      assert.deepEqual(result.slice(2), links);
-    });
-    it("should handle empty slots in the pinned list", () => {
-      const pinned = [
-        null,
-        { url: "http://github.com/mozilla/activity-stream", title: "moz/a-s" },
-        null,
-        null,
-        { url: "http://example.com", title: "example" },
-      ];
-      const result = insertPinned(links, pinned);
-      for (let index of [1, 4]) {
-        assert.equal(result[index].url, pinned[index].url);
-        assert.ok(result[index].isPinned);
-        assert.equal(result[index].pinIndex, index);
-      }
-      result.splice(4, 1);
-      result.splice(1, 1);
-      assert.deepEqual(result, links);
-    });
-    it("should handle a pinned site past the end of the list of links", () => {
-      const pinned = [];
-      pinned[11] = {
-        url: "http://github.com/mozilla/activity-stream",
-        title: "moz/a-s",
-      };
-      const result = insertPinned([], pinned);
-      assert.equal(result[11].url, pinned[11].url);
-      assert.isTrue(result[11].isPinned);
-      assert.equal(result[11].pinIndex, 11);
-    });
-    it("should unpin previously pinned links no longer in the pinned list", () => {
-      const pinned = [];
-      links[2].isPinned = true;
-      links[2].pinIndex = 2;
-      const result = insertPinned(links, pinned);
-      assert.notProperty(result[2], "isPinned");
-      assert.notProperty(result[2], "pinIndex");
-    });
-    it("should handle a link present in both the links and pinned list", () => {
-      const pinned = [links[7]];
-      const result = insertPinned(links, pinned);
-      assert.equal(links.length, result.length);
-    });
-    it("should not modify the original data", () => {
-      const pinned = [{ url: "http://example.com" }];
-
-      insertPinned(links, pinned);
-
-      assert.equal(typeof pinned[0].isPinned, "undefined");
-    });
-  });
   describe("Pocket", () => {
     it("should return INITIAL_STATE by default", () => {
       assert.equal(

browser/components/newtab/test/unit/unit-entry.js

@@ -551,6 +551,9 @@ const TEST_GLOBAL = {
     removeExpirationFilter() {},
   },
   Logger: FakeLogger,
+  LinksCache: class {},
+  FaviconFeed: class {},
+
   getFxAccountsSingleton() {},
   AboutNewTab: {},
   Glean: {

browser/components/newtab/test/xpcshell/test_TopSitesFeed.js

@@ -22,9 +22,8 @@ ChromeUtils.defineESModuleGetters(this, {
   Screenshots: "resource://activity-stream/lib/Screenshots.sys.mjs",
   Sampling: "resource://gre/modules/components-utils/Sampling.sys.mjs",
   SearchService: "resource://gre/modules/SearchService.sys.mjs",
-  TOP_SITES_DEFAULT_ROWS: "resource://activity-stream/common/Reducers.sys.mjs",
+  TOP_SITES_DEFAULT_ROWS: "resource:///modules/topsites/constants.mjs",
-  TOP_SITES_MAX_SITES_PER_ROW:
+  TOP_SITES_MAX_SITES_PER_ROW: "resource:///modules/topsites/constants.mjs",
-    "resource://activity-stream/common/Reducers.sys.mjs",
 });
 
 const FAKE_FAVICON = "data987";

browser/components/newtab/webpack.system-addon.config.js

@@ -26,6 +26,10 @@ module.exports = (env = {}) => ({
           new RegExp("^resource://activity-stream/"),
           path.join(__dirname, "./"),
         ],
+        [
+          new RegExp("^resource:///modules/topsites/"),
+          path.join(__dirname, "../topsites/"),
+        ],
       ],
     }),
     new webpack.BannerPlugin(

browser/components/topsites/TopSites.sys.mjs

@@ -3,15 +3,12 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 import { TippyTopProvider } from "resource:///modules/topsites/TippyTopProvider.sys.mjs";
-import {
-  insertPinned,
-  TOP_SITES_MAX_SITES_PER_ROW,
-} from "resource://activity-stream/common/Reducers.sys.mjs";
 import { Dedupe } from "resource://activity-stream/common/Dedupe.sys.mjs";
 import {
   shortURL,
   shortHostname,
 } from "resource://activity-stream/lib/ShortURL.sys.mjs";
+import { TOP_SITES_MAX_SITES_PER_ROW } from "resource:///modules/topsites/constants.mjs";
 
 import {
   CUSTOM_SEARCH_SHORTCUTS,
@@ -39,6 +36,7 @@ ChromeUtils.defineLazyGetter(lazy, "log", () => {
 });
 
 export const DEFAULT_TOP_SITES = [];
+
 const FRECENCY_THRESHOLD = 100 + 1; // 1 visit (skip first-run/one-time pages)
 const MIN_FAVICON_SIZE = 96;
 const PINNED_FAVICON_PROPS_TO_MIGRATE = [
@@ -1088,4 +1086,41 @@ class _TopSites {
   }
 }
 
+/**
+ * insertPinned - Inserts pinned links in their specified slots
+ *
+ * @param {Array} links list of links
+ * @param {Array} pinned list of pinned links
+ * @returns {Array} resulting list of links with pinned links inserted
+ */
+export function insertPinned(links, pinned) {
+  // Remove any pinned links
+  const pinnedUrls = pinned.map(link => link && link.url);
+  let newLinks = links.filter(link =>
+    link ? !pinnedUrls.includes(link.url) : false
+  );
+  newLinks = newLinks.map(link => {
+    if (link && link.isPinned) {
+      delete link.isPinned;
+      delete link.pinIndex;
+    }
+    return link;
+  });
+
+  // Then insert them in their specified location
+  pinned.forEach((val, index) => {
+    if (!val) {
+      return;
+    }
+    let link = Object.assign({}, val, { isPinned: true, pinIndex: index });
+    if (index > newLinks.length) {
+      newLinks[index] = link;
+    } else {
+      newLinks.splice(index, 0, link);
+    }
+  });
+
+  return newLinks;
+}
+
 export const TopSites = new _TopSites();

browser/components/topsites/constants.mjs

@@ -0,0 +1,6 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+export const TOP_SITES_DEFAULT_ROWS = 1;
+export const TOP_SITES_MAX_SITES_PER_ROW = 8;

browser/components/topsites/moz.build

@@ -5,6 +5,7 @@
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 EXTRA_JS_MODULES.topsites += [
+    "constants.mjs",
     "TippyTopProvider.sys.mjs",
     "TopSites.sys.mjs",
 ]

browser/components/topsites/test/unit/test_top_sites.js

@@ -3,9 +3,8 @@
 
 "use strict";
 
-const { TopSites, DEFAULT_TOP_SITES } = ChromeUtils.importESModule(
+const { TopSites, insertPinned, DEFAULT_TOP_SITES } =
-  "resource:///modules/topsites/TopSites.sys.mjs"
+  ChromeUtils.importESModule("resource:///modules/topsites/TopSites.sys.mjs");
-);
 
 const { actionTypes: at } = ChromeUtils.importESModule(
   "resource://activity-stream/common/Actions.mjs"
@@ -22,9 +21,8 @@ ChromeUtils.defineESModuleGetters(this, {
   Screenshots: "resource://activity-stream/lib/Screenshots.sys.mjs",
   SearchService: "resource://gre/modules/SearchService.sys.mjs",
   TestUtils: "resource://testing-common/TestUtils.sys.mjs",
-  TOP_SITES_DEFAULT_ROWS: "resource://activity-stream/common/Reducers.sys.mjs",
+  TOP_SITES_DEFAULT_ROWS: "resource:///modules/topsites/constants.mjs",
-  TOP_SITES_MAX_SITES_PER_ROW:
+  TOP_SITES_MAX_SITES_PER_ROW: "resource:///modules/topsites/constants.mjs",
-    "resource://activity-stream/common/Reducers.sys.mjs",
 });
 
 const FAKE_FAVICON = "data987";
@@ -2504,3 +2502,99 @@ add_task(async function test_updatePinnedSearchShortcuts() {
 
   sandbox.restore();
 });
+
+add_task(async function test_insertPinned() {
+  info("#insertPinned");
+
+  function createLinks(count) {
+    return new Array(count).fill(null).map((v, i) => ({ url: `site${i}.com` }));
+  }
+
+  info("should place pinned links where they belong");
+  {
+    let links = createLinks(12);
+    const pinned = [
+      { url: "http://github.com/mozilla/activity-stream", title: "moz/a-s" },
+      { url: "http://example.com", title: "example" },
+    ];
+
+    const result = insertPinned(links, pinned);
+    for (let index of [0, 1]) {
+      Assert.equal(result[index].url, pinned[index].url, "Pinned URL matches");
+      Assert.ok(result[index].isPinned, "Link is marked as pinned");
+      Assert.equal(result[index].pinIndex, index, "Pin index is correct");
+    }
+    Assert.deepEqual(result.slice(2), links, "Remaining links are unchanged");
+  }
+
+  info("should handle empty slots in the pinned list");
+  {
+    let links = createLinks(12);
+    const pinned = [
+      null,
+      { url: "http://github.com/mozilla/activity-stream", title: "moz/a-s" },
+      null,
+      null,
+      { url: "http://example.com", title: "example" },
+    ];
+
+    const result = insertPinned(links, pinned);
+    for (let index of [1, 4]) {
+      Assert.equal(result[index].url, pinned[index].url, "Pinned URL matches");
+      Assert.ok(result[index].isPinned, "Link is marked as pinned");
+      Assert.equal(result[index].pinIndex, index, "Pin index is correct");
+    }
+    result.splice(4, 1);
+    result.splice(1, 1);
+    Assert.deepEqual(result, links, "Remaining links are unchanged");
+  }
+
+  info("should handle a pinned site past the end of the list of links");
+  {
+    const pinned = [];
+    pinned[11] = {
+      url: "http://github.com/mozilla/activity-stream",
+      title: "moz/a-s",
+    };
+
+    const result = insertPinned([], pinned);
+    Assert.equal(result[11].url, pinned[11].url, "Pinned URL matches");
+    Assert.ok(result[11].isPinned, "Link is marked as pinned");
+    Assert.equal(result[11].pinIndex, 11, "Pin index is correct");
+  }
+
+  info("should unpin previously pinned links no longer in the pinned list");
+  {
+    let links = createLinks(12);
+    const pinned = [];
+    links[2].isPinned = true;
+    links[2].pinIndex = 2;
+
+    const result = insertPinned(links, pinned);
+    Assert.ok(!result[2].isPinned, "isPinned property removed");
+    Assert.ok(!result[2].pinIndex, "pinIndex property removed");
+  }
+
+  info("should handle a link present in both the links and pinned list");
+  {
+    let links = createLinks(12);
+    const pinned = [links[7]];
+
+    const result = insertPinned(links, pinned);
+    Assert.equal(links.length, result.length, "Length of links is unchanged");
+  }
+
+  info("should not modify the original data");
+  {
+    let links = createLinks(12);
+    const pinned = [{ url: "http://example.com" }];
+
+    insertPinned(links, pinned);
+
+    Assert.equal(
+      typeof pinned[0].isPinned,
+      "undefined",
+      "Pinned data is not mutated"
+    );
+  }
+});

browser/components/urlbar/UrlbarProviderTopSites.sys.mjs

@@ -19,9 +19,8 @@ ChromeUtils.defineESModuleGetters(lazy, {
   AboutNewTab: "resource:///modules/AboutNewTab.sys.mjs",
   PlacesUtils: "resource://gre/modules/PlacesUtils.sys.mjs",
   TopSites: "resource:///modules/topsites/TopSites.sys.mjs",
-  TOP_SITES_DEFAULT_ROWS: "resource://activity-stream/common/Reducers.sys.mjs",
+  TOP_SITES_DEFAULT_ROWS: "resource:///modules/topsites/constants.mjs",
-  TOP_SITES_MAX_SITES_PER_ROW:
+  TOP_SITES_MAX_SITES_PER_ROW: "resource:///modules/topsites/constants.mjs",
-    "resource://activity-stream/common/Reducers.sys.mjs",
   UrlbarPrefs: "resource:///modules/UrlbarPrefs.sys.mjs",
   UrlbarProviderOpenTabs: "resource:///modules/UrlbarProviderOpenTabs.sys.mjs",
   UrlbarResult: "resource:///modules/UrlbarResult.sys.mjs",
@@ -158,7 +157,7 @@ class ProviderTopSites extends UrlbarProvider {
     }
 
     // This is done here, rather than in the global scope, because
-    // TOP_SITES_DEFAULT_ROWS causes the import of Reducers.sys.mjs, and we want to
+    // TOP_SITES_DEFAULT_ROWS causes import of topsites constants.mjs, and we want to
     // do that only when actually querying for Top Sites.
     if (this.topSitesRows === undefined) {
       XPCOMUtils.defineLazyPreferenceGetter(