Bug 1610298 - Snippets preview messages don't belong to a provider r=k88hudson

Differential Revision: https://phabricator.services.mozilla.com/D60414

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Andrei Oprea 2020-01-29 07:45:58 +00:00
Родитель 7e8b6e2ef8
Коммит de60fd5486
7 изменённых файлов: 184 добавлений и 46 удалений

Просмотреть файл

@ -1282,6 +1282,7 @@ pref("browser.newtabpage.activity-stream.asrouter.providers.message-groups", "{\
// 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

Просмотреть файл

@ -68,11 +68,12 @@ const TRAILHEAD_CONFIG = {
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-admin.mozilla.org": PREVIEW_SNIPPETS_ID,
};
const SNIPPETS_ENDPOINT_WHITELIST =
"browser.newtab.activity-stream.asrouter.whitelistHosts";
@ -603,7 +604,7 @@ class _ASRouter {
const previousProviders = this.state.providers;
const providers = [
// If we have added a `preview` provider, hold onto it
...previousProviders.filter(p => p.id === "preview"),
...previousProviders.filter(p => p.id === PREVIEW_SNIPPETS_ID),
// The provider should be enabled and not have a user preference set to false
...ASRouterPreferences.providers.filter(
p =>
@ -698,6 +699,9 @@ class _ASRouter {
* @returns bool
*/
hasGroupsEnabled(groups = []) {
if (groups.includes(PREVIEW_SNIPPETS_ID)) {
return true;
}
return this.state.groups
.filter(({ id }) => groups.includes(id))
.every(({ enabled }) => enabled);
@ -710,10 +714,7 @@ class _ASRouter {
*/
isExcludedByProvider(message) {
const provider = this.state.providers.find(p => p.id === message.provider);
if (!provider) {
return true;
}
if (provider.exclude) {
if (provider && provider.exclude) {
return provider.exclude.includes(message.id);
}
return false;
@ -849,7 +850,7 @@ class _ASRouter {
}
// We don't want to cache preview endpoints, remove them after messages are fetched
await this.setState(this._removePreviewEndpoint(newState));
await this.setState(this._disablePreviewEndpoint(newState));
await this.cleanupImpressions();
}
}
@ -1757,7 +1758,7 @@ class _ASRouter {
// `preview` so that the updateCycle is 0
return additionalHosts.reduce(
(whitelist_hosts, host) => {
whitelist_hosts[host] = "preview";
whitelist_hosts[host] = PREVIEW_SNIPPETS_ID;
Services.console.logStringMessage(`Adding ${host} to whitelist hosts.`);
return whitelist_hosts;
},
@ -1777,27 +1778,38 @@ class _ASRouter {
});
}
_removePreviewEndpoint(state) {
state.providers = state.providers.filter(p => p.id !== "preview");
_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;
}
return state;
}
async _addPreviewEndpoint(url, portID) {
// 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)
) {
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)) {
this.dispatchToAS(
// When you view a preview snippet we want to hide all real content
ac.OnlyToOneContent({ type: at.SNIPPETS_PREVIEW_MODE }, portID)
);
providers.push({
id: "preview",
type: "remote",
url,
updateCycleInMs: 0,
});
previewProvider.enabled = true;
previewProvider.url = url;
providers.push(previewProvider);
await this.setState({ providers });
}
}
@ -2020,7 +2032,9 @@ class _ASRouter {
await this.loadMessagesFromAllProviders();
if (endpoint) {
message = await this.handleMessageRequest({ provider: "preview" });
message = await this.handleMessageRequest({
provider: PREVIEW_SNIPPETS_ID,
});
// We don't want to cache preview messages, remove them after we selected the message to show
if (message) {
@ -2168,12 +2182,7 @@ class _ASRouter {
await this.setMessageById(action.data.id, target, true, action);
break;
case "ADMIN_CONNECT_STATE":
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);

Просмотреть файл

@ -3,6 +3,7 @@ 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

Просмотреть файл

@ -64,3 +64,38 @@ test_newtab({
);
},
});
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"
);
}
);
});

Просмотреть файл

@ -0,0 +1,25 @@
{
"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"
}
]
}

Просмотреть файл

@ -50,6 +50,7 @@ function fakeExecuteUserAction(action) {
return fakeAsyncMessage({ data: action, type: "USER_ACTION" });
}
// eslint-disable-next-line max-statements
describe("ASRouter", () => {
let Router;
let globals;
@ -345,7 +346,11 @@ describe("ASRouter", () => {
getStringPrefStub.returns('["whitelist.com"]');
await createRouterAndInit();
assert.propertyVal(Router.WHITELIST_HOSTS, "whitelist.com", "preview");
assert.propertyVal(
Router.WHITELIST_HOSTS,
"whitelist.com",
"snippets-preview"
);
// Should still include the defaults
assert.lengthOf(Object.keys(Router.WHITELIST_HOSTS), 3);
});
@ -357,7 +362,7 @@ describe("ASRouter", () => {
assert.propertyVal(
Router.WHITELIST_HOSTS,
"snippets-admin.mozilla.org",
"preview"
"snippets-preview"
);
assert.propertyVal(
Router.WHITELIST_HOSTS,
@ -1156,6 +1161,28 @@ describe("ASRouter", () => {
});
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(() => ({
@ -1636,6 +1663,18 @@ describe("ASRouter", () => {
});
});
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({
@ -1647,17 +1686,6 @@ describe("ASRouter", () => {
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({
@ -2013,12 +2041,12 @@ describe("ASRouter", () => {
it("should return the preview message if that's available and remove it from Router.state", async () => {
const expectedObj = {
id: "foo",
groups: ["preview"],
provider: "preview",
groups: ["snippets-preview"],
provider: "snippets-preview",
};
Router.setState({
messages: [expectedObj],
providers: [{ id: "preview" }],
providers: [{ id: "snippets-preview" }],
});
await Router.sendNewTabMessage(channel, { endpoint: "foo.com" });
@ -2029,7 +2057,7 @@ describe("ASRouter", () => {
{ type: "SET_MESSAGE", data: expectedObj }
);
assert.isUndefined(
Router.state.messages.find(m => m.provider === "preview")
Router.state.messages.find(m => m.provider === "snippets-preview")
);
});
it("should call _getBundledMessages if we request a message that needs to be bundled", async () => {
@ -3519,6 +3547,23 @@ describe("ASRouter", () => {
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);
});
});
});
@ -4038,4 +4083,25 @@ describe("ASRouter", () => {
});
});
});
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);
});
});
});

Просмотреть файл

@ -9,6 +9,7 @@ user_pref("devtools.console.stdout.chrome", true);
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", "");