modifying DWDS Injector to always inject client and introduce runMainAtStart flag

[jyameo]

Updated DWDS to always inject the client and use runMainAtStart flag to control whether main() execution is deferred or runs immediately.

Related to dart-lang/sdk#60289.

[srujzs]

I believe this _trySendEvent would eventually run main again, which we don't want. Then again, maybe it won't if it's web server? I'm not sure, but maybe something to investigate.

[jyameo]

This connectRequest establishes a socket connection to DWDS and is managed here. Eventually, main will be triggered from DWDS: https://github.com/dart-lang/webdev/blob/main/dwds/lib/src/handlers/dev_handler.dart#L286-L297.

This is how it's currently working for Chromium devices, so I recommend leaving this logic unchanged for now, since we're focusing on an issue with web-server devices rather than Chromium ones.

[nshahan]

I don't think of using flutter to launch the "web-server" device and connected chromium devices as being exclusive of each other. Users can and do connect chrome or other chromium browsers to an app that was launched using -d web-server.

[srujzs]

Eventually, main will be triggered from DWDS

Right, that's what I was referring to. I believe if you specify "runMainAtStart" and open the web server URL using Chrome, main might possibly get run twice now - one when you immediately run main and the other when the connect request succeeds.

[jyameo]

Hi @srujzs @nshahan, to address the issue above, I’ve updated the injected client to always send a connect request to DWDS. This ensures web-server can still support using the Dart Debug Extension to connect to the app. I’ll also be reusing this connection for the upcoming socket-based hot reload work.

To avoid running main twice, I added a markAsCompleted method in app_connection.dart. This can be used to prevent a second main invocation. In my follow-up PR in the Flutter repo, I’ll call this method when runMainAtStart is true, so we avoid running the app twice.

I’ve tested running the app with both web-server and chrome configurations, and I validated that the app runs on both Chrome and Safari without calling main() twice. I also made sure that using the Dart Debug Extension still works as intended and does not cause main() to be called twice.

Let me know if there are any questions or concerns!

[srujzs]

Thanks Jessy!

I think we may be adding a patch on a patch here which makes me nervous. We are in the current situation because:

• We start paused with the DDC library bundle format. We did this so that we can enable DWDS.
• Because the injected client immediately runs main and doesn't send a ConnectRequest when using Firefox or Safari, it works in those browsers. In Chrome, we send a ConnectRequest instead, waiting for main to be run. Because we start paused, nothing sends a resume request, so we never run main.

So I believe we're trying to work around --start-paused. If that flag is never passed, then we can do the ConnectRequest fine and DWDS will start main eventually. That also might be what doing a flutter run with the AMD format and web-server might be doing. If the flag is passed is passed, we'd be doing the right thing by pausing until resume. With this change, it looks like we just ignore --start-paused and start immediately with Chrome, which seems contradicting - runMainAtStart and start-paused is a strange combination. This is a revision of my last bullet-point (execute main immediately), but I believe it's less confusing and more consistent with current behavior.

The better answer might be to address enabling DWDS always in Flutter tools (along with the injected client) when using the library bundle format and remove the need for --start-paused so we don't introduce flags to work around its limitations.

I know there's been a lot of back and forth and I apologize for the complexity. I'd be happy to discuss this during the reload discussion on Thursday if it makes this easier to iterate on.

[jyameo]

Thanks for sharing this. I’ll be OOO until next Tuesday, so let’s sync up to discuss this when I’m back next week.

[jyameo]

There are some test failures with test/frontend_server_callstack_test.dart: shared context | callStack | expression evaluation succeeds on parent frame (failed).

The test seems to be passing when I run it locally so I'm not sure why it's failing in the CI. Any ideas? @bkonyi @srujzs

[nshahan]

I have a fix out for the existing test failures that should clear them up. Feel free to land your change before that makes it's way in though.
https://dart-review.googlesource.com/c/sdk/+/431951