Skip to content

Commit fa68d4e

Browse files
bmeurerDevtools-frontend LUCI CQ
authored andcommitted
[sources] Add nudge for Automatic Workspace Folders feature.
When the Automatic Workspace Folders feature is available but not utilized by the developer, display a message to nudge developers to look into adopting `com.chrome.devtools.json` for this (per UX seed[^1]). [^1]: http://go/chrome-devtools:automatic-workspace-folders-ux Bug: 404170628 Change-Id: I7d6020c7862e68edfb74635df4574b6fb4adaa1a Screenshot: http://screen/PCS4kAnxuTmfwCv.png Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6438855 Auto-Submit: Benedikt Meurer <[email protected]> Reviewed-by: Alex Rudenko <[email protected]> Commit-Queue: Alex Rudenko <[email protected]>
1 parent f32c639 commit fa68d4e

File tree

7 files changed

+241
-2
lines changed

7 files changed

+241
-2
lines changed

front_end/models/persistence/AutomaticFileSystemManager.test.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ describe('AutomaticFileSystemManager', () => {
2121
it('initially doesn\'t report an automatic file system', () => {
2222
const inspectorFrontendHost = sinon.createStubInstance(Host.InspectorFrontendHost.InspectorFrontendHostStub);
2323
const projectSettingsModel = sinon.createStubInstance(ProjectSettings.ProjectSettingsModel.ProjectSettingsModel);
24+
sinon.stub(projectSettingsModel, 'availability').value('available');
2425
sinon.stub(projectSettingsModel, 'projectSettings').value({});
2526

2627
const manager = AutomaticFileSystemManager.instance({
@@ -51,6 +52,7 @@ describe('AutomaticFileSystemManager', () => {
5152
it('attempts to automatically connect the file system initially', () => {
5253
const inspectorFrontendHost = sinon.createStubInstance(Host.InspectorFrontendHost.InspectorFrontendHostStub);
5354
const projectSettingsModel = sinon.createStubInstance(ProjectSettings.ProjectSettingsModel.ProjectSettingsModel);
55+
sinon.stub(projectSettingsModel, 'availability').value('available');
5456
sinon.stub(projectSettingsModel, 'projectSettings').value({workspace: {root, uuid}});
5557

5658
const manager = AutomaticFileSystemManager.instance({
@@ -67,6 +69,7 @@ describe('AutomaticFileSystemManager', () => {
6769
it('reflects state correctly when automatic connection succeeds', async () => {
6870
const inspectorFrontendHost = sinon.createStubInstance(Host.InspectorFrontendHost.InspectorFrontendHostStub);
6971
const projectSettingsModel = sinon.createStubInstance(ProjectSettings.ProjectSettingsModel.ProjectSettingsModel);
72+
sinon.stub(projectSettingsModel, 'availability').value('available');
7073
sinon.stub(projectSettingsModel, 'projectSettings').value({workspace: {root, uuid}});
7174

7275
const manager = AutomaticFileSystemManager.instance({
@@ -86,6 +89,7 @@ describe('AutomaticFileSystemManager', () => {
8689
it('reflects state correctly when automatic connection fails', async () => {
8790
const inspectorFrontendHost = sinon.createStubInstance(Host.InspectorFrontendHost.InspectorFrontendHostStub);
8891
const projectSettingsModel = sinon.createStubInstance(ProjectSettings.ProjectSettingsModel.ProjectSettingsModel);
92+
sinon.stub(projectSettingsModel, 'availability').value('available');
8993
sinon.stub(projectSettingsModel, 'projectSettings').value({workspace: {root, uuid}});
9094

9195
const manager = AutomaticFileSystemManager.instance({
@@ -105,6 +109,7 @@ describe('AutomaticFileSystemManager', () => {
105109
it('performs first-time setup of automatic file system correctly', async () => {
106110
const inspectorFrontendHost = sinon.createStubInstance(Host.InspectorFrontendHost.InspectorFrontendHostStub);
107111
const projectSettingsModel = sinon.createStubInstance(ProjectSettings.ProjectSettingsModel.ProjectSettingsModel);
112+
sinon.stub(projectSettingsModel, 'availability').value('available');
108113
sinon.stub(projectSettingsModel, 'projectSettings').value({workspace: {root, uuid}});
109114
const manager = AutomaticFileSystemManager.instance({
110115
forceNew: true,
@@ -134,6 +139,7 @@ describe('AutomaticFileSystemManager', () => {
134139
it('correctly disconnects automatic file systems', async () => {
135140
const inspectorFrontendHost = sinon.createStubInstance(Host.InspectorFrontendHost.InspectorFrontendHostStub);
136141
const projectSettingsModel = sinon.createStubInstance(ProjectSettings.ProjectSettingsModel.ProjectSettingsModel);
142+
sinon.stub(projectSettingsModel, 'availability').value('available');
137143
sinon.stub(projectSettingsModel, 'projectSettings').value({workspace: {root, uuid}});
138144
const manager = AutomaticFileSystemManager.instance({
139145
forceNew: true,
@@ -153,4 +159,51 @@ describe('AutomaticFileSystemManager', () => {
153159
assert.strictEqual(manager.automaticFileSystem, automaticFileSystem);
154160
assert.deepEqual(manager.automaticFileSystem, {root, uuid, state: 'disconnected'});
155161
});
162+
163+
it('reports unavailable when `devToolsAutomaticFileSystems` is off', () => {
164+
const hostConfig = {devToolsAutomaticFileSystems: {enabled: false}};
165+
const inspectorFrontendHost = sinon.createStubInstance(Host.InspectorFrontendHost.InspectorFrontendHostStub);
166+
const projectSettingsModel = sinon.createStubInstance(ProjectSettings.ProjectSettingsModel.ProjectSettingsModel);
167+
168+
const manager = AutomaticFileSystemManager.instance({
169+
forceNew: true,
170+
hostConfig,
171+
inspectorFrontendHost,
172+
projectSettingsModel,
173+
});
174+
175+
assert.strictEqual(manager.availability, 'unavailable');
176+
});
177+
178+
it('reports available when project settings are available', () => {
179+
const inspectorFrontendHost = sinon.createStubInstance(Host.InspectorFrontendHost.InspectorFrontendHostStub);
180+
const projectSettingsModel = sinon.createStubInstance(ProjectSettings.ProjectSettingsModel.ProjectSettingsModel);
181+
sinon.stub(projectSettingsModel, 'availability').value('available');
182+
sinon.stub(projectSettingsModel, 'projectSettings').value({});
183+
184+
const manager = AutomaticFileSystemManager.instance({
185+
forceNew: true,
186+
hostConfig,
187+
inspectorFrontendHost,
188+
projectSettingsModel,
189+
});
190+
191+
assert.strictEqual(manager.availability, 'available');
192+
});
193+
194+
it('reports unavailable when project settings are unavailable', () => {
195+
const inspectorFrontendHost = sinon.createStubInstance(Host.InspectorFrontendHost.InspectorFrontendHostStub);
196+
const projectSettingsModel = sinon.createStubInstance(ProjectSettings.ProjectSettingsModel.ProjectSettingsModel);
197+
sinon.stub(projectSettingsModel, 'availability').value('unavailable');
198+
sinon.stub(projectSettingsModel, 'projectSettings').value({});
199+
200+
const manager = AutomaticFileSystemManager.instance({
201+
forceNew: true,
202+
hostConfig,
203+
inspectorFrontendHost,
204+
projectSettingsModel,
205+
});
206+
207+
assert.strictEqual(manager.availability, 'unavailable');
208+
});
156209
});

front_end/models/persistence/AutomaticFileSystemManager.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,16 @@ export interface AutomaticFileSystem {
1717
state: 'disconnected'|'connecting'|'connected';
1818
}
1919

20+
/**
21+
* Indicates the availability of the Automatic Workspace Folders feature.
22+
*
23+
* `'available'` means that the feature is enabled and the project settings
24+
* are also available. It doesn't indicate whether or not the page is actually
25+
* providing a `com.chrome.devtools.json` or not, and whether or not that file
26+
* (if it exists) provides workspace information.
27+
*/
28+
export type AutomaticFileSystemAvailability = 'available'|'unavailable';
29+
2030
let automaticFileSystemManagerInstance: AutomaticFileSystemManager|undefined;
2131

2232
/**
@@ -26,6 +36,7 @@ let automaticFileSystemManagerInstance: AutomaticFileSystemManager|undefined;
2636
*/
2737
export class AutomaticFileSystemManager extends Common.ObjectWrapper.ObjectWrapper<EventTypes> {
2838
#automaticFileSystem: AutomaticFileSystem|null;
39+
#availability: AutomaticFileSystemAvailability = 'unavailable';
2940
#inspectorFrontendHost: Host.InspectorFrontendHostAPI.InspectorFrontendHostAPI;
3041
#projectSettingsModel: ProjectSettings.ProjectSettingsModel.ProjectSettingsModel;
3142

@@ -38,6 +49,21 @@ export class AutomaticFileSystemManager extends Common.ObjectWrapper.ObjectWrapp
3849
return this.#automaticFileSystem;
3950
}
4051

52+
/**
53+
* Yields the availability of the Automatic Workspace Folders feature.
54+
*
55+
* `'available'` means that the feature is enabled and the project settings
56+
* are also available. It doesn't indicate whether or not the page is actually
57+
* providing a `com.chrome.devtools.json` or not, and whether or not that file
58+
* (if it exists) provides workspace information.
59+
*
60+
* @return `'available'` if the feature is available and the project settings
61+
* feature is also available, otherwise `'unavailable'`.
62+
*/
63+
get availability(): AutomaticFileSystemAvailability {
64+
return this.#availability;
65+
}
66+
4167
/**
4268
* @internal
4369
*/
@@ -50,6 +76,9 @@ export class AutomaticFileSystemManager extends Common.ObjectWrapper.ObjectWrapp
5076
this.#inspectorFrontendHost = inspectorFrontendHost;
5177
this.#projectSettingsModel = projectSettingsModel;
5278
if (hostConfig.devToolsAutomaticFileSystems?.enabled) {
79+
this.#projectSettingsModel.addEventListener(
80+
ProjectSettings.ProjectSettingsModel.Events.AVAILABILITY_CHANGED, this.#availabilityChanged, this);
81+
this.#availabilityChanged({data: this.#projectSettingsModel.availability});
5382
this.#projectSettingsModel.addEventListener(
5483
ProjectSettings.ProjectSettingsModel.Events.PROJECT_SETTINGS_CHANGED, this.#projectSettingsChanged, this);
5584
this.#projectSettingsChanged({data: this.#projectSettingsModel.projectSettings});
@@ -94,10 +123,22 @@ export class AutomaticFileSystemManager extends Common.ObjectWrapper.ObjectWrapp
94123
}
95124

96125
#dispose(): void {
126+
this.#projectSettingsModel.removeEventListener(
127+
ProjectSettings.ProjectSettingsModel.Events.AVAILABILITY_CHANGED, this.#availabilityChanged, this);
97128
this.#projectSettingsModel.removeEventListener(
98129
ProjectSettings.ProjectSettingsModel.Events.PROJECT_SETTINGS_CHANGED, this.#projectSettingsChanged, this);
99130
}
100131

132+
#availabilityChanged(
133+
event: Common.EventTarget.EventTargetEvent<ProjectSettings.ProjectSettingsModel.ProjectSettingsAvailability>):
134+
void {
135+
const availability = event.data;
136+
if (this.#availability !== availability) {
137+
this.#availability = availability;
138+
this.dispatchEventToListeners(Events.AVAILABILITY_CHANGED, this.#availability);
139+
}
140+
}
141+
101142
#projectSettingsChanged(
102143
event: Common.EventTarget.EventTargetEvent<ProjectSettings.ProjectSettingsModel.ProjectSettings>): void {
103144
const projectSettings = event.data;
@@ -172,11 +213,18 @@ export const enum Events {
172213
* `AutomaticFileSystemManager` changes.
173214
*/
174215
AUTOMATIC_FILE_SYSTEM_CHANGED = 'AutomaticFileSystemChanged',
216+
217+
/**
218+
* Emitted whenever the `availability` property of the
219+
* `AutomaticFileSystemManager` changes.
220+
*/
221+
AVAILABILITY_CHANGED = 'AvailabilityChanged',
175222
}
176223

177224
/**
178225
* @internal
179226
*/
180227
export interface EventTypes {
181228
[Events.AUTOMATIC_FILE_SYSTEM_CHANGED]: Readonly<AutomaticFileSystem>|null;
229+
[Events.AVAILABILITY_CHANGED]: AutomaticFileSystemAvailability;
182230
}

front_end/models/project_settings/ProjectSettingsModel.test.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,4 +207,54 @@ describe('ProjectSettingsModel', () => {
207207

208208
assert.isTrue(targetManager.addEventListener.notCalled);
209209
});
210+
211+
it('reports unavailable when `devToolsWellKnown` is disabled', () => {
212+
const hostConfig = {devToolsWellKnown: {enabled: false}};
213+
const pageResourceLoader = sinon.createStubInstance(SDK.PageResourceLoader.PageResourceLoader);
214+
const targetManager = sinon.createStubInstance(SDK.TargetManager.TargetManager);
215+
216+
const projectSettingsModel = ProjectSettingsModel.instance({
217+
forceNew: true,
218+
hostConfig,
219+
pageResourceLoader,
220+
targetManager,
221+
});
222+
223+
assert.strictEqual(projectSettingsModel.availability, 'unavailable');
224+
});
225+
226+
it('reports available for local origins', async () => {
227+
const hostConfig = {devToolsWellKnown: {enabled: true}};
228+
const pageResourceLoader = sinon.createStubInstance(SDK.PageResourceLoader.PageResourceLoader);
229+
const targetManager = sinon.createStubInstance(SDK.TargetManager.TargetManager);
230+
231+
const target = sinon.createStubInstance(SDK.Target.Target);
232+
targetManager.primaryPageTarget.returns(target);
233+
234+
const resourceTreeModel = sinon.createStubInstance(SDK.ResourceTreeModel.ResourceTreeModel);
235+
target.model.withArgs(SDK.ResourceTreeModel.ResourceTreeModel).returns(resourceTreeModel);
236+
237+
const url = urlString`http://localhost:5713/.well-known/appspecific/com.chrome.devtools.json`;
238+
const frameId = 'frame1';
239+
const initiatorUrl = urlString`http://localhost:5713/index.html`;
240+
const frame = sinon.createStubInstance(SDK.ResourceTreeModel.ResourceTreeFrame);
241+
resourceTreeModel.mainFrame = frame;
242+
sinon.stub(frame, 'securityOriginDetails').get(() => ({isLocalhost: true}));
243+
sinon.stub(frame, 'url').get(() => initiatorUrl);
244+
sinon.stub(frame, 'id').get(() => frameId);
245+
246+
const content = '{"workspace":{"root":"/home/foo","uuid":"8f7b028c-0323-485f-bcb9-b404edc0f186"}}';
247+
pageResourceLoader.loadResource.withArgs(url, sinon.match({target, frameId, initiatorUrl}))
248+
.returns(Promise.resolve({content}));
249+
250+
const projectSettingsModel = ProjectSettingsModel.instance({
251+
forceNew: true,
252+
hostConfig,
253+
pageResourceLoader,
254+
targetManager,
255+
});
256+
await projectSettingsModel.projectSettingsPromise;
257+
258+
assert.strictEqual(projectSettingsModel.availability, 'available');
259+
});
210260
});

front_end/models/project_settings/ProjectSettingsModel.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,15 @@ export interface ProjectSettings {
6060
readonly workspace?: {readonly root: Platform.DevToolsPath.RawPathString, readonly uuid: string};
6161
}
6262

63+
/**
64+
* Indicates the availability of the project settings feature.
65+
*
66+
* `'available'` means that the feature is enabled, the origin of the inspected
67+
* page is `localhost`. It doesn't however indicate whether or not the page is
68+
* actually providing a `com.chrome.devtools.json` or not.
69+
*/
70+
export type ProjectSettingsAvailability = 'available'|'unavailable';
71+
6372
const EMPTY_PROJECT_SETTINGS: ProjectSettings = Object.freeze({});
6473
const IDLE_PROMISE: Promise<void> = Promise.resolve();
6574

@@ -68,9 +77,24 @@ let projectSettingsModelInstance: ProjectSettingsModel|undefined;
6877
export class ProjectSettingsModel extends Common.ObjectWrapper.ObjectWrapper<EventTypes> {
6978
readonly #pageResourceLoader: SDK.PageResourceLoader.PageResourceLoader;
7079
readonly #targetManager: SDK.TargetManager.TargetManager;
80+
#availability: ProjectSettingsAvailability = 'unavailable';
7181
#projectSettings: ProjectSettings = EMPTY_PROJECT_SETTINGS;
7282
#promise: Promise<void> = IDLE_PROMISE;
7383

84+
/**
85+
* Yields the availability of the project settings feature.
86+
*
87+
* `'available'` means that the feature is enabled, the origin of the inspected
88+
* page is `localhost`. It doesn't however indicate whether or not the page is
89+
* actually providing a `com.chrome.devtools.json` or not.
90+
*
91+
* @return `'available'` if the feature is enabled and the inspected page is
92+
* `localhost`, otherwise `'unavailable'`.
93+
*/
94+
get availability(): ProjectSettingsAvailability {
95+
return this.#availability;
96+
}
97+
7498
/**
7599
* Yields the current project settings.
76100
*
@@ -169,8 +193,16 @@ export class ProjectSettingsModel extends Common.ObjectWrapper.ObjectWrapper<Eve
169193
async #loadAndValidateProjectSettings(target: SDK.Target.Target): Promise<ProjectSettings> {
170194
const frame = target.model(SDK.ResourceTreeModel.ResourceTreeModel)?.mainFrame;
171195
if (!isLocalFrame(frame)) {
196+
if (this.#availability !== 'unavailable') {
197+
this.#availability = 'unavailable';
198+
this.dispatchEventToListeners(Events.AVAILABILITY_CHANGED, this.#availability);
199+
}
172200
return EMPTY_PROJECT_SETTINGS;
173201
}
202+
if (this.#availability !== 'available') {
203+
this.#availability = 'available';
204+
this.dispatchEventToListeners(Events.AVAILABILITY_CHANGED, this.#availability);
205+
}
174206
const initiatorUrl = frame.url;
175207
const frameId = frame.id;
176208
let url = WELL_KNOWN_DEVTOOLS_JSON_PATH;
@@ -199,10 +231,27 @@ export class ProjectSettingsModel extends Common.ObjectWrapper.ObjectWrapper<Eve
199231
}
200232
}
201233

234+
/**
235+
* Events emitted by the `ProjectSettingsModel`.
236+
*/
202237
export const enum Events {
238+
/**
239+
* Emitted whenever the `availability` property of the
240+
* `ProjectSettingsModel` changes.
241+
*/
242+
AVAILABILITY_CHANGED = 'AvailabilityChanged',
243+
244+
/**
245+
* Emitted whenever the `projectSettings` property of the
246+
* `ProjectSettingsModel` changes.
247+
*/
203248
PROJECT_SETTINGS_CHANGED = 'ProjectSettingsChanged',
204249
}
205250

251+
/**
252+
* @internal
253+
*/
206254
export interface EventTypes {
255+
[Events.AVAILABILITY_CHANGED]: ProjectSettingsAvailability;
207256
[Events.PROJECT_SETTINGS_CHANGED]: ProjectSettings;
208257
}

0 commit comments

Comments
 (0)