Skip to content

Commit 4996fdb

Browse files
kzarkzar
committed
Bug 1861445 - Add the runtime.onPerformanceWarning WebExtension event r=zombie,robwu
When an extension's content script is very slow and causes a webpage to hang noticeably, a warning banner is displayed to the user. It would be useful to also notify the extension developer when that happens, so that they can address the issue. Let's add a new event runtime.onPerformanceWarning that can be dispatched when the browser needs to warn an extension of runtime performance issues. For now, let's just dispatch that event when the slow extension warning banner is displayed to the user. See also w3c/webextensions#456 Differential Revision: https://phabricator.services.mozilla.com/D194708
1 parent 859effd commit 4996fdb

File tree

5 files changed

+240
-0
lines changed

5 files changed

+240
-0
lines changed

‎browser/components/extensions/test/browser/browser.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,8 @@ run-if = ["crashreporter"]
381381

382382
["browser_ext_request_permissions.js"]
383383

384+
["browser_ext_runtime_onPerformanceWarning.js"]
385+
384386
["browser_ext_runtime_openOptionsPage.js"]
385387

386388
["browser_ext_runtime_openOptionsPage_uninstall.js"]
@@ -455,6 +457,7 @@ https_first_disabled = true
455457
skip-if = [
456458
"debug",
457459
"asan",
460+
"tsan" # Bug 1874317
458461
]
459462

460463
["browser_ext_tab_runtimeConnect.js"]

‎browser/components/extensions/test/browser/browser_ext_runtime_onPerformanceWarning.js

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
/* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
2+
/* vim: set sts=2 sw=2 et tw=80: */
3+
"use strict";
4+
5+
const {
6+
Management: {
7+
global: { tabTracker },
8+
},
9+
} = ChromeUtils.importESModule("resource://gre/modules/Extension.sys.mjs");
10+
11+
const {
12+
ExtensionUtils: { promiseObserved },
13+
} = ChromeUtils.importESModule("resource://gre/modules/ExtensionUtils.sys.mjs");
14+
15+
class TestHangReport {
16+
constructor(addonId, scriptBrowser) {
17+
this.addonId = addonId;
18+
this.scriptBrowser = scriptBrowser;
19+
this.QueryInterface = ChromeUtils.generateQI(["nsIHangReport"]);
20+
}
21+
22+
userCanceled() {}
23+
terminateScript() {}
24+
25+
isReportForBrowserOrChildren(frameLoader) {
26+
return (
27+
!this.scriptBrowser || this.scriptBrowser.frameLoader === frameLoader
28+
);
29+
}
30+
}
31+
32+
function dispatchHangReport(extensionId, scriptBrowser) {
33+
const hangObserved = promiseObserved("process-hang-report");
34+
35+
Services.obs.notifyObservers(
36+
new TestHangReport(extensionId, scriptBrowser),
37+
"process-hang-report"
38+
);
39+
40+
return hangObserved;
41+
}
42+
43+
function background() {
44+
let onPerformanceWarningDetails = null;
45+
46+
browser.runtime.onPerformanceWarning.addListener(details => {
47+
onPerformanceWarningDetails = details;
48+
});
49+
50+
browser.test.onMessage.addListener(message => {
51+
if (message === "get-on-performance-warning-details") {
52+
browser.test.sendMessage(
53+
"on-performance-warning-details",
54+
onPerformanceWarningDetails
55+
);
56+
onPerformanceWarningDetails = null;
57+
}
58+
});
59+
}
60+
61+
async function expectOnPerformanceWarningDetails(
62+
extension,
63+
expectedOnPerformanceWarningDetails
64+
) {
65+
extension.sendMessage("get-on-performance-warning-details");
66+
67+
let actualOnPerformanceWarningDetails = await extension.awaitMessage(
68+
"on-performance-warning-details"
69+
);
70+
Assert.deepEqual(
71+
actualOnPerformanceWarningDetails,
72+
expectedOnPerformanceWarningDetails,
73+
expectedOnPerformanceWarningDetails
74+
? "runtime.onPerformanceWarning fired with correct details"
75+
: "runtime.onPerformanceWarning didn't fire"
76+
);
77+
}
78+
79+
add_task(async function test_should_fire_on_process_hang_report() {
80+
const description =
81+
"Slow extension content script caused a page hang, user was warned.";
82+
83+
const extension = ExtensionTestUtils.loadExtension({ background });
84+
await extension.startup();
85+
86+
const notificationPromise = BrowserTestUtils.waitForGlobalNotificationBar(
87+
window,
88+
"process-hang"
89+
);
90+
91+
const tabs = await Promise.all([
92+
BrowserTestUtils.openNewForegroundTab(gBrowser),
93+
BrowserTestUtils.openNewForegroundTab(gBrowser),
94+
]);
95+
96+
// Warning event shouldn't have fired initially.
97+
await expectOnPerformanceWarningDetails(extension, null);
98+
99+
// Hang report fired for the extension and first tab. Warning event with first
100+
// tab ID expected.
101+
await dispatchHangReport(extension.id, tabs[0].linkedBrowser);
102+
await expectOnPerformanceWarningDetails(extension, {
103+
category: "content_script",
104+
severity: "high",
105+
description,
106+
tabId: tabTracker.getId(tabs[0]),
107+
});
108+
109+
// Hang report fired for different extension, no warning event expected.
110+
await dispatchHangReport("wrong-addon-id", tabs[0].linkedBrowser);
111+
await expectOnPerformanceWarningDetails(extension, null);
112+
113+
// Non-extension hang report fired, no warning event expected.
114+
await dispatchHangReport(null, tabs[0].linkedBrowser);
115+
await expectOnPerformanceWarningDetails(extension, null);
116+
117+
// Hang report fired for the extension and second tab. Warning event with
118+
// second tab ID expected.
119+
await dispatchHangReport(extension.id, tabs[1].linkedBrowser);
120+
await expectOnPerformanceWarningDetails(extension, {
121+
category: "content_script",
122+
severity: "high",
123+
description,
124+
tabId: tabTracker.getId(tabs[1]),
125+
});
126+
127+
// Hang report fired for the extension with no associated tab. Warning event
128+
// with no tab ID expected.
129+
await dispatchHangReport(extension.id, null);
130+
await expectOnPerformanceWarningDetails(extension, {
131+
category: "content_script",
132+
severity: "high",
133+
description,
134+
});
135+
136+
await Promise.all(tabs.map(BrowserTestUtils.removeTab));
137+
await extension.unload();
138+
139+
// Wait for the process-hang warning bar to be displayed, then ensure it's
140+
// cleared to avoid clobbering other tests.
141+
const notification = await notificationPromise;
142+
Assert.ok(notification.isConnected, "Notification still present");
143+
notification.buttonContainer.querySelector("[label='Stop']").click();
144+
});

‎toolkit/components/extensions/parent/ext-runtime.js

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44

55
"use strict";
66

7+
// This file expects tabTracker to be defined in the global scope (e.g.
8+
// by ext-browser.js or ext-android.js).
9+
/* global tabTracker */
10+
711
var { ExtensionParent } = ChromeUtils.importESModule(
812
"resource://gre/modules/ExtensionParent.sys.mjs"
913
);
@@ -83,6 +87,43 @@ this.runtime = class extends ExtensionAPIPersistent {
8387
},
8488
};
8589
},
90+
onPerformanceWarning({ fire }) {
91+
let { extension } = this;
92+
93+
let observer = (subject, topic) => {
94+
let report = subject.QueryInterface(Ci.nsIHangReport);
95+
96+
if (report?.addonId !== extension.id) {
97+
return;
98+
}
99+
100+
const performanceWarningEventDetails = {
101+
category: "content_script",
102+
severity: "high",
103+
description:
104+
"Slow extension content script caused a page hang, user was warned.",
105+
};
106+
107+
let scriptBrowser = report.scriptBrowser;
108+
let nativeTab =
109+
scriptBrowser?.ownerGlobal.gBrowser?.getTabForBrowser(scriptBrowser);
110+
if (nativeTab) {
111+
performanceWarningEventDetails.tabId = tabTracker.getId(nativeTab);
112+
}
113+
114+
fire.async(performanceWarningEventDetails);
115+
};
116+
117+
Services.obs.addObserver(observer, "process-hang-report");
118+
return {
119+
unregister: () => {
120+
Services.obs.removeObserver(observer, "process-hang-report");
121+
},
122+
convert(_fire, context) {
123+
fire = _fire;
124+
},
125+
};
126+
},
86127
};
87128

88129
getAPI(context) {
@@ -171,6 +212,13 @@ this.runtime = class extends ExtensionAPIPersistent {
171212
},
172213
}).api(),
173214

215+
onPerformanceWarning: new EventManager({
216+
context,
217+
module: "runtime",
218+
event: "onPerformanceWarning",
219+
extensionApi: this,
220+
}).api(),
221+
174222
reload: async () => {
175223
if (extension.upgrade) {
176224
// If there is a pending update, install it now.

‎toolkit/components/extensions/schemas/runtime.json

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,18 @@
162162
"allowedContexts": ["content", "devtools"],
163163
"description": "The reason that the event is being dispatched. 'app_update' is used when the restart is needed because the application is updated to a newer version. 'os_update' is used when the restart is needed because the browser/OS is updated to a newer version. 'periodic' is used when the system runs for more than the permitted uptime set in the enterprise policy.",
164164
"enum": ["app_update", "os_update", "periodic"]
165+
},
166+
{
167+
"id": "OnPerformanceWarningCategory",
168+
"type": "string",
169+
"enum": ["content_script"],
170+
"description": "The performance warning event category, e.g. 'content_script'."
171+
},
172+
{
173+
"id": "OnPerformanceWarningSeverity",
174+
"type": "string",
175+
"enum": ["low", "medium", "high"],
176+
"description": "The performance warning event severity. Will be 'high' for serious and user-visible issues."
165177
}
166178
],
167179
"properties": {
@@ -677,6 +689,36 @@
677689
"description": "The reason that the event is being dispatched."
678690
}
679691
]
692+
},
693+
{
694+
"name": "onPerformanceWarning",
695+
"type": "function",
696+
"description": "Fired when a runtime performance issue is detected with the extension. Observe this event to be proactively notified of runtime performance problems with the extension.",
697+
"parameters": [
698+
{
699+
"type": "object",
700+
"name": "details",
701+
"properties": {
702+
"category": {
703+
"$ref": "OnPerformanceWarningCategory",
704+
"description": "The performance warning event category, e.g. 'content_script'."
705+
},
706+
"severity": {
707+
"$ref": "OnPerformanceWarningSeverity",
708+
"description": "The performance warning event severity, e.g. 'high'."
709+
},
710+
"tabId": {
711+
"type": "integer",
712+
"optional": true,
713+
"description": "The $(ref:tabs.Tab) that the performance warning relates to, if any."
714+
},
715+
"description": {
716+
"type": "string",
717+
"description": "An explanation of what the warning means, and hopefully how to address it."
718+
}
719+
}
720+
}
721+
]
680722
}
681723
]
682724
}

‎toolkit/components/extensions/test/mochitest/test_ext_all_apis.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,13 +95,16 @@ let expectedBackgroundApis = [
9595
"runtime.onConnectExternal",
9696
"runtime.onInstalled",
9797
"runtime.onMessageExternal",
98+
"runtime.onPerformanceWarning",
9899
"runtime.onStartup",
99100
"runtime.onSuspend",
100101
"runtime.onSuspendCanceled",
101102
"runtime.onUpdateAvailable",
102103
"runtime.openOptionsPage",
103104
"runtime.reload",
104105
"runtime.setUninstallURL",
106+
"runtime.OnPerformanceWarningCategory",
107+
"runtime.OnPerformanceWarningSeverity",
105108
"theme.getCurrent",
106109
"theme.onUpdated",
107110
"types.LevelOfControl",

0 commit comments

Comments
 (0)