Don't reuse embedded windows in launchDevTools (#2489)

Fixes Dart-Code/Dart-Code#2931.

packages/devtools_app/lib/src/framework_controller.dart

@@ -23,7 +23,7 @@ class FrameworkController {
   final StreamController<Uri> _connectedController =
       StreamController.broadcast();
   final StreamController _disconnectedController = StreamController.broadcast();
-  final StreamController<String> _pageChangeController =
+  final StreamController<PageChangeEvent> _pageChangeController =
       StreamController.broadcast();
 
   /// Show the indicated page.
@@ -50,13 +50,11 @@ class FrameworkController {
   Stream<Uri> get onConnected => _connectedController.stream;
 
   /// Notifies when the current page changes.
-  ///
-  /// This notifies with the page ID.
-  Stream<String> get onPageChange => _pageChangeController.stream;
+  Stream<PageChangeEvent> get onPageChange => _pageChangeController.stream;
 
   /// Notify the controller that the current page has changed.
-  void notifyPageChange(String pageId) {
-    _pageChangeController.add(pageId);
+  void notifyPageChange(PageChangeEvent page) {
+    _pageChangeController.add(page);
   }
 
   /// Notifies when a device disconnects from DevTools.
@@ -79,3 +77,10 @@ class ConnectVmEvent {
   final Uri serviceProtocolUri;
   final bool notify;
 }
+
+class PageChangeEvent {
+  PageChangeEvent(this.id, this.embedded);
+
+  final String id;
+  final bool embedded;
+}

packages/devtools_app/lib/src/scaffold.dart

@@ -204,7 +204,9 @@ class DevToolsScaffoldState extends State<DevToolsScaffold>
 
         // Send the page change info to the framework controller (it can then
         // send it on to the devtools server, if one is connected).
-        frameworkController.notifyPageChange(screen?.screenId);
+        frameworkController.notifyPageChange(
+          PageChangeEvent(screen?.screenId, widget.embed),
+        );
 
         // If the tab index is 0 and the current route has no page ID (eg. we're
         // at the URL /?uri= with no page ID), those are equivalent pages but
@@ -222,7 +224,9 @@ class DevToolsScaffoldState extends State<DevToolsScaffold>
     });
 
     // Broadcast the initial page.
-    frameworkController.notifyPageChange(_currentScreen.value.screenId);
+    frameworkController.notifyPageChange(
+      PageChangeEvent(_currentScreen.value.screenId, widget.embed),
+    );
   }
 
   /// Connects to the VM with the given URI. This request usually comes from the

packages/devtools_app/lib/src/server_api_client.dart

@@ -10,6 +10,7 @@ import 'package:http/http.dart' as http;
 import 'config_specific/logger/logger.dart';
 import 'config_specific/notifications/notifications.dart';
 import 'config_specific/sse/sse_shim.dart';
+import 'framework_controller.dart';
 import 'globals.dart';
 
 /// This class coordinates the connection between the DevTools server and the
@@ -72,8 +73,8 @@ class DevToolsServerConnection {
       _notifyConnected(vmServiceUri);
     });
 
-    frameworkController.onPageChange.listen((pageId) {
-      _notifyCurrentPage(pageId);
+    frameworkController.onPageChange.listen((page) {
+      _notifyCurrentPage(page);
     });
 
     frameworkController.onDisconnected.listen((_) {
@@ -162,8 +163,14 @@ class DevToolsServerConnection {
     _callMethod('connected', {'uri': vmServiceUri.toString()});
   }
 
-  void _notifyCurrentPage(String pageId) {
-    _callMethod('currentPage', {'id': pageId});
+  void _notifyCurrentPage(PageChangeEvent page) {
+    _callMethod(
+      'currentPage',
+      {
+        'id': page.id,
+        if (page.embedded != null) 'embedded': page.embedded,
+      },
+    );
   }
 
   void _notifyDisconnected() {

packages/devtools_app/test/integration_tests/server_test.dart

@@ -357,6 +357,40 @@ void main() {
         expect(serverResponse['clients'], hasLength(1));
       }, timeout: const Timeout.factor(10));
 
+      test('Server does not reuses DevTools instance if embedded', () async {
+        // Register the VM.
+        await _send('vm.register', {'uri': appFixture.serviceUri.toString()});
+
+        // Spawn an embedded version of DevTools in a browser.
+        final event = await serverStartedEvent;
+        final devToolsUri =
+            'http://${event['params']['host']}:${event['params']['port']}';
+        final launchUrl = '$devToolsUri/?embed=true&page=logging'
+            '&uri=${Uri.encodeQueryComponent(appFixture.serviceUri.toString())}';
+        final chrome = await Chrome.locate().start(url: launchUrl);
+        try {
+          {
+            final serverResponse =
+                await _waitForClients(requiredConnectionState: true);
+            expect(serverResponse['clients'], hasLength(1));
+          }
+
+          // Send a request to the server to launch and ensure it did
+          // not reuse the existing connection. Launch it on a different page
+          // so we can easily tell once this one has connected.
+          final launchResponse = await _sendLaunchDevToolsRequest(
+              useVmService: useVmService, reuseWindows: true, page: 'memory');
+          expect(launchResponse['reused'], isFalse);
+
+          // Ensure there's now two connections.
+          final serverResponse = await _waitForClients(
+              requiredConnectionState: true, requiredPage: 'memory');
+          expect(serverResponse['clients'], hasLength(2));
+        } finally {
+          chrome?.kill();
+        }
+      }, timeout: const Timeout.factor(10));
+
       test('Server reuses DevTools instance if not connected to a VM',
           () async {
         // Register the VM.

packages/devtools_server/lib/src/client_manager.dart

@@ -45,17 +45,23 @@ class ClientManager {
   /// and it disconnected, then started debugging again, we want to reuse
   /// the open DevTools window).
   DevToolsClient findReusableClient() {
-    return _clients.firstWhere((c) => !c.hasConnection, orElse: () => null);
+    return _clients.firstWhere(
+      (c) => !c.hasConnection && !c.embedded,
+      orElse: () => null,
+    );
   }
 
   /// Finds a client that may already be connected to this VM Service.
-  DevToolsClient findExistingConnectedClient(Uri vmServiceUri) {
+  DevToolsClient findExistingConnectedReusableClient(Uri vmServiceUri) {
     // Checking the whole URI will fail if DevTools converted it from HTTP to
     // WS, so just check the host, port and first segment of path (token).
     return _clients.firstWhere(
-        (c) =>
-            c.hasConnection && _areSameVmServices(c.vmServiceUri, vmServiceUri),
-        orElse: () => null);
+      (c) =>
+          c.hasConnection &&
+          !c.embedded &&
+          _areSameVmServices(c.vmServiceUri, vmServiceUri),
+      orElse: () => null,
+    );
   }
 
   bool _areSameVmServices(Uri uri1, Uri uri2) {
@@ -86,6 +92,7 @@ class DevToolsClient {
           return;
         case 'currentPage':
           _currentPage = request['params']['id'];
+          _embedded = request['params']['embedded'] == true;
           _respond(request);
           return;
         case 'disconnected':
@@ -163,12 +170,14 @@ class DevToolsClient {
   }
 
   final SseConnection _connection;
-  Uri _vmServiceUri;
 
+  Uri _vmServiceUri;
   Uri get vmServiceUri => _vmServiceUri;
-
   bool get hasConnection => _vmServiceUri != null;
-  String _currentPage;
 
+  String _currentPage;
   String get currentPage => _currentPage;
+
+  bool _embedded = false;
+  bool get embedded => _embedded;
 }

packages/devtools_server/lib/src/server.dart

@@ -703,6 +703,7 @@ Future<void> _handleClientsList(
             .map((c) => {
                   'hasConnection': c.hasConnection,
                   'currentPage': c.currentPage,
+                  'embedded': c.embedded,
                   'vmServiceUri': c.vmServiceUri?.toString(),
                 })
             .toList()
@@ -719,7 +720,8 @@ Future<bool> _tryReuseExistingDevToolsInstance(
 ) async {
   // First try to find a client that's already connected to this VM service,
   // and just send the user a notification for that one.
-  final existingClient = clients.findExistingConnectedClient(vmServiceUri);
+  final existingClient =
+      clients.findExistingConnectedReusableClient(vmServiceUri);
   if (existingClient != null) {
     try {
       await existingClient.showPage(page);