Conversation
…ged as I have make the complier xcode
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for logging Apple StoreKit 2 transactions on iOS by adding a new LogAppleTransaction API, including a Swift bridge for native interaction and updated CMake configurations to support Swift compilation. Several issues were identified in the CMake configuration, including a regression for Windows builds, incorrect platform detection logic for tvOS, non-portable header inclusion, and reliance on Xcode-specific variables that break other generators like Ninja.
AustinBenoit
added
the
tests-requested: full
github-actions
bot
added
tests: in-progress
❌ Integration test FAILEDRequested by @AustinBenoit on commit 66cb596
Add flaky tests to go/fpl-cpp-flake-tracker |
firebase-workflow-trigger
bot
removed
the
tests: in-progress
AustinBenoit
added
tests-requested: full
github-actions
bot
added
tests: in-progress
firebase-workflow-trigger
bot
removed
the
tests: in-progress
AustinBenoit
added
type: feature request
github-actions
bot
added
tests: in-progress
firebase-workflow-trigger
bot
removed
the
tests: in-progress
…ocumentation This commit resolves several known compiler and runtime issues when compiling C++ alongside modern Swift (Concurrency) using Xcode 15+ for deployment on iOS 15/16: 1. `swift_task_alloc` (EXC_BAD_ACCESS): Mixing Swift Concurrency with `@objc` interfaces and C++ pointers on older runtimes causes memory segmentation faults. We introduced an Objective-C++ Block Barrier to keep C pointers on the C++ side (`analytics_ios.mm`) and used a detached `Task` with a pure Swift struct (`AppleTransactionVerifier.swift`) to safely isolate the async `Transaction.all` loop from the bridging boundary. 2. `AsyncIteratorProtocol` (Witness Table Corruption): Using a `for await` loop within an `@objc` class on iOS 15/16 triggers a known Xcode 15+ compiler bug leading to a runtime crash. We bypassed this by extracting the logic into a pure Swift struct and replacing the `for await` loop with manual `while let ... = await iterator.next()` iteration. 3. `__libcpp_verbose_abort`: Xcode 15 requires this symbol for C++ exception handling, but it is missing from the iOS 15/16 system `libc++`. We disabled verbose abort globally via `CMakeLists.txt` and provided a weak fallback implementation in `app_framework.cc` for integration tests to prevent a `dyld` launch crash. 4. `libswift_Concurrency.dylib`: The Swift compiler sometimes fails to weak-link the Concurrency backward compatibility library in Objective-C++ projects. We now natively inject `-Wl,-weak-lswift_Concurrency` into all integration test Xcode projects during `setup_integration_tests.py`. Internal dialog comments have been cleaned up and replaced with professional explanations suitable for open-source maintainers. Documentation in `release_build_files/readme.md` has been updated to provide clear instructions for manual C++ integrators to apply these linker/compiler flags if they experience similar crashes.
|
/gemini review |
AustinBenoit
added
tests-requested: full
github-actions
bot
added
tests: in-progress
There was a problem hiding this comment.
Code Review
This pull request introduces the LogAppleTransaction API to Firebase Analytics, enabling the logging of StoreKit 2 transactions on iOS and tvOS. The implementation includes a Swift-based verification layer using AppleTransactionWorker and bridges it to the C++ SDK. Additionally, the PR incorporates critical workarounds for Xcode 15+ compatibility on older iOS versions (15 and 16), specifically addressing potential crashes related to Swift Concurrency and __libcpp_verbose_abort. Build system updates across multiple modules ensure proper Swift/C++ interop and framework linking. Review feedback suggests addressing a potential memory leak in the iOS asynchronous completion handling and improving the portability of the CMake configuration by removing hardcoded framework paths.
| return Future<void>(api, future_handle.get()); | ||
| } | ||
|
|
||
| SafeFutureHandle<void>* handle_ptr = new SafeFutureHandle<void>(future_handle); |
There was a problem hiding this comment.
The SafeFutureHandle<void> is allocated on the heap using new, but there is a risk of a memory leak if the verifyWithTransactionId completion block is never executed. While the current Swift implementation appears to always call the completion handler, it is safer to use a mechanism that ensures cleanup even if the asynchronous task fails to complete or if the app state changes unexpectedly.
| set(analytics_framework_dir "${pods_dir}/FirebaseAnalytics/Frameworks/FirebaseAnalytics.xcframework/${analytics_slice}") | ||
| set(measurement_framework_dir "${pods_dir}/GoogleAppMeasurement/Frameworks/GoogleAppMeasurement.xcframework/${analytics_slice}") |
There was a problem hiding this comment.
The paths for FirebaseAnalytics.xcframework and GoogleAppMeasurement.xcframework are hardcoded based on a specific CocoaPods directory structure. This may cause build failures if the environment uses a different version of the Firebase Apple SDK or a different dependency management setup. Consider using CMake variables or a more flexible discovery mechanism to locate these frameworks.
firebase-workflow-trigger
bot
removed
the
tests: in-progress
1 participant
Description
StoreKit2 support for Apple devices. Make the LogAppleTransaction a no-op for Android and iOS
Testing
Added in a new integration test and steps to manually test the transactions.
Performed the manual testing as describe:
Type of Change
Place an
xthe applicable box:Notes
Release Notessection ofrelease_build_files/readme.md.