dart-lang / sdk

The Dart SDK, including the VM, dart2js, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
9.97k stars 1.54k forks source link

[CP] beta - devtools server can reuse disconnected DevTools instance #56128

Open elliette opened 1 week ago

elliette commented 1 week ago

Commit(s) to merge

https://dart-review.googlesource.com/c/sdk/+/374298

Target

beta

Prepared changelist for beta/stable

https://dart-review.googlesource.com/c/sdk/+/374460

Issue Description

https://github.com/flutter/devtools/issues/8008

When a user launched DevTools from their IDE in a new window, disconnected from their app, and then tried to relaunch DevTools, they would see an infinite spinner on the opened window instead of a DevTools connection to their app.

What is the fix

Fix: https://github.com/flutter/devtools/pull/8009

Makes sure that DevTools receives a connect event from the devtools_server, even after disconnection.

Why cherry-pick

This prevents users from re-connecting to DevTools in an existing browser window.

Risk

Low. Only changes are in DevTools to fix the issue described above:

https://github.com/flutter/devtools/compare/v2.37.0..v2.37.1

The test that caught the regression has been confirmed fixed after https://dart-review.googlesource.com/c/sdk/+/374298, see https://github.com/dart-lang/sdk/issues/56111#issuecomment-2206939914

Issue link(s)

https://github.com/flutter/devtools/issues/8008

Extra Info

No response

dart-github-bot commented 1 week ago

Summary: DevTools users experienced an infinite spinner when relaunching DevTools in a new window after disconnecting from their app. The issue was caused by DevTools not receiving a connect event from the devtools_server after disconnection. The fix ensures that DevTools receives a connect event even after disconnection.

vsmenon commented 1 week ago

@kenzieschmoll - do you also recommend cherry picking?

kenzieschmoll commented 1 week ago

Yes, I do. CP LGTM. This regression has not yet made its way to stable, so it is important that we prevent the bug from going out to stable users.

itsjustkevin commented 1 week ago

@kenzieschmoll could you review the CL? @elliette after review by @kenzieschmoll you can go ahead and merge to beta!

elliette commented 1 week ago

Thanks! It's been merged