fix(firebase_storage): clean up Apple task listeners

[Wackymax]

Description

Firebase Storage can invoke an old Flutter event sink after its listener is cancelled: removing an observer does not retract Firebase callbacks already queued for delivery. Storage also declares a stream-handler registry without populating it, does not publish its iOS plugin for detach callbacks, and does not participate in Firebase Core reinitialization cleanup.

This change gives each listen an optional sink that is cleared before observer removal. A queued callback from that listen then stops without reaching either the cancelled sink or a later subscription. Sink access and delivery are serialized on the main queue; native callbacks already on main are delivered without an extra queue hop. Terminal delivery clears the listener and observers before sending its final event.

The plugin retains handlers, uses their standard onCancel callback for cleanup, publishes on iOS, and registers with Firebase Core so repeated initialization clears old channels and task-handle maps. Cleanup does not explicitly cancel native transfers and preserves emulator configuration. Storage's library version constant is included in the existing version-generation script.

The generic TaskEventDispatcher, generation counters, helper-only tests, and Ruby project-mutating harness have been removed. Real Firebase/Flutter tests now live in a checked-in RunnerTests target under the example's normal Runner scheme. The test bundle uses the host app's plugin code rather than linking duplicate static package copies. The example integration setup also avoids redundant default-app initialization and uses initialized native options for secondary apps.

This does not fix the retained-engine destroyContext() / shell-reset crash. That path does not invoke detachFromEngine. It is tracked separately in flutter/flutter#126671, with a tested no-Firebase reproduction and disposal control. The demonstrated failure addressed here is stale native callback delivery and listener cleanup; no end-user UI failure or fix for those shutdown crashes is claimed.

Validation

• Native example RunnerTests: 9 XCTest cases passed on an iOS 26.5 simulator with Xcode 27. Tests use real Firebase upload tasks and real Flutter engines, covering cancellation with a queued callback, relistening, background callback delivery/invalidation, immediate sink release, terminal success/failure, engine disposal, and repeated Firebase Core initialization followed by a fresh successful upload.
• Guard negative control: disabling the sink invalidation made both live-engine cancellation assertions fail. Restoring it passed the full native suite.
• Full Storage emulator integration: iOS 89 passed / 11 existing skips; macOS 87 passed / 13 existing skips.
• Storage unit suites: 84 Dart tests, 58 platform-interface tests, and 5 web tests passed (147 total).
• Strict analysis of all four Storage packages passed with dart run melos exec --scope='firebase_storage*' -c 1 -- dart analyze . --fatal-infos. Strict analysis of the updated version-generation script also passed.
• Swift formatting/lint, Dart formatting, shell syntax, and git diff --check passed. Full native plugin/example compilation is exercised by the native and emulator suites; existing unrelated native compiler warnings remain.
• melos bootstrap completed. Flutter-generated project migrations are excluded from the patch; only the checked-in test target/scheme changes are retained.
• First-time contributor entry and source license headers retained. No package version or changelog bump.

The earlier full-workspace analyzer run encountered missing Firebase AI example configuration and an unrelated Firebase Performance Web telemetry network failure. Affected Storage packages and the updated script pass strict analysis.

Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide). (Real handler/SDK listener behavior, disposal, and Firebase Core reinitialization are covered; the excluded shell-reset path is documented above.)
• All existing and new tests are passing. (Affected Storage unit/native suites and both Apple emulator suites pass; existing platform skips retained.)
• I updated/added relevant documentation (doc comments with ///).
• The analyzer (melos run analyze) does not report any problems on my PR. (Scoped strict analysis passes; unrelated full-workspace issues are detailed above.)
• I read and followed the Flutter Style Guide.
• I signed the CLA.
• I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

• Yes, this is a breaking change.
• No, this is not a breaking change.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[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 @Wackymax, thanks for the PR. Will review this and provide feedback.

[SelaseKay]

Hi @Wackymax, a couple of questions before we go further.

1. The crash this cites is not the path detachFromEngine covers.

#8919 / #11001 are NSInternalInconsistencyException: Sending a message before the FlutterEngine has been run. On iOS that is the destroyContext / shell-reset path (applicationWillTerminate, and more recently sceneWillDisconnect → destroyContext). Flutter does not call detachFromEngine there: flutter/flutter#126671.

This PR says that path is out of scope, then still hangs the work on those issues. publish + detachFromEngine only runs when Flutter actually detaches the plugin (engine dealloc / add-to-app teardown). That is a different lifecycle than the crash reports.

Can you clarify what user-visible failure this is meant to fix, if not #8919 / #11001? If the intent is only “clean up when Flutter does detach,” that’s fine, but we should drop those issue links (or mark them related-but-unfixed) so we don’t claim a shutdown crash this hook cannot reach.

2. The useful Storage gap looks much smaller than the dispatcher.

Auth and Firestore already: retain stream handlers, publish the plugin, and cancel/invalidate them in detachFromEngine (and on hot restart via didReinitializeFirebaseCore). Storage declared streamHandlers and never stored the handler in registerTask, and never published. Fixing that would match existing Apple plugins.

That does not obviously need a new TaskEventDispatcher, generation counters, or a custom Xcode-project-mutating test harness. Why is the Auth/Firestore pattern not enough here? If there is a Storage-specific race (removeObserver vs an already-queued Firebase callback on a live engine), can you show that independently of engine disposal — e.g. cancel/onCancel while a callback is queued — rather than tying it to detachFromEngine?

[Wackymax]

Hi @SelaseKay, thanks for taking the time to look at this. You are right about the engine lifecycle. I went back and tested it with a small Flutter app without Firebase, and destroyContext() still crashes if the engine stays alive and a callback tries to use the old sink. I've removed the claim that this PR fixes that and posted the reproduction on the Flutter issue.

I also had a look at the cancellation case with the engine still running. With a real paused upload, cancelling the listener before the queued callback runs still calls the old sink. The same happens if I cancel and start listening again straight away. Both tests fail with the current upstream code and pass with this change. That confirms the stale callback, although I haven't reproduced a user-visible problem from that case alone.

The difference I found with Firestore is that its native SDK mutes the listener when it is removed. Storage's removeObserver removes it from the dictionary, but callbacks that are already queued can still run.

I've simplified the implementation as well. The dispatcher is gone and each listener now just holds a sink that gets cleared before removing the observers. Cleanup uses the existing onCancel method, including on engine detach and Firebase Core reinitialisation. There is no explicit transfer cancellation added.

I removed the Ruby harness and moved the native tests into the example's RunnerTests target. All 9 native tests pass, including reinitialising through Firebase Core and uploading again afterwards. The Dart/web tests, iOS and macOS emulator suites, and analysis also pass. Thanks again for the feedback, it helped narrow down what this change actually solves.

[SelaseKay]

Hi @Wackymax, thanks for applying the requested changes. I've requested a review from @Lyokone. Let's wait for his feedback. Also, can you have the tests you added run in CI? Thanks again.

[russellwheatley]

Hi @Wackymax, one thing from me while this is with @Lyokone.

The 9 RunnerTests cases still aren't in CI. The green iOS/macOS jobs are the Dart emulator suite, they don't run xcodebuild test. Those XCTests are the only coverage of the queued-callback race, so they need a CI hook

[Wackymax]

Hi @SelaseKay @russellwheatley, sorry I missed wiring these into CI. You are right, the green iOS/macOS jobs were only running the Dart tests.

I've added the native tests to the Storage iOS job in 9abe498. It runs the existing script with the Storage emulator, so all 9 RunnerTests cases now run through xcodebuild test and a failure fails the job. It also uploads the XCTest results, and changes to the test script will trigger the iOS job.

The workflow checks pass locally and I've checked that failures are passed through correctly. The new CI run is waiting for maintainer approval. Thanks for catching this.