Bug 1423506 - Tippytop does not honor preference browser.chrome.site_icons. r?k88hudson
MozReview-Commit-ID: CKNY8tfniKq
--- a/browser/extensions/activity-stream/lib/FaviconFeed.jsm
+++ b/browser/extensions/activity-stream/lib/FaviconFeed.jsm
@@ -5,38 +5,40 @@
const {utils: Cu} = Components;
Cu.import("resource://gre/modules/XPCOMUtils.jsm");
Cu.importGlobalProperties(["fetch", "URL"]);
const {actionTypes: at} = Cu.import("resource://activity-stream/common/Actions.jsm", {});
const {PersistentCache} = Cu.import("resource://activity-stream/lib/PersistentCache.jsm", {});
-const {Prefs} = Cu.import("resource://activity-stream/lib/ActivityStreamPrefs.jsm", {});
const {getDomain} = Cu.import("resource://activity-stream/lib/TippyTopProvider.jsm", {});
XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils",
"resource://gre/modules/PlacesUtils.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "Services",
"resource://gre/modules/Services.jsm");
const FIVE_MINUTES = 5 * 60 * 1000;
const ONE_DAY = 24 * 60 * 60 * 1000;
const TIPPYTOP_UPDATE_TIME = ONE_DAY;
const TIPPYTOP_RETRY_DELAY = FIVE_MINUTES;
this.FaviconFeed = class FaviconFeed {
constructor() {
this.tippyTopNextUpdate = 0;
this.cache = new PersistentCache("tippytop", true);
- this.prefs = new Prefs();
this._sitesByDomain = null;
this.numRetries = 0;
}
+ get endpoint() {
+ return this.store.getState().Prefs.values["tippyTop.service.endpoint"];
+ }
+
async loadCachedData() {
const data = await this.cache.get("sites");
if (data && "_timestamp" in data) {
this._sitesByDomain = data;
this.tippyTopNextUpdate = data._timestamp + TIPPYTOP_UPDATE_TIME;
}
}
@@ -46,17 +48,17 @@ this.FaviconFeed = class FaviconFeed {
}
}
async refresh() {
let headers = new Headers();
if (this._sitesByDomain && this._sitesByDomain._etag) {
headers.set("If-None-Match", this._sitesByDomain._etag);
}
- let {data, etag, status} = await this.loadFromURL(this.prefs.get("tippyTop.service.endpoint"), headers);
+ let {data, etag, status} = await this.loadFromURL(this.endpoint, headers);
let failedUpdate = false;
if (status === 200) {
this._sitesByDomain = this._sitesArrayToObjectByDomain(data);
this._sitesByDomain._etag = etag;
} else if (status !== 304) {
failedUpdate = true;
}
let delay = TIPPYTOP_UPDATE_TIME;
@@ -108,16 +110,21 @@ this.FaviconFeed = class FaviconFeed {
// If _sitesByDomain is still a Promise, no data was loaded from cache or fetch.
this._sitesByDomain = {};
}
resolve(this._sitesByDomain);
}));
}
async fetchIcon(url) {
+ // Avoid initializing and fetching icons if prefs are turned off
+ if (!this.shouldFetchIcons) {
+ return;
+ }
+
const sitesByDomain = await this.getSitesByDomain();
const domain = getDomain(url);
if (domain in sitesByDomain) {
let iconUri = Services.io.newURI(sitesByDomain[domain].image_url);
// The #tippytop is to be able to identify them for telemetry.
iconUri.ref = "tippytop";
PlacesUtils.favicons.setAndFetchFaviconForPage(
Services.io.newURI(url),
@@ -125,16 +132,23 @@ this.FaviconFeed = class FaviconFeed {
false,
PlacesUtils.favicons.FAVICON_LOAD_NON_PRIVATE,
null,
Services.scriptSecurityManager.getSystemPrincipal()
);
}
}
+ /**
+ * Determine if we should be fetching and saving icons.
+ */
+ get shouldFetchIcons() {
+ return this.endpoint && Services.prefs.getBoolPref("browser.chrome.site_icons");
+ }
+
onAction(action) {
switch (action.type) {
case at.SYSTEM_TICK:
if (this._sitesByDomain) {
// No need to refresh if we haven't been initialized.
this.maybeRefresh();
}
break;
--- a/browser/extensions/activity-stream/test/unit/lib/FaviconFeed.test.js
+++ b/browser/extensions/activity-stream/test/unit/lib/FaviconFeed.test.js
@@ -1,54 +1,54 @@
"use strict";
-const injector = require("inject!lib/FaviconFeed.jsm");
-const {FakePrefs, GlobalOverrider} = require("test/unit/utils");
+const {FaviconFeed} = require("lib/FaviconFeed.jsm");
+const {GlobalOverrider} = require("test/unit/utils");
const {actionTypes: at} = require("common/Actions.jsm");
+const FAKE_ENDPOINT = "https://foo.com/";
+
describe("FaviconFeed", () => {
- let FaviconFeed;
let feed;
let globals;
let sandbox;
let clock;
+ let siteIconsPref;
beforeEach(() => {
- FakePrefs.prototype.prefs["tippyTop.service.endpoint"] = "https://foo.com/";
clock = sinon.useFakeTimers();
globals = new GlobalOverrider();
sandbox = globals.sandbox;
globals.set("PlacesUtils", {
favicons: {
setAndFetchFaviconForPage: sandbox.spy(),
FAVICON_LOAD_NON_PRIVATE: 1
}
});
+ siteIconsPref = true;
+ sandbox.stub(global.Services.prefs, "getBoolPref")
+ .withArgs("browser.chrome.site_icons").callsFake(() => siteIconsPref);
let fetchStub = globals.sandbox.stub();
globals.set("fetch", fetchStub);
fetchStub.resolves({
ok: true,
status: 200,
json: () => Promise.resolve([{
"domains": ["facebook.com"],
"image_url": "https://www.facebook.com/icon.png"
}, {
"domains": ["gmail.com", "mail.google.com"],
"image_url": "https://iconserver.com/gmail.png"
}])
});
- ({FaviconFeed} = injector({"lib/ActivityStreamPrefs.jsm": {Prefs: FakePrefs}}));
feed = new FaviconFeed();
feed.store = {
dispatch: sinon.spy(),
getState() { return this.state; },
- state: {
- Prefs: {},
- TippyTop: {initialized: false, sitesByDomain: {}}
- }
+ state: {Prefs: {values: {"tippyTop.service.endpoint": FAKE_ENDPOINT}}}
};
});
afterEach(() => {
clock.restore();
globals.restore();
});
it("should create a TopStoriesFeed", () => {
@@ -117,17 +117,17 @@ describe("FaviconFeed", () => {
});
});
describe("#refresh", () => {
it("should loadFromURL with the right URL from prefs", async () => {
feed.loadFromURL = sinon.spy(() => ({data: []}));
await feed.refresh();
assert.calledOnce(feed.loadFromURL);
- assert.calledWith(feed.loadFromURL, FakePrefs.prototype.prefs["tippyTop.service.endpoint"]);
+ assert.calledWith(feed.loadFromURL, FAKE_ENDPOINT);
});
it("should set _sitesByDomain if new sites are returned from loadFromURL", async () => {
const data = {
data: [
{"domains": ["mozilla.org"], "image_url": "https://mozilla.org/icon.png"},
{"domains": ["facebook.com"], "image_url": "https://facebook.com/icon.png"}
],
etag: "etag1234567890",
@@ -189,35 +189,62 @@ describe("FaviconFeed", () => {
feed.loadFromURL = sinon.spy(url => ({data: [], status: 200}));
await feed.refresh();
assert.equal(0, feed.numRetries);
assert.equal(24 * 60 * 60 * 1000, feed.tippyTopNextUpdate);
});
});
describe("#fetchIcon", () => {
+ let domain;
+ let url;
+ beforeEach(() => {
+ domain = "mozilla.org";
+ url = `https://${domain}/`;
+ feed._sitesByDomain = {[domain]: {url, image_url: `${url}/icon.png`}};
+ });
+
it("should setAndFetchFaviconForPage if the url is in the TippyTop data", async () => {
- let url = "https://mozilla.org";
- feed._sitesByDomain = {"mozilla.org": {url, image_url: `${url}/icon.png`}};
await feed.fetchIcon(url);
+
assert.calledOnce(global.PlacesUtils.favicons.setAndFetchFaviconForPage);
assert.calledWith(global.PlacesUtils.favicons.setAndFetchFaviconForPage,
{spec: url},
{ref: "tippytop", spec: `${url}/icon.png`},
false,
global.PlacesUtils.favicons.FAVICON_LOAD_NON_PRIVATE,
null,
undefined);
});
+ it("should NOT setAndFetchFaviconForPage if site_icons pref is false", async () => {
+ siteIconsPref = false;
+
+ await feed.fetchIcon(url);
+
+ assert.notCalled(global.PlacesUtils.favicons.setAndFetchFaviconForPage);
+ });
+ it("should NOT setAndFetchFaviconForPage if the endpoint is empty", async () => {
+ feed.store.state.Prefs.values["tippyTop.service.endpoint"] = "";
+
+ await feed.fetchIcon(url);
+
+ assert.notCalled(global.PlacesUtils.favicons.setAndFetchFaviconForPage);
+ });
it("should NOT setAndFetchFaviconForPage if the url is NOT in the TippyTop data", async () => {
- let url = "https://mozilla.org";
- feed.store.state.TippyTop.sitesByDomain["mozilla.org"] = {url, image_url: `${url}/icon.png`};
await feed.fetchIcon("https://example.com");
+
assert.notCalled(global.PlacesUtils.favicons.setAndFetchFaviconForPage);
});
+ it("should cause sites to initialize with fetched sites if no sites", async () => {
+ delete feed._sitesByDomain;
+
+ await feed.fetchIcon(url);
+
+ assert.containsAllKeys(feed._sitesByDomain, ["facebook.com", "gmail.com", "mail.google.com"]);
+ });
});
describe("#onAction", () => {
it("should maybeRefresh on SYSTEM_TICK if initialized", async () => {
feed._sitesByDomain = {"mozilla.org": {}};
feed.maybeRefresh = sinon.spy();
feed.onAction({type: at.SYSTEM_TICK});
assert.calledOnce(feed.maybeRefresh);