Merge branch 'master' of github.com:flutter/devtools into offline2

Author: polina-c

packages/devtools_app/integration_test/test/live_connection/service_connection_test.dart

@@ -11,6 +11,7 @@ import 'package:devtools_app/src/service/service_extensions.dart' as extensions;
 import 'package:devtools_app_shared/service.dart';
 import 'package:devtools_app_shared/ui.dart';
 import 'package:devtools_app_shared/utils.dart';
+import 'package:devtools_shared/devtools_shared.dart';
 import 'package:devtools_test/helpers.dart';
 import 'package:devtools_test/integration_test.dart';
 import 'package:flutter/widgets.dart';
@@ -398,8 +399,8 @@ Future<void> _serviceExtensionAvailable(String extensionName) async {
 
   final completer = Completer<void>();
   void listener() {
-    if (listenable.value && !completer.isCompleted) {
-      completer.complete();
+    if (listenable.value) {
+      completer.safeComplete();
     }
   }
 

packages/devtools_app/lib/src/extensions/embedded/_view_web.dart

@@ -9,6 +9,7 @@ import 'package:devtools_app_shared/ui.dart';
 import 'package:devtools_app_shared/utils.dart';
 import 'package:devtools_extensions/api.dart';
 import 'package:devtools_extensions/utils.dart';
+import 'package:devtools_shared/devtools_shared.dart';
 import 'package:flutter/material.dart';
 import 'package:web/web.dart';
 
@@ -268,9 +269,7 @@ class _ExtensionIFrameController extends DisposableController
 
     switch (event.type) {
       case DevToolsExtensionEventType.pong:
-        if (!_extensionHandlerReady.isCompleted) {
-          _extensionHandlerReady.complete();
-        }
+        _extensionHandlerReady.safeComplete();
         break;
       case DevToolsExtensionEventType.vmServiceConnection:
         updateVmServiceConnection(

packages/devtools_app/lib/src/service/vm_service_wrapper.dart

@@ -13,6 +13,7 @@ import 'package:dap/dap.dart' as dap;
 import 'package:dds_service_extensions/dap.dart';
 import 'package:dds_service_extensions/dds_service_extensions.dart';
 import 'package:devtools_app_shared/service.dart';
+import 'package:devtools_shared/devtools_shared.dart';
 import 'package:flutter/foundation.dart';
 import 'package:logging/logging.dart';
 import 'package:vm_service/vm_service.dart';
@@ -380,8 +381,8 @@ class VmServiceWrapper extends VmService {
 
     void futureComplete() {
       activeFutures.remove(trackedFuture);
-      if (activeFutures.isEmpty && !_allFuturesCompleter.isCompleted) {
-        _allFuturesCompleter.complete(true);
+      if (activeFutures.isEmpty) {
+        _allFuturesCompleter.safeComplete(true);
       }
     }
 

packages/devtools_app/test/legacy_integration_tests/integration.dart

@@ -9,6 +9,7 @@ import 'dart:async';
 import 'dart:convert';
 import 'dart:io';
 
+import 'package:devtools_shared/devtools_shared.dart';
 import 'package:devtools_shared/devtools_test_utils.dart';
 import 'package:flutter_test/flutter_test.dart';
 import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart'
@@ -302,12 +303,12 @@ class WebBuildFixture {
 
       // Serving `web` on http://localhost:8080
       if (line.contains('Serving `web`')) {
-        if (!hasUrl.isCompleted) {
-          final String url = line.substring(line.indexOf('http://'));
-          hasUrl.complete(url);
-        } else {
-          print('Ignoring "Serving..." notification because already completed');
-        }
+        hasUrl.safeComplete(
+          line.substring(line.indexOf('http://')),
+          () => print(
+            'Ignoring "Serving..." notification because already completed',
+          ),
+        );
       }
     });
 

packages/devtools_app_shared/lib/src/service/isolate_state.dart

@@ -5,6 +5,7 @@
 import 'dart:async';
 import 'dart:core';
 
+import 'package:devtools_shared/devtools_shared.dart';
 import 'package:flutter/foundation.dart';
 import 'package:meta/meta.dart';
 import 'package:vm_service/vm_service.dart' hide Error;
@@ -43,11 +44,10 @@ class IsolateState {
 
   void dispose() {
     _isolateNow = null;
-    if (!_isolateLoadCompleter.isCompleted) {
-      _isolateLoadCompleter.complete(null);
-    } else {
-      _isolateLoadCompleter = Completer()..complete(null);
-    }
+    _isolateLoadCompleter.safeComplete(
+      null,
+      () => _isolateLoadCompleter = Completer()..complete(null),
+    );
   }
 
   void handleDebugEvent(String? kind) {

packages/devtools_app_shared/lib/src/service/service_extension_manager.dart

@@ -5,6 +5,7 @@
 import 'dart:async';
 import 'dart:core';
 
+import 'package:devtools_shared/devtools_shared.dart';
 import 'package:flutter/foundation.dart';
 import 'package:logging/logging.dart';
 import 'package:vm_service/vm_service.dart' hide Error;
@@ -436,11 +437,7 @@ final class ServiceExtensionManager with DisposerMixin {
     // service extensions that might be registered.
     _performActionAndClearMap<Completer<bool>>(
       _maybeRegisteringServiceExtensions,
-      action: (completer) {
-        if (!completer.isCompleted) {
-          completer.complete(false);
-        }
-      },
+      action: (completer) => completer.safeComplete(false),
     );
 
     _performActionAndClearMap<ValueNotifier<bool>>(

packages/devtools_shared/CHANGELOG.md

@@ -1,6 +1,7 @@
 # 9.1.0-wip
 * Return valid extensions from the `apiServeAvailableExtensions` endpoint even when
 an exception is thrown.
+* Add utility extension methods on `Completer`: `safeComplete` and `safeCompleteError`.
 
 # 9.0.1
 * Restructure `devtools_extensions.dart` and `devtools_extensions_io.dart` libraries.

packages/devtools_shared/lib/devtools_shared.dart

@@ -15,3 +15,4 @@ export 'src/utils/compare.dart';
 export 'src/utils/file_utils.dart';
 export 'src/utils/retry.dart';
 export 'src/utils/semantic_version.dart';
+export 'src/utils/utils.dart';

packages/devtools_shared/lib/src/service/service.dart

@@ -12,6 +12,7 @@ import 'package:vm_service/vm_service.dart';
 import 'package:web_socket_channel/web_socket_channel.dart';
 
 import '../sse/sse_shim.dart';
+import '../utils/utils.dart';
 
 Future<T> _connectWithSse<T extends VmService>({
   required Uri uri,
@@ -87,11 +88,7 @@ Future<T> connect<T extends VmService>({
 }) {
   final connectedCompleter = Completer<T>();
 
-  void onError(Object? error) {
-    if (!connectedCompleter.isCompleted) {
-      connectedCompleter.completeError(error!);
-    }
-  }
+  void onError(Object? error) => connectedCompleter.safeCompleteError(error!);
 
   // Connects to a VM Service but does not verify the connection was fully
   // successful.
@@ -118,11 +115,7 @@ Future<T> connect<T extends VmService>({
   }
 
   connectHelper().then(
-    (service) {
-      if (!connectedCompleter.isCompleted) {
-        connectedCompleter.complete(service);
-      }
-    },
+    (service) => connectedCompleter.safeComplete(service),
     onError: onError,
   );
   finishedCompleter.future.then((_) {

packages/devtools_shared/lib/src/utils/utils.dart

@@ -0,0 +1,28 @@
+// Copyright 2024 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'dart:async';
+
+extension CompleterExtension<T> on Completer<T> {
+  /// Completes this completer with optional [value] only if it has not already
+  /// been completed.
+  ///
+  /// If the completer has already been completed, [orElse] will be called if it
+  /// is not null,
+  void safeComplete([T? value, void Function()? orElse]) {
+    if (!isCompleted) {
+      complete(value);
+    } else {
+      orElse?.call();
+    }
+  }
+
+  /// Completes this completer with [error] and an optional [stackTrace] only if
+  /// it has not already been completed.
+  void safeCompleteError(Object error, [StackTrace? stackTrace]) {
+    if (!isCompleted) {
+      completeError(error, stackTrace);
+    }
+  }
+}

packages/devtools_shared/test/utils/utils_test.dart

@@ -0,0 +1,89 @@
+// Copyright 2024 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'dart:async';
+
+import 'package:devtools_shared/devtools_shared.dart';
+import 'package:test/test.dart';
+
+void main() {
+  group('CompleterExtension', () {
+    late Completer<int> completer;
+
+    setUp(() {
+      completer = Completer<int>();
+    });
+
+    group('safeComplete', () {
+      int? value;
+      setUp(() {
+        value = null;
+        unawaited(completer.future.then((v) => value = v));
+      });
+
+      test('completes successfully', () async {
+        expect(completer.isCompleted, false);
+        expect(value, isNull);
+
+        completer.safeComplete(5);
+        await completer.future;
+        expect(completer.isCompleted, true);
+        expect(value, 5);
+      });
+
+      test('does not complete if already completed', () async {
+        expect(completer.isCompleted, false);
+        expect(value, isNull);
+        completer.complete(1);
+        await completer.future;
+
+        expect(completer.isCompleted, true);
+        expect(value, 1);
+
+        completer.safeComplete(5);
+        expect(completer.isCompleted, true);
+        expect(value, 1);
+
+        String? elseValue;
+        completer.safeComplete(3, () => elseValue = 'hit orElse');
+        expect(completer.isCompleted, true);
+        expect(value, 1);
+        expect(elseValue, 'hit orElse');
+      });
+    });
+
+    group('safeCompleteError', () {
+      late Future<int> errorFuture;
+      ({Object error, StackTrace? st})? error;
+
+      setUp(() {
+        unawaited(
+          errorFuture = completer.future.onError((e, st) {
+            error = (error: e!, st: st);
+            return -1;
+          }),
+        );
+      });
+
+      test('completes successfully', () async {
+        completer.safeCompleteError('This is an error');
+        await errorFuture;
+        expect(completer.isCompleted, true);
+        expect(error?.error, 'This is an error');
+      });
+
+      test('does not complete if already completed', () async {
+        completer.safeCompleteError('This is an error');
+        await errorFuture;
+        expect(completer.isCompleted, true);
+        expect(error?.error, 'This is an error');
+
+        completer.safeCompleteError('This is a different error');
+        await errorFuture;
+        expect(completer.isCompleted, true);
+        expect(error?.error, 'This is an error');
+      });
+    });
+  });
+}