Skip to content

#167410: _initCalled completed twice #9694

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

#167410: _initCalled completed twice #9694

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7a44a91
#167410: _initCalled completed twice
srivats22 Jul 29, 2025
d061d6a
Fixing build issues
srivats22 Jul 29, 2025
3ca9fc4
Fixing test issues
srivats22 Jul 31, 2025
7781af6
Merge branch 'main' of https://github.com/flutter/packages into 167410
srivats22 Aug 20, 2025
a954909
resolved merge conflicts
srivats22 Aug 20, 2025
6ef759f
dart format
srivats22 Aug 20, 2025
e6adf2d
Merge branch 'flutter:main' into 167410
srivats22 Sep 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/google_sign_in/google_sign_in_web/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 1.0.1

* Fixes Bad state: Future already completed on the web.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sounds like there was a bug in the plugin that was fixed, which is not the case. This should say something like "Removes initialization checks, since initialization handling has moved to the app-facing package."


## NEXT

* Updates minimum supported SDK version to Flutter 3.29/Dart 3.7.
Expand Down
29 changes: 0 additions & 29 deletions ...s/google_sign_in/google_sign_in_web/example/integration_test/google_sign_in_web_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,35 +90,6 @@ void main() {
);
}, throwsAssertionError);
});

testWidgets('must be called for most of the API to work', (_) async {
expect(() async {
await plugin.attemptLightweightAuthentication(
const AttemptLightweightAuthenticationParameters(),
);
}, throwsStateError);

expect(() async {
await plugin.clientAuthorizationTokensForScopes(
const ClientAuthorizationTokensForScopesParameters(
request: AuthorizationRequestDetails(
scopes: <String>[],
userId: null,
email: null,
promptIfUnauthorized: false,
),
),
);
}, throwsStateError);

expect(() async {
await plugin.signOut(const SignOutParams());
}, throwsStateError);

expect(() async {
await plugin.disconnect(const DisconnectParams());
}, throwsStateError);
});
});

group('support queries', () {
Expand Down
33 changes: 5 additions & 28 deletions packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,9 @@ class GoogleSignInPlugin extends GoogleSignInPlatform {
@visibleForTesting GisSdkClient? debugOverrideGisSdkClient,
@visibleForTesting
StreamController<AuthenticationEvent>? debugAuthenticationController,
}) : _gisSdkClient = debugOverrideGisSdkClient,
_authenticationController =
debugAuthenticationController ??
StreamController<AuthenticationEvent>.broadcast() {
}) : _gisSdkClient = debugOverrideGisSdkClient,
_authenticationController = debugAuthenticationController ??
StreamController<AuthenticationEvent>.broadcast() {
autoDetectedClientId = web.document
.querySelector(clientIdMetaSelector)
?.getAttribute(clientIdAttributeName);
Expand All @@ -68,8 +67,6 @@ class GoogleSignInPlugin extends GoogleSignInPlatform {

// A future that completes when the JS loader is done.
late Future<void> _jsSdkLoadedFuture;
// A future that completes when the `init` call is done.
Completer<void>? _initCalled;

// A StreamController to communicate status changes from the GisSdkClient.
final StreamController<AuthenticationEvent> _authenticationController;
Expand All @@ -89,28 +86,14 @@ class GoogleSignInPlugin extends GoogleSignInPlatform {
return _gisSdkClient!;
}

// This method throws if init or initWithParams hasn't been called at some
// point in the past. It is used by the [initialized] getter to ensure that
// users can't await on a Future that will never resolve.
void _assertIsInitCalled() {
if (_initCalled == null) {
throw StateError(
'GoogleSignInPlugin::init() or GoogleSignInPlugin::initWithParams() '
'must be called before any other method in this plugin.',
);
}
}

/// A future that resolves when the plugin is fully initialized.
///
/// This ensures that the SDK has been loaded, and that the `init` method
/// has finished running.
@visibleForTesting
Future<void> get initialized {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method should just be removed.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will work on it and push the updated changes by today if possible

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing the function completely won't work cause the renderButton uses the initialized function as the future

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That code is wrong now though; initialized is now equivalent to just _jsSdkLoadedFuture in this PR, but the code in renderButton that is being guarded requires _gisSdkClient to be non-null, and there's no order guarantee between those two things.

If we want to keep renderButton callable before initialization (I'm skeptical that that's actually important with the new API structure), then the internals need to be restructured to make that actually work. But that can be done with an internal future that doesn't have a getter wrapping it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood so how should I proceed??

_assertIsInitCalled();
return Future.wait<void>(<Future<void>>[
_jsSdkLoadedFuture,
_initCalled!.future,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is the right fix.

There's a chance _jsSdkLoadedFuture completes and app code starts using the plugin before _gisSdkClient has been set (in init()).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As of google_sign_in 7.x it is a programming error at the app-facing package level for app to code to fail to (indirectly) await this layer's init().

]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can become:

Future<void> get initialized {
  _assertIsInitCalled();
  return _initCalled!.future; // `init()` already awaits `_jsSdkLoadedFuture` so there's no need to wait for it here.
}

}

Expand All @@ -133,12 +116,8 @@ class GoogleSignInPlugin extends GoogleSignInPlatform {
' or pass clientId when initializing GoogleSignIn',
);

assert(
params.serverClientId == null,
'serverClientId is not supported on Web.',
);

_initCalled = Completer<void>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may work by putting the completer in a local variable:

final initCalled = _initCalled = Completer<void>();

and later:

initCalled.complete(); // Use the local completer, not `_initCalled`

assert(params.serverClientId == null,
'serverClientId is not supported on Web.');

await _jsSdkLoadedFuture;

Expand All @@ -149,8 +128,6 @@ class GoogleSignInPlugin extends GoogleSignInPlatform {
authenticationController: _authenticationController,
loggingEnabled: kDebugMode,
);

_initCalled!.complete(); // Signal that `init` is fully done.
}

@override
Expand Down
2 changes: 1 addition & 1 deletion packages/google_sign_in/google_sign_in_web/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ description: Flutter plugin for Google Sign-In, a secure authentication system
for signing in with a Google account on Android, iOS and Web.
repository: https://github.com/flutter/packages/tree/main/packages/google_sign_in/google_sign_in_web
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+google_sign_in%22
version: 1.0.0
version: 1.0.1

environment:
sdk: ^3.7.0
Expand Down