Bug 1653932 - Add pref for top site defaults from remote settings. r=mikedeboer
☠☠ backed out by 466ec9e1891a ☠ ☠
authorDão Gottwald <dao@mozilla.com>
Fri, 24 Jul 2020 14:13:08 +0000
changeset 542054 bdedb095df0f3f9ba3a5fbb354b751882a184070
parent 542053 3ec1fb624570f73be6ebe42fc2d52065dfdb655c
child 542055 695cbcabf7fe8ce0bdb218baccd3758e416b2907
push id122600
push userdgottwald@mozilla.com
push dateFri, 24 Jul 2020 14:14:09 +0000
treeherderautoland@bdedb095df0f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmikedeboer
bugs1653932
milestone80.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 1653932 - Add pref for top site defaults from remote settings. r=mikedeboer Differential Revision: https://phabricator.services.mozilla.com/D84532
browser/app/profile/firefox.js
browser/components/newtab/lib/Screenshots.jsm
browser/components/newtab/lib/TopSitesFeed.jsm
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -1286,16 +1286,18 @@ pref("browser.menu.showCharacterEncoding
 
 // Allow using tab-modal prompts when possible.
 pref("prompts.tab_modal.enabled", true);
 
 // Whether prompts should be content modal (1) tab modal (2) or window modal(3) by default
 // This is a fallback value for when prompt callers do not specify a modalType.
 pref("prompts.defaultModalType", 3);
 
+pref("browser.topsites.useRemoteSetting", false);
+
 // Activates preloading of the new tab url.
 pref("browser.newtab.preload", true);
 
 // Activity Stream prefs that control to which page to redirect
 #ifndef RELEASE_OR_BETA
   pref("browser.newtabpage.activity-stream.debug", false);
 #endif
 
--- a/browser/components/newtab/lib/Screenshots.jsm
+++ b/browser/components/newtab/lib/Screenshots.jsm
@@ -116,19 +116,27 @@ this.Screenshots = {
    * @param property {string} Name of property on object to set
    @ @param onScreenshot {function} Callback for when the screenshot loads
    */
   async maybeCacheScreenshot(link, url, property, onScreenshot) {
     // If there are only private windows open, do not collect screenshots
     if (!this._shouldGetScreenshots()) {
       return;
     }
+    // __sharedCache may not exist yet for links from default top sites that
+    // don't have a default tippy top icon.
+    // eslint-disable-next-line no-unsanitized/property
+    link.__sharedCache ??= {
+      updateLink(prop, val) {
+        link[prop] = val;
+      },
+    };
+    const cache = link.__sharedCache;
     // Nothing to do if we already have a pending screenshot or
     // if a previous request failed and returned null.
-    const cache = link.__sharedCache;
     if (cache.fetchingScreenshot || link[property] !== undefined) {
       return;
     }
 
     // Save the promise to the cache so other links get it immediately
     cache.fetchingScreenshot = this.getScreenshotForURL(url);
 
     // Clean up now that we got the screenshot
--- a/browser/components/newtab/lib/TopSitesFeed.jsm
+++ b/browser/components/newtab/lib/TopSitesFeed.jsm
@@ -89,16 +89,19 @@ const SEARCH_FILTERS = [
 let SEARCH_TILE_OVERRIDE_PREFS = new Map();
 for (let searchProvider of ["amazon", "google"]) {
   SEARCH_TILE_OVERRIDE_PREFS.set(
     `browser.newtabpage.searchTileOverride.${searchProvider}.url`,
     searchProvider
   );
 }
 
+const REMOTE_SETTING_DEFAULTS_PREF = "browser.topsites.useRemoteSetting";
+const REMOTE_SETTING_OVERRIDE_PREF = "browser.topsites.default";
+
 function getShortURLForCurrentSearch() {
   const url = shortURL({ url: Services.search.defaultEngine.searchForm });
   return url;
 }
 
 this.TopSitesFeed = class TopSitesFeed {
   constructor() {
     this._tippyTopProvider = new TippyTopProvider();
@@ -120,58 +123,93 @@ this.TopSitesFeed = class TopSitesFeed {
       ...CACHED_LINK_PROPS_TO_MIGRATE,
       ...PINNED_FAVICON_PROPS_TO_MIGRATE,
     ]);
     PageThumbs.addExpirationFilter(this);
   }
 
   init() {
     // If the feed was previously disabled PREFS_INITIAL_VALUES was never received
-    this.refreshDefaults(
-      this.store.getState().Prefs.values[DEFAULT_SITES_PREF]
-    );
+    this._readDefaults();
     this._storage = this.store.dbStorage.getDbTable("sectionPrefs");
     this.refresh({ broadcast: true, isStartup: true });
     Services.obs.addObserver(this, "browser-search-engine-modified");
     for (let [pref] of SEARCH_TILE_OVERRIDE_PREFS) {
       Services.prefs.addObserver(pref, this);
     }
+    Services.prefs.addObserver(REMOTE_SETTING_DEFAULTS_PREF, this);
+    Services.prefs.addObserver(REMOTE_SETTING_OVERRIDE_PREF, this);
   }
 
   uninit() {
     PageThumbs.removeExpirationFilter(this);
     Services.obs.removeObserver(this, "browser-search-engine-modified");
     for (let [pref] of SEARCH_TILE_OVERRIDE_PREFS) {
       Services.prefs.removeObserver(pref, this);
     }
+    Services.prefs.removeObserver(REMOTE_SETTING_DEFAULTS_PREF, this);
+    Services.prefs.removeObserver(REMOTE_SETTING_OVERRIDE_PREF, this);
   }
 
   observe(subj, topic, data) {
-    // We should update the current top sites if the search engine has been changed since
-    // the search engine that gets filtered out of top sites has changed.
-    if (
-      topic === "browser-search-engine-modified" &&
-      data === "engine-default" &&
-      this.store.getState().Prefs.values[FILTER_DEFAULT_SEARCH_PREF]
-    ) {
-      delete this._currentSearchHostname;
-      this._currentSearchHostname = getShortURLForCurrentSearch();
-      this.refresh({ broadcast: true });
-    } else if (
-      topic === "nsPref:changed" &&
-      SEARCH_TILE_OVERRIDE_PREFS.has(data)
-    ) {
-      this.refresh({ broadcast: true });
+    switch (topic) {
+      case "browser-search-engine-modified":
+        // We should update the current top sites if the search engine has been changed since
+        // the search engine that gets filtered out of top sites has changed.
+        if (
+          data === "engine-default" &&
+          this.store.getState().Prefs.values[FILTER_DEFAULT_SEARCH_PREF]
+        ) {
+          delete this._currentSearchHostname;
+          this._currentSearchHostname = getShortURLForCurrentSearch();
+          this.refresh({ broadcast: true });
+        }
+        break;
+      case "nsPref:changed":
+        if (
+          data === REMOTE_SETTING_DEFAULTS_PREF ||
+          data === REMOTE_SETTING_OVERRIDE_PREF
+        ) {
+          this._readDefaults();
+          this.refresh({ broadcast: true });
+        } else if (SEARCH_TILE_OVERRIDE_PREFS.has(data)) {
+          this.refresh({ broadcast: true });
+        }
+        break;
     }
   }
 
   _dedupeKey(site) {
     return site && site.hostname;
   }
 
+  /**
+   * _readDefaults - sets DEFAULT_TOP_SITES
+   */
+  _readDefaults() {
+    this._useRemoteSetting = Services.prefs.getBoolPref(
+      REMOTE_SETTING_DEFAULTS_PREF
+    );
+
+    if (!this._useRemoteSetting) {
+      this.refreshDefaults(
+        this.store.getState().Prefs.values[DEFAULT_SITES_PREF]
+      );
+      return;
+    }
+
+    let sites;
+    try {
+      sites = Services.prefs.getStringPref(REMOTE_SETTING_OVERRIDE_PREF);
+    } catch (e) {}
+    // Placeholder for the actual remote setting (bug 1653937).
+    sites ??= "https://mozilla.org,https://firefox.com";
+    this.refreshDefaults(sites);
+  }
+
   refreshDefaults(sites) {
     // Clear out the array of any previous defaults
     DEFAULT_TOP_SITES.length = 0;
 
     // Add default sites if any based on the pref
     if (sites) {
       for (const url of sites.split(",")) {
         const site = {
@@ -879,17 +917,19 @@ this.TopSitesFeed = class TopSitesFeed {
       case at.PLACES_LINK_BLOCKED:
         this.frecentCache.expire();
         this.pinnedCache.expire();
         this.refresh({ broadcast: true });
         break;
       case at.PREF_CHANGED:
         switch (action.data.name) {
           case DEFAULT_SITES_PREF:
-            this.refreshDefaults(action.data.value);
+            if (!this._useRemoteSetting) {
+              this.refreshDefaults(action.data.value);
+            }
             break;
           case ROWS_PREF:
           case FILTER_DEFAULT_SEARCH_PREF:
           case SEARCH_SHORTCUTS_SEARCH_ENGINES_PREF:
             this.refresh({ broadcast: true });
             break;
           case SEARCH_SHORTCUTS_EXPERIMENT:
             if (action.data.value) {
@@ -901,17 +941,19 @@ this.TopSitesFeed = class TopSitesFeed {
         }
         break;
       case at.UPDATE_SECTION_PREFS:
         if (action.data.id === SECTION_ID) {
           this.updateSectionPrefs(action.data.value);
         }
         break;
       case at.PREFS_INITIAL_VALUES:
-        this.refreshDefaults(action.data[DEFAULT_SITES_PREF]);
+        if (!this._useRemoteSetting) {
+          this.refreshDefaults(action.data[DEFAULT_SITES_PREF]);
+        }
         break;
       case at.TOP_SITES_PIN:
         this.pin(action);
         break;
       case at.TOP_SITES_UNPIN:
         this.unpin(action);
         break;
       case at.TOP_SITES_INSERT: