Skip to content

Fix bugs with ext.flutter.activeDevToolsServerAddress - #74423

Merged
christopherfujino merged 3 commits into
flutter:masterfrom
kenzieschmoll:dt-server
Jan 21, 2021
Merged

christopherfujino merged 3 commits into
flutter:masterfrom
kenzieschmoll:dt-server

Conversation

@kenzieschmoll

@kenzieschmoll kenzieschmoll commented Jan 21, 2021 •

Copy link
Copy Markdown
Member

Fixes #74407

A completer was attempting to fire again after hot restarting - this PR fixes that. We also were not properly restoring the state of ext.flutter.activeDevToolsServerAddress on hot restart, so this PR fixes that issue as well.

Working on adding tests to verify fix.

@flutter-dashboard flutter-dashboard Bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Jan 21, 2021
@flutter-dashboard

Copy link
Copy Markdown

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Comment thread packages/flutter_tools/lib/src/resident_runner.dart

@jonahwilliams jonahwilliams left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jonahwilliams

Copy link
Copy Markdown
Contributor

Fixes #74431

@christopherfujino

Copy link
Copy Markdown
Contributor

@kenzieschmoll is it safe for me to land this now? This is one of the pre-requisites for making flutter-build going green.

@kenzieschmoll

Copy link
Copy Markdown
Member Author

Yes safe to land.

@christopherfujino
christopherfujino merged commit 75dc45a into flutter:master Jan 21, 2021
@christopherfujino

Copy link
Copy Markdown
Contributor

Thanks @kenzieschmoll. Landed on red flutter-build to fix a test.

if (!silent) {
globals.printStatus('Restarted application in ${getElapsedAsMilliseconds(timer.elapsed)}.');
}
unawaited(maybeCallDevToolsUriServiceExtension());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to have a comment saying why it's ok to ignore errors from this future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crash: "Bad state: Future already completed" when hot restarting

4 participants