Bug 1413650 - Capture loaded page instead of relying on background screenshot r=Mardak
☠☠ backed out by 9b78a31f929a ☠ ☠
authorUrsula Sarracini
Tue, 07 Nov 2017 16:48:07 -0500
changeset 694867 44f0f3508112bb23ab23fb1c90ab062768ee959c
parent 694866 eae1b436e4a63c4fcdd3553ad1ffdb26c3e527b6
child 694868 b2dd86a66344a2717f5cae026ac47247be525712
push id88276
push userbmo:lchang@mozilla.com
push dateWed, 08 Nov 2017 12:05:35 +0000
reviewersMardak
bugs1413650
milestone58.0a1
Bug 1413650 - Capture loaded page instead of relying on background screenshot r=Mardak MozReview-Commit-ID: 7O6c9uI6P8P
browser/base/content/browser-thumbnails.js
toolkit/components/thumbnails/test/browser.ini
toolkit/components/thumbnails/test/browser_thumbnails_bg_topsites.js
--- a/browser/base/content/browser-thumbnails.js
+++ b/browser/base/content/browser-thumbnails.js
@@ -39,17 +39,16 @@ var gBrowserThumbnails = {
   _topSiteURLsRefreshTimer: null,
 
   /**
    * List of tab events we want to listen for.
    */
   _tabEvents: ["TabClose", "TabSelect"],
 
   init: function Thumbnails_init() {
-    PageThumbs.addExpirationFilter(this);
     gBrowser.addTabsProgressListener(this);
     Services.prefs.addObserver(this.PREF_DISK_CACHE_SSL, this);
     Services.prefs.addObserver(this.PREF_ACTIVITY_STREAM_ENABLED, this);
 
     this._sslDiskCacheEnabled =
       Services.prefs.getBoolPref(this.PREF_DISK_CACHE_SSL);
     this._activityStreamEnabled =
       Services.prefs.getBoolPref(this.PREF_ACTIVITY_STREAM_ENABLED);
@@ -57,17 +56,16 @@ var gBrowserThumbnails = {
     this._tabEvents.forEach(function(aEvent) {
       gBrowser.tabContainer.addEventListener(aEvent, this);
     }, this);
 
     this._timeouts = new WeakMap();
   },
 
   uninit: function Thumbnails_uninit() {
-    PageThumbs.removeExpirationFilter(this);
     gBrowser.removeTabsProgressListener(this);
     Services.prefs.removeObserver(this.PREF_DISK_CACHE_SSL, this);
     Services.prefs.removeObserver(this.PREF_ACTIVITY_STREAM_ENABLED, this);
 
     if (this._topSiteURLsRefreshTimer) {
       this._topSiteURLsRefreshTimer.cancel();
       this._topSiteURLsRefreshTimer = null;
     }
@@ -99,46 +97,36 @@ var gBrowserThumbnails = {
       case this.PREF_DISK_CACHE_SSL:
         this._sslDiskCacheEnabled =
           Services.prefs.getBoolPref(this.PREF_DISK_CACHE_SSL);
         break;
       case this.PREF_ACTIVITY_STREAM_ENABLED:
         this._activityStreamEnabled =
           Services.prefs.getBoolPref(this.PREF_ACTIVITY_STREAM_ENABLED);
         // Get the new top sites
-        XPCOMUtils.defineLazyGetter(this, "_topSiteURLs", getTopSiteURLs);
+        this.clearTopSiteURLCache();
         break;
     }
   },
 
-  /**
-   * clearTopSiteURLCache is only ever called if we've created an nsITimer,
-   * which only happens if we've loaded the tiles top sites. Therefore we only
-   * need to clear the tiles top sites (and not activity stream's top sites)
-   */
   clearTopSiteURLCache: function Thumbnails_clearTopSiteURLCache() {
     if (this._topSiteURLsRefreshTimer) {
       this._topSiteURLsRefreshTimer.cancel();
       this._topSiteURLsRefreshTimer = null;
     }
     // Delete the defined property
     delete this._topSiteURLs;
     XPCOMUtils.defineLazyGetter(this, "_topSiteURLs", getTopSiteURLs);
   },
 
   notify: function Thumbnails_notify(timer) {
     gBrowserThumbnails._topSiteURLsRefreshTimer = null;
     gBrowserThumbnails.clearTopSiteURLCache();
   },
 
-  async filterForThumbnailExpiration(aCallback) {
-    const topSites = await this._topSiteURLs;
-    aCallback(topSites);
-  },
-
   /**
    * State change progress listener for all tabs.
    */
   onStateChange: function Thumbnails_onStateChange(aBrowser, aWebProgress,
                                                    aRequest, aStateFlags, aStatus) {
     if (aStateFlags & Ci.nsIWebProgressListener.STATE_STOP &&
         aStateFlags & Ci.nsIWebProgressListener.STATE_IS_NETWORK)
       this._delayedCapture(aBrowser);
@@ -207,28 +195,33 @@ var gBrowserThumbnails = {
     } else {
       // idle callback dispatched
       window.cancelIdleCallback(timeoutData.id);
     }
   }
 };
 
 async function getTopSiteURLs() {
+  // The _topSiteURLs getter can be expensive to run, but its return value can
+  // change frequently on new profiles, so as a compromise we cache its return
+  // value as a lazy getter for 1 minute every time it's called.
+  gBrowserThumbnails._topSiteURLsRefreshTimer =
+    Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
+  gBrowserThumbnails._topSiteURLsRefreshTimer.initWithCallback(gBrowserThumbnails,
+                                                               60 * 1000,
+                                                               Ci.nsITimer.TYPE_ONE_SHOT);
   let sites = [];
   if (gBrowserThumbnails._activityStreamEnabled) {
-    sites = await NewTabUtils.activityStreamLinks.getTopSites();
+    // Get both the top sites returned by the query, and also any pinned sites
+    // that the user might have added manually that also need a screenshot.
+    // Also include top sites that don't have rich icons
+    let topSites = await NewTabUtils.activityStreamLinks.getTopSites();
+    sites.push(...topSites.filter(link => !(link.faviconSize >= 96)));
+    sites.push(...NewTabUtils.pinnedLinks.links);
   } else {
-    // The _topSiteURLs getter can be expensive to run, but its return value can
-    // change frequently on new profiles, so as a compromise we cache its return
-    // value as a lazy getter for 1 minute every time it's called.
-    gBrowserThumbnails._topSiteURLsRefreshTimer =
-      Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
-    gBrowserThumbnails._topSiteURLsRefreshTimer.initWithCallback(gBrowserThumbnails,
-                                                                 60 * 1000,
-                                                                 Ci.nsITimer.TYPE_ONE_SHOT);
     sites = NewTabUtils.links.getLinks();
   }
   return sites.reduce((urls, link) => {
     if (link) urls.push(link.url);
     return urls;
   }, []);
 }
 
--- a/toolkit/components/thumbnails/test/browser.ini
+++ b/toolkit/components/thumbnails/test/browser.ini
@@ -24,16 +24,17 @@ skip-if = !crashreporter
 [browser_thumbnails_bg_destroy_browser.js]
 [browser_thumbnails_bg_no_cookies_sent.js]
 [browser_thumbnails_bg_no_cookies_stored.js]
 [browser_thumbnails_bg_no_auth_prompt.js]
 [browser_thumbnails_bg_no_alert.js]
 [browser_thumbnails_bg_no_duplicates.js]
 [browser_thumbnails_bg_captureIfMissing.js]
 [browser_thumbnails_bg_image_capture.js]
+[browser_thumbnails_bg_topsites.js]
 [browser_thumbnails_bug726727.js]
 [browser_thumbnails_bug727765.js]
 [browser_thumbnails_bug818225.js]
 [browser_thumbnails_capture.js]
 skip-if = os == "mac" && !debug # bug 1314039
 [browser_thumbnails_expiration.js]
 [browser_thumbnails_privacy.js]
 [browser_thumbnails_redirect.js]
new file mode 100644
--- /dev/null
+++ b/toolkit/components/thumbnails/test/browser_thumbnails_bg_topsites.js
@@ -0,0 +1,42 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+const image1x1 = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAAAAAA6fptVAAAACklEQVQI12NgAAAAAgAB4iG8MwAAAABJRU5ErkJggg==";
+const image96x96 = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAGAAAAAJCAYAAADNYymqAAAAKklEQVR42u3RMQEAAAjDMFCO9CGDg1RC00lN6awGAACADQAACAAAAXjXApPGFm+IdJG9AAAAAElFTkSuQmCC";
+const baseURL = "http://mozilla${i}.com/";
+
+function* runTests() {
+  // Add 3 top sites - 2 visits each so it can pass frecency threshold of the top sites query
+  for (let i = 1; i <= 3; i++) {
+    yield PlacesTestUtils.addVisits(baseURL.replace('${i}', i));
+    yield PlacesTestUtils.addVisits(baseURL.replace('${i}', i));
+  }
+
+  // Add favicon data for 2 of the top sites
+  let faviconData = new Map();
+  faviconData.set("http://mozilla1.com/", image1x1);
+  faviconData.set("http://mozilla2.com/", image96x96);
+  yield PlacesTestUtils.addFavicons(faviconData);
+
+  // Sanity check that we've successfully added all 3 urls to top sites
+  let links = yield NewTabUtils.activityStreamLinks.getTopSites();
+  is(links[0].url, baseURL.replace('${i}', 3), "Top site has been successfully added");
+  is(links[1].url, baseURL.replace('${i}', 2), "Top site has been successfully added");
+  is(links[2].url, baseURL.replace('${i}', 1), "Top site has been successfully added");
+
+  // Now, add a pinned site so we can also fetch a screenshot for that
+  const pinnedSite = {url: baseURL.replace('${i}', 4)};
+  NewTabUtils.pinnedLinks.pin(pinnedSite, 0);
+
+  // Check that the correct sites will capture screenshots
+  gBrowserThumbnails.clearTopSiteURLCache();
+  let topSites = yield gBrowserThumbnails._topSiteURLs;
+  ok(topSites.includes("http://mozilla1.com/"), "Top site did not have a rich icon - get a screenshot");
+  ok(topSites.includes("http://mozilla3.com/"), "Top site did not have an icon - get a screenshot");
+  ok(topSites.includes("http://mozilla4.com/"), "Site is pinned - get a screenshot");
+  ok(!topSites.includes("http://mozilla2.com/"), "Top site had a rich icon - do not get a screenshot");
+
+  // Clean up
+  NewTabUtils.pinnedLinks.unpin(pinnedSite);
+  yield PlacesTestUtils.clearHistory();
+}