author | Arthur Iakab <aiakab@mozilla.com> |
Wed, 29 Jan 2020 10:48:58 +0200 | |
changeset 512051 | 0d06c43eb924e3e733a6347a02f0750b112a19c3 |
parent 512050 | 8d0d054e5d99d943c3036182215d65ff971dab50 |
child 512052 | 1de45bb038fca66a8413838e3118f3b47559f995 |
push id | 37068 |
push user | nerli@mozilla.com |
push date | Wed, 29 Jan 2020 15:51:04 +0000 |
treeherder | mozilla-central@019ae805259f [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
bugs | 1610298 |
milestone | 74.0a1 |
backs out | 28a9739ba4c402c2877f21e2e268bc862aa6e9f1 |
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 @@ -1277,17 +1277,16 @@ pref("browser.newtabpage.activity-stream // ASRouter provider configuration pref("browser.newtabpage.activity-stream.asrouter.providers.cfr", "{\"id\":\"cfr\",\"enabled\":true,\"type\":\"remote-settings\",\"bucket\":\"cfr\",\"frequency\":{\"custom\":[{\"period\":\"daily\",\"cap\":1}]},\"categories\":[\"cfrAddons\",\"cfrFeatures\"],\"updateCycleInMs\":3600000}"); pref("browser.newtabpage.activity-stream.asrouter.providers.whats-new-panel", "{\"id\":\"whats-new-panel\",\"enabled\":true,\"type\":\"remote-settings\",\"bucket\":\"whats-new-panel\",\"updateCycleInMs\":3600000}"); pref("browser.newtabpage.activity-stream.asrouter.providers.message-groups", "{\"id\":\"message-groups\",\"enabled\":true,\"type\":\"remote-settings\",\"bucket\":\"message-groups\",\"updateCycleInMs\":3600000}"); // This url, if changed, MUST continue to point to an https url. Pulling arbitrary content to inject into // this page over http opens us up to a man-in-the-middle attack that we'd rather not face. If you are a downstream // repackager of this code using an alternate snippet url, please keep your users safe pref("browser.newtabpage.activity-stream.asrouter.providers.snippets", "{\"id\":\"snippets\",\"enabled\":true,\"type\":\"remote\",\"url\":\"https://snippets.cdn.mozilla.net/%STARTPAGE_VERSION%/%NAME%/%VERSION%/%APPBUILDID%/%BUILD_TARGET%/%LOCALE%/%CHANNEL%/%OS_VERSION%/%DISTRIBUTION%/%DISTRIBUTION_VERSION%/\",\"updateCycleInMs\":14400000}"); -pref("browser.newtabpage.activity-stream.asrouter.providers.snippets-preview", "{\"id\":\"snippets-preview\",\"enabled\":true,\"type\":\"remote\",\"url\":\"\",\"updateCycleInMs\":0}"); #ifdef NIGHTLY_BUILD pref("browser.newtabpage.activity-stream.asrouter.useReleaseSnippets", true); #endif // 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);
--- a/browser/components/newtab/lib/ASRouter.jsm +++ b/browser/components/newtab/lib/ASRouter.jsm @@ -63,22 +63,21 @@ const { AttributionCode } = ChromeUtils. const TRAILHEAD_CONFIG = { DID_SEE_ABOUT_WELCOME_PREF: "trailhead.firstrun.didSeeAboutWelcome", DYNAMIC_TRIPLET_BUNDLE_LENGTH: 3, }; const INCOMING_MESSAGE_NAME = "ASRouter:child-to-parent"; const OUTGOING_MESSAGE_NAME = "ASRouter:parent-to-child"; const ONE_DAY_IN_MS = 24 * 60 * 60 * 1000; -const PREVIEW_SNIPPETS_ID = "snippets-preview"; // List of hosts for endpoints that serve router messages. // Key is allowed host, value is a name for the endpoint host. const DEFAULT_WHITELIST_HOSTS = { "activity-stream-icons.services.mozilla.com": "production", - "snippets-admin.mozilla.org": PREVIEW_SNIPPETS_ID, + "snippets-admin.mozilla.org": "preview", }; const SNIPPETS_ENDPOINT_WHITELIST = "browser.newtab.activity-stream.asrouter.whitelistHosts"; // Max possible impressions cap for any message const MAX_MESSAGE_LIFETIME_CAP = 100; const LOCAL_MESSAGE_PROVIDERS = { OnboardingMessageProvider, @@ -599,17 +598,17 @@ class _ASRouter { } } // Fetch and decode the message provider pref JSON, and update the message providers _updateMessageProviders() { const previousProviders = this.state.providers; const providers = [ // If we have added a `preview` provider, hold onto it - ...previousProviders.filter(p => p.id === PREVIEW_SNIPPETS_ID), + ...previousProviders.filter(p => p.id === "preview"), // The provider should be enabled and not have a user preference set to false ...ASRouterPreferences.providers.filter( p => p.enabled && (ASRouterPreferences.getUserPreference(p.id) !== false && // Provider is enabled or if provider has multiple categories // check that at least one category is enabled (!p.categories || @@ -694,32 +693,32 @@ class _ASRouter { } /** * Check all provided groups are enabled * @param groups Set of groups to verify * @returns bool */ hasGroupsEnabled(groups = []) { - if (groups.includes(PREVIEW_SNIPPETS_ID)) { - return true; - } return this.state.groups .filter(({ id }) => groups.includes(id)) .every(({ enabled }) => enabled); } /** * Verify that the provider block the message through the `exclude` field * @param message Message to verify * @returns bool */ isExcludedByProvider(message) { const provider = this.state.providers.find(p => p.id === message.provider); - if (provider && provider.exclude) { + if (!provider) { + return true; + } + if (provider.exclude) { return provider.exclude.includes(message.id); } return false; } /** * Fetch all message groups and update Router.state.groups * There are 3 types of groups: @@ -845,17 +844,17 @@ class _ASRouter { } // We don't need these listeners, but they may have previously been // initialised, so uninitialise them for (const triggerID of unseenListeners) { ASRouterTriggerListeners.get(triggerID).uninit(); } // We don't want to cache preview endpoints, remove them after messages are fetched - await this.setState(this._disablePreviewEndpoint(newState)); + await this.setState(this._removePreviewEndpoint(newState)); await this.cleanupImpressions(); } } async _maybeUpdateL10nAttachment() { const { localeInUse } = this.state.localeInUse; const newLocale = Services.locale.appLocaleAsBCP47; if (newLocale !== localeInUse) { @@ -1753,17 +1752,17 @@ class _ASRouter { if (!additionalHosts.length) { return DEFAULT_WHITELIST_HOSTS; } // If there are additional hosts we want to whitelist, add them as // `preview` so that the updateCycle is 0 return additionalHosts.reduce( (whitelist_hosts, host) => { - whitelist_hosts[host] = PREVIEW_SNIPPETS_ID; + whitelist_hosts[host] = "preview"; Services.console.logStringMessage(`Adding ${host} to whitelist hosts.`); return whitelist_hosts; }, { ...DEFAULT_WHITELIST_HOSTS } ); } // To be passed to ASRouterTriggerListeners @@ -1773,48 +1772,37 @@ class _ASRouter { return; } await this.onMessage({ target, data: { type: "TRIGGER", data: { trigger } }, }); } - _disablePreviewEndpoint(state) { - const previewProvider = this.state.providers.find( - p => p.id === PREVIEW_SNIPPETS_ID - ); - if (previewProvider && previewProvider.enabled) { - const providers = this.state.providers.filter( - p => p.id !== PREVIEW_SNIPPETS_ID - ); - previewProvider.url = ""; - previewProvider.enabled = false; - providers.push(previewProvider); - state.providers = providers; - } - + _removePreviewEndpoint(state) { + state.providers = state.providers.filter(p => p.id !== "preview"); return state; } async _addPreviewEndpoint(url, portID) { - const providers = [...this.state.providers].filter( - p => p.id !== PREVIEW_SNIPPETS_ID - ); - const previewProvider = this.state.providers.find( - p => p.id === PREVIEW_SNIPPETS_ID - ); - if (this._validPreviewEndpoint(url)) { + // When you view a preview snippet we want to hide all real content + const providers = [...this.state.providers]; + if ( + this._validPreviewEndpoint(url) && + !providers.find(p => p.url === url) + ) { this.dispatchToAS( - // When you view a preview snippet we want to hide all real content ac.OnlyToOneContent({ type: at.SNIPPETS_PREVIEW_MODE }, portID) ); - previewProvider.enabled = true; - previewProvider.url = url; - providers.push(previewProvider); + providers.push({ + id: "preview", + type: "remote", + url, + updateCycleInMs: 0, + }); await this.setState({ providers }); } } // Windows specific calls to write attribution data // Used by `forceAttribution` to set required targeting attributes for // RTAMO messages. This should only be called from within about:newtab#asrouter /* istanbul ignore next */ @@ -2027,19 +2015,17 @@ class _ASRouter { if (endpoint) { await this._addPreviewEndpoint(endpoint.url, target.portID); } // Load all messages await this.loadMessagesFromAllProviders(); if (endpoint) { - message = await this.handleMessageRequest({ - provider: PREVIEW_SNIPPETS_ID, - }); + message = await this.handleMessageRequest({ provider: "preview" }); // We don't want to cache preview messages, remove them after we selected the message to show if (message) { await this.setState(state => ({ messages: state.messages.filter(m => m.id !== message.id), })); } } else { @@ -2177,17 +2163,22 @@ class _ASRouter { this._storage.set("messageBlockList", messageBlockList); return { messageBlockList }; }); break; case "OVERRIDE_MESSAGE": await this.setMessageById(action.data.id, target, true, action); break; case "ADMIN_CONNECT_STATE": - await this._updateAdminState(target); + if (action.data && action.data.endpoint) { + this._addPreviewEndpoint(action.data.endpoint.url, target.portID); + await this.loadMessagesFromAllProviders(); + } else { + await this._updateAdminState(target); + } break; case "IMPRESSION": await this.addImpression(action.data); break; case "DOORHANGER_TELEMETRY": case "TOOLBAR_BADGE_TELEMETRY": case "TOOLBAR_PANEL_TELEMETRY": if (this.dispatchToAS) {
--- a/browser/components/newtab/test/browser/browser.ini +++ b/browser/components/newtab/test/browser/browser.ini @@ -1,14 +1,13 @@ [DEFAULT] support-files = blue_page.html red_page.html head.js - snippet.json prefs = browser.newtabpage.activity-stream.debug=false browser.newtabpage.activity-stream.discoverystream.enabled=true browser.newtabpage.activity-stream.discoverystream.endpoints=data: browser.newtabpage.activity-stream.feeds.section.topstories=true browser.newtabpage.activity-stream.feeds.section.topstories.options={"provider_name":""} [browser_aboutwelcome.js]
--- a/browser/components/newtab/test/browser/browser_asrouter_snippets.js +++ b/browser/components/newtab/test/browser/browser_asrouter_snippets.js @@ -59,43 +59,8 @@ test_newtab({ ); ok( !content.document.querySelector(".below-search-snippet"), "Should not find any snippets below search" ); }, }); - -add_task(async () => { - ASRouter._validPreviewEndpoint = () => true; - await BrowserTestUtils.withNewTab( - { - gBrowser, - url: - "about:newtab?endpoint=https://example.com/browser/browser/components/newtab/test/browser/snippet.json", - }, - async browser => { - let text = await SpecialPowers.spawn(browser, [], async () => { - await ContentTaskUtils.waitForCondition( - () => content.document.querySelector(".activity-stream"), - `Should render Activity Stream` - ); - await ContentTaskUtils.waitForCondition( - () => - content.document.querySelector( - "#footer-asrouter-container .SimpleSnippet" - ), - "Should find the snippet inside the footer container" - ); - - return content.document.querySelector("#footer-asrouter-container") - .innerText; - }); - - Assert.equal( - text, - "On January 30th Nightly will introduce dedicated profiles, making it simpler to run different installations of Firefox side by side. Learn what this means for you.", - "Snippet content match" - ); - } - ); -});
deleted file mode 100644 --- a/browser/components/newtab/test/browser/snippet.json +++ /dev/null @@ -1,25 +0,0 @@ -{ - "messages": [ - { - "weight": 50, - "id": "10533", - "template": "simple_snippet", - "template_version": "1.0.0", - "content": { - "icon": "", - "text": "On January 30th Nightly will introduce dedicated profiles, making it simpler to run different installations of Firefox side by side. <link0> Learn what this means for you</link0>.", - "tall": false, - "do_not_autoblock": false, - "links": { - "link0": { - "url": "https://support.mozilla.org/kb/dedicated-profiles-firefox-installation/?utm_source=desktop-snippet&utm_medium=snippet&utm_campaign=nightly_profile_management&utm_term=10533&utm_content=nightly" - } - } - }, - "campaign": "nightly-profile-management", - "targeting": "true", - "provider_url": "https://snippets.cdn.mozilla.net/6/Firefox/66.0a1/20190122215349/Darwin_x86_64-gcc3/en-US/default/Darwin%2018.0.0/default/default/", - "provider": "snippets" - } - ] -}
--- a/browser/components/newtab/test/unit/asrouter/ASRouter.test.js +++ b/browser/components/newtab/test/unit/asrouter/ASRouter.test.js @@ -45,17 +45,16 @@ const USE_REMOTE_L10N_PREF = function fakeAsyncMessage(action) { return { data: action, target: new FakeRemotePageManager() }; } // Create a message that looks like a user action function fakeExecuteUserAction(action) { return fakeAsyncMessage({ data: action, type: "USER_ACTION" }); } -// eslint-disable-next-line max-statements describe("ASRouter", () => { let Router; let globals; let channel; let sandbox; let messageBlockList; let providerBlockList; let messageImpressions; @@ -341,33 +340,29 @@ describe("ASRouter", () => { Router.state.messages, FAKE_LOCAL_MESSAGES.length + FAKE_REMOTE_MESSAGES.length ); }); it("should load additional whitelisted hosts", async () => { getStringPrefStub.returns('["whitelist.com"]'); await createRouterAndInit(); - assert.propertyVal( - Router.WHITELIST_HOSTS, - "whitelist.com", - "snippets-preview" - ); + assert.propertyVal(Router.WHITELIST_HOSTS, "whitelist.com", "preview"); // Should still include the defaults assert.lengthOf(Object.keys(Router.WHITELIST_HOSTS), 3); }); it("should fallback to defaults if pref parsing fails", async () => { getStringPrefStub.returns("err"); await createRouterAndInit(); assert.lengthOf(Object.keys(Router.WHITELIST_HOSTS), 2); assert.propertyVal( Router.WHITELIST_HOSTS, "snippets-admin.mozilla.org", - "snippets-preview" + "preview" ); assert.propertyVal( Router.WHITELIST_HOSTS, "activity-stream-icons.services.mozilla.com", "production" ); }); it("should set this.dispatchToAS to the third parameter passed to .init()", async () => { @@ -1156,38 +1151,16 @@ describe("ASRouter", () => { messageBlockList: ["foocampaign"], })); const result = await Router.handleMessageRequest({ provider: "snippets", }); assert.isNull(result); }); - it("should allow messages with undefined providers (like 'preview')", async () => { - sandbox.stub(Router, "loadMessagesFromAllProviders"); - await Router.setState(() => ({ - providers: [{ id: "snippets", exclude: ["foo"] }], - })); - - await Router.setState(() => ({ - messages: [ - { - id: "foo", - provider: "snippets-preview", - groups: ["snippets-preview"], - }, - ], - messageBlockList: ["foocampaign"], - })); - - const result = await Router.handleMessageRequest({ - provider: "snippets-preview", - }); - assert.propertyVal(result, "provider", "snippets-preview"); - }); it("should not return a message if the frequency cap has been hit", async () => { sandbox.stub(Router, "isBelowFrequencyCaps").returns(false); await Router.setState(() => ({ messages: [{ id: "foo", provider: "snippets" }], })); const result = await Router.handleMessageRequest({ provider: "snippets", }); @@ -1658,39 +1631,38 @@ describe("ASRouter", () => { assert.calledWith( msg.target.sendAsyncMessage, PARENT_TO_CHILD_MESSAGE_NAME, { type: "CLEAR_ALL" } ); }); }); describe("#_addPreviewEndpoint", () => { - beforeEach(async () => { - await Router.setState(state => { - const providers = [...state.providers]; - const snippetsPreview = { - id: "snippets-preview", - enabled: false, - type: "remote", - }; - providers.push(snippetsPreview); - return { providers }; - }); - }); it("should make a request to the provided endpoint on NEWTAB_MESSAGE_REQUEST", async () => { const url = "https://snippets-admin.mozilla.org/foo"; const msg = fakeAsyncMessage({ type: "NEWTAB_MESSAGE_REQUEST", data: { endpoint: { url } }, }); await Router.onMessage(msg); assert.calledWith(global.fetch, url); assert.lengthOf(Router.state.providers.filter(p => p.url === url), 0); }); + it("should make a request to the provided endpoint on ADMIN_CONNECT_STATE and remove the endpoint", async () => { + const url = "https://snippets-admin.mozilla.org/foo"; + const msg = fakeAsyncMessage({ + type: "ADMIN_CONNECT_STATE", + data: { endpoint: { url } }, + }); + await Router.onMessage(msg); + + assert.calledWith(global.fetch, url); + assert.lengthOf(Router.state.providers.filter(p => p.url === url), 0); + }); it("should dispatch SNIPPETS_PREVIEW_MODE when adding a preview endpoint", async () => { const url = "https://snippets-admin.mozilla.org/foo"; const msg = fakeAsyncMessage({ type: "NEWTAB_MESSAGE_REQUEST", data: { endpoint: { url } }, }); await Router.onMessage(msg); @@ -2036,33 +2008,33 @@ describe("ASRouter", () => { Router.sendNewTabMessage, sinon.match.instanceOf(FakeRemotePageManager), data ); }); it("should return the preview message if that's available and remove it from Router.state", async () => { const expectedObj = { id: "foo", - groups: ["snippets-preview"], - provider: "snippets-preview", + groups: ["preview"], + provider: "preview", }; Router.setState({ messages: [expectedObj], - providers: [{ id: "snippets-preview" }], + providers: [{ id: "preview" }], }); await Router.sendNewTabMessage(channel, { endpoint: "foo.com" }); assert.calledWith( channel.sendAsyncMessage, PARENT_TO_CHILD_MESSAGE_NAME, { type: "SET_MESSAGE", data: expectedObj } ); assert.isUndefined( - Router.state.messages.find(m => m.provider === "snippets-preview") + Router.state.messages.find(m => m.provider === "preview") ); }); it("should call _getBundledMessages if we request a message that needs to be bundled", async () => { sandbox.stub(Router, "_getBundledMessages").resolves(); // forcefully pick a message which needs to be bundled (the second message in FAKE_LOCAL_MESSAGES) const [, testMessage] = Router.state.messages; const msg = fakeAsyncMessage({ type: "OVERRIDE_MESSAGE", @@ -3542,33 +3514,16 @@ describe("ASRouter", () => { messageImpressions = { foo: [0], bar: [] }; await createRouterAndInit([ { id: "onboarding", type: "local", messages, enabled: true }, ]); assert.notCalled(Router._storage.set); assert.deepEqual(Router.state.messageImpressions, messageImpressions); }); - it("should call setState before calling cleanupImpressions", async () => { - // The call order in `loadMessagesFromAllProviders` is important - const messages = [ - { id: "foo", frequency: { lifetime: 10 } }, - { id: "bar", frequency: { lifetime: 10 } }, - ]; - const setStateStub = sandbox.stub(Router, "setState"); - const cleanupImpressionsStub = sandbox.stub( - Router, - "cleanupImpressions" - ); - await createRouterAndInit([ - { id: "onboarding", type: "local", messages, enabled: true }, - ]); - - cleanupImpressionsStub.calledImmediatelyAfter(setStateStub); - }); }); }); describe("handle targeting errors", () => { beforeEach(() => { sandbox.stub(Router, "loadMessagesFromAllProviders"); }); it("should dispatch an event when a targeting expression throws an error", async () => { @@ -4078,30 +4033,9 @@ describe("ASRouter", () => { assert.calledOnce(Router.setGroupState); assert.calledWithExactly(Router.setGroupState, { id: "unblock", value: true, }); }); }); - describe("#loadMessagesForProvider", () => { - it("should fetch json from url", async () => { - let result = await MessageLoaderUtils.loadMessagesForProvider({ - location: "http://fake.com/endpoint", - type: "json", - }); - - assert.property(result, "messages"); - assert.lengthOf(result.messages, FAKE_REMOTE_MESSAGES.length); - }); - it("should catch errors", async () => { - fetchStub.throws(); - let result = await MessageLoaderUtils.loadMessagesForProvider({ - location: "http://fake.com/endpoint", - type: "json", - }); - - assert.property(result, "messages"); - assert.lengthOf(result.messages, 0); - }); - }); });
--- a/testing/profiles/common/user.js +++ b/testing/profiles/common/user.js @@ -4,17 +4,16 @@ user_pref("app.update.checkInstallTime", user_pref("app.update.disabledForTesting", true); user_pref("browser.chrome.guess_favicon", false); user_pref("browser.dom.window.dump.enabled", true); user_pref("devtools.console.stdout.chrome", true); // Use a python-eval-able empty JSON array even though asrouter expects plain object user_pref("browser.newtabpage.activity-stream.asrouter.providers.cfr", "[]"); user_pref("browser.newtabpage.activity-stream.asrouter.providers.cfr-fxa", "[]"); user_pref("browser.newtabpage.activity-stream.asrouter.providers.snippets", "[]"); -user_pref("browser.newtabpage.activity-stream.asrouter.providers.snippets-preview", "[]"); user_pref("browser.newtabpage.activity-stream.feeds.section.topstories", false); user_pref("browser.newtabpage.activity-stream.feeds.snippets", false); user_pref("browser.newtabpage.activity-stream.tippyTop.service.endpoint", ""); user_pref("browser.newtabpage.activity-stream.discoverystream.config", "[]"); // For Activity Stream firstrun page, use an empty string to avoid fetching. user_pref("browser.newtabpage.activity-stream.fxaccounts.endpoint", ""); // Background thumbnails in particular cause grief, and disabling thumbnails