-
Notifications
You must be signed in to change notification settings - Fork 83
[MV3 Debug Extension] Extension sets the ide
query parameter for the DevTools URI
#1943
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -544,28 +544,40 @@ class DevHandler { | |
extensionDebugConnections.add(DebugConnection(appServices)); | ||
_servicesByAppId[appId] = appServices; | ||
} | ||
// If we don't have a DevTools instance, then are connecting to an IDE. | ||
// Therefore return early instead of opening DevTools: | ||
if (_devTools == null) return; | ||
|
||
final encodedUri = await appServices.debugService.encodedUri; | ||
|
||
appServices.dwdsStats.updateLoadTime( | ||
debuggerStart: debuggerStart, devToolsStart: DateTime.now()); | ||
|
||
if (_devTools != null) { | ||
// If we only want the URI, this means we are embedding Dart DevTools in | ||
// Chrome DevTools. Therefore return early. | ||
if (devToolsRequest.uriOnly ?? false) { | ||
final devToolsUri = _constructDevToolsUri( | ||
encodedUri, | ||
ideQueryParam: 'ChromeDevTools', | ||
); | ||
extensionDebugger.sendEvent('dwds.devtoolsUri', devToolsUri); | ||
return; | ||
} | ||
final devToolsUri = _constructDevToolsUri( | ||
// TODO(elliette): Remove handling requests from the MV2 extension after | ||
// MV3 release. | ||
// If we only want the URI, this means the Dart Debug Extension should | ||
// handle how to open it. Therefore return early before opening a new | ||
// tab or window: | ||
if (devToolsRequest.uriOnly ?? false) { | ||
// The MV3 extension is responsible for adding the IDE query | ||
// parameter to the DevTools URI. | ||
final devToolsUri = (devToolsRequest.isMv3Extension ?? false) | ||
? _constructDevToolsUri(encodedUri) | ||
: _constructDevToolsUri( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can the original extension also add the IDE query parameter, so we don't have to split? Or is it too much change? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We would have to do a new release of the old extension, and still handle transition period while it is being released (so there would still be a split). I've added a TODO to remove the old code once the MV3 extension is released. |
||
encodedUri, | ||
ideQueryParam: 'ChromeDevTools', | ||
); | ||
return extensionDebugger.sendEvent('dwds.devtoolsUri', devToolsUri); | ||
} | ||
|
||
// Otherwise, launch DevTools in a new tab / window: | ||
await _launchDevTools( | ||
extensionDebugger, | ||
_constructDevToolsUri( | ||
encodedUri, | ||
ideQueryParam: 'DebugExtension', | ||
); | ||
await _launchDevTools(extensionDebugger, devToolsUri); | ||
} | ||
), | ||
); | ||
}); | ||
} | ||
|
||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious - do they have to be nullable, or can we made them non-nulllable and false by default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think they make sense to be nullable, since they aren't applicable to non-extension requests. This is the same as
tabUrl
andcontextId
(above) which are also null. Added a comment.