Skip to content

Commit 5a2c2b1

Browse files
szuendDevtools-frontend LUCI CQ
authored andcommitted
[common] Store Settings global on the global context
Since we pass the `DevToolsContext` of a universe to the TargetManager, SDKModel instances will be able to pull Settings from there instead of using the global. Bug: 490892816 Change-Id: I54eb8d698c440ed6913bed2fd7ff0236a13d3ed3 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/7649035 Reviewed-by: Kim-Anh Tran <kimanh@chromium.org> Commit-Queue: Simon Zünd <szuend@chromium.org>
1 parent e86ad58 commit 5a2c2b1

2 files changed

Lines changed: 16 additions & 10 deletions

File tree

front_end/core/common/Settings.ts

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@ import {
2222
SettingType,
2323
} from './SettingRegistration.js';
2424

25-
let settingsInstance: Settings|undefined;
26-
2725
export interface SettingsCreationOptions {
2826
syncedStorage: SettingsStorage;
2927
globalStorage: SettingsStorage;
@@ -85,7 +83,7 @@ export class Settings {
8583
}
8684

8785
static hasInstance(): boolean {
88-
return typeof settingsInstance !== 'undefined';
86+
return Root.DevToolsContext.globalInstance().has(Settings);
8987
}
9088

9189
static instance(opts: {
@@ -107,20 +105,26 @@ export class Settings {
107105
logSettingAccess,
108106
runSettingsMigration
109107
} = opts;
110-
if (!settingsInstance || forceNew) {
108+
if (!Root.DevToolsContext.globalInstance().has(Settings) || forceNew) {
111109
if (!syncedStorage || !globalStorage || !localStorage || !settingRegistrations) {
112110
throw new Error(`Unable to create settings: global and local storage must be provided: ${new Error().stack}`);
113111
}
114112

115-
settingsInstance = new Settings(
116-
{syncedStorage, globalStorage, localStorage, settingRegistrations, logSettingAccess, runSettingsMigration});
113+
Root.DevToolsContext.globalInstance().set(Settings, new Settings({
114+
syncedStorage,
115+
globalStorage,
116+
localStorage,
117+
settingRegistrations,
118+
logSettingAccess,
119+
runSettingsMigration
120+
}));
117121
}
118122

119-
return settingsInstance;
123+
return Root.DevToolsContext.globalInstance().get(Settings);
120124
}
121125

122126
static removeInstance(): void {
123-
settingsInstance = undefined;
127+
Root.DevToolsContext.globalInstance().delete(Settings);
124128
}
125129

126130
private registerModuleSetting(setting: Setting<unknown>): void {

front_end/foundation/Universe.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,14 @@ export class Universe {
1717
readonly context = new Root.DevToolsContext.DevToolsContext();
1818

1919
constructor(options: CreationOptions) {
20-
// TODO(crbug.com/458180550): Store instance on a "DevToolsContext" instead.
21-
// For now the global is fine as we don't anticipate the MCP server to change settings.
20+
// TODO(crbug.com/458180550): Store instance only on this.context instead.
21+
// For now the global is required as not everything in foundation cleanly
22+
// reads from the scoped `Settings` instance.
2223
const settings = Common.Settings.Settings.instance({
2324
forceNew: true,
2425
...options.settingsCreationOptions,
2526
});
27+
this.context.set(Common.Settings.Settings, settings);
2628

2729
const targetManager = new SDK.TargetManager.TargetManager(this.context, options.overrideAutoStartModels);
2830
this.context.set(SDK.TargetManager.TargetManager, targetManager);

0 commit comments

Comments
 (0)