Skip to content

Commit c453ebe

Browse files
ellietteelliette
authored
[MV3 Debug Extension] Don't connect to DWDS if we already have a debugger attached (#1903)
1 parent 7575ebd commit c453ebe

File tree

3 files changed

+185
-71
lines changed

3 files changed

+185
-71
lines changed

‎dwds/debug_extension_mv3/web/background.dart‎

Lines changed: 2 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import 'logger.dart';
2020
import 'messaging.dart';
2121
import 'storage.dart';
2222
import 'utils.dart';
23-
import 'web_api.dart';
2423

2524
void main() {
2625
_registerListeners();
@@ -53,50 +52,13 @@ void _registerListeners() {
5352

5453
// Detect clicks on the Dart Debug Extension icon.
5554
chrome.action.onClicked.addListener(allowInterop(
56-
(Tab tab) => _startDebugSession(
55+
(Tab tab) => attachDebugger(
5756
tab.id,
5857
trigger: Trigger.extensionIcon,
5958
),
6059
));
6160
}
6261

63-
Future<void> _startDebugSession(
64-
int tabId, {
65-
required Trigger trigger,
66-
}) async {
67-
final isAuthenticated = await _authenticateUser(tabId);
68-
if (!isAuthenticated) return;
69-
70-
maybeCreateLifelinePort(tabId);
71-
attachDebugger(tabId, trigger: trigger);
72-
}
73-
74-
Future<bool> _authenticateUser(int tabId) async {
75-
final isAlreadyAuthenticated = await _fetchIsAuthenticated(tabId);
76-
if (isAlreadyAuthenticated) return true;
77-
final debugInfo = await _fetchDebugInfo(tabId);
78-
final authUrl = debugInfo?.authUrl;
79-
if (authUrl == null) {
80-
_showWarningNotification('Cannot authenticate user.');
81-
return false;
82-
}
83-
final isAuthenticated = await _sendAuthRequest(authUrl);
84-
if (isAuthenticated) {
85-
await setStorageObject<String>(
86-
type: StorageObject.isAuthenticated,
87-
value: '$isAuthenticated',
88-
tabId: tabId,
89-
);
90-
} else {
91-
sendConnectFailureMessage(
92-
ConnectFailureReason.authentication,
93-
dartAppTabId: tabId,
94-
);
95-
await createTab(authUrl, inNewWindow: false);
96-
}
97-
return isAuthenticated;
98-
}
99-
10062
void _handleRuntimeMessages(
10163
dynamic jsRequest, MessageSender sender, Function sendResponse) async {
10264
if (jsRequest is! String) return;
@@ -150,7 +112,7 @@ void _handleRuntimeMessages(
150112
final newState = debugStateChange.newState;
151113
final tabId = debugStateChange.tabId;
152114
if (newState == DebugStateChange.startDebugging) {
153-
_startDebugSession(tabId, trigger: Trigger.extensionPanel);
115+
attachDebugger(tabId, trigger: Trigger.extensionPanel);
154116
}
155117
});
156118
}
@@ -198,33 +160,6 @@ Future<DebugInfo?> _fetchDebugInfo(int tabId) {
198160
);
199161
}
200162

201-
Future<bool> _fetchIsAuthenticated(int tabId) async {
202-
final authenticated = await fetchStorageObject<String>(
203-
type: StorageObject.isAuthenticated,
204-
tabId: tabId,
205-
);
206-
return authenticated == 'true';
207-
}
208-
209-
Future<bool> _sendAuthRequest(String authUrl) async {
210-
final response = await fetchRequest(authUrl);
211-
final responseBody = response.body ?? '';
212-
return responseBody.contains('Dart Debug Authentication Success!');
213-
}
214-
215-
void _showWarningNotification(String message) {
216-
chrome.notifications.create(
217-
/*notificationId*/ null,
218-
NotificationOptions(
219-
title: '[Error] Dart Debug Extension',
220-
message: message,
221-
iconUrl: 'static_assets/dart.png',
222-
type: 'basic',
223-
),
224-
/*callback*/ null,
225-
);
226-
}
227-
228163
Future<Tab?> _getTab() async {
229164
final query = QueryInfo(active: true, currentWindow: true);
230165
final tabs = List<Tab>.from(await promiseToFuture(chrome.tabs.query(query)));

‎dwds/debug_extension_mv3/web/debug_session.dart‎

Lines changed: 115 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,39 @@ enum Trigger {
8080
extensionIcon,
8181
}
8282

83-
void attachDebugger(int dartAppTabId, {required Trigger trigger}) {
83+
enum DebuggerLocation {
84+
angularDartDevTools,
85+
chromeDevTools,
86+
dartDevTools,
87+
ide;
88+
89+
String get displayName {
90+
switch (this) {
91+
case DebuggerLocation.angularDartDevTools:
92+
return 'AngularDart DevTools';
93+
case DebuggerLocation.chromeDevTools:
94+
return 'Chrome DevTools';
95+
case DebuggerLocation.dartDevTools:
96+
return 'a Dart DevTools tab';
97+
case DebuggerLocation.ide:
98+
return 'an IDE';
99+
}
100+
}
101+
}
102+
103+
void attachDebugger(int dartAppTabId, {required Trigger trigger}) async {
104+
// Check if a debugger is already attached:
105+
final existingDebuggerLocation = _debuggerLocation(dartAppTabId);
106+
if (existingDebuggerLocation != null) {
107+
return _showWarningNotification(
108+
'Already debugging in ${existingDebuggerLocation.displayName}.',
109+
);
110+
}
111+
112+
// Verify that the user is authenticated:
113+
final isAuthenticated = await _authenticateUser(dartAppTabId);
114+
if (!isAuthenticated) return;
115+
84116
_tabIdToTrigger[dartAppTabId] = trigger;
85117
_registerDebugEventListeners();
86118
chrome.debugger.attach(
@@ -154,11 +186,15 @@ String _translateChromeError(String chromeErrorMessage) {
154186

155187
Future<void> _onDebuggerEvent(
156188
Debuggee source, String method, Object? params) async {
189+
final tabId = source.tabId;
157190
maybeForwardMessageToAngularDartDevTools(
158191
method: method, params: params, tabId: source.tabId);
159192

160193
if (method == 'Runtime.executionContextCreated') {
161-
return _maybeConnectToDwds(source.tabId, params);
194+
// Only try to connect to DWDS if we don't already have a debugger instance:
195+
if (_debuggerLocation(tabId) == null) {
196+
return _maybeConnectToDwds(source.tabId, params);
197+
}
162198
}
163199

164200
return _forwardChromeDebuggerEventToDwds(source, method, params);
@@ -183,7 +219,7 @@ Future<void> _maybeConnectToDwds(int tabId, Object? params) async {
183219
);
184220
if (!connected) {
185221
debugWarn('Failed to connect to DWDS for $contextOrigin.');
186-
sendConnectFailureMessage(ConnectFailureReason.unknown,
222+
_sendConnectFailureMessage(ConnectFailureReason.unknown,
187223
dartAppTabId: tabId);
188224
}
189225
}
@@ -373,7 +409,7 @@ void _removeDebugSession(_DebugSession debugSession) {
373409
}
374410
}
375411

376-
void sendConnectFailureMessage(ConnectFailureReason reason,
412+
void _sendConnectFailureMessage(ConnectFailureReason reason,
377413
{required int dartAppTabId}) async {
378414
final json = jsonEncode(serializers.serialize(ConnectFailure((b) => b
379415
..tabId = dartAppTabId
@@ -409,6 +445,81 @@ _DebugSession? _debugSessionForTab(tabId, {required TabType type}) {
409445
}
410446
}
411447

448+
Future<bool> _authenticateUser(int tabId) async {
449+
final isAlreadyAuthenticated = await _fetchIsAuthenticated(tabId);
450+
if (isAlreadyAuthenticated) return true;
451+
final debugInfo = await fetchStorageObject<DebugInfo>(
452+
type: StorageObject.debugInfo,
453+
tabId: tabId,
454+
);
455+
final authUrl = debugInfo?.authUrl;
456+
if (authUrl == null) {
457+
_showWarningNotification('Cannot authenticate user.');
458+
return false;
459+
}
460+
final isAuthenticated = await _sendAuthRequest(authUrl);
461+
if (isAuthenticated) {
462+
await setStorageObject<String>(
463+
type: StorageObject.isAuthenticated,
464+
value: '$isAuthenticated',
465+
tabId: tabId,
466+
);
467+
} else {
468+
_sendConnectFailureMessage(
469+
ConnectFailureReason.authentication,
470+
dartAppTabId: tabId,
471+
);
472+
await createTab(authUrl, inNewWindow: false);
473+
}
474+
return isAuthenticated;
475+
}
476+
477+
Future<bool> _fetchIsAuthenticated(int tabId) async {
478+
final authenticated = await fetchStorageObject<String>(
479+
type: StorageObject.isAuthenticated,
480+
tabId: tabId,
481+
);
482+
return authenticated == 'true';
483+
}
484+
485+
Future<bool> _sendAuthRequest(String authUrl) async {
486+
final response = await fetchRequest(authUrl);
487+
final responseBody = response.body ?? '';
488+
return responseBody.contains('Dart Debug Authentication Success!');
489+
}
490+
491+
void _showWarningNotification(String message) {
492+
chrome.notifications.create(
493+
/*notificationId*/ null,
494+
NotificationOptions(
495+
title: '[Error] Dart Debug Extension',
496+
message: message,
497+
iconUrl: 'static_assets/dart.png',
498+
type: 'basic',
499+
),
500+
/*callback*/ null,
501+
);
502+
}
503+
504+
DebuggerLocation? _debuggerLocation(int dartAppTabId) {
505+
final debugSession = _debugSessionForTab(dartAppTabId, type: TabType.dartApp);
506+
final trigger = _tabIdToTrigger[dartAppTabId];
507+
if (debugSession == null || trigger == null) return null;
508+
509+
switch (trigger) {
510+
case Trigger.extensionIcon:
511+
if (debugSession.devToolsTabId != null) {
512+
return DebuggerLocation.dartDevTools;
513+
} else {
514+
return DebuggerLocation.ide;
515+
}
516+
case Trigger.angularDartDevTools:
517+
return DebuggerLocation.angularDartDevTools;
518+
case Trigger.extensionPanel:
519+
return DebuggerLocation.chromeDevTools;
520+
}
521+
}
522+
412523
/// Construct an [ExtensionEvent] from [method] and [params].
413524
ExtensionEvent _extensionEventFor(String method, dynamic params) {
414525
return ExtensionEvent((b) => b

‎dwds/test/puppeteer/extension_test.dart‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,62 @@ void main() async {
238238
// Verify that the Dart DevTools tab closes:
239239
await devToolsTabTarget.onClose;
240240
});
241+
242+
test('Clicking extension icon while debugging shows warning', () async {
243+
final appUrl = context.appUrl;
244+
final devToolsUrlFragment =
245+
useSse ? 'debugger?uri=sse' : 'debugger?uri=ws';
246+
// Navigate to the Dart app:
247+
final appTab =
248+
await navigateToPage(browser, url: appUrl, isNew: true);
249+
// Click on the Dart Debug Extension icon:
250+
await workerEvalDelay();
251+
await clickOnExtensionIcon(worker);
252+
// Wait for Dart Devtools to open:
253+
final devToolsTabTarget = await browser.waitForTarget(
254+
(target) => target.url.contains(devToolsUrlFragment));
255+
// There should be no warning notifications:
256+
var chromeNotifications = await worker.evaluate(_getNotifications());
257+
expect(chromeNotifications, isEmpty);
258+
// Navigate back to Dart app:
259+
await navigateToPage(browser, url: appUrl, isNew: false);
260+
// Click on the Dart Debug Extension icon again:
261+
await workerEvalDelay();
262+
await clickOnExtensionIcon(worker);
263+
await workerEvalDelay();
264+
// There should now be a warning notificiation:
265+
chromeNotifications = await worker.evaluate(_getNotifications());
266+
expect(chromeNotifications, isNotEmpty);
267+
// Close the Dart app and the associated Dart DevTools:
268+
await appTab.close();
269+
await devToolsTabTarget.onClose;
270+
});
271+
272+
test('Refreshing the Dart app does not open a new Dart DevTools',
273+
() async {
274+
final appUrl = context.appUrl;
275+
final devToolsUrlFragment =
276+
useSse ? 'debugger?uri=sse' : 'debugger?uri=ws';
277+
// Navigate to the Dart app:
278+
final appTab =
279+
await navigateToPage(browser, url: appUrl, isNew: true);
280+
// Click on the Dart Debug Extension icon:
281+
await workerEvalDelay();
282+
await clickOnExtensionIcon(worker);
283+
// Verify that the Dart DevTools tab is open:
284+
final devToolsTabTarget = await browser.waitForTarget(
285+
(target) => target.url.contains(devToolsUrlFragment));
286+
expect(devToolsTabTarget.type, equals('page'));
287+
// Refresh the app tab:
288+
await appTab.reload();
289+
// Verify that we don't open a new Dart DevTools on page refresh:
290+
final devToolsTargets = browser.targets
291+
.where((target) => target.url.contains(devToolsUrlFragment));
292+
expect(devToolsTargets.length, equals(1));
293+
// Close the Dart app and the associated Dart DevTools:
294+
await appTab.close();
295+
await devToolsTabTarget.onClose;
296+
});
241297
});
242298
}
243299

@@ -673,6 +729,18 @@ String _clearStorageJs() {
673729
''';
674730
}
675731

732+
String _getNotifications() {
733+
return '''
734+
async () => {
735+
return new Promise((resolve, reject) => {
736+
chrome.notifications.getAll((notifications) => {
737+
resolve(notifications);
738+
});
739+
});
740+
}
741+
''';
742+
}
743+
676744
// TODO(https://github.com/dart-lang/webdev/issues/1787): Compare to golden
677745
// images. Currently golden comparison is not set up, since this is only run
678746
// locally, not as part of our CI test suite.

0 commit comments

Comments
 (0)