Skip to content

Commit 91617af

Browse files
bmeurerDevtools-frontend LUCI CQ
authored and
Devtools-frontend LUCI CQ
committed
[models] AutomaticFileSystemManager must listen to FileSystemRemoved.
When a file system is removed by the user, the `AutomaticFileSystemManager` has to update its state back to 'disconnected'. Bug: 395562934 Change-Id: I0b89e46fc1a0d8c79e9ce393f904c9a8a299fd78 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6447053 Auto-Submit: Benedikt Meurer <bmeurer@chromium.org> Reviewed-by: Wolfgang Beyer <wolfi@chromium.org> Commit-Queue: Wolfgang Beyer <wolfi@chromium.org>
1 parent 1de87e3 commit 91617af

File tree

4 files changed

+90
-47
lines changed

4 files changed

+90
-47
lines changed

front_end/core/host/InspectorFrontendHostAPI.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// found in the LICENSE file.
44

55
import type * as Platform from '../../core/platform/platform.js';
6+
import type * as Common from '../common/common.js';
67
import type * as Root from '../root/root.js';
78

89
export enum Events {
@@ -242,6 +243,8 @@ export interface EventTypes {
242243
}
243244

244245
export interface InspectorFrontendHostAPI {
246+
events: Common.EventTarget.EventTarget<EventTypes>;
247+
245248
connectAutomaticFileSystem(
246249
fileSystemPath: Platform.DevToolsPath.RawPathString,
247250
fileSystemUUID: string,

front_end/models/persistence/AutomaticFileSystemManager.test.ts

Lines changed: 68 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,28 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5+
import * as Common from '../../core/common/common.js';
56
import * as Host from '../../core/host/host.js';
67
import type * as Platform from '../../core/platform/platform.js';
78
import * as ProjectSettings from '../project_settings/project_settings.js';
89

910
import * as Persistence from './persistence.js';
1011

12+
function createStubInstances(
13+
availability: ProjectSettings.ProjectSettingsModel.ProjectSettingsAvailability,
14+
projectSettings: ProjectSettings.ProjectSettingsModel.ProjectSettings,
15+
) {
16+
const inspectorFrontendHost =
17+
sinon.createStubInstance(class extends Host.InspectorFrontendHost.InspectorFrontendHostStub {
18+
override events = sinon.createStubInstance(Common.ObjectWrapper.ObjectWrapper);
19+
});
20+
inspectorFrontendHost.events = sinon.createStubInstance(Common.ObjectWrapper.ObjectWrapper);
21+
const projectSettingsModel = sinon.createStubInstance(ProjectSettings.ProjectSettingsModel.ProjectSettingsModel);
22+
sinon.stub(projectSettingsModel, 'availability').value(availability);
23+
sinon.stub(projectSettingsModel, 'projectSettings').value(projectSettings);
24+
return {inspectorFrontendHost, projectSettingsModel};
25+
}
26+
1127
describe('Persistence', () => {
1228
describe('AutomaticFileSystemManager', () => {
1329
describe('AutomaticFileSystemManager', () => {
@@ -22,11 +38,7 @@ describe('Persistence', () => {
2238
});
2339

2440
it('initially doesn\'t report an automatic file system', () => {
25-
const inspectorFrontendHost = sinon.createStubInstance(Host.InspectorFrontendHost.InspectorFrontendHostStub);
26-
const projectSettingsModel =
27-
sinon.createStubInstance(ProjectSettings.ProjectSettingsModel.ProjectSettingsModel);
28-
sinon.stub(projectSettingsModel, 'availability').value('available');
29-
sinon.stub(projectSettingsModel, 'projectSettings').value({});
41+
const {inspectorFrontendHost, projectSettingsModel} = createStubInstances('available', {});
3042

3143
const manager = AutomaticFileSystemManager.instance({
3244
forceNew: true,
@@ -40,9 +52,7 @@ describe('Persistence', () => {
4052

4153
it('doesn\'t listen to project settings changes when `devToolsAutomaticFileSystems` is off', () => {
4254
const hostConfig = {devToolsAutomaticFileSystems: {enabled: false}};
43-
const inspectorFrontendHost = sinon.createStubInstance(Host.InspectorFrontendHost.InspectorFrontendHostStub);
44-
const projectSettingsModel =
45-
sinon.createStubInstance(ProjectSettings.ProjectSettingsModel.ProjectSettingsModel);
55+
const {inspectorFrontendHost, projectSettingsModel} = createStubInstances('available', {});
4656

4757
AutomaticFileSystemManager.instance({
4858
forceNew: true,
@@ -54,12 +64,24 @@ describe('Persistence', () => {
5464
sinon.assert.notCalled(projectSettingsModel.addEventListener);
5565
});
5666

67+
it('listens to FileSystemRemoved events', () => {
68+
const {inspectorFrontendHost, projectSettingsModel} = createStubInstances('available', {});
69+
70+
const automaticFileSystemManager = AutomaticFileSystemManager.instance({
71+
forceNew: true,
72+
hostConfig,
73+
inspectorFrontendHost,
74+
projectSettingsModel,
75+
});
76+
77+
sinon.assert.calledOnceWithMatch(
78+
inspectorFrontendHost.events.addEventListener, Host.InspectorFrontendHostAPI.Events.FileSystemRemoved,
79+
sinon.match.func, automaticFileSystemManager);
80+
});
81+
5782
it('attempts to automatically connect the file system initially', () => {
58-
const inspectorFrontendHost = sinon.createStubInstance(Host.InspectorFrontendHost.InspectorFrontendHostStub);
59-
const projectSettingsModel =
60-
sinon.createStubInstance(ProjectSettings.ProjectSettingsModel.ProjectSettingsModel);
61-
sinon.stub(projectSettingsModel, 'availability').value('available');
62-
sinon.stub(projectSettingsModel, 'projectSettings').value({workspace: {root, uuid}});
83+
const {inspectorFrontendHost, projectSettingsModel} =
84+
createStubInstances('available', {workspace: {root, uuid}});
6385

6486
const manager = AutomaticFileSystemManager.instance({
6587
forceNew: true,
@@ -74,11 +96,8 @@ describe('Persistence', () => {
7496
});
7597

7698
it('reflects state correctly when automatic connection succeeds', async () => {
77-
const inspectorFrontendHost = sinon.createStubInstance(Host.InspectorFrontendHost.InspectorFrontendHostStub);
78-
const projectSettingsModel =
79-
sinon.createStubInstance(ProjectSettings.ProjectSettingsModel.ProjectSettingsModel);
80-
sinon.stub(projectSettingsModel, 'availability').value('available');
81-
sinon.stub(projectSettingsModel, 'projectSettings').value({workspace: {root, uuid}});
99+
const {inspectorFrontendHost, projectSettingsModel} =
100+
createStubInstances('available', {workspace: {root, uuid}});
82101

83102
const manager = AutomaticFileSystemManager.instance({
84103
forceNew: true,
@@ -95,11 +114,8 @@ describe('Persistence', () => {
95114
});
96115

97116
it('reflects state correctly when automatic connection fails', async () => {
98-
const inspectorFrontendHost = sinon.createStubInstance(Host.InspectorFrontendHost.InspectorFrontendHostStub);
99-
const projectSettingsModel =
100-
sinon.createStubInstance(ProjectSettings.ProjectSettingsModel.ProjectSettingsModel);
101-
sinon.stub(projectSettingsModel, 'availability').value('available');
102-
sinon.stub(projectSettingsModel, 'projectSettings').value({workspace: {root, uuid}});
117+
const {inspectorFrontendHost, projectSettingsModel} =
118+
createStubInstances('available', {workspace: {root, uuid}});
103119

104120
const manager = AutomaticFileSystemManager.instance({
105121
forceNew: true,
@@ -116,11 +132,8 @@ describe('Persistence', () => {
116132
});
117133

118134
it('performs first-time setup of automatic file system correctly', async () => {
119-
const inspectorFrontendHost = sinon.createStubInstance(Host.InspectorFrontendHost.InspectorFrontendHostStub);
120-
const projectSettingsModel =
121-
sinon.createStubInstance(ProjectSettings.ProjectSettingsModel.ProjectSettingsModel);
122-
sinon.stub(projectSettingsModel, 'availability').value('available');
123-
sinon.stub(projectSettingsModel, 'projectSettings').value({workspace: {root, uuid}});
135+
const {inspectorFrontendHost, projectSettingsModel} =
136+
createStubInstances('available', {workspace: {root, uuid}});
124137
const manager = AutomaticFileSystemManager.instance({
125138
forceNew: true,
126139
hostConfig,
@@ -147,11 +160,8 @@ describe('Persistence', () => {
147160
});
148161

149162
it('correctly disconnects automatic file systems', async () => {
150-
const inspectorFrontendHost = sinon.createStubInstance(Host.InspectorFrontendHost.InspectorFrontendHostStub);
151-
const projectSettingsModel =
152-
sinon.createStubInstance(ProjectSettings.ProjectSettingsModel.ProjectSettingsModel);
153-
sinon.stub(projectSettingsModel, 'availability').value('available');
154-
sinon.stub(projectSettingsModel, 'projectSettings').value({workspace: {root, uuid}});
163+
const {inspectorFrontendHost, projectSettingsModel} =
164+
createStubInstances('available', {workspace: {root, uuid}});
155165
const manager = AutomaticFileSystemManager.instance({
156166
forceNew: true,
157167
hostConfig,
@@ -171,11 +181,31 @@ describe('Persistence', () => {
171181
assert.deepEqual(manager.automaticFileSystem, {root, uuid, state: 'disconnected'});
172182
});
173183

184+
it('reflects disconnected state correctly when the file system is removed', async () => {
185+
const {inspectorFrontendHost, projectSettingsModel} =
186+
createStubInstances('available', {workspace: {root, uuid}});
187+
const manager = AutomaticFileSystemManager.instance({
188+
forceNew: true,
189+
hostConfig,
190+
inspectorFrontendHost,
191+
projectSettingsModel,
192+
});
193+
const [, fileSystemRemoved] = inspectorFrontendHost.events.addEventListener.lastCall.args;
194+
const [, , , setupCallback] = inspectorFrontendHost.connectAutomaticFileSystem.lastCall.args;
195+
setupCallback({success: true});
196+
await manager.once(AUTOMATIC_FILE_SYSTEM_CHANGED);
197+
const automaticFileSystemPromise = manager.once(AUTOMATIC_FILE_SYSTEM_CHANGED);
198+
199+
fileSystemRemoved.call(manager, {data: root});
200+
201+
const automaticFileSystem = await automaticFileSystemPromise;
202+
assert.strictEqual(manager.automaticFileSystem, automaticFileSystem);
203+
assert.deepEqual(manager.automaticFileSystem, {root, uuid, state: 'disconnected'});
204+
});
205+
174206
it('reports unavailable when `devToolsAutomaticFileSystems` is off', () => {
175207
const hostConfig = {devToolsAutomaticFileSystems: {enabled: false}};
176-
const inspectorFrontendHost = sinon.createStubInstance(Host.InspectorFrontendHost.InspectorFrontendHostStub);
177-
const projectSettingsModel =
178-
sinon.createStubInstance(ProjectSettings.ProjectSettingsModel.ProjectSettingsModel);
208+
const {inspectorFrontendHost, projectSettingsModel} = createStubInstances('available', {});
179209

180210
const manager = AutomaticFileSystemManager.instance({
181211
forceNew: true,
@@ -188,11 +218,7 @@ describe('Persistence', () => {
188218
});
189219

190220
it('reports available when project settings are available', () => {
191-
const inspectorFrontendHost = sinon.createStubInstance(Host.InspectorFrontendHost.InspectorFrontendHostStub);
192-
const projectSettingsModel =
193-
sinon.createStubInstance(ProjectSettings.ProjectSettingsModel.ProjectSettingsModel);
194-
sinon.stub(projectSettingsModel, 'availability').value('available');
195-
sinon.stub(projectSettingsModel, 'projectSettings').value({});
221+
const {inspectorFrontendHost, projectSettingsModel} = createStubInstances('available', {});
196222

197223
const manager = AutomaticFileSystemManager.instance({
198224
forceNew: true,
@@ -205,11 +231,7 @@ describe('Persistence', () => {
205231
});
206232

207233
it('reports unavailable when project settings are unavailable', () => {
208-
const inspectorFrontendHost = sinon.createStubInstance(Host.InspectorFrontendHost.InspectorFrontendHostStub);
209-
const projectSettingsModel =
210-
sinon.createStubInstance(ProjectSettings.ProjectSettingsModel.ProjectSettingsModel);
211-
sinon.stub(projectSettingsModel, 'availability').value('unavailable');
212-
sinon.stub(projectSettingsModel, 'projectSettings').value({});
234+
const {inspectorFrontendHost, projectSettingsModel} = createStubInstances('unavailable', {});
213235

214236
const manager = AutomaticFileSystemManager.instance({
215237
forceNew: true,

front_end/models/persistence/AutomaticFileSystemManager.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
// found in the LICENSE file.
44

55
import * as Common from '../../core/common/common.js';
6-
import type * as Host from '../../core/host/host.js';
6+
import * as Host from '../../core/host/host.js';
77
import type * as Platform from '../../core/platform/platform.js';
88
import type * as Root from '../../core/root/root.js';
99
import * as ProjectSettings from '../project_settings/project_settings.js';
@@ -76,6 +76,8 @@ export class AutomaticFileSystemManager extends Common.ObjectWrapper.ObjectWrapp
7676
this.#inspectorFrontendHost = inspectorFrontendHost;
7777
this.#projectSettingsModel = projectSettingsModel;
7878
if (hostConfig.devToolsAutomaticFileSystems?.enabled) {
79+
this.#inspectorFrontendHost.events.addEventListener(
80+
Host.InspectorFrontendHostAPI.Events.FileSystemRemoved, this.#fileSystemRemoved, this);
7981
this.#projectSettingsModel.addEventListener(
8082
ProjectSettings.ProjectSettingsModel.Events.AVAILABILITY_CHANGED, this.#availabilityChanged, this);
8183
this.#availabilityChanged({data: this.#projectSettingsModel.availability});
@@ -123,6 +125,8 @@ export class AutomaticFileSystemManager extends Common.ObjectWrapper.ObjectWrapp
123125
}
124126

125127
#dispose(): void {
128+
this.#inspectorFrontendHost.events.removeEventListener(
129+
Host.InspectorFrontendHostAPI.Events.FileSystemRemoved, this.#fileSystemRemoved, this);
126130
this.#projectSettingsModel.removeEventListener(
127131
ProjectSettings.ProjectSettingsModel.Events.AVAILABILITY_CHANGED, this.#availabilityChanged, this);
128132
this.#projectSettingsModel.removeEventListener(
@@ -139,6 +143,19 @@ export class AutomaticFileSystemManager extends Common.ObjectWrapper.ObjectWrapp
139143
}
140144
}
141145

146+
#fileSystemRemoved(event: Common.EventTarget.EventTargetEvent<Platform.DevToolsPath.RawPathString>): void {
147+
if (this.#automaticFileSystem === null) {
148+
return;
149+
}
150+
if (this.#automaticFileSystem.root === event.data) {
151+
this.#automaticFileSystem = Object.freeze({
152+
...this.#automaticFileSystem,
153+
state: 'disconnected',
154+
});
155+
this.dispatchEventToListeners(Events.AUTOMATIC_FILE_SYSTEM_CHANGED, this.#automaticFileSystem);
156+
}
157+
}
158+
142159
#projectSettingsChanged(
143160
event: Common.EventTarget.EventTargetEvent<ProjectSettings.ProjectSettingsModel.ProjectSettings>): void {
144161
const projectSettings = event.data;

front_end/testing/AiAssistanceHelpers.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,7 @@ export const setupAutomaticFileSystem = (options: {hasFileSystem: boolean} = {
224224
const uuid = '549bbf9b-48b2-4af7-aebd-d3ba68993094';
225225
const hostConfig = {devToolsAutomaticFileSystems: {enabled: true}};
226226
const inspectorFrontendHost = sinon.createStubInstance(Host.InspectorFrontendHost.InspectorFrontendHostStub);
227+
inspectorFrontendHost.events = sinon.createStubInstance(Common.ObjectWrapper.ObjectWrapper);
227228
const projectSettingsModel = sinon.createStubInstance(ProjectSettings.ProjectSettingsModel.ProjectSettingsModel);
228229
sinon.stub(projectSettingsModel, 'availability').value('available');
229230
sinon.stub(projectSettingsModel, 'projectSettings').value(options.hasFileSystem ? {workspace: {root, uuid}} : {});

0 commit comments

Comments
 (0)