Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-10-09 03:15:11 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1680273 - Move ASRouter telemetry call from child actor to parent r=k88hudson

Browse Source 
Differential Revision: https://phabricator.services.mozilla.com/D98472
This commit is contained in:
Andrei Oprea 2020-12-10 19:19:45 +00:00
parent 13ce8a64aa
commit b0cf793df1
9 changed files with 119 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
browser/components/newtab/actors/ASRouterChild.jsm
View File

@ -9,17 +9,12 @@ const EXPORTED_SYMBOLS = ["ASRouterChild"];
const { MESSAGE_TYPE_LIST, MESSAGE_TYPE_HASH: msg } = ChromeUtils.import(
"resource://activity-stream/common/ActorConstants.jsm"
);
const { ASRouterTelemetry } = ChromeUtils.import(
"resource://activity-stream/lib/ASRouterTelemetry.jsm"
);
const VALID_TYPES = new Set(MESSAGE_TYPE_LIST);
class ASRouterChild extends JSWindowActorChild {
constructor() {
super();
this.observers = new Set();
this.telemetry = new ASRouterTelemetry({ isParentProcess: false });
}
didDestroy() {
@ -92,15 +87,6 @@ class ASRouterChild extends JSWindowActorChild {
asRouterMessage({ type, data }) {
if (VALID_TYPES.has(type)) {
switch (type) {
// these messages are telemetry and can be done client side
case msg.AS_ROUTER_TELEMETRY_USER_EVENT:
case msg.TOOLBAR_BADGE_TELEMETRY:
case msg.TOOLBAR_PANEL_TELEMETRY:
case msg.MOMENTS_PAGE_TELEMETRY:
case msg.DOORHANGER_TELEMETRY: {
return this.telemetry.sendTelemetry({ type, data });
}
// these messages don't need a repsonse
case msg.DISABLE_PROVIDER:
case msg.ENABLE_PROVIDER:
case msg.EXPIRE_QUERY_CACHE:

6
browser/components/newtab/lib/ASRouterParentProcessMessageHandler.jsm
View File

@ -30,7 +30,6 @@ class ASRouterParentProcessMessageHandler {
handleCFRAction({ type, data }, browser) {
switch (type) {
case msg.AS_ROUTER_TELEMETRY_USER_EVENT:
case msg.TOOLBAR_BADGE_TELEMETRY:
case msg.TOOLBAR_PANEL_TELEMETRY:
case msg.MOMENTS_PAGE_TELEMETRY:
@ -45,6 +44,11 @@ class ASRouterParentProcessMessageHandler {
handleMessage(name, data, { id: tabId, browser } = { browser: null }) {
switch (name) {
case msg.AS_ROUTER_TELEMETRY_USER_EVENT:
return this.handleTelemetry({
type: msg.AS_ROUTER_TELEMETRY_USER_EVENT,
data,
});
case msg.BLOCK_MESSAGE_BY_ID: {
// Block the message but don't dismiss it in case the action taken has
// another state that needs to be visible

22
browser/components/newtab/lib/TelemetryFeed.jsm
View File

@ -118,18 +118,15 @@ XPCOMUtils.defineLazyGetter(
);
this.TelemetryFeed = class TelemetryFeed {
constructor({ isParentProcess = true } = {}) {
constructor() {
this.sessions = new Map();
this._prefs = new Prefs();
this._impressionId = this.getOrCreateImpressionId();
this._aboutHomeSeen = false;
this._classifySite = classifySite;
this._addWindowListeners = this._addWindowListeners.bind(this);
this._browserOpenNewtabStart = null;
this.handleEvent = this.handleEvent.bind(this);
if (isParentProcess && !this._prefs.get(PREF_IMPRESSION_ID)) {
const id = String(gUUIDGenerator.generateUUID());
this._prefs.set(PREF_IMPRESSION_ID, id);
}
}
get telemetryEnabled() {
@ -231,8 +228,13 @@ this.TelemetryFeed = class TelemetryFeed {
return pinnedTabs;
}
get _impressionId() {
return this._prefs.get(PREF_IMPRESSION_ID);
getOrCreateImpressionId() {
let impressionId = this._prefs.get(PREF_IMPRESSION_ID);
if (!impressionId) {
impressionId = String(gUUIDGenerator.generateUUID());
this._prefs.set(PREF_IMPRESSION_ID, impressionId);
}
return impressionId;
}
browserOpenNewtabStart() {
@ -689,10 +691,8 @@ this.TelemetryFeed = class TelemetryFeed {
* Per Bug 1485069, all the metrics for Snippets in AS router use client_id in
* all the release channels
*/
applySnippetsPolicy(ping) {
// XXX Bug 1677723
// ping.client_id = await this.telemetryClientId;
ping.client_id = this._impressionId;
async applySnippetsPolicy(ping) {
ping.client_id = await this.telemetryClientId;
delete ping.action;
return { ping, pingType: "snippets" };
}

13
browser/components/newtab/test/browser/browser_asrouter_cfr.js
View File

@ -11,6 +11,10 @@ const { CFRMessageProvider } = ChromeUtils.import(
"resource://activity-stream/lib/CFRMessageProvider.jsm"
);
const { TelemetryFeed } = ChromeUtils.import(
"resource://activity-stream/lib/TelemetryFeed.jsm"
);
const createDummyRecommendation = ({
action,
category,
@ -218,6 +222,10 @@ add_task(async function setup() {
});
add_task(async function test_cfr_notification_show() {
const sendPingStub = sinon.stub(
TelemetryFeed.prototype,
"sendStructuredIngestionEvent"
);
// addRecommendation checks that scheme starts with http and host matches
let browser = gBrowser.selectedBrowser;
BrowserTestUtils.loadURI(browser, "http://example.com/");
@ -269,6 +277,11 @@ add_task(async function test_cfr_notification_show() {
0,
"Should have removed the notification"
);
Assert.ok(sendPingStub.callCount >= 1, "Recorded some events");
let cfrPing = sendPingStub.args.find(args => args[2] === "cfr");
Assert.equal(cfrPing[0].source, "CFR", "Got a CFR event");
sendPingStub.restore();
});
add_task(async function test_cfr_notification_show() {

66
browser/components/newtab/test/browser/browser_asrouter_snippets.js
View File

@ -4,6 +4,10 @@ const { ASRouter } = ChromeUtils.import(
"resource://activity-stream/lib/ASRouter.jsm"
);
const { TelemetryFeed } = ChromeUtils.import(
"resource://activity-stream/lib/TelemetryFeed.jsm"
);
add_task(async function render_below_search_snippet() {
ASRouter._validPreviewEndpoint = () => true;
await BrowserTestUtils.withNewTab(
@ -125,3 +129,65 @@ add_task(async function render_preview_snippet() {
}
);
});
add_task(async function test_snippets_telemetry() {
await SpecialPowers.pushPrefEnv({
set: [
[
"browser.newtabpage.activity-stream.asrouter.providers.snippets",
`{"id":"snippets","enabled":true,"type":"remote","url":"https://example.com/browser/browser/components/newtab/test/browser/snippet.json","updateCycleInMs":0}`,
],
["browser.newtabpage.activity-stream.feeds.snippets", true],
],
});
const sendPingStub = sinon.stub(
TelemetryFeed.prototype,
"sendStructuredIngestionEvent"
);
await BrowserTestUtils.withNewTab(
{
gBrowser,
// Work around any issues caching might introduce by navigating to
// about blank first
url: "about:blank",
},
async browser => {
await BrowserTestUtils.loadURI(browser, "about:home");
await BrowserTestUtils.browserLoaded(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 .SimpleSnippet"
).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"
);
}
);
Assert.ok(sendPingStub.callCount >= 1, "We registered some pings");
const snippetsPing = sendPingStub.args.find(args => args[2] === "snippets");
Assert.ok(snippetsPing, "Found the snippets ping");
Assert.equal(
snippetsPing[0].event,
"IMPRESSION",
"It's the correct ping type"
);
sendPingStub.restore();
});

23
browser/components/newtab/test/unit/asrouter/ASRouterChild.test.js
View File

@ -30,29 +30,6 @@ describe("ASRouterChild", () => {
asRouterChild = null;
});
describe("asRouterMessage", () => {
describe("sends telemetry types to telemetry", () => {
[
msg.AS_ROUTER_TELEMETRY_USER_EVENT,
msg.TOOLBAR_BADGE_TELEMETRY,
msg.TOOLBAR_PANEL_TELEMETRY,
msg.MOMENTS_PAGE_TELEMETRY,
msg.DOORHANGER_TELEMETRY,
].forEach(type => {
it(`type ${type}`, () => {
asRouterChild.asRouterMessage({
type,
data: {
something: 1,
},
});
sandbox.assert.calledOnce(asRouterChild.telemetry.sendTelemetry);
sandbox.assert.calledWith(asRouterChild.telemetry.sendTelemetry, {
type,
data: { something: 1 },
});
});
});
});
describe("uses sendAsyncMessage for types that don't need an async response", () => {
[
msg.DISABLE_PROVIDER,

22
browser/components/newtab/test/unit/asrouter/ASRouterParentProcessMessageHandler.test.js
View File

@ -121,7 +121,7 @@ describe("ASRouterParentProcessMessageHandler", () => {
].forEach(type => {
it(`telemetry type "${type}" is sent to telemetry`, () => {
handler.handleCFRAction({
type: msg.AS_ROUTER_TELEMETRY_USER_EVENT,
type,
data: { id: 1 },
});
assert.calledOnce(config.sendTelemetry);
@ -129,7 +129,25 @@ describe("ASRouterParentProcessMessageHandler", () => {
});
});
});
describe("handleMessage", () => {
describe("#handleMessage", () => {
it("#default: should throw for unknown msg types", () => {
handler.handleMessage("err").then(
() => assert.fail("It should not succeed"),
() => assert.ok(true)
);
});
describe("#AS_ROUTER_TELEMETRY_USER_EVENT", () => {
it("should route AS_ROUTER_TELEMETRY_USER_EVENT to handleTelemetry", async () => {
const data = { data: "foo" };
await handler.handleMessage(msg.AS_ROUTER_TELEMETRY_USER_EVENT, data);
assert.calledOnce(handler.handleTelemetry);
assert.calledWithExactly(handler.handleTelemetry, {
type: msg.AS_ROUTER_TELEMETRY_USER_EVENT,
data,
});
});
});
describe("BLOCK_MESSAGE_BY_ID action", () => {
it("with preventDismiss returns false", async () => {
const result = await handler.handleMessage(msg.BLOCK_MESSAGE_BY_ID, {

15
browser/components/newtab/test/unit/lib/TelemetryFeed.test.js
View File

@ -101,16 +101,8 @@ describe("TelemetryFeed", () => {
ASRouterPreferences.uninit();
});
describe("#init", () => {
it("should set preferences in parent process", () => {
const testInstance = new TelemetryFeed({ isParentProcess: true });
// unfortuntely testing gUUIDGenerator.generateUUID is not possible here
// but we still need test coverage
assert.isDefined(testInstance);
});
it("should not set preferences in content process", () => {
const testInstance = new TelemetryFeed({ isParentProcess: false });
// unfortuntely testing gUUIDGenerator.generateUUID is not possible here
// but we still need test coverage
it("should create an instance", () => {
const testInstance = new TelemetryFeed();
assert.isDefined(testInstance);
});
it("should add .pingCentre, a PingCentre instance", () => {
@ -901,8 +893,7 @@ describe("TelemetryFeed", () => {
const { ping, pingType } = await instance.applySnippetsPolicy(data);
assert.equal(pingType, "snippets");
// XXX Bug 1677723
assert.propertyVal(ping, "client_id", FAKE_UUID);
assert.propertyVal(ping, "client_id", FAKE_TELEMETRY_ID);
assert.propertyVal(ping, "message_id", "snippets_message_01");
});
});

9
browser/modules/PingCentre.jsm
View File

@ -33,9 +33,6 @@ const STRUCTURED_INGESTION_SEND_TIMEOUT = 30 * 1000; // 30 seconds
const FHR_UPLOAD_ENABLED_PREF = "datareporting.healthreport.uploadEnabled";
const IS_MAIN_PROCESS =
Services.appinfo.processType === Services.appinfo.PROCESS_TYPE_DEFAULT;
/**
* Observe various notifications and send them to a telemetry endpoint.
*
@ -82,12 +79,8 @@ class PingCentre {
}
_createExperimentsPayload() {
let experiments = {};
// XXX Workaround for bug 1677723
if (!IS_MAIN_PROCESS) {
return experiments;
}
let activeExperiments = TelemetryEnvironment.getActiveExperiments();
let experiments = {};
for (let experimentID in activeExperiments) {
if (
activeExperiments[experimentID] &&