fix(firestore): don't attach a native snapshot listener cancelled while registering

[chiliec]

Description

Fixes #18724.

snapshots() on MethodChannelDocumentReference / MethodChannelQuery and MethodChannelFirebaseFirestore.snapshotsInSync() await the pigeon registration call inside onListen, and only then subscribe to the event channel. If the subscription is cancelled during that await, onCancel sees a null snapshotStreamSubscription and does nothing, and onListen still subscribes when the call returns. The native listener then attaches and is never removed.

This adds a listen generation counter to each of the three: onCancel bumps it, and onListen returns without subscribing to the event channel if the generation changed during the await. A counter is used instead of a bool so the broadcast controller being re-listened while the first await is still pending is also handled. onCancel also clears the stored subscription.

Tests

New group in test/method_channel_firestore_test.dart. The pigeon mock holds documentReferenceSnapshot / querySnapshot on a completer, the test does listen().cancel(), completes the observer id, and asserts that nothing was sent on the event channel:

• DocumentReference.snapshots() and Query.snapshots(): fail on main (Actual: ['listen']), pass with this change.
• A control test checks that a listener that is not cancelled still sends listen and then cancel.

snapshotsInSync() gets the same fix but has no test. Its event channel passes a non-codec-encodable firestore argument, and that breaks the mock messenger.

cd packages/cloud_firestore/cloud_firestore_platform_interface
flutter test test/method_channel_firestore_test.dart   # 4/4 pass (2 fail on main)
TZ=UTC flutter test                                    # 49/49 pass
flutter analyze                                        # No issues found
dart format --set-exit-if-changed lib test             # 0 changed

(timestamp_test.dart "dates before 1970" also fails on unmodified main when the local timezone is non-UTC, so I ran the full suite with TZ=UTC.)

I did not run this on an Android or iOS device. The fix only touches the Dart method-channel layer.

[gemini-code-assist]

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

[SelaseKay]

Hi @chiliec, thanks for the PR.

Two tests worth adding, neither blocks this:

1. Re-listen while the first pigeon call is still pending. listen, cancel, listen again, complete the first observer id, then a second id. Only the second channel should receive listen.
2. A cancel-path test for snapshotsInSync(). The generation check returns before receiveGuardedBroadcastStream, so that test does not have to encode the firestore argument.
   The pigeon call still registers a native stream handler before it returns. An early return never calls onCancel on that handler, so it stays in the plugin map until the engine detaches. addSnapshotListener is not called, so there is no watch target. A normal listen/cancel already leaves that map entry behind.
   LGTM.

[chiliec]

Thanks for the review, @SelaseKay! I've added both tests you requested to method_channel_firestore_test.dart:

1. Re-listen while the first pigeon call is still pending — subscribes, cancels, then re-subscribes while the first setup future is pending, and completes the first (stale) observer id followed by a second. Asserts that only the second channel receives the native listen — the stale first completion does not attach.
2. snapshotsInSync() cancel-path — subscribes then cancels before snapshotsInSyncSetup completes, and asserts the native snapshotsInSync stream is never attached. As you noted, the generation check returns before receiveGuardedBroadcastStream, so it doesn't need to encode the firestore argument.

All tests pass headless via flutter test test/method_channel_firestore_test.dart (6 passing: the 4 existing + these 2) and flutter analyze is clean.

[SelaseKay]

Hi @chiliec, CI is failing. Kindly run melos format-ci to resolve formatting issues.

[chiliec]

Thanks @SelaseKay — done. Ran dart format on the touched files; it was a single-line reflow in the test file (method_channel_firestore_test.dart). The format check should be green now.