Port bug 1524593 - nsISearchService (aka nsIBrowserSearchService, previously) refactor to be mostly an asynchronous, in preparation of WebExtension engines

2019-02-04 16:21:55 +01:00 · 2019-02-04 16:21:55 +01:00 · 20319dd55d
--- a/lib/ASRouterTargeting.jsm
+++ b/lib/ASRouterTargeting.jsm
@ -238,19 +238,14 @@ const TargetingGetters = {
  get searchEngines() {
    return new Promise(resolve => {
      // Note: calling init ensures this code is only executed after Search has been initialized
-      Services.search.init(rv => {
-        if (Components.isSuccessCode(rv)) {
-          let engines = Services.search.getVisibleEngines();
-          resolve({
-            current: Services.search.defaultEngine.identifier,
-            installed: engines
-              .map(engine => engine.identifier)
-              .filter(engine => engine),
-          });
-        } else {
-          resolve({installed: [], current: ""});
-        }
-      });
+      Services.search.getVisibleEngines().then(engines => {
+        resolve({
+          current: Services.search.defaultEngine.identifier,
+          installed: engines
+            .map(engine => engine.identifier)
+            .filter(engine => engine),
+        });
+      }).catch(() => resolve({installed: [], current: ""}));
    });
  },
  get isDefaultBrowser() {
--- a/lib/SearchShortcuts.jsm
+++ b/lib/SearchShortcuts.jsm
@ -37,8 +37,8 @@ this.getSearchProvider = getSearchProvider;

 // Check topsite against predefined list of valid search engines
 // https://searchfox.org/mozilla-central/rev/ca869724246f4230b272ed1c8b9944596e80d920/toolkit/components/search/nsSearchService.js#939
-function checkHasSearchEngine(keyword) {
-  return Services.search.getDefaultEngines()
+async function checkHasSearchEngine(keyword) {
+  return (await Services.search.getDefaultEngines())
    .find(e => e.wrappedJSObject._internalAliases.includes(keyword));
 }
 this.checkHasSearchEngine = checkHasSearchEngine;
--- a/lib/SnippetsFeed.jsm
+++ b/lib/SnippetsFeed.jsm
@ -71,20 +71,14 @@ this.SnippetsFeed = class SnippetsFeed {
  getSelectedSearchEngine() {
    return new Promise(resolve => {
      // Note: calling init ensures this code is only executed after Search has been initialized
-      Services.search.init(rv => {
-        // istanbul ignore else
-        if (Components.isSuccessCode(rv)) {
-          let engines = Services.search.getVisibleEngines();
-          resolve({
-            searchEngineIdentifier: Services.search.defaultEngine.identifier,
-            engines: engines
-              .filter(engine => engine.identifier)
-              .map(engine => `${TARGET_SEARCHENGINE_PREFIX}${engine.identifier}`),
-          });
-        } else {
-          resolve({engines: [], searchEngineIdentifier: ""});
-        }
-      });
+      Services.search.getVisibleEngines().then(engines => {
+        resolve({
+          searchEngineIdentifier: Services.search.defaultEngine.identifier,
+          engines: engines
+            .filter(engine => engine.identifier)
+            .map(engine => `${TARGET_SEARCHENGINE_PREFIX}${engine.identifier}`),
+        });
+      }).catch(() => resolve({engines: [], searchEngineIdentifier: ""}));
    });
  }

--- a/lib/TopSitesFeed.jsm
+++ b/lib/TopSitesFeed.jsm
@ -180,9 +180,7 @@ this.TopSitesFeed = class TopSitesFeed {
        Array(emptySlots).fill(null)
      );

-      await new Promise(resolve => Services.search.init(resolve));
-
-      const tryToInsertSearchShortcut = shortcut => {
+      const tryToInsertSearchShortcut = async shortcut => {
        const nextAvailable = pinnedSites.indexOf(null);
        // Only add a search shortcut if the site isn't already pinned, we
        // haven't previously inserted it, there's space to pin it, and the
@ -191,16 +189,18 @@ this.TopSitesFeed = class TopSitesFeed {
          !pinnedSites.find(s => s && s.hostname === shortcut.shortURL) &&
          !prevInsertedShortcuts.includes(shortcut.shortURL) &&
          nextAvailable > -1 &&
-          checkHasSearchEngine(shortcut.keyword)
+          await checkHasSearchEngine(shortcut.keyword)
        ) {
-          const site = this.topSiteToSearchTopSite({url: shortcut.url});
+          const site = await this.topSiteToSearchTopSite({url: shortcut.url});
          this._pinSiteAt(site, nextAvailable);
          pinnedSites[nextAvailable] = site;
          newInsertedShortcuts.push(shortcut.shortURL);
        }
      };

-      shouldPin.forEach(shortcut => tryToInsertSearchShortcut(shortcut));
+      for (let shortcut of shouldPin) {
+        await tryToInsertSearchShortcut(shortcut);
+      }

      if (newInsertedShortcuts.length) {
        this.store.dispatch(ac.SetPref(SEARCH_SHORTCUTS_HAVE_PINNED_PREF, prevInsertedShortcuts.concat(newInsertedShortcuts).join(",")));
@ -216,43 +216,42 @@ this.TopSitesFeed = class TopSitesFeed {
    const searchShortcutsExperiment = this.store.getState().Prefs.values[SEARCH_SHORTCUTS_EXPERIMENT];
    // We must wait for search services to initialize in order to access default
    // search engine properties without triggering a synchronous initialization
-    await new Promise(resolve => Services.search.init(resolve));
+    await Services.search.init();

-    // Get all frecent sites from history
-    const frecent = (await this.frecentCache.request({
+    // Get all frecent sites from history.
+    let frecent = [];
+    const cache = await this.frecentCache.request({
      // We need to overquery due to the top 5 alexa search + default search possibly being removed
      numItems: numItems + SEARCH_FILTERS.length + 1,
      topsiteFrecency: FRECENCY_THRESHOLD,
-    }))
-    .reduce((validLinks, link) => {
+    });
+    for (let link of cache) {
      const hostname = shortURL(link);
      if (!this.isExperimentOnAndLinkFilteredSearch(hostname)) {
-        validLinks.push({
-          ...(searchShortcutsExperiment ? this.topSiteToSearchTopSite(link) : link),
+        frecent.push({
+          ...(searchShortcutsExperiment ? await this.topSiteToSearchTopSite(link) : link),
          hostname,
        });
      }
-      return validLinks;
-    }, []);
+    }

-    // Remove any defaults that have been blocked
-    const notBlockedDefaultSites = DEFAULT_TOP_SITES
-      .reduce((topsites, link) => {
-        const searchProvider = getSearchProvider(shortURL(link));
-        if (NewTabUtils.blockedLinks.isBlocked({url: link.url})) {
-          return topsites;
-        } else if (this.isExperimentOnAndLinkFilteredSearch(link.hostname)) {
-          return topsites;
-          // If we've previously blocked a search shortcut, remove the default top site
-          // that matches the hostname
-        } else if (searchProvider && NewTabUtils.blockedLinks.isBlocked({url: searchProvider.url})) {
-          return topsites;
-        }
-        return [
-          ...topsites,
-          searchShortcutsExperiment ? this.topSiteToSearchTopSite(link) : link,
-        ];
-      }, []);
+    // Remove any defaults that have been blocked.
+    let notBlockedDefaultSites = [];
+    for (let link of DEFAULT_TOP_SITES) {
+      const searchProvider = getSearchProvider(shortURL(link));
+      if (NewTabUtils.blockedLinks.isBlocked({url: link.url})) {
+        continue;
+      } else if (this.isExperimentOnAndLinkFilteredSearch(link.hostname)) {
+        continue;
+        // If we've previously blocked a search shortcut, remove the default top site
+        // that matches the hostname
+      } else if (searchProvider && NewTabUtils.blockedLinks.isBlocked({url: searchProvider.url})) {
+        continue;
+      }
+      notBlockedDefaultSites.push(
+        searchShortcutsExperiment ? await this.topSiteToSearchTopSite(link) : link,
+      );
+    }

    // Get pinned links augmented with desired properties
    let plainPinned = await this.pinnedCache.request();
@ -379,8 +378,7 @@ this.TopSitesFeed = class TopSitesFeed {
    }

    // Populate the state with available search shortcuts
-    await new Promise(resolve => Services.search.init(resolve));
-    const searchShortcuts = Services.search.getDefaultEngines().reduce((result, engine) => {
+    const searchShortcuts = (await Services.search.getDefaultEngines()).reduce((result, engine) => {
      const shortcut = CUSTOM_SEARCH_SHORTCUTS.find(s => engine.wrappedJSObject._internalAliases.includes(s.keyword));
      if (shortcut) {
        result.push(this._tippyTopProvider.processSite({...shortcut}));
@ -393,9 +391,9 @@ this.TopSitesFeed = class TopSitesFeed {
    }));
  }

-  topSiteToSearchTopSite(site) {
+  async topSiteToSearchTopSite(site) {
    const searchProvider = getSearchProvider(shortURL(site));
-    if (!searchProvider || !checkHasSearchEngine(searchProvider.keyword)) {
+    if (!searchProvider || !await checkHasSearchEngine(searchProvider.keyword)) {
      return site;
    }
    return {
--- a/test/browser/browser_asrouter_targeting.js
+++ b/test/browser/browser_asrouter_targeting.js
@ -188,7 +188,7 @@ add_task(async function check_needsUpdate() {

 add_task(async function checksearchEngines() {
  const result = await ASRouterTargeting.Environment.searchEngines;
-  const expectedInstalled = Services.search.getVisibleEngines()
+  const expectedInstalled = (await Services.search.getVisibleEngines())
    .map(engine => engine.identifier)
    .sort()
    .join(",");
@ -198,14 +198,14 @@ add_task(async function checksearchEngines() {
    "searchEngines.installed should be an array of visible search engines");
  ok(result.current && typeof result.current === "string",
    "searchEngines.current should be a truthy string");
-  is(result.current, Services.search.defaultEngine.identifier,
+  is(result.current, (await Services.search.getDefault()).identifier,
    "searchEngines.current should be the current engine name");

-  const message = {id: "foo", targeting: `searchEngines[.current == ${Services.search.defaultEngine.identifier}]`};
+  const message = {id: "foo", targeting: `searchEngines[.current == ${(await Services.search.getDefault()).identifier}]`};
  is(await ASRouterTargeting.findMatchingMessage({messages: [message]}), message,
    "should select correct item by searchEngines.current");

-  const message2 = {id: "foo", targeting: `searchEngines[${Services.search.getVisibleEngines()[0].identifier} in .installed]`};
+  const message2 = {id: "foo", targeting: `searchEngines[${(await Services.search.getVisibleEngines())[0].identifier} in .installed]`};
  is(await ASRouterTargeting.findMatchingMessage({messages: [message2]}), message2,
    "should select correct item by searchEngines.installed");
 });
--- a/test/unit/lib/TopSitesFeed.test.js
+++ b/test/unit/lib/TopSitesFeed.test.js
@ -1124,7 +1124,7 @@ describe("Top Sites Feed", () => {
  describe("improvesearch.noDefaultSearchTile experiment", () => {
    const NO_DEFAULT_SEARCH_TILE_PREF = "improvesearch.noDefaultSearchTile";
    beforeEach(() => {
-      sandbox.stub(global.Services.search, "defaultEngine").value({identifier: "google", searchForm: "google.com"});
+      global.Services.search.getDefault = async () => {identifier: "google", searchForm: "google.com"};
      feed.store.state.Prefs.values[NO_DEFAULT_SEARCH_TILE_PREF] = true;
    });
    it("should filter out alexa top 5 search from the default sites", async () => {
@ -1162,7 +1162,7 @@ describe("Top Sites Feed", () => {
    });
    it("should call refresh and set ._currentSearchHostname to the new engine hostname when the the default search engine has been set", () => {
      sinon.stub(feed, "refresh");
-      sandbox.stub(global.Services.search, "defaultEngine").value({identifier: "ddg", searchForm: "duckduckgo.com"});
+      global.Services.search.getDefault = async () => {identifier: "ddg", searchForm: "duckduckgo.com"};
      feed.observe(null, "browser-search-engine-modified", "engine-current");
      assert.equal(feed._currentSearchHostname, "duckduckgo");
      assert.calledOnce(feed.refresh);
@ -1187,7 +1187,7 @@ describe("Top Sites Feed", () => {
        {wrappedJSObject: {_internalAliases: ["@google"]}},
        {wrappedJSObject: {_internalAliases: ["@amazon"]}},
      ];
-      global.Services.search.getDefaultEngines = () => searchEngines;
+      global.Services.search.getDefaultEngines = async () => searchEngines;
      fakeNewTabUtils.pinnedLinks.pin = sinon.stub().callsFake((site, index) => {
        fakeNewTabUtils.pinnedLinks.links[index] = site;
      });
@ -1367,7 +1367,7 @@ describe("Top Sites Feed", () => {

      it("should not pin a shortcut if the corresponding search engine is not available", async () => {
        // Make Amazon search engine unavailable
-        global.Services.search.getDefaultEngines = () => [{wrappedJSObject: {_internalAliases: ["@google"]}}];
+        global.Services.search.getDefaultEngines = async () => [{wrappedJSObject: {_internalAliases: ["@google"]}}];
        fakeNewTabUtils.pinnedLinks.links.fill(null);
        await feed._maybeInsertSearchShortcuts(fakeNewTabUtils.pinnedLinks.links);
        assert.notOk(fakeNewTabUtils.pinnedLinks.links.find(s => s && s.url === "https://amazon.com"));