☠☠ backed out by bb240664f5cb ☠ ☠ | |
author | Gavin Lazar Suntop <gavin@gsuntop.com> |
Fri, 23 Jul 2021 01:07:38 +0000 | |
changeset 586512 | cfd1a5096364a4a03e0dc04277f27cff357ea432 |
parent 586511 | 9bbaeeac02061196dd56b394843e05897ead1e2b |
child 586513 | 42fcc35abbd8a2caddd5b8d3932501e30d977669 |
push id | 146778 |
push user | gsuntop@getpocket.com |
push date | Fri, 23 Jul 2021 03:41:45 +0000 |
treeherder | autoland@cfd1a5096364 [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | thecount |
bugs | 1717891 |
milestone | 92.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
|
--- a/browser/app/profile/firefox.js +++ b/browser/app/profile/firefox.js @@ -1475,16 +1475,17 @@ pref("browser.newtabpage.activity-stream // The pref that controls if ASRouter uses the remote fluent files. // It's enabled by default, but could be disabled to force ASRouter to use the local files. pref("browser.newtabpage.activity-stream.asrouter.useRemoteL10n", true); // These prefs control if Discovery Stream is enabled. pref("browser.newtabpage.activity-stream.discoverystream.enabled", true); pref("browser.newtabpage.activity-stream.discoverystream.hardcoded-basic-layout", false); +pref("browser.newtabpage.activity-stream.discoverystream.spoc-positions", "2,4,11,20"); pref("browser.newtabpage.activity-stream.discoverystream.spocs-endpoint", ""); pref("browser.newtabpage.activity-stream.discoverystream.spocs-endpoint-query", ""); // List of regions that do not get stories, regardless of locale-list-config. pref("browser.newtabpage.activity-stream.discoverystream.region-stories-block", "FR"); // List of locales that get stories, regardless of region-stories-config. #ifdef NIGHTLY_BUILD pref("browser.newtabpage.activity-stream.discoverystream.locale-list-config", "en-US,en-CA,en-GB");
--- a/browser/components/newtab/lib/DiscoveryStreamFeed.jsm +++ b/browser/components/newtab/lib/DiscoveryStreamFeed.jsm @@ -404,32 +404,63 @@ this.DiscoveryStreamFeed = class Discove for (let [key, val] of params.entries()) { urlObject.searchParams.append(key, val); } return urlObject.toString(); } + parseSpocPositions(csvPositions) { + let spocPositions; + + // Only accept parseable non-negative integers + try { + spocPositions = csvPositions.map(index => { + let parsedInt = parseInt(index, 10); + + if (!isNaN(parsedInt) && parsedInt >= 0) { + return parsedInt; + } + + throw new Error("Bad input"); + }); + } catch (e) { + // Catch spoc positions that are not numbers or negative, and do nothing. + // We have hard coded backup positions. + spocPositions = undefined; + } + + return spocPositions; + } + async loadLayout(sendUpdate, isStartup) { let layoutResp = {}; let url = ""; + if (!this.config.hardcoded_layout) { layoutResp = await this.fetchLayout(isStartup); } if (!layoutResp || !layoutResp.layout) { const isBasic = this.config.hardcoded_basic_layout || this.store.getState().Prefs.values[PREF_HARDCODED_BASIC_LAYOUT] || this.store.getState().Prefs.values[PREF_REGION_BASIC_LAYOUT]; + let spocPositions = this.store + .getState() + .Prefs.values?.pocketConfig?.spocPositions?.split(`,`); + // Set a hardcoded layout if one is needed. // Changing values in this layout in memory object is unnecessary. - layoutResp = getHardcodedLayout(isBasic); + layoutResp = getHardcodedLayout( + isBasic, + this.parseSpocPositions(spocPositions) + ); } sendUpdate({ type: at.DISCOVERY_STREAM_LAYOUT_UPDATE, data: layoutResp, meta: { isStartup, }, @@ -1530,16 +1561,24 @@ this.DiscoveryStreamFeed = class Discove } }); if (changed) { this.writeDataPref(pref, impressions); } } + onPocketConfigChanged() { + // Update layout, and reload any off screen tabs. + // This does not change any existing open tabs. + // It also doesn't update any spoc or rec data, just the layout. + const dispatch = action => this.store.dispatch(ac.AlsoToPreloaded(action)); + this.loadLayout(dispatch, false); + } + async onPrefChangedAction(action) { switch (action.data.name) { case PREF_CONFIG: case PREF_ENABLED: case PREF_HARDCODED_BASIC_LAYOUT: case PREF_SPOCS_ENDPOINT: case PREF_SPOCS_ENDPOINT_QUERY: // This is a config reset directly related to Discovery Stream pref. @@ -1765,28 +1804,31 @@ this.DiscoveryStreamFeed = class Discove if (flight_id) { this.recordBlockFlightId(flight_id); } }); break; } case at.PREF_CHANGED: await this.onPrefChangedAction(action); + if (action.data.name === "pocketConfig") { + await this.onPocketConfigChanged(action.data.value); + } break; } } }; // This function generates a hardcoded layout each call. // This is because modifying the original object would // persist across pref changes and system_tick updates. // // NOTE: There is some branching logic in the template based on `isBasicLayout` // -getHardcodedLayout = isBasicLayout => ({ +getHardcodedLayout = (isBasicLayout, spocPositions = [2, 4, 11, 20]) => ({ lastUpdate: Date.now(), spocs: { url: "https://spocs.getpocket.com/spocs", }, layout: [ { width: 12, components: [ @@ -1862,30 +1904,19 @@ getHardcodedLayout = isBasicLayout => ({ }, feed: { embed_reference: null, url: "https://getpocket.cdn.mozilla.net/v3/firefox/global-recs?version=3&consumer_key=$apiKey&locale_lang=$locale®ion=$region&count=30", }, spocs: { probability: 1, - positions: [ - { - index: 2, - }, - { - index: 4, - }, - { - index: 11, - }, - { - index: 20, - }, - ], + positions: spocPositions.map(position => { + return { index: position }; + }), }, }, { type: "Navigation", properties: { alignment: "left-align", links: [ {
--- a/browser/components/newtab/lib/PrefsFeed.jsm +++ b/browser/components/newtab/lib/PrefsFeed.jsm @@ -21,16 +21,17 @@ XPCOMUtils.defineLazyModuleGetters(this, Region: "resource://gre/modules/Region.jsm", }); this.PrefsFeed = class PrefsFeed { constructor(prefMap) { this._prefMap = prefMap; this._prefs = new Prefs(); this.onExperimentUpdated = this.onExperimentUpdated.bind(this); + this.onPocketExperimentUpdated = this.onPocketExperimentUpdated.bind(this); } onPrefChanged(name, value) { const prefItem = this._prefMap.get(name); if (prefItem) { this.store.dispatch( ac[prefItem.skipBroadcast ? "OnlyToMain" : "BroadcastToContent"]({ type: at.PREF_CHANGED, @@ -72,19 +73,36 @@ this.PrefsFeed = class PrefsFeed { data: { name: "featureConfig", value, }, }) ); } + /** + * Handler for Pocket specific experiment data updates. + */ + onPocketExperimentUpdated(event, reason) { + const value = NimbusFeatures.pocketNewtab.getAllVariables() || {}; + this.store.dispatch( + ac.BroadcastToContent({ + type: at.PREF_CHANGED, + data: { + name: "pocketConfig", + value, + }, + }) + ); + } + init() { this._prefs.observeBranch(this); NimbusFeatures.newtab.onUpdate(this.onExperimentUpdated); + NimbusFeatures.pocketNewtab.onUpdate(this.onPocketExperimentUpdated); this._storage = this.store.dbStorage.getDbTable("sectionPrefs"); // Get the initial value of each activity stream pref const values = {}; for (const name of this._prefMap.keys()) { values[name] = this._prefs.get(name); } @@ -150,16 +168,17 @@ this.PrefsFeed = class PrefsFeed { ); values["urlbar.placeholderName"] = placeholderPrefValue; this._prefMap.set("urlbar.placeholderName", { value: placeholderPrefValue, }); // Add experiment values and default values values.featureConfig = NimbusFeatures.newtab.getValue() || {}; + values.pocketConfig = NimbusFeatures.pocketNewtab.getAllVariables() || {}; this._setBoolPref(values, "logowordmark.alwaysVisible", false); this._setBoolPref(values, "feeds.section.topstories", false); this._setBoolPref(values, "discoverystream.enabled", false); this._setBoolPref(values, "discoverystream.isCollectionDismissible", false); this._setBoolPref(values, "discoverystream.hardcoded-basic-layout", false); this._setBoolPref(values, "discoverystream.recs.personalized", false); this._setBoolPref(values, "discoverystream.spocs.personalized", false); this._setStringPref(
--- a/browser/components/newtab/test/unit/lib/DiscoveryStreamFeed.test.js +++ b/browser/components/newtab/test/unit/lib/DiscoveryStreamFeed.test.js @@ -224,16 +224,28 @@ describe("DiscoveryStreamFeed", () => { const result = feed.getOrCreateImpressionId(); assert.equal(result, "from get"); assert.calledOnce(global.Services.prefs.getCharPref); }); }); + describe("#parseSpocPositions", () => { + it("should return an equivalent array for an array of non negative integers", async () => { + assert.deepEqual(feed.parseSpocPositions([0, 2, 3]), [0, 2, 3]); + }); + it("should return undefined for an array containing negative integers", async () => { + assert.equal(feed.parseSpocPositions([-2, 2, 3]), undefined); + }); + it("should return undefined for an undefined input", async () => { + assert.equal(feed.parseSpocPositions(undefined), undefined); + }); + }); + describe("#loadLayout", () => { it("should fetch data and populate the cache if it is empty", async () => { const resp = { layout: ["foo", "bar"] }; const fakeCache = {}; sandbox.stub(feed.cache, "get").returns(Promise.resolve(fakeCache)); sandbox.stub(feed.cache, "set").returns(Promise.resolve()); fetchStub.resolves({ ok: true, json: () => Promise.resolve(resp) }); @@ -2077,16 +2089,26 @@ describe("DiscoveryStreamFeed", () => { await feed.onAction({ type: at.PREF_CHANGED, data: { name: "showSponsored" }, }); assert.calledOnce(feed.loadSpocs); }); + it("should fire onPocketConfigChanged when pocketConfig pref changes", async () => { + sandbox.stub(feed, "onPocketConfigChanged").returns(Promise.resolve()); + + await feed.onAction({ + type: at.PREF_CHANGED, + data: { name: "pocketConfig", value: false }, + }); + + assert.calledOnce(feed.onPocketConfigChanged); + }); it("should call clearSpocs when sponsored content is turned off", async () => { sandbox.stub(feed, "clearSpocs").returns(Promise.resolve()); await feed.onAction({ type: at.PREF_CHANGED, data: { name: "showSponsored", value: false }, }); @@ -2162,16 +2184,26 @@ describe("DiscoveryStreamFeed", () => { sandbox.stub(feed, "checkIfAnyCacheExpired").resolves(true); sandbox.stub(feed, "refreshAll").resolves(); await feed.onAction({ type: at.SYSTEM_TICK }); assert.calledWith(feed.refreshAll, { updateOpenTabs: false }); }); }); + describe("#onPocketConfigChanged", () => { + it("should call loadLayout when Pocket config changes", async () => { + sandbox.stub(feed, "loadLayout").callsFake(dispatch => dispatch("foo")); + sandbox.stub(feed.store, "dispatch"); + await feed.onPocketConfigChanged(); + assert.calledOnce(feed.loadLayout); + assert.calledWith(feed.store.dispatch, ac.AlsoToPreloaded("foo")); + }); + }); + describe("#onAction: PREF_SHOW_SPONSORED", () => { it("should call loadSpocs when preference changes", async () => { sandbox.stub(feed, "loadSpocs").resolves(); sandbox.stub(feed.store, "dispatch"); await feed.onAction({ type: at.PREF_CHANGED, data: { name: "showSponsored" },
--- a/browser/components/newtab/test/unit/lib/PrefsFeed.test.js +++ b/browser/components/newtab/test/unit/lib/PrefsFeed.test.js @@ -148,16 +148,34 @@ describe("PrefsFeed", () => { assert.calledWith( feed.store.dispatch, ac.BroadcastToContent({ type: at.PREF_CHANGED, data: { name: "foo", value: 2 }, }) ); }); + it("should send a PREF_CHANGED actions when onPocketExperimentUpdated is called", () => { + sandbox.stub(global.NimbusFeatures.newtab, "getAllVariables").returns({ + prefsButtonIcon: "icon-new", + }); + feed.onPocketExperimentUpdated(); + assert.calledWith( + feed.store.dispatch, + ac.BroadcastToContent({ + type: at.PREF_CHANGED, + data: { + name: "pocketConfig", + value: { + prefsButtonIcon: "icon-new", + }, + }, + }) + ); + }); it("should send a PREF_CHANGED actions when onExperimentUpdated is called", () => { sandbox.stub(global.NimbusFeatures.newtab, "getValue").returns({ prefsButtonIcon: "icon-new", }); feed.onExperimentUpdated(); assert.calledWith( feed.store.dispatch, ac.BroadcastToContent({
--- a/browser/components/newtab/test/unit/unit-entry.js +++ b/browser/components/newtab/test/unit/unit-entry.js @@ -461,16 +461,17 @@ const TEST_GLOBAL = { getExperiment() {}, on: () => {}, off: () => {}, }, NimbusFeatures: { newtab: { isEnabled() {}, getValue() {}, + getAllVariables() {}, onUpdate() {}, off() {}, }, }, TelemetryEnvironment: { setExperimentActive() {}, currentEnvironment: { profile: { @@ -504,14 +505,15 @@ const TEST_GLOBAL = { PageThumbs: { addExpirationFilter() {}, removeExpirationFilter() {}, }, gUUIDGenerator: { generateUUID: () => "{foo-123-foo}", }, }; +TEST_GLOBAL.NimbusFeatures.pocketNewtab = TEST_GLOBAL.NimbusFeatures.newtab; overrider.set(TEST_GLOBAL); describe("activity-stream", () => { after(() => overrider.restore()); files.forEach(file => req(file)); });
--- a/toolkit/components/nimbus/FeatureManifest.js +++ b/toolkit/components/nimbus/FeatureManifest.js @@ -108,16 +108,27 @@ const FeatureManifest = { description: "Enable the customization panel inside of the newtab", }, prefsButtonIcon: { type: "string", description: "Icon url to use for the preferences button", }, }, }, + pocketNewtab: { + description: "The Pocket section in newtab", + isEarlyStartup: true, + variables: { + spocPositions: { + type: "string", + fallbackPref: + "browser.newtabpage.activity-stream.discoverystream.spoc-positions", + }, + }, + }, "password-autocomplete": { description: "A special autocomplete UI for password fields.", variables: { directMigrateSingleProfile: { type: "boolean", description: "Enable direct migration?", }, },