Skip to content

Commit a88e571

Browse files
pfaffeDevtools-frontend LUCI CQ
authored andcommitted
Tidy up ExtensionServer helpers
- Eliminate second path to add extensions for tests: We can use the regular extension registration path in tests except for the iframe. Pull the iframe setup into a separate method that tests can stub away. - Correctly reset the devtools API object between tests. With the switch to new headless we were always hitting the catch in cleanup(). Bug: none Change-Id: I2d23f15690d9e04afac8df9190e222cb442dc5a4 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4664805 Commit-Queue: Philip Pfaffe <[email protected]> Reviewed-by: Danil Somsikov <[email protected]>
1 parent b60bc49 commit a88e571

File tree

2 files changed

+37
-35
lines changed

2 files changed

+37
-35
lines changed

front_end/models/extensions/ExtensionServer.ts

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -979,18 +979,15 @@ export class ExtensionServer extends Common.ObjectWrapper.ObjectWrapper<EventTyp
979979
}
980980
}
981981

982-
addExtensionForTest(extensionInfo: Host.InspectorFrontendHostAPI.ExtensionDescriptor, origin: string): boolean
983-
|undefined {
984-
const name = extensionInfo.name || `Extension ${origin}`;
985-
const hostsPolicy = HostsPolicy.create(extensionInfo.hostsPolicy);
986-
if (!hostsPolicy) {
987-
return false;
988-
}
989-
this.registeredExtensions.set(origin, {name, hostsPolicy});
990-
return true;
982+
addExtensionFrame({startPage, name}: Host.InspectorFrontendHostAPI.ExtensionDescriptor): void {
983+
const iframe = document.createElement('iframe');
984+
iframe.src = startPage;
985+
iframe.dataset.devtoolsExtension = name;
986+
iframe.style.display = 'none';
987+
document.body.appendChild(iframe); // Only for main window.
991988
}
992989

993-
private addExtension(extensionInfo: Host.InspectorFrontendHostAPI.ExtensionDescriptor): boolean|undefined {
990+
addExtension(extensionInfo: Host.InspectorFrontendHostAPI.ExtensionDescriptor): boolean|undefined {
994991
const startPage = extensionInfo.startPage;
995992

996993
const inspectedURL = SDK.TargetManager.TargetManager.instance().primaryPageTarget()?.inspectedURL() ?? '';
@@ -1018,12 +1015,7 @@ export class ExtensionServer extends Common.ObjectWrapper.ObjectWrapper<EventTyp
10181015
const name = extensionInfo.name || `Extension ${extensionOrigin}`;
10191016
this.registeredExtensions.set(extensionOrigin, {name, hostsPolicy});
10201017
}
1021-
1022-
const iframe = document.createElement('iframe');
1023-
iframe.src = startPage;
1024-
iframe.dataset.devtoolsExtension = extensionInfo.name;
1025-
iframe.style.display = 'none';
1026-
document.body.appendChild(iframe); // Only for main window.
1018+
this.addExtensionFrame(extensionInfo);
10271019
} catch (e) {
10281020
console.error('Failed to initialize extension ' + startPage + ':' + e);
10291021
return false;

test/unittests/front_end/models/extensions/helpers.ts

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,50 +3,60 @@
33
// found in the LICENSE file.
44

55
import {type Chrome} from '../../../../../extension-api/ExtensionAPI.js';
6-
import type * as Host from '../../../../../front_end/core/host/host.js';
6+
import * as Host from '../../../../../front_end/core/host/host.js';
77
import * as Extensions from '../../../../../front_end/models/extensions/extensions.js';
8-
import {describeWithEnvironment} from '../../helpers/EnvironmentHelpers.js';
8+
import {describeWithEnvironment, setupActionRegistry} from '../../helpers/EnvironmentHelpers.js';
99

1010
interface ExtensionContext {
1111
chrome: Partial<Chrome.DevTools.Chrome>;
12+
extensionDescriptor: Extensions.ExtensionAPI.ExtensionDescriptor;
1213
}
1314

1415
export function describeWithDevtoolsExtension(
1516
title: string, extension: Partial<Host.InspectorFrontendHostAPI.ExtensionDescriptor>,
1617
fn: (this: Mocha.Suite, context: ExtensionContext) => void) {
18+
const extensionDescriptor = {
19+
startPage: `${window.location.origin}/blank.html`,
20+
name: 'TestExtension',
21+
exposeExperimentalAPIs: true,
22+
...extension,
23+
};
1724
const context: ExtensionContext = {
25+
extensionDescriptor,
1826
chrome: {},
1927
};
2028

2129
function setup() {
2230
const server = Extensions.ExtensionServer.ExtensionServer.instance({forceNew: true});
23-
const extensionDescriptor = {
24-
startPage: 'blank.html',
25-
name: 'TestExtension',
26-
exposeExperimentalAPIs: true,
27-
...extension,
28-
};
29-
server.addExtensionForTest(extensionDescriptor, window.location.origin);
30-
const chrome: Partial<Chrome.DevTools.Chrome> = {};
31-
(window as {chrome?: Partial<Chrome.DevTools.Chrome>}).chrome = chrome;
32-
self.injectedExtensionAPI(extensionDescriptor, 'main', 'dark', [], () => {}, 1, window);
33-
context.chrome = chrome;
31+
sinon.stub(server, 'addExtensionFrame');
32+
33+
sinon.stub(Host.InspectorFrontendHost.InspectorFrontendHostInstance, 'setInjectedScriptForOrigin')
34+
.callsFake((origin, _script) => {
35+
if (origin === window.location.origin) {
36+
const chrome: Partial<Chrome.DevTools.Chrome> = {};
37+
(window as {chrome?: Partial<Chrome.DevTools.Chrome>}).chrome = chrome;
38+
self.injectedExtensionAPI(extensionDescriptor, 'main', 'dark', [], () => {}, 1, window);
39+
context.chrome = chrome;
40+
}
41+
});
42+
server.addExtension(extensionDescriptor);
3443
}
3544

3645
function cleanup() {
37-
try {
38-
delete (window as {chrome?: Chrome.DevTools.Chrome}).chrome;
39-
} catch {
40-
// Eat errors in headful mode
41-
}
46+
const chrome: Partial<Chrome.DevTools.Chrome> = {};
47+
(window as {chrome?: Partial<Chrome.DevTools.Chrome>}).chrome = chrome;
48+
context.chrome = chrome;
4249
}
4350

4451
return describe(`with-extension-${title}`, function() {
4552
beforeEach(cleanup);
4653
beforeEach(setup);
4754
afterEach(cleanup);
4855

49-
describeWithEnvironment(title, fn.bind(this, context));
56+
describeWithEnvironment(title, function() {
57+
setupActionRegistry();
58+
fn.call(this, context);
59+
});
5060
});
5161
}
5262

0 commit comments

Comments
 (0)