#167410: _initCalled completed twice #9694

srivats22 · 2025-07-29T15:15:04Z

This PR Fixes: flutter/flutter#167410, where _initCalled was being performed twice on the web

Based on the discussion comments I have removed the calles to _initCalled in the google_sign_in_web package

Pre-Review Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩² ↩³

flutter-dashboard · 2025-07-29T15:15:07Z

It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

gemini-code-assist

Code Review

The pull request removes the _initCalled completer, which introduces a race condition. The init method should be made idempotent to handle being called multiple times. The CHANGELOG.md entry should be a complete sentence ending with a period.

srivats22 · 2025-07-29T21:20:21Z

Hi,

any idea or pointers on how to fix this:

The following TestFailure was thrown running a test (but after
the test had completed):
Expected: throws <Instance of 'StateError'>
  Actual: <Closure: () => Future<Null> from: () => {
                            let t$goto = 0, t$completer =
async._makeAsyncAwaitCompleter(T.Null());
                            var t$36asyncBody =
async._wrapJsFunctionForAsync((t$errorCode, t$result) => {
                              if (t$errorCode === 1) return
async._asyncRethrow(t$result, t$completer);
                              while (true)
                                switch (t$goto) {
                                  case 0:
                                    // Function start
                                    t$goto = 2;
                                    return
async._asyncAwait(t$36$35plugin$35get().disconnect(C[8] ||
CT.C8), t$36asyncBody, t$completer);
                                  case 2:
                                    // returning from await.
                                    // implicit return
                                    return
async._asyncReturn(null, t$completer);
                                }
                            });
                            return
async._asyncStartSync(t$36asyncBody, t$completer);
                          }>
   Which: returned a Future that emitted <null>

When the exception was thrown, this was the stack:

its failing for the same reason in 3 of the tests and the repo check I know the issue which I will fix...

stuartmorgan-g · 2025-07-31T12:49:59Z

I'm not sure I understand the question. The expectations that check that calling a method without calling init throws a StateError are failing because you removed the code that throws the StateError if a method is called without calling init.

srivats22 · 2025-07-31T12:52:51Z

Oh so the changes in the PR is incorrect? Or something else needs to change?

stuartmorgan-g · 2025-07-31T14:28:08Z

Tests that expect that the web implementation asserts init completion everywhere are no longer valid if the web implementation no longer asserts init completion. Intentional behavioral changes often require changing tests.

srivats22 · 2025-07-31T14:39:03Z

Understood let me look at the code and see where the initCompleted is still be called in the test and remove those... Last I checked wasn't able to find any

stuartmorgan-g · 2025-08-19T18:33:17Z

From triage: Is this ready for review?

srivats22 · 2025-08-19T18:35:08Z

Yes its ready for review looks like I might need to fix the conflicts which I will get to

mdebbar

Thanks for contributing this fix!

Some thoughts:

When the app calls init() twice (potentially with different params), it becomes a tricky situation. We already have the first init() in flight while the second init() is being executed.

I think we should let both init() calls continue to completion. Any calls to await initialized; should await the latest init() call.

init(InitParameters(clientId: 111));
init(InitParameters(clientId: 222));
await initialized;
// At this point, the plugin should be ready with clientId 222.

@stuartmorgan-g what do other platforms do in this case?

packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart

srivats22 · 2025-08-20T22:47:54Z

I made the changes based on what was put in the issue... What I got from it was since the logic has been moved outside the package the code for init seems redundant... I might have miss understood let me take a look based on the comments and try and incorporate it

stuartmorgan-g · 2025-08-21T12:16:31Z

@stuartmorgan-g what do other platforms do in this case?

Nothing; in the new version of google_sign_in, clients explicitly call an initialize method (rather than it being an internal, implicit thing on most platforms, but semi-exposed on web, as was the case before), and the docs say they must do so exactly once. It's a programming error by the client to write the code you've shown there.

If we think clients need explicit Errors when doing the wrong thing, that should be added to the app-facing package, rather than handled in each implementation. This code is a legacy of the exposed-for-web nature of the previous init code.

(Also, clients should basically never call the platform interface methods directly. That's not how most federated plugins are designed.)

stuartmorgan-g · 2025-09-03T14:24:33Z

packages/google_sign_in/google_sign_in_web/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 1.0.1
+
+* Fixes Bad state: Future already completed on the web.


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."

packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart

stuartmorgan-g · 2025-09-03T14:31:44Z

packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart

  /// This ensures that the SDK has been loaded, and that the `init` method
  /// has finished running.
  @visibleForTesting
  Future<void> get initialized {


This method should just be removed.

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

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

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.

Understood so how should I proceed??

@mdebbar Do you want renderButton to be callable before the plugin client has run initialize at the app-facing package level?

From triage: Ping @mdebbar on the question above.

I looked at iOS and Android implementations and I didn't see explicit errors being thrown if init() is called twice.

The documentation says: or calling this method more than once, will result in undefined behavior. I would say throwing an error qualifies as "undefined behavior" :)

Here's my suggestion:

Make initialized a private property _initialized to be used by methods in this file, and returned from init() to be used by users.

Throw if init() is called more than once.

_gisSdkClient should become late final GisSdkClient _gisSdkClient; so that it can only be set once, and it throws if accessed before initialization.

mdebbar

Sorry for the delay in my response. I gave this some more thought and looked at the documentation and other platforms. See my suggestion in the comment below.

mdebbar · 2025-10-27T15:13:28Z

packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart

  /// This ensures that the SDK has been loaded, and that the `init` method
  /// has finished running.
  @visibleForTesting
  Future<void> get initialized {


I looked at iOS and Android implementations and I didn't see explicit errors being thrown if init() is called twice.

The documentation says: or calling this method more than once, will result in undefined behavior. I would say throwing an error qualifies as "undefined behavior" :)

Here's my suggestion:

Make initialized a private property _initialized to be used by methods in this file, and returned from init() to be used by users.

Throw if init() is called more than once.

_gisSdkClient should become late final GisSdkClient _gisSdkClient; so that it can only be set once, and it throws if accessed before initialization.

srivats22 · 2025-10-27T15:18:33Z

Thank you, I will work on it with the directions provided and update the PR

mdebbar · 2025-11-04T19:59:23Z

packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart

+    if (_initCalled.isCompleted) {
+      throw StateError(
+        'init() has already been called. Calling init() more than once results in undefined behavior.',
+      );
+    }


This will prevent:

await plugin.init(...); await plugin.init(...); // This will throw.

But will not prevent this:

plugin.init(...); plugin.init(...); // This will NOT throw.

One way to solve it is to introduce a new boolean:

Suggested change

if (_initCalled.isCompleted) {

throw StateError(

'init() has already been called. Calling init() more than once results in undefined behavior.',

);

}

if (_isInitCalled) {

throw StateError(

'init() has already been called. Calling init() more than once results in undefined behavior.',

);

}

_isInitCalled = true;

mdebbar · 2025-11-04T20:10:06Z

packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart

-  @visibleForTesting
-  Future<void> get initialized {
-    _assertIsInitCalled();
+  Future<void> get _initialized {


To avoid the unnecessary creation of new futures:

Future<void> get _initialized => _initCalled!.future;

packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart

srivats22 · 2025-11-05T19:41:09Z

Is it ok if I close this PR and open a new one?

stuartmorgan-g · 2025-11-06T14:25:22Z

@srivats22 What would the goal of doing that be? Making it harder to find all the review context makes continuing the review much harder.

srivats22 · 2025-11-06T14:44:49Z

@srivats22 What would the goal of doing that be? Making it harder to find all the review context makes continuing the review much harder.

Oh ok no worries I will just push to this... Was just thinking the number of commits were increasing hence...

stuartmorgan-g · 2025-12-02T19:24:29Z

From triage: @srivats22 Are you still planning on updating this PR based on the review feedback?

srivats22 · 2025-12-02T19:26:42Z

From triage: @srivats22 Are you still planning on updating this PR based on the review feedback?

Hi,

yes I am still working on it... I had posted a question:
Quick question on this, should _isInitCalled be a local boolean? or how should I procceed on it? Once I get the answer I will try and finalize it by this week

stuartmorgan-g · 2025-12-03T14:45:41Z

I had posted a question: Quick question on this, should _isInitCalled be a local boolean?

I don't see anywhere in the discussion above where you asked that; the only comment since the last review was when you asked about creating a new PR.

You'll need to provide more context for the question, since I'm not sure what you mean. I don't see how a local variable would be relevant to the previous discussion.

srivats22 · 2025-12-03T14:49:05Z

I had posted a question: Quick question on this, should _isInitCalled be a local boolean?

I don't see anywhere in the discussion above where you asked that; the only comment since the last review was when you asked about creating a new PR.

You'll need to provide more context for the question, since I'm not sure what you mean. I don't see how a local variable would be relevant to the previous discussion.

Sorry I had accidently put it as a review comment have attached a screenshot of the same

mdebbar · 2025-12-03T15:01:30Z

@srivats22 yes _isInitCalled should be private and serve a single purpose: detect when init() is called more than once.

mdebbar

Code changes look good to me! Thanks for addressing all the concerns and being patient with us :)

My remaining comments are about CHANGELOG and code comments.

packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart

packages/google_sign_in/google_sign_in_web/CHANGELOG.md

…web.dart Co-authored-by: Mouad Debbar <[email protected]>

…67410

srivats22 · 2025-12-05T15:10:15Z

How do I fix the build failures?

mdebbar · 2025-12-05T15:15:05Z

...ages/google_sign_in/google_sign_in_web/example/integration_test/google_sign_in_web_test.dart

+    testWidgets('throws if init is called twice', (_) async {
+      await plugin.init(
+        const InitParameters(clientId: 'some-non-null-client-id'),
+      );

-      expect(plugin.initialized, completes);
+      // Calling init() a second time should throw state error
+      expect(
+        () => plugin.init(
+          const InitParameters(clientId: 'some-non-null-client-id'),
+        ),
+        throwsStateError,
+      );


Please add another test for multiple synchronous calls:

testWidgets('throws if init is called twice synchronously', (_) async { final firstInit = plugin.init( const InitParameters(clientId: 'some-non-null-client-id'), ); // Calling init() a second time synchronously should throw state error expect( () => plugin.init( const InitParameters(clientId: 'some-non-null-client-id'), ), throwsStateError, ); await firstInit; });

mdebbar · 2025-12-05T15:17:11Z

packages/google_sign_in/google_sign_in_web/CHANGELOG.md

+## 1.1.1
+
+* Throws a more actionable error when init is called more than once


This needs to be combined with the items from NEXT section:

## 1.1.1 * Throws a more actionable error when init is called more than once * Updates minimum supported SDK version to Flutter 3.32/Dart 3.8.

#167410: _initCalled completed twice

7a44a91

srivats22 requested a review from ditman as a code owner July 29, 2025 15:15

github-actions bot added p: google_sign_in platform-web labels Jul 29, 2025

gemini-code-assist bot reviewed Jul 29, 2025

View reviewed changes

Fixing build issues

d061d6a

stuartmorgan-g self-requested a review July 29, 2025 18:23

Fixing test issues

3ca9fc4

stuartmorgan-g added the triage-web Should be looked at in web triage label Aug 19, 2025

srivats22 added 3 commits August 20, 2025 07:43

Merge branch 'main' of https://github.com/flutter/packages into 167410

7781af6

resolved merge conflicts

a954909

dart format

6ef759f

mdebbar requested review from mdebbar and removed request for ditman August 20, 2025 18:15

mdebbar reviewed Aug 20, 2025

View reviewed changes

stuartmorgan-g requested changes Sep 3, 2025

View reviewed changes

Merge branch 'flutter:main' into 167410

e6adf2d

mdebbar reviewed Oct 27, 2025

View reviewed changes

PR update based on review comment

3ff7372

mdebbar requested changes Nov 4, 2025

View reviewed changes

srivats22 added 2 commits December 3, 2025 10:28

Merge branch 'main' into 167410

df0a6f8

Addressing PR comments

b9ac25c

mdebbar reviewed Dec 3, 2025

View reviewed changes

packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart Outdated Show resolved Hide resolved

packages/google_sign_in/google_sign_in_web/CHANGELOG.md Outdated Show resolved Hide resolved

srivats22 and others added 3 commits December 3, 2025 11:30

Update packages/google_sign_in/google_sign_in_web/lib/google_sign_in_…

777c0eb

…web.dart Co-authored-by: Mouad Debbar <[email protected]>

Updating changelog

5900a70

Merge branch '167410' of https://github.com/srivats22/packages into 1…

0d27936

…67410

mdebbar reviewed Dec 5, 2025

View reviewed changes

srivats22 added 2 commits December 5, 2025 10:59

fix test failiure and add new test

5905948

Merge branch 'main' into 167410

9f6261e

		@@ -1,3 +1,7 @@
		## 1.0.1

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

		## 1.1.1

		* Throws a more actionable error when init is called more than once

#167410: _initCalled completed twice #9694

Are you sure you want to change the base?

#167410: _initCalled completed twice #9694

Uh oh!

Conversation

srivats22 commented Jul 29, 2025

Pre-Review Checklist

Footnotes

Uh oh!

flutter-dashboard bot commented Jul 29, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

srivats22 commented Jul 29, 2025 • edited by stuartmorgan-g Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stuartmorgan-g commented Jul 31, 2025

Uh oh!

srivats22 commented Jul 31, 2025

Uh oh!

stuartmorgan-g commented Jul 31, 2025

Uh oh!

srivats22 commented Jul 31, 2025

Uh oh!

stuartmorgan-g commented Aug 19, 2025

Uh oh!

srivats22 commented Aug 19, 2025

Uh oh!

mdebbar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

srivats22 commented Aug 20, 2025

Uh oh!

stuartmorgan-g commented Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdebbar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

srivats22 commented Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

srivats22 commented Nov 5, 2025

Uh oh!

stuartmorgan-g commented Nov 6, 2025

Uh oh!

srivats22 commented Nov 6, 2025

Uh oh!

stuartmorgan-g commented Dec 2, 2025

srivats22 commented Jul 29, 2025 •

edited by stuartmorgan-g

Loading

stuartmorgan-g commented Aug 21, 2025 •

edited

Loading

srivats22 commented Oct 27, 2025 •

edited

Loading