Support hot reload over websocket

[jyameo]

• Added WebSocket-based hot reload support: reloadSources in ChromeProxyService and DevHandler can now handle hot reload requests and responses over WebSockets (protected by bool flag useWebSocket).
• Refactored the injected client to use a reusable function for handling hot reload requests and responses over WebSockets.

Related to #2605

[srujzs]

Thanks Jessy! This generally looks good % my comment about requests coming in too quick.

[srujzs]

The way the hot reload works in the client is it looks at the hotReloadSourcesUri which contains all the changed files and their libraries and then does an XHR. If whatever app that calls DWDS sends requests too quickly, it may be a possible that a hot reload request reads a version of the file that is no longer the correct version.

For example, the app sends a request and then sets the data in that URI. The app sends a second request with new data in that URI before the first request finishes. Then the first request is reading the new data instead.

[jyameo]

I see. This makes sense. If I add a request/response mechanism for fetchLibrariesForHotReload, similar to how I handle HotReloadRequest with a Completer, would that help resolve the possible race condition you described? Essentially by sending a FetchLibrariesForHotReloadRequest and waiting for its response (using a Completer), we can ensure that the client fetches the correct set of changed libraries before proceeding to the hot reload step. What do you think about this approach?

[srujzs]

I think we want to have the completer wait for both fetchLibrariesForHotReload and hotReload. The former may return the right paths now, but those paths may be replaced with new compiled files before hotReload gets to execute if we don't wait for hotReload as well.

But I believe that should work. This more or less "blocks" the request queue until the previous request is fully finished.

[srujzs]

Sorry Jessy, I may have led you astray here and we should discuss this a bit more before we settle on an implementation.

I don't think the current code makes much of a difference as we just split the requests into two.

Going back to the original code you had, I'm realizing we just await the completer we created to finish before we return from reloadSources. This is a little confusing to me, as that means we're waiting for the hot reload to finish for every request. In that case, we don't really need a map of completers, we could just have one pending completer. There's also no race condition here because there's only one request ever being processed.

I'm assuming this isn't what we want, though, and we do want DWDS to handle multiple requests coming in without blocking (but maybe not, in which case the original code works!). In the Flutter tools case, we always await reloadSources to finish when a reload request comes in. How does the workflow look like for Firebase Studio?

[biggs0125]

Based on the comments above it seems like these both need to happen before we 'ack' the hot reload request. I'm not clear on a couple things:

1. Why do these need to be 2 separate functions if we need to make sure they're called together anyway?
2. Once the future from fetchLibrariesForHotReload completes the files are loaded into the page and we have an ID associated with the request so we can avoid mixing up files. Why do we need to wait for the client to finish applying the hot reload patch (i.e. calling hotReload) to 'ack' the server and unblock it to process further requests?

cc @srujzs

[srujzs]

1. In the non-websocket case, we use the list of libraries to disable breakpoints before we continue the hot reload. Doing this after the full hot reload means we may come across a previous breakpoint (maybe in a captured closure?) before we have the chance to disable all the breakpoints.

Once breakpoints work for hot reload, we'll definitely need this to be two methods so that the debugger can place breakpoints in the newly loaded libraries before they're added to the runtime (between hotReloadStart and hotReloadEnd).

2. the files are loaded into the page

Not yet. This just fetches the list, but doesn't load the files into the page until a later call to hotReload. Once breakpoints work, that will be true though.

and we have an ID associated with the request so we can avoid mixing up files. Why do we need to wait for the client to finish applying the hot reload patch (i.e. calling hotReload) to 'ack' the server and unblock it to process further requests?

We don't pipe that ID to the DDC embedder though. I'm imagining a scenario where the first request fetches the list, maybe even loads the files onto the page, but before we call hotReloadEnd, the second request also fetches its list and loads the files onto the page, essentially clobbering the information in the embedder that we need for the first hotReloadEnd. I believe that's possible due to the async boundaries. We could have an ID associated with the request within the embedder and it could do some of that state management, but that doesn't exist today. We rate-limited hot restart requests in the past (_hotRestartRunIdCache), so we may be able to do something similar.