-
Notifications
You must be signed in to change notification settings - Fork 84
Implement the VM service getFlagList
API
#2438
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 4 commits
4398291
ce05071
1c4b800
dfff191
c41bafa
296cbc3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -92,12 +92,16 @@ class ChromeProxyService implements VmServiceInterface { | |
|
||
StreamSubscription<ConsoleAPIEvent>? _consoleSubscription; | ||
|
||
bool _pauseIsolatesOnStart = false; | ||
/// The flags that can be set at runtime via [setFlag]. | ||
final Map<String, bool> _supportedVmServiceFlags = { | ||
_pauseIsolatesOnStartFlag: false, | ||
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. Is pause-on-exit not supported? 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. pause-on-exit is not supported - because the web doesn't have "real" isolates we mimic the pause-on-start behavior by waiting to run the app's main method until we get a resume event. However, I can't think of a way we could mimic pause-on-exit. |
||
}; | ||
|
||
/// The value of the [_pauseIsolatesOnStartFlag]. | ||
/// | ||
/// This value can be updated at runtime via [setFlag]. | ||
bool get pauseIsolatesOnStart => _pauseIsolatesOnStart; | ||
bool get pauseIsolatesOnStart => | ||
_supportedVmServiceFlags[_pauseIsolatesOnStartFlag] ?? false; | ||
|
||
final _resumeAfterRestartEventsController = | ||
StreamController<String>.broadcast(); | ||
|
@@ -758,9 +762,22 @@ ${globalToolConfiguration.loadStrategy.loadModuleSnippet}("dart_sdk").developer. | |
} | ||
|
||
@override | ||
Future<FlagList> getFlagList() async { | ||
// VM flags do not apply to web apps. | ||
return FlagList(flags: []); | ||
Future<FlagList> getFlagList() { | ||
return wrapInErrorHandlerAsync( | ||
'getFlagList', | ||
_getFlagList, | ||
); | ||
} | ||
|
||
Future<FlagList> _getFlagList() { | ||
final flags = _supportedVmServiceFlags.entries.map<Flag>( | ||
(entry) => Flag( | ||
name: entry.key, | ||
valueAsString: '${entry.value}', | ||
), | ||
); | ||
|
||
return Future.value(FlagList(flags: flags.toList())); | ||
} | ||
|
||
@override | ||
|
@@ -1214,14 +1231,12 @@ ${globalToolConfiguration.loadStrategy.loadModuleSnippet}("dart_sdk").developer. | |
); | ||
|
||
Future<Success> _setFlag(String name, String value) async { | ||
if (!_supportedVmServiceFlags.contains(name)) { | ||
if (!_supportedVmServiceFlags.containsKey(name)) { | ||
return _rpcNotSupportedFuture('setFlag'); | ||
} | ||
|
||
if (name == _pauseIsolatesOnStartFlag) { | ||
assert(value == 'true' || value == 'false'); | ||
_pauseIsolatesOnStart = value == 'true'; | ||
} | ||
assert(value == 'true' || value == 'false'); | ||
_supportedVmServiceFlags[name] = value == 'true'; | ||
|
||
return Success(); | ||
} | ||
|
@@ -1699,8 +1714,3 @@ const _stderrTypes = ['error']; | |
const _stdoutTypes = ['log', 'info', 'warning']; | ||
|
||
const _pauseIsolatesOnStartFlag = 'pause_isolates_on_start'; | ||
|
||
/// The flags that can be set at runtime via [setFlag]. | ||
const _supportedVmServiceFlags = { | ||
_pauseIsolatesOnStartFlag, | ||
}; |
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.
Is the name of this map a bit misleading now? It sounds like a set of supported flags, but based on the comment and code below, it appears to be holding the actual flag values now too. (
_currentVmServiceFlags
or something?)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.
Good point, changed to _currentVmServiceFlags!