From fc5aa8eca9d1240d895ca0bf25a46cc2aeb2bd87 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Wed, 25 Jun 2025 19:22:45 +0000 Subject: [PATCH 1/3] Initialize Analytics C SDK with AppOptions on desktop This change updates the desktop analytics initialization to use the newly required Options struct for the Windows C API. - In `analytics/src/analytics_desktop.cc`: - `firebase::analytics::Initialize(const App& app)` now retrieves `app_id` and `package_name` from `app.options()`. - It calls `GoogleAnalytics_Options_Create()` to create the options struct. - Populates `app_id`, `package_name`, and sets a default for `analytics_collection_enabled_at_first_launch`. - Calls `GoogleAnalytics_Initialize()` with the populated options struct. - String lifetimes for `app_id` and `package_name` are handled by creating local `std::string` copies before passing their `c_str()` to the C API. --- analytics/src/analytics_desktop.cc | 48 +++++++++++++++++++++++++----- 1 file changed, 41 insertions(+), 7 deletions(-) diff --git a/analytics/src/analytics_desktop.cc b/analytics/src/analytics_desktop.cc index 5fd7da9afd..20beab17da 100644 --- a/analytics/src/analytics_desktop.cc +++ b/analytics/src/analytics_desktop.cc @@ -29,7 +29,8 @@ #if defined(_WIN32) #include -#include "analytics/src/analytics_windows.h" +// analytics_windows.h is not strictly needed if only using C API via analytics_desktop_dynamic.h +// #include "analytics/src/analytics_windows.h" #endif // defined(_WIN32) namespace firebase { @@ -39,6 +40,13 @@ namespace analytics { #define ANALYTICS_DLL_FILENAME L"analytics_win.dll" static HMODULE g_analytics_module = 0; +// It's generally safer to use local std::string copies within Initialize +// for app_id and package_name to ensure their lifetime if Initialize +// could theoretically be called multiple times with different App objects, +// or if AppOptions getters returned temporaries. +// Given AppOptions structure, direct use of app.options().app_id() etc. +// within the Initialize call *should* be safe as App outlives the call, +// but local copies are more robust. #endif // defined(_WIN32) // Future data for analytics. @@ -49,10 +57,7 @@ static int g_fake_instance_id = 0; // Initializes the Analytics desktop API. // This function must be called before any other Analytics methods. void Initialize(const App& app) { - // The 'app' parameter is not directly used by the underlying Google Analytics - // C API for Windows for global initialization. It's included for API - // consistency with other Firebase platforms. - (void)app; + // app parameter is now used to retrieve AppOptions. g_initialized = true; internal::RegisterTerminateOnDefaultAppDestroy(); @@ -72,7 +77,7 @@ void Initialize(const App& app) { if (g_analytics_module) { int num_loaded = FirebaseAnalytics_LoadDynamicFunctions( - g_analytics_module); // Ensure g_analytics_module is used + g_analytics_module); if (num_loaded < FirebaseAnalytics_DynamicFunctionCount) { LogWarning( "Analytics: Failed to load functions from Google Analytics " @@ -81,12 +86,41 @@ void Initialize(const App& app) { FirebaseAnalytics_UnloadDynamicFunctions(); FreeLibrary(g_analytics_module); g_analytics_module = 0; + // Do not proceed with C API initialization if functions didn't load } else { LogInfo("Analytics: Loaded Google Analytics module."); + + // Initialize Google Analytics C API + std::string current_app_id = app.options().app_id(); + std::string current_package_name = app.options().package_name(); + + GoogleAnalytics_Options* c_options = GoogleAnalytics_Options_Create(); + if (!c_options) { + LogError("Analytics: Failed to create GoogleAnalytics_Options."); + } else { + c_options->app_id = current_app_id.c_str(); + c_options->package_name = current_package_name.c_str(); + c_options->analytics_collection_enabled_at_first_launch = true; + // c_options->reserved is initialized by GoogleAnalytics_Options_Create + + LogInfo("Analytics: Initializing Google Analytics C API with App ID: %s, Package Name: %s", + c_options->app_id ? c_options->app_id : "null", + c_options->package_name ? c_options->package_name : "null"); + + if (!GoogleAnalytics_Initialize(c_options)) { + LogError("Analytics: Failed to initialize Google Analytics C API."); + // GoogleAnalytics_Initialize destroys c_options automatically if created by _Create + } else { + LogInfo("Analytics: Google Analytics C API initialized successfully."); + } + } } + } else { + // LogWarning for g_analytics_module load failure is handled by VerifyAndLoadAnalyticsLibrary + g_analytics_module = 0; // Ensure it's null if loading failed } } -#endif +#endif // defined(_WIN32) } namespace internal { From e3e8a08fb42cdeb9b24461996485399e40c10e30 Mon Sep 17 00:00:00 2001 From: Jon Simantov Date: Wed, 25 Jun 2025 12:30:35 -0700 Subject: [PATCH 2/3] Format code. --- analytics/src/analytics_desktop.cc | 35 ++++++++++++++--------- analytics/src/analytics_desktop_dynamic.c | 3 +- analytics/src/analytics_windows.cc | 28 ++++++++++-------- 3 files changed, 39 insertions(+), 27 deletions(-) diff --git a/analytics/src/analytics_desktop.cc b/analytics/src/analytics_desktop.cc index 20beab17da..87a938f261 100644 --- a/analytics/src/analytics_desktop.cc +++ b/analytics/src/analytics_desktop.cc @@ -29,8 +29,8 @@ #if defined(_WIN32) #include -// analytics_windows.h is not strictly needed if only using C API via analytics_desktop_dynamic.h -// #include "analytics/src/analytics_windows.h" +// analytics_windows.h is not strictly needed if only using C API via +// analytics_desktop_dynamic.h #include "analytics/src/analytics_windows.h" #endif // defined(_WIN32) namespace firebase { @@ -67,8 +67,9 @@ void Initialize(const App& app) { #if defined(_WIN32) if (!g_analytics_module) { std::vector allowed_hashes; - for (int i=0; i < FirebaseAnalytics_KnownWindowsDllHashCount; i++) { - allowed_hashes.push_back(std::string(FirebaseAnalytics_KnownWindowsDllHashes[i])); + for (int i = 0; i < FirebaseAnalytics_KnownWindowsDllHashCount; i++) { + allowed_hashes.push_back( + std::string(FirebaseAnalytics_KnownWindowsDllHashes[i])); } g_analytics_module = @@ -76,8 +77,8 @@ void Initialize(const App& app) { ANALYTICS_DLL_FILENAME, allowed_hashes); if (g_analytics_module) { - int num_loaded = FirebaseAnalytics_LoadDynamicFunctions( - g_analytics_module); + int num_loaded = + FirebaseAnalytics_LoadDynamicFunctions(g_analytics_module); if (num_loaded < FirebaseAnalytics_DynamicFunctionCount) { LogWarning( "Analytics: Failed to load functions from Google Analytics " @@ -101,23 +102,29 @@ void Initialize(const App& app) { c_options->app_id = current_app_id.c_str(); c_options->package_name = current_package_name.c_str(); c_options->analytics_collection_enabled_at_first_launch = true; - // c_options->reserved is initialized by GoogleAnalytics_Options_Create + // c_options->reserved is initialized by + // GoogleAnalytics_Options_Create - LogInfo("Analytics: Initializing Google Analytics C API with App ID: %s, Package Name: %s", - c_options->app_id ? c_options->app_id : "null", - c_options->package_name ? c_options->package_name : "null"); + LogInfo( + "Analytics: Initializing Google Analytics C API with App ID: %s, " + "Package Name: %s", + c_options->app_id ? c_options->app_id : "null", + c_options->package_name ? c_options->package_name : "null"); if (!GoogleAnalytics_Initialize(c_options)) { LogError("Analytics: Failed to initialize Google Analytics C API."); - // GoogleAnalytics_Initialize destroys c_options automatically if created by _Create + // GoogleAnalytics_Initialize destroys c_options automatically if + // created by _Create } else { - LogInfo("Analytics: Google Analytics C API initialized successfully."); + LogInfo( + "Analytics: Google Analytics C API initialized successfully."); } } } } else { - // LogWarning for g_analytics_module load failure is handled by VerifyAndLoadAnalyticsLibrary - g_analytics_module = 0; // Ensure it's null if loading failed + // LogWarning for g_analytics_module load failure is handled by + // VerifyAndLoadAnalyticsLibrary + g_analytics_module = 0; // Ensure it's null if loading failed } } #endif // defined(_WIN32) diff --git a/analytics/src/analytics_desktop_dynamic.c b/analytics/src/analytics_desktop_dynamic.c index 82e70e820d..56d7508c34 100644 --- a/analytics/src/analytics_desktop_dynamic.c +++ b/analytics/src/analytics_desktop_dynamic.c @@ -18,7 +18,8 @@ #include -// A nice big chunk of stub memory that can be returned by stubbed Create methods. +// A nice big chunk of stub memory that can be returned by stubbed Create +// methods. static char g_stub_memory[256] = {0}; // clang-format off diff --git a/analytics/src/analytics_windows.cc b/analytics/src/analytics_windows.cc index 1e3991e381..5398d8136c 100644 --- a/analytics/src/analytics_windows.cc +++ b/analytics/src/analytics_windows.cc @@ -99,11 +99,12 @@ static std::string CalculateFileSha256(HANDLE hFile) { DWORD dwError = GetLastError(); LogError(LOG_TAG "CalculateFileSha256.SetFilePointer failed. Error: %u", dwError); - return ""; // Return empty string on failure + return ""; // Return empty string on failure } // Acquire Crypto Provider. - // Using CRYPT_VERIFYCONTEXT for operations that don't require private key access. + // Using CRYPT_VERIFYCONTEXT for operations that don't require private key + // access. if (!CryptAcquireContextW(&hProv, NULL, NULL, PROV_RSA_AES, CRYPT_VERIFYCONTEXT)) { DWORD dwError = GetLastError(); @@ -152,7 +153,8 @@ static std::string CalculateFileSha256(HANDLE hFile) { // --- Get the binary hash value --- DWORD cbHashValue = 0; DWORD dwCount = sizeof(DWORD); - if (!CryptGetHashParam(hHash, HP_HASHSIZE, (BYTE*)&cbHashValue, &dwCount, 0)) { + if (!CryptGetHashParam(hHash, HP_HASHSIZE, (BYTE*)&cbHashValue, &dwCount, + 0)) { DWORD dwError = GetLastError(); LogError(LOG_TAG "CalculateFileSha256.CryptGetHashParam (HP_HASHSIZE) failed. " @@ -179,12 +181,13 @@ static std::string CalculateFileSha256(HANDLE hFile) { // --- Convert the binary hash to a hex string --- DWORD hex_string_size = 0; if (!CryptBinaryToStringA(binary_hash_value.data(), binary_hash_value.size(), - CRYPT_STRING_HEXRAW | CRYPT_STRING_NOCRLF, - NULL, &hex_string_size)) { + CRYPT_STRING_HEXRAW | CRYPT_STRING_NOCRLF, NULL, + &hex_string_size)) { DWORD dwError = GetLastError(); - LogError(LOG_TAG - "CalculateFileSha256.CryptBinaryToStringA (size) failed. Error: %u", - dwError); + LogError( + LOG_TAG + "CalculateFileSha256.CryptBinaryToStringA (size) failed. Error: %u", + dwError); CryptDestroyHash(hHash); CryptReleaseContext(hProv, 0); return ""; @@ -196,7 +199,8 @@ static std::string CalculateFileSha256(HANDLE hFile) { &hex_hash_string[0], &hex_string_size)) { DWORD dwError = GetLastError(); LogError(LOG_TAG - "CalculateFileSha256.CryptBinaryToStringA (conversion) failed. Error: %u", + "CalculateFileSha256.CryptBinaryToStringA (conversion) failed. " + "Error: %u", dwError); CryptDestroyHash(hHash); CryptReleaseContext(hProv, 0); @@ -286,9 +290,9 @@ HMODULE VerifyAndLoadAnalyticsLibrary( if (calculated_hash == expected_hash) { hash_matched = true; break; - } - else { - LogDebug(LOG_TAG "Hash mismatch: got %s expected %s", calculated_hash.c_str(), expected_hash.c_str()); + } else { + LogDebug(LOG_TAG "Hash mismatch: got %s expected %s", + calculated_hash.c_str(), expected_hash.c_str()); } } From 9a65e98d2120adc18f7f1473d6d5a5ab9d1e50ef Mon Sep 17 00:00:00 2001 From: Jon Simantov Date: Wed, 25 Jun 2025 12:47:14 -0700 Subject: [PATCH 3/3] Fix build issues. --- analytics/generate_windows_stubs.py | 1 + analytics/src/analytics_desktop.cc | 4 +--- analytics/src/analytics_desktop_dynamic.c | 3 +-- analytics/src/analytics_desktop_dynamic.h | 2 ++ 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/analytics/generate_windows_stubs.py b/analytics/generate_windows_stubs.py index 4fdd34f7bd..3ecf6a1043 100755 --- a/analytics/generate_windows_stubs.py +++ b/analytics/generate_windows_stubs.py @@ -159,6 +159,7 @@ def generate_function_pointers(dll_file_path, header_file_path, output_h_path, o f.write(f"// Generated from {os.path.basename(header_file_path)} by {os.path.basename(sys.argv[0])}\n\n") f.write(f"#ifndef {header_guard}\n") f.write(f"#define {header_guard}\n\n") + f.write(f"#define ANALYTICS_API // filter out from header copy\n\n") f.write("#include // needed for bool type in pure C\n\n") f.write("// --- Copied from original header ---\n") diff --git a/analytics/src/analytics_desktop.cc b/analytics/src/analytics_desktop.cc index 87a938f261..fca5d9cc4d 100644 --- a/analytics/src/analytics_desktop.cc +++ b/analytics/src/analytics_desktop.cc @@ -28,9 +28,7 @@ #if defined(_WIN32) #include - -// analytics_windows.h is not strictly needed if only using C API via -// analytics_desktop_dynamic.h #include "analytics/src/analytics_windows.h" +#include "analytics_windows.h" #endif // defined(_WIN32) namespace firebase { diff --git a/analytics/src/analytics_desktop_dynamic.c b/analytics/src/analytics_desktop_dynamic.c index 56d7508c34..82e70e820d 100644 --- a/analytics/src/analytics_desktop_dynamic.c +++ b/analytics/src/analytics_desktop_dynamic.c @@ -18,8 +18,7 @@ #include -// A nice big chunk of stub memory that can be returned by stubbed Create -// methods. +// A nice big chunk of stub memory that can be returned by stubbed Create methods. static char g_stub_memory[256] = {0}; // clang-format off diff --git a/analytics/src/analytics_desktop_dynamic.h b/analytics/src/analytics_desktop_dynamic.h index 75abbcc53d..55567bd1ba 100644 --- a/analytics/src/analytics_desktop_dynamic.h +++ b/analytics/src/analytics_desktop_dynamic.h @@ -17,6 +17,8 @@ #ifndef FIREBASE_ANALYTICS_SRC_WINDOWS_ANALYTICS_DESKTOP_DYNAMIC_H_ #define FIREBASE_ANALYTICS_SRC_WINDOWS_ANALYTICS_DESKTOP_DYNAMIC_H_ +#define ANALYTICS_API // filter out from header copy + #include // needed for bool type in pure C // --- Copied from original header ---