[ DWDS ] Launch DDS when using the web socket proxy service #2706
Conversation
DDS is needed to serve DevTools, so the web socket proxy service should be setup to launch DDS at startup. This change also includes some significant refactors to reduce the amount of duplicate code that could be shared by the Chrome and web socket service implementations.
| requestStream: requestStream, | ||
| requestSink: requestSink, | ||
| dwdsStats: dwdsStats, | ||
| clientFuture: clientCompleter.future, |
There was a problem hiding this comment.
here I see that you no longer pass a clientFuture argument. Could this maybe cause an exception when we await the clientFuture downstream (ie. In maybeHandleServiceExtensionRequestImpl)?
I think we should pass the _clientCompleter.future from ChromeDwdsVmClient to the base class when we initialize it. WDYT?
There was a problem hiding this comment.
Correct, because it doesn't need to be passed down any further and is only needed for the ChromeDwdsVmClient. If you look at this line, you'll see we still have the completion logic that runs after this initialize() method completes.
|
@bkonyi This is a great refactor! I just added a few comment for you to review. Thanks Ben |
|
Landing as the failures are due to a upstream bug unrelated to this change. |
DDS is needed to serve DevTools, so the web socket proxy service should be setup to launch DDS at startup.
This change also includes some significant refactors to reduce the amount of duplicate code that could be shared by the Chrome and web socket service implementations.