[google_sign_in] Fix chained async methods in error handling zones (flutter#2059)

Author: mehmetf

packages/google_sign_in/CHANGELOG.md

@@ -1,3 +1,8 @@
+## 4.0.8
+
+* Get rid of `MethodCompleter` and serialize async actions using chained futures.
+  This prevents a bug when sign in methods are being used in error handling zones.
+
 ## 4.0.7
 
 * Switch from using `api` to `implementation` for dependency on `play-services-auth`,

packages/google_sign_in/lib/google_sign_in.dart

@@ -230,50 +230,57 @@ class GoogleSignIn {
   }
 
   Future<void> _ensureInitialized() {
-    if (_initialization == null) {
-      _initialization = channel.invokeMethod<void>('init', <String, dynamic>{
-        'signInOption': (signInOption ?? SignInOption.standard).toString(),
-        'scopes': scopes ?? <String>[],
-        'hostedDomain': hostedDomain,
-      })
-        ..catchError((dynamic _) {
-          // Invalidate initialization if it errored out.
-          _initialization = null;
-        });
-    }
-    return _initialization;
+    return _initialization ??=
+        channel.invokeMethod<void>('init', <String, dynamic>{
+      'signInOption': (signInOption ?? SignInOption.standard).toString(),
+      'scopes': scopes ?? <String>[],
+      'hostedDomain': hostedDomain,
+    })
+          ..catchError((dynamic _) {
+            // Invalidate initialization if it errored out.
+            _initialization = null;
+          });
   }
 
-  /// Keeps track of the most recently scheduled method call.
-  _MethodCompleter _lastMethodCompleter;
+  /// The most recently scheduled method call.
+  Future<void> _lastMethodCall;
+
+  /// Returns a [Future] that completes with a success after [future], whether
+  /// it completed with a value or an error.
+  static Future<void> _waitFor(Future<void> future) {
+    final Completer<void> completer = Completer<void>();
+    future.whenComplete(completer.complete).catchError((dynamic _) {
+      // Ignore if previous call completed with an error.
+    });
+    return completer.future;
+  }
 
   /// Adds call to [method] in a queue for execution.
   ///
   /// At most one in flight call is allowed to prevent concurrent (out of order)
   /// updates to [currentUser] and [onCurrentUserChanged].
-  Future<GoogleSignInAccount> _addMethodCall(String method) {
-    if (_lastMethodCompleter == null) {
-      _lastMethodCompleter = _MethodCompleter(method)
-        ..complete(_callMethod(method));
-      return _lastMethodCompleter.future;
+  Future<GoogleSignInAccount> _addMethodCall(String method) async {
+    Future<GoogleSignInAccount> response;
+    if (_lastMethodCall == null) {
+      response = _callMethod(method);
+    } else {
+      response = _lastMethodCall.then((_) {
+        // If after the last completed call `currentUser` is not `null` and requested
+        // method is a sign in method, re-use the same authenticated user
+        // instead of making extra call to the native side.
+        const List<String> kSignInMethods = <String>[
+          'signIn',
+          'signInSilently'
+        ];
+        if (kSignInMethods.contains(method) && _currentUser != null) {
+          return _currentUser;
+        } else {
+          return _callMethod(method);
+        }
+      });
     }
-
-    final _MethodCompleter completer = _MethodCompleter(method);
-    _lastMethodCompleter.future.whenComplete(() {
-      // If after the last completed call currentUser is not null and requested
-      // method is a sign in method, re-use the same authenticated user
-      // instead of making extra call to the native side.
-      const List<String> kSignInMethods = <String>['signIn', 'signInSilently'];
-      if (kSignInMethods.contains(method) && _currentUser != null) {
-        completer.complete(_currentUser);
-      } else {
-        completer.complete(_callMethod(method));
-      }
-    }).catchError((dynamic _) {
-      // Ignore if previous call completed with an error.
-    });
-    _lastMethodCompleter = completer;
-    return _lastMethodCompleter.future;
+    _lastMethodCall = _waitFor(response);
+    return response;
   }
 
   /// The currently signed in account, or null if the user is signed out.
@@ -296,12 +303,17 @@ class GoogleSignIn {
   /// returned Future completes with [PlatformException] whose `code` can be
   /// either [kSignInRequiredError] (when there is no authenticated user) or
   /// [kSignInFailedError] (when an unknown error occurred).
-  Future<GoogleSignInAccount> signInSilently({bool suppressErrors = true}) {
-    final Future<GoogleSignInAccount> result = _addMethodCall('signInSilently');
-    if (suppressErrors) {
-      return result.catchError((dynamic _) => null);
+  Future<GoogleSignInAccount> signInSilently(
+      {bool suppressErrors = true}) async {
+    try {
+      return await _addMethodCall('signInSilently');
+    } catch (_) {
+      if (suppressErrors) {
+        return null;
+      } else {
+        rethrow;
+      }
     }
-    return result;
   }
 
   /// Returns a future that resolves to whether a user is currently signed in.
@@ -334,26 +346,3 @@ class GoogleSignIn {
   /// authentication.
   Future<GoogleSignInAccount> disconnect() => _addMethodCall('disconnect');
 }
-
-class _MethodCompleter {
-  _MethodCompleter(this.method);
-
-  final String method;
-  final Completer<GoogleSignInAccount> _completer =
-      Completer<GoogleSignInAccount>();
-
-  Future<void> complete(FutureOr<GoogleSignInAccount> value) async {
-    if (value is Future<GoogleSignInAccount>) {
-      try {
-        _completer.complete(await value);
-      } catch (e, stacktrace) {
-        _completer.completeError(e, stacktrace);
-      }
-    } else {
-      _completer.complete(value);
-    }
-  }
-
-  bool get isCompleted => _completer.isCompleted;
-  Future<GoogleSignInAccount> get future => _completer.future;
-}

packages/google_sign_in/pubspec.yaml

@@ -3,7 +3,7 @@ description: Flutter plugin for Google Sign-In, a secure authentication system
   for signing in with a Google account on Android and iOS.
 author: Flutter Team <flutter-dev@googlegroups.com>
 homepage: https://github.com/flutter/plugins/tree/master/packages/google_sign_in
-version: 4.0.7
+version: 4.0.8
 
 flutter:
   plugin:

packages/google_sign_in/test/google_sign_in_test.dart

@@ -45,7 +45,11 @@ void main() {
       responses = Map<String, dynamic>.from(kDefaultResponses);
       channel.setMockMethodCallHandler((MethodCall methodCall) {
         log.add(methodCall);
-        return Future<dynamic>.value(responses[methodCall.method]);
+        final dynamic response = responses[methodCall.method];
+        if (response != null && response is Exception) {
+          return Future<dynamic>.error('$response');
+        }
+        return Future<dynamic>.value(response);
       });
       googleSignIn = GoogleSignIn();
       log.clear();
@@ -142,6 +146,27 @@ void main() {
       ]);
     });
 
+    test('signIn works even if a previous call throws error in other zone',
+        () async {
+      responses['signInSilently'] = Exception('Not a user');
+      await runZoned(() async {
+        expect(await googleSignIn.signInSilently(), isNull);
+      }, onError: (dynamic e, dynamic st) {});
+      expect(await googleSignIn.signIn(), isNotNull);
+      expect(
+        log,
+        <Matcher>[
+          isMethodCall('init', arguments: <String, dynamic>{
+            'signInOption': 'SignInOption.standard',
+            'scopes': <String>[],
+            'hostedDomain': null,
+          }),
+          isMethodCall('signInSilently', arguments: null),
+          isMethodCall('signIn', arguments: null),
+        ],
+      );
+    });
+
     test('concurrent calls of the same method trigger sign in once', () async {
       final List<Future<GoogleSignInAccount>> futures =
           <Future<GoogleSignInAccount>>[
@@ -170,7 +195,7 @@ void main() {
     });
 
     test('can sign in after previously failed attempt', () async {
-      responses['signInSilently'] = <String, dynamic>{'error': 'Not a user'};
+      responses['signInSilently'] = Exception('Not a user');
       expect(await googleSignIn.signInSilently(), isNull);
       expect(await googleSignIn.signIn(), isNotNull);
       expect(