From 10930e047d54921a3ccfa1a1c91c9b4725841134 Mon Sep 17 00:00:00 2001 From: "almostmatt@google.com" Date: Thu, 11 May 2023 17:16:03 -0400 Subject: [PATCH 01/18] Fix for gma fail to initialize without firebase app --- gma/integration_test/src/integration_test.cc | 28 ++++++++++++++++++++ gma/src/android/gma_android.cc | 12 ++++----- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/gma/integration_test/src/integration_test.cc b/gma/integration_test/src/integration_test.cc index 616857980b..354b8eeca8 100644 --- a/gma/integration_test/src/integration_test.cc +++ b/gma/integration_test/src/integration_test.cc @@ -169,6 +169,12 @@ class FirebaseGmaUITest : public FirebaseGmaTest { void SetUp() override; }; +class FirebaseGmaMinimalTest : public FirebaseTest { + public: + FirebaseGmaMinimalTest(); + ~FirebaseGmaMinimalTest() override; +}; + // Runs GMA Tests on methods and functions that should be run // before GMA initializes. class FirebaseGmaPreInitializationTests : public FirebaseGmaTest { @@ -283,6 +289,10 @@ firebase::gma::AdRequest FirebaseGmaTest::GetAdRequest() { return request; } +FirebaseGmaMinimalTest::FirebaseGmaMinimalTest() {} + +FirebaseGmaMinimalTest::~FirebaseGmaMinimalTest() {} + FirebaseGmaUITest::FirebaseGmaUITest() {} FirebaseGmaUITest::~FirebaseGmaUITest() {} @@ -322,6 +332,24 @@ void FirebaseGmaPreInitializationTests::SetUpTestSuite() { // Test cases below. +TEST_F(FirebaseGmaMinimalTest, TestInitializeGmaWithoutFirebase) { + + LogDebug("Initializing GMA without a Firebase App."); + firebase::InitResult result; +#if defined(ANDROID) + ::firebase::gma::Initialize( + app_framework::GetJniEnv(), + app_framework::GetActivity(), + &result); +#else // !defined(ANDROID) + ::firebase::gma::Initialize(&result); +#endif // defined(ANDROID) + EXPECT_EQ(result, ::firebase::kInitResultSuccess); + LogDebug("Successfully initialized GMA."); + LogDebug("Shutdown GMA."); + firebase::gma::Terminate(); +} + TEST_F(FirebaseGmaPreInitializationTests, TestDisableMediationInitialization) { // Note: This test should be disabled or put in an entirely different test // binrary if we ever wish to test mediation in this application. diff --git a/gma/src/android/gma_android.cc b/gma/src/android/gma_android.cc index ae072ef4af..1ecc89c234 100644 --- a/gma/src/android/gma_android.cc +++ b/gma/src/android/gma_android.cc @@ -283,11 +283,7 @@ Future Initialize(JNIEnv* env, jobject activity, env->GetJavaVM(&g_java_vm); } - // GMA requires Google Play services if the class - // "com.google.android.gms.ads.internal.ClientApi" does not exist. - if (!util::FindClass(env, "com/google/android/gms/ads/internal/ClientApi") && - google_play_services::CheckAvailability(env, activity) != - google_play_services::kAvailabilityAvailable) { + if (!util::Initialize(env, activity)) { if (init_result_out) { *init_result_out = kInitResultFailedMissingDependency; } @@ -296,7 +292,11 @@ Future Initialize(JNIEnv* env, jobject activity, return Future(); } - if (!util::Initialize(env, activity)) { + // GMA requires Google Play services if the class + // "com.google.android.gms.ads.internal.ClientApi" does not exist. + if (!util::FindClass(env, "com/google/android/gms/ads/internal/ClientApi") && + google_play_services::CheckAvailability(env, activity) != + google_play_services::kAvailabilityAvailable) { if (init_result_out) { *init_result_out = kInitResultFailedMissingDependency; } From efcde40fcd62a8fc1e857d8567ddd72d6044013c Mon Sep 17 00:00:00 2001 From: "almostmatt@google.com" Date: Thu, 11 May 2023 17:16:15 -0400 Subject: [PATCH 02/18] format --- gma/integration_test/src/integration_test.cc | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/gma/integration_test/src/integration_test.cc b/gma/integration_test/src/integration_test.cc index 354b8eeca8..dbff30478a 100644 --- a/gma/integration_test/src/integration_test.cc +++ b/gma/integration_test/src/integration_test.cc @@ -333,15 +333,12 @@ void FirebaseGmaPreInitializationTests::SetUpTestSuite() { // Test cases below. TEST_F(FirebaseGmaMinimalTest, TestInitializeGmaWithoutFirebase) { - LogDebug("Initializing GMA without a Firebase App."); firebase::InitResult result; #if defined(ANDROID) - ::firebase::gma::Initialize( - app_framework::GetJniEnv(), - app_framework::GetActivity(), - &result); -#else // !defined(ANDROID) + ::firebase::gma::Initialize(app_framework::GetJniEnv(), + app_framework::GetActivity(), &result); +#else // !defined(ANDROID) ::firebase::gma::Initialize(&result); #endif // defined(ANDROID) EXPECT_EQ(result, ::firebase::kInitResultSuccess); From 8f154d425a5adc2a8c1735b7667af1bafb180ee7 Mon Sep 17 00:00:00 2001 From: "almostmatt@google.com" Date: Thu, 11 May 2023 17:19:30 -0400 Subject: [PATCH 03/18] release notes --- release_build_files/readme.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/release_build_files/readme.md b/release_build_files/readme.md index a1dd7f949d..f552036f53 100644 --- a/release_build_files/readme.md +++ b/release_build_files/readme.md @@ -627,6 +627,10 @@ workflow use only during the development of your app, not for publicly shipping code. ## Release Notes +### Upcoming release +- Changes + - GMA (Android): Fixed a crash when Initializing GMA without a Firebase App. + ### 11.0.1 - Changes - Auth (iOS): Fixed a crash in `Credential::is_valid()` when an `AuthResult` From 903ad7466b1f59b2b95bdb48ee1946dde4ed486b Mon Sep 17 00:00:00 2001 From: "almostmatt@google.com" Date: Thu, 11 May 2023 18:26:23 -0400 Subject: [PATCH 04/18] Add the same wait to gma terminate as the other test cases --- gma/integration_test/src/integration_test.cc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/gma/integration_test/src/integration_test.cc b/gma/integration_test/src/integration_test.cc index dbff30478a..6ffda28d8a 100644 --- a/gma/integration_test/src/integration_test.cc +++ b/gma/integration_test/src/integration_test.cc @@ -343,6 +343,11 @@ TEST_F(FirebaseGmaMinimalTest, TestInitializeGmaWithoutFirebase) { #endif // defined(ANDROID) EXPECT_EQ(result, ::firebase::kInitResultSuccess); LogDebug("Successfully initialized GMA."); + // Workaround: GMA does some of its initialization in the main + // thread, so if you terminate it too quickly after initialization + // it can cause issues. Add a small delay here in case most of the + // tests are skipped. + ProcessEvents(1000); LogDebug("Shutdown GMA."); firebase::gma::Terminate(); } From d705263d4f4efe1cf9173cdcc7ac2dfa503d22e6 Mon Sep 17 00:00:00 2001 From: "almostmatt@google.com" Date: Wed, 26 Jul 2023 17:31:41 -0400 Subject: [PATCH 05/18] minor update to test --- gma/integration_test/src/integration_test.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/gma/integration_test/src/integration_test.cc b/gma/integration_test/src/integration_test.cc index 176e11bb5a..ee93ae8835 100644 --- a/gma/integration_test/src/integration_test.cc +++ b/gma/integration_test/src/integration_test.cc @@ -307,6 +307,10 @@ firebase::Variant FirebaseGmaTest::GetVariantMap() { return variant_map; } +FirebaseGmaMinimalTest::FirebaseGmaMinimalTest() {} + +FirebaseGmaMinimalTest::~FirebaseGmaMinimalTest() {} + FirebaseGmaUITest::FirebaseGmaUITest() {} FirebaseGmaUITest::~FirebaseGmaUITest() {} @@ -347,6 +351,8 @@ void FirebaseGmaPreInitializationTests::SetUpTestSuite() { // Test cases below. TEST_F(FirebaseGmaMinimalTest, TestInitializeGmaWithoutFirebase) { + // TODO(almostmatt): Disabling mediation initialization to not mess up next test on iOS. + firebase::gma::DisableMediationInitialization(); LogDebug("Initializing GMA without a Firebase App."); firebase::InitResult result; #if defined(ANDROID) From 01682d171f2f6e74f5631564a48c8cba0ce9648d Mon Sep 17 00:00:00 2001 From: "almostmatt@google.com" Date: Thu, 27 Jul 2023 18:15:26 -0400 Subject: [PATCH 06/18] Temporarily debug what is happening in the github runner --- gma/integration_test/src/integration_test.cc | 2 +- gma/src/android/gma_android.cc | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/gma/integration_test/src/integration_test.cc b/gma/integration_test/src/integration_test.cc index ee93ae8835..b107f7b3ba 100644 --- a/gma/integration_test/src/integration_test.cc +++ b/gma/integration_test/src/integration_test.cc @@ -351,7 +351,7 @@ void FirebaseGmaPreInitializationTests::SetUpTestSuite() { // Test cases below. TEST_F(FirebaseGmaMinimalTest, TestInitializeGmaWithoutFirebase) { - // TODO(almostmatt): Disabling mediation initialization to not mess up next test on iOS. + // TODO(almostmatt): Disabling mediation initialization to not mess up next test on iOS. firebase::gma::DisableMediationInitialization(); LogDebug("Initializing GMA without a Firebase App."); firebase::InitResult result; diff --git a/gma/src/android/gma_android.cc b/gma/src/android/gma_android.cc index 8f6bbc2294..cad36ff272 100644 --- a/gma/src/android/gma_android.cc +++ b/gma/src/android/gma_android.cc @@ -146,6 +146,7 @@ void CallInitializeGoogleMobileAds(void* data) { bool jni_env_exists = (env != nullptr); FIREBASE_ASSERT(jni_env_exists); + LogDebug("almostmatt - calling gma_initialization_helper kInitializeGma"); jobject activity = call_data->activity_global; env->CallStaticVoidMethod(gma_initialization_helper::GetClass(), gma_initialization_helper::GetMethodId( @@ -250,6 +251,7 @@ static AdapterInitializationStatus PopulateAdapterInitializationStatus( // Initializes the Google Mobile Ads SDK using the MobileAds.initialize() // method. The GMA app ID is retrieved from the App's android manifest. Future InitializeGoogleMobileAds(JNIEnv* env) { + LogDebug("almostmatt - calling InitializeGoogleMobileAds"); Future future_to_return; { MutexLock lock(g_future_impl_mutex); @@ -273,6 +275,7 @@ Future InitializeGoogleMobileAds(JNIEnv* env) { Future Initialize(const ::firebase::App& app, InitResult* init_result_out) { FIREBASE_ASSERT(!g_initialized); + LogDebug("almostmatt - calling initialize with app"); g_app = &app; return Initialize(g_app->GetJNIEnv(), g_app->activity(), init_result_out); } @@ -280,8 +283,10 @@ Future Initialize(const ::firebase::App& app, Future Initialize(JNIEnv* env, jobject activity, InitResult* init_result_out) { FIREBASE_ASSERT(!g_initialized); + LogDebug("almostmatt - calling initialize without app"); if (g_java_vm == nullptr) { + LogDebug("almostmatt - getting java vm"); env->GetJavaVM(&g_java_vm); } @@ -289,6 +294,8 @@ Future Initialize(JNIEnv* env, jobject activity, if (init_result_out) { *init_result_out = kInitResultFailedMissingDependency; } + LogDebug("almostmatt - failed util initialize."); + // Need to return an invalid Future, because without GMA initialized, // there is no ReferenceCountedFutureImpl to hold an actual Future instance. return Future(); @@ -302,6 +309,7 @@ Future Initialize(JNIEnv* env, jobject activity, if (init_result_out) { *init_result_out = kInitResultFailedMissingDependency; } + LogDebug("almostmatt - failed to find ClientApi class."); // Need to return an invalid Future, because without GMA initialized, // there is no ReferenceCountedFutureImpl to hold an actual Future instance. return Future(); @@ -314,6 +322,7 @@ Future Initialize(JNIEnv* env, jobject activity, firebase_gma::gma_resources_data, firebase_gma::gma_resources_size)); + LogDebug("almostmatt - initialize - cache methods and register natives"); if (!(mobile_ads::CacheMethodIds(env, activity) && ad_request_builder::CacheMethodIds(env, activity) && adapter_response_info::CacheMethodIds(env, activity) && @@ -351,6 +360,7 @@ Future Initialize(JNIEnv* env, jobject activity, rewarded_ad_helper::CacheMethodIds(env, activity) && load_ad_error::CacheMethodIds(env, activity) && gma::RegisterNatives())) { + LogDebug("almostmatt - failed cache methods. releasing classes."); ReleaseClasses(env); util::Terminate(env); if (init_result_out) { @@ -367,6 +377,7 @@ Future Initialize(JNIEnv* env, jobject activity, g_initialized = true; g_activity = env->NewGlobalRef(activity); + LogDebug("almostmatt - initialize ok. calling InitializeGoogleMobileAds"); Future future = InitializeGoogleMobileAds(env); RegisterTerminateOnDefaultAppDestroy(); @@ -695,6 +706,7 @@ void ReleaseClasses(JNIEnv* env) { bool IsInitialized() { return g_initialized; } void Terminate() { + LogDebug("almostmatt - normal Terminate."); if (!g_initialized) { LogWarning("GMA already shut down"); return; @@ -716,6 +728,7 @@ void Terminate() { env->DeleteGlobalRef(g_activity); g_activity = nullptr; + LogDebug("almostmatt - normal Terminate. releasing classes."); ReleaseClasses(env); util::Terminate(env); } @@ -841,6 +854,7 @@ AdValue::PrecisionType ConvertAndroidPrecisionTypeToCPPPrecisionType( static void JNICALL GmaInitializationHelper_initializationCompleteCallback( JNIEnv* env, jclass clazz, jobject j_initialization_status) { + LogDebug("almostmatt - called initializationCompleteCallback."); AdapterInitializationStatus adapter_status = PopulateAdapterInitializationStatus(j_initialization_status); { @@ -1282,6 +1296,8 @@ bool RegisterNatives() { }; JNIEnv* env = GetJNI(); + + LogDebug("almostmatt - registering natives"); return ad_inspector_helper::RegisterNatives( env, kAdInspectorHelperMethods, FIREBASE_ARRAYSIZE(kAdInspectorHelperMethods)) && From 8b1155e34e3481a6acc5deb7eed7f812f4d2eb01 Mon Sep 17 00:00:00 2001 From: "almostmatt@google.com" Date: Thu, 27 Jul 2023 18:39:48 -0400 Subject: [PATCH 07/18] debugging - warn instead of debug to be sure it logs --- gma/src/android/gma_android.cc | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/gma/src/android/gma_android.cc b/gma/src/android/gma_android.cc index cad36ff272..8bb20ccd7d 100644 --- a/gma/src/android/gma_android.cc +++ b/gma/src/android/gma_android.cc @@ -146,7 +146,7 @@ void CallInitializeGoogleMobileAds(void* data) { bool jni_env_exists = (env != nullptr); FIREBASE_ASSERT(jni_env_exists); - LogDebug("almostmatt - calling gma_initialization_helper kInitializeGma"); + LogWarning("almostmatt - calling gma_initialization_helper kInitializeGma"); jobject activity = call_data->activity_global; env->CallStaticVoidMethod(gma_initialization_helper::GetClass(), gma_initialization_helper::GetMethodId( @@ -251,7 +251,7 @@ static AdapterInitializationStatus PopulateAdapterInitializationStatus( // Initializes the Google Mobile Ads SDK using the MobileAds.initialize() // method. The GMA app ID is retrieved from the App's android manifest. Future InitializeGoogleMobileAds(JNIEnv* env) { - LogDebug("almostmatt - calling InitializeGoogleMobileAds"); + LogWarning("almostmatt - calling InitializeGoogleMobileAds"); Future future_to_return; { MutexLock lock(g_future_impl_mutex); @@ -275,7 +275,7 @@ Future InitializeGoogleMobileAds(JNIEnv* env) { Future Initialize(const ::firebase::App& app, InitResult* init_result_out) { FIREBASE_ASSERT(!g_initialized); - LogDebug("almostmatt - calling initialize with app"); + LogWarning("almostmatt - calling initialize with app"); g_app = &app; return Initialize(g_app->GetJNIEnv(), g_app->activity(), init_result_out); } @@ -283,10 +283,10 @@ Future Initialize(const ::firebase::App& app, Future Initialize(JNIEnv* env, jobject activity, InitResult* init_result_out) { FIREBASE_ASSERT(!g_initialized); - LogDebug("almostmatt - calling initialize without app"); + LogWarning("almostmatt - calling initialize without app"); if (g_java_vm == nullptr) { - LogDebug("almostmatt - getting java vm"); + LogWarning("almostmatt - getting java vm"); env->GetJavaVM(&g_java_vm); } @@ -294,7 +294,7 @@ Future Initialize(JNIEnv* env, jobject activity, if (init_result_out) { *init_result_out = kInitResultFailedMissingDependency; } - LogDebug("almostmatt - failed util initialize."); + LogWarning("almostmatt - failed util initialize."); // Need to return an invalid Future, because without GMA initialized, // there is no ReferenceCountedFutureImpl to hold an actual Future instance. @@ -309,7 +309,7 @@ Future Initialize(JNIEnv* env, jobject activity, if (init_result_out) { *init_result_out = kInitResultFailedMissingDependency; } - LogDebug("almostmatt - failed to find ClientApi class."); + LogWarning("almostmatt - failed to find ClientApi class."); // Need to return an invalid Future, because without GMA initialized, // there is no ReferenceCountedFutureImpl to hold an actual Future instance. return Future(); @@ -322,7 +322,7 @@ Future Initialize(JNIEnv* env, jobject activity, firebase_gma::gma_resources_data, firebase_gma::gma_resources_size)); - LogDebug("almostmatt - initialize - cache methods and register natives"); + LogWarning("almostmatt - initialize - cache methods and register natives"); if (!(mobile_ads::CacheMethodIds(env, activity) && ad_request_builder::CacheMethodIds(env, activity) && adapter_response_info::CacheMethodIds(env, activity) && @@ -360,7 +360,7 @@ Future Initialize(JNIEnv* env, jobject activity, rewarded_ad_helper::CacheMethodIds(env, activity) && load_ad_error::CacheMethodIds(env, activity) && gma::RegisterNatives())) { - LogDebug("almostmatt - failed cache methods. releasing classes."); + LogWarning("almostmatt - failed cache methods. releasing classes."); ReleaseClasses(env); util::Terminate(env); if (init_result_out) { @@ -377,7 +377,7 @@ Future Initialize(JNIEnv* env, jobject activity, g_initialized = true; g_activity = env->NewGlobalRef(activity); - LogDebug("almostmatt - initialize ok. calling InitializeGoogleMobileAds"); + LogWarning("almostmatt - initialize ok. calling InitializeGoogleMobileAds"); Future future = InitializeGoogleMobileAds(env); RegisterTerminateOnDefaultAppDestroy(); @@ -706,7 +706,7 @@ void ReleaseClasses(JNIEnv* env) { bool IsInitialized() { return g_initialized; } void Terminate() { - LogDebug("almostmatt - normal Terminate."); + LogWarning("almostmatt - normal Terminate."); if (!g_initialized) { LogWarning("GMA already shut down"); return; @@ -728,7 +728,7 @@ void Terminate() { env->DeleteGlobalRef(g_activity); g_activity = nullptr; - LogDebug("almostmatt - normal Terminate. releasing classes."); + LogWarning("almostmatt - normal Terminate. releasing classes."); ReleaseClasses(env); util::Terminate(env); } @@ -854,7 +854,7 @@ AdValue::PrecisionType ConvertAndroidPrecisionTypeToCPPPrecisionType( static void JNICALL GmaInitializationHelper_initializationCompleteCallback( JNIEnv* env, jclass clazz, jobject j_initialization_status) { - LogDebug("almostmatt - called initializationCompleteCallback."); + LogWarning("almostmatt - called initializationCompleteCallback."); AdapterInitializationStatus adapter_status = PopulateAdapterInitializationStatus(j_initialization_status); { @@ -1297,7 +1297,7 @@ bool RegisterNatives() { JNIEnv* env = GetJNI(); - LogDebug("almostmatt - registering natives"); + LogWarning("almostmatt - registering natives"); return ad_inspector_helper::RegisterNatives( env, kAdInspectorHelperMethods, FIREBASE_ARRAYSIZE(kAdInspectorHelperMethods)) && From f6fb224abaa20c160562915c9a12fa4db403c133 Mon Sep 17 00:00:00 2001 From: "almostmatt@google.com" Date: Fri, 28 Jul 2023 13:05:27 -0400 Subject: [PATCH 08/18] try with new test case disabled --- gma/integration_test/src/integration_test.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gma/integration_test/src/integration_test.cc b/gma/integration_test/src/integration_test.cc index b107f7b3ba..01b1533249 100644 --- a/gma/integration_test/src/integration_test.cc +++ b/gma/integration_test/src/integration_test.cc @@ -350,6 +350,8 @@ void FirebaseGmaPreInitializationTests::SetUpTestSuite() { // Test cases below. +/* +// TODO(almostmatt): enable this test. TEST_F(FirebaseGmaMinimalTest, TestInitializeGmaWithoutFirebase) { // TODO(almostmatt): Disabling mediation initialization to not mess up next test on iOS. firebase::gma::DisableMediationInitialization(); @@ -371,6 +373,7 @@ TEST_F(FirebaseGmaMinimalTest, TestInitializeGmaWithoutFirebase) { LogDebug("Shutdown GMA."); firebase::gma::Terminate(); } +*/ TEST_F(FirebaseGmaPreInitializationTests, TestDisableMediationInitialization) { // Note: This test should be disabled or put in an entirely different test From 134ae68bfb359401c20ddddc18f90d2f45f79953 Mon Sep 17 00:00:00 2001 From: "almostmatt@google.com" Date: Tue, 1 Aug 2023 13:20:31 -0400 Subject: [PATCH 09/18] wait for initialize future in test and more debugging --- gma/integration_test/src/integration_test.cc | 24 ++++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/gma/integration_test/src/integration_test.cc b/gma/integration_test/src/integration_test.cc index 01b1533249..6d0a7a5f39 100644 --- a/gma/integration_test/src/integration_test.cc +++ b/gma/integration_test/src/integration_test.cc @@ -199,18 +199,23 @@ void PauseForVisualInspectionAndCallbacks() { void InitializeGma(firebase::App* shared_app) { LogDebug("Initializing GMA."); + LogDebug("almostmatt - Initializing GMA. using a module initializer"); ::firebase::ModuleInitializer initializer; initializer.Initialize(shared_app, nullptr, [](::firebase::App* app, void* /* userdata */) { + LogDebug("almostmatt - Try to initialize GMA"); LogDebug("Try to initialize GMA"); firebase::InitResult result; ::firebase::gma::Initialize(*app, &result); return result; }); + + LogDebug("almostmatt - waiting for Module initializer."); FirebaseGmaTest::WaitForCompletion(initializer.InitializeLastResult(), "Initialize"); + LogDebug("almostmatt - finished waiting for Module initializer."); ASSERT_EQ(initializer.InitializeLastResult().error(), 0) << initializer.InitializeLastResult().error_message(); @@ -238,7 +243,9 @@ void FirebaseGmaTest::TearDownTestSuite() { // thread, so if you terminate it too quickly after initialization // it can cause issues. Add a small delay here in case most of the // tests are skipped. + LogDebug("almostmatt - processing events 1000 before shutdown"); ProcessEvents(1000); + LogDebug("almostmatt - finished processing events 1000 before shutdown"); LogDebug("Shutdown GMA."); firebase::gma::Terminate(); LogDebug("Shutdown Firebase App."); @@ -348,10 +355,6 @@ void FirebaseGmaPreInitializationTests::SetUpTestSuite() { #endif // defined(ANDROID) } -// Test cases below. - -/* -// TODO(almostmatt): enable this test. TEST_F(FirebaseGmaMinimalTest, TestInitializeGmaWithoutFirebase) { // TODO(almostmatt): Disabling mediation initialization to not mess up next test on iOS. firebase::gma::DisableMediationInitialization(); @@ -369,11 +372,22 @@ TEST_F(FirebaseGmaMinimalTest, TestInitializeGmaWithoutFirebase) { // thread, so if you terminate it too quickly after initialization // it can cause issues. Add a small delay here in case most of the // tests are skipped. + + LogDebug("almostmatt - waiting for gma initialize future to be finished"); + auto initialize_future = firebase::gma::InitializeLastResult(); + WaitForCompletion(initialize_future, "gma::Initialize"); + LogDebug("almostmatt - finished waiting for gma initialize future to be finished"); + + ASSERT_EQ(initializer.InitializeLastResult().error(), 0) + << initializer.InitializeLastResult().error_message(); + + LogDebug("almostmatt - processing events 1000 before shutdown"); ProcessEvents(1000); + LogDebug("almostmatt - finished processing events 1000 before shutdown"); LogDebug("Shutdown GMA."); firebase::gma::Terminate(); + LogDebug("almostmatt - finished calling gma terminate"); } -*/ TEST_F(FirebaseGmaPreInitializationTests, TestDisableMediationInitialization) { // Note: This test should be disabled or put in an entirely different test From 445d144e62a393b3b9cc7a0e232c46a7606565eb Mon Sep 17 00:00:00 2001 From: "almostmatt@google.com" Date: Tue, 1 Aug 2023 13:47:57 -0400 Subject: [PATCH 10/18] minor fix --- gma/integration_test/src/integration_test.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/gma/integration_test/src/integration_test.cc b/gma/integration_test/src/integration_test.cc index 6d0a7a5f39..7b1c2ede87 100644 --- a/gma/integration_test/src/integration_test.cc +++ b/gma/integration_test/src/integration_test.cc @@ -378,8 +378,7 @@ TEST_F(FirebaseGmaMinimalTest, TestInitializeGmaWithoutFirebase) { WaitForCompletion(initialize_future, "gma::Initialize"); LogDebug("almostmatt - finished waiting for gma initialize future to be finished"); - ASSERT_EQ(initializer.InitializeLastResult().error(), 0) - << initializer.InitializeLastResult().error_message(); + ASSERT_EQ(initialize_future.error(), 0) << initialize_future.error_message(); LogDebug("almostmatt - processing events 1000 before shutdown"); ProcessEvents(1000); From f0217eb7963cef4ab53934c2071b1d9797c7c802 Mon Sep 17 00:00:00 2001 From: "almostmatt@google.com" Date: Tue, 1 Aug 2023 14:34:34 -0400 Subject: [PATCH 11/18] wait for future instead of waiting 1000 --- gma/integration_test/src/integration_test.cc | 21 +++++++------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/gma/integration_test/src/integration_test.cc b/gma/integration_test/src/integration_test.cc index 7b1c2ede87..efa6ec266f 100644 --- a/gma/integration_test/src/integration_test.cc +++ b/gma/integration_test/src/integration_test.cc @@ -239,13 +239,13 @@ void FirebaseGmaTest::SetUpTestSuite() { } void FirebaseGmaTest::TearDownTestSuite() { - // Workaround: GMA does some of its initialization in the main - // thread, so if you terminate it too quickly after initialization - // it can cause issues. Add a small delay here in case most of the - // tests are skipped. - LogDebug("almostmatt - processing events 1000 before shutdown"); - ProcessEvents(1000); - LogDebug("almostmatt - finished processing events 1000 before shutdown"); + // GMA does some of its initialization in the main thread, so if you terminate + // it before initialization has completed, it can cause issues. + // Wait for any pending initialization to be completed. + LogDebug("almostmatt - teardown - waiting for any past initialization to be completed"); + auto initialize_future = firebase::gma::InitializeLastResult(); + WaitForCompletion(initialize_future, "gma::Initialize"); + LogDebug("almostmatt - finished waiting for any past initialization before terminate"); LogDebug("Shutdown GMA."); firebase::gma::Terminate(); LogDebug("Shutdown Firebase App."); @@ -368,10 +368,6 @@ TEST_F(FirebaseGmaMinimalTest, TestInitializeGmaWithoutFirebase) { #endif // defined(ANDROID) EXPECT_EQ(result, ::firebase::kInitResultSuccess); LogDebug("Successfully initialized GMA."); - // Workaround: GMA does some of its initialization in the main - // thread, so if you terminate it too quickly after initialization - // it can cause issues. Add a small delay here in case most of the - // tests are skipped. LogDebug("almostmatt - waiting for gma initialize future to be finished"); auto initialize_future = firebase::gma::InitializeLastResult(); @@ -380,9 +376,6 @@ TEST_F(FirebaseGmaMinimalTest, TestInitializeGmaWithoutFirebase) { ASSERT_EQ(initialize_future.error(), 0) << initialize_future.error_message(); - LogDebug("almostmatt - processing events 1000 before shutdown"); - ProcessEvents(1000); - LogDebug("almostmatt - finished processing events 1000 before shutdown"); LogDebug("Shutdown GMA."); firebase::gma::Terminate(); LogDebug("almostmatt - finished calling gma terminate"); From b772b2b7b7210c57c8e51d44cbc4b6c19fc5b938 Mon Sep 17 00:00:00 2001 From: "almostmatt@google.com" Date: Tue, 1 Aug 2023 14:48:31 -0400 Subject: [PATCH 12/18] Remove debug logs --- gma/integration_test/src/integration_test.cc | 18 +++--------------- gma/src/android/gma_android.cc | 14 -------------- 2 files changed, 3 insertions(+), 29 deletions(-) diff --git a/gma/integration_test/src/integration_test.cc b/gma/integration_test/src/integration_test.cc index efa6ec266f..311eb023f6 100644 --- a/gma/integration_test/src/integration_test.cc +++ b/gma/integration_test/src/integration_test.cc @@ -199,23 +199,17 @@ void PauseForVisualInspectionAndCallbacks() { void InitializeGma(firebase::App* shared_app) { LogDebug("Initializing GMA."); - LogDebug("almostmatt - Initializing GMA. using a module initializer"); ::firebase::ModuleInitializer initializer; initializer.Initialize(shared_app, nullptr, [](::firebase::App* app, void* /* userdata */) { - LogDebug("almostmatt - Try to initialize GMA"); LogDebug("Try to initialize GMA"); firebase::InitResult result; ::firebase::gma::Initialize(*app, &result); return result; }); - - - LogDebug("almostmatt - waiting for Module initializer."); FirebaseGmaTest::WaitForCompletion(initializer.InitializeLastResult(), "Initialize"); - LogDebug("almostmatt - finished waiting for Module initializer."); ASSERT_EQ(initializer.InitializeLastResult().error(), 0) << initializer.InitializeLastResult().error_message(); @@ -242,10 +236,8 @@ void FirebaseGmaTest::TearDownTestSuite() { // GMA does some of its initialization in the main thread, so if you terminate // it before initialization has completed, it can cause issues. // Wait for any pending initialization to be completed. - LogDebug("almostmatt - teardown - waiting for any past initialization to be completed"); auto initialize_future = firebase::gma::InitializeLastResult(); WaitForCompletion(initialize_future, "gma::Initialize"); - LogDebug("almostmatt - finished waiting for any past initialization before terminate"); LogDebug("Shutdown GMA."); firebase::gma::Terminate(); LogDebug("Shutdown Firebase App."); @@ -356,7 +348,8 @@ void FirebaseGmaPreInitializationTests::SetUpTestSuite() { } TEST_F(FirebaseGmaMinimalTest, TestInitializeGmaWithoutFirebase) { - // TODO(almostmatt): Disabling mediation initialization to not mess up next test on iOS. + // Don't initialize mediation in this test so that a later test can still + // verify that Mediation has not been initialized. firebase::gma::DisableMediationInitialization(); LogDebug("Initializing GMA without a Firebase App."); firebase::InitResult result; @@ -367,18 +360,13 @@ TEST_F(FirebaseGmaMinimalTest, TestInitializeGmaWithoutFirebase) { ::firebase::gma::Initialize(&result); #endif // defined(ANDROID) EXPECT_EQ(result, ::firebase::kInitResultSuccess); - LogDebug("Successfully initialized GMA."); - - LogDebug("almostmatt - waiting for gma initialize future to be finished"); auto initialize_future = firebase::gma::InitializeLastResult(); WaitForCompletion(initialize_future, "gma::Initialize"); - LogDebug("almostmatt - finished waiting for gma initialize future to be finished"); - ASSERT_EQ(initialize_future.error(), 0) << initialize_future.error_message(); + LogDebug("Successfully initialized GMA."); LogDebug("Shutdown GMA."); firebase::gma::Terminate(); - LogDebug("almostmatt - finished calling gma terminate"); } TEST_F(FirebaseGmaPreInitializationTests, TestDisableMediationInitialization) { diff --git a/gma/src/android/gma_android.cc b/gma/src/android/gma_android.cc index 8bb20ccd7d..398941769e 100644 --- a/gma/src/android/gma_android.cc +++ b/gma/src/android/gma_android.cc @@ -146,7 +146,6 @@ void CallInitializeGoogleMobileAds(void* data) { bool jni_env_exists = (env != nullptr); FIREBASE_ASSERT(jni_env_exists); - LogWarning("almostmatt - calling gma_initialization_helper kInitializeGma"); jobject activity = call_data->activity_global; env->CallStaticVoidMethod(gma_initialization_helper::GetClass(), gma_initialization_helper::GetMethodId( @@ -251,7 +250,6 @@ static AdapterInitializationStatus PopulateAdapterInitializationStatus( // Initializes the Google Mobile Ads SDK using the MobileAds.initialize() // method. The GMA app ID is retrieved from the App's android manifest. Future InitializeGoogleMobileAds(JNIEnv* env) { - LogWarning("almostmatt - calling InitializeGoogleMobileAds"); Future future_to_return; { MutexLock lock(g_future_impl_mutex); @@ -275,7 +273,6 @@ Future InitializeGoogleMobileAds(JNIEnv* env) { Future Initialize(const ::firebase::App& app, InitResult* init_result_out) { FIREBASE_ASSERT(!g_initialized); - LogWarning("almostmatt - calling initialize with app"); g_app = &app; return Initialize(g_app->GetJNIEnv(), g_app->activity(), init_result_out); } @@ -283,10 +280,8 @@ Future Initialize(const ::firebase::App& app, Future Initialize(JNIEnv* env, jobject activity, InitResult* init_result_out) { FIREBASE_ASSERT(!g_initialized); - LogWarning("almostmatt - calling initialize without app"); if (g_java_vm == nullptr) { - LogWarning("almostmatt - getting java vm"); env->GetJavaVM(&g_java_vm); } @@ -294,7 +289,6 @@ Future Initialize(JNIEnv* env, jobject activity, if (init_result_out) { *init_result_out = kInitResultFailedMissingDependency; } - LogWarning("almostmatt - failed util initialize."); // Need to return an invalid Future, because without GMA initialized, // there is no ReferenceCountedFutureImpl to hold an actual Future instance. @@ -309,7 +303,6 @@ Future Initialize(JNIEnv* env, jobject activity, if (init_result_out) { *init_result_out = kInitResultFailedMissingDependency; } - LogWarning("almostmatt - failed to find ClientApi class."); // Need to return an invalid Future, because without GMA initialized, // there is no ReferenceCountedFutureImpl to hold an actual Future instance. return Future(); @@ -322,7 +315,6 @@ Future Initialize(JNIEnv* env, jobject activity, firebase_gma::gma_resources_data, firebase_gma::gma_resources_size)); - LogWarning("almostmatt - initialize - cache methods and register natives"); if (!(mobile_ads::CacheMethodIds(env, activity) && ad_request_builder::CacheMethodIds(env, activity) && adapter_response_info::CacheMethodIds(env, activity) && @@ -360,7 +352,6 @@ Future Initialize(JNIEnv* env, jobject activity, rewarded_ad_helper::CacheMethodIds(env, activity) && load_ad_error::CacheMethodIds(env, activity) && gma::RegisterNatives())) { - LogWarning("almostmatt - failed cache methods. releasing classes."); ReleaseClasses(env); util::Terminate(env); if (init_result_out) { @@ -377,7 +368,6 @@ Future Initialize(JNIEnv* env, jobject activity, g_initialized = true; g_activity = env->NewGlobalRef(activity); - LogWarning("almostmatt - initialize ok. calling InitializeGoogleMobileAds"); Future future = InitializeGoogleMobileAds(env); RegisterTerminateOnDefaultAppDestroy(); @@ -706,7 +696,6 @@ void ReleaseClasses(JNIEnv* env) { bool IsInitialized() { return g_initialized; } void Terminate() { - LogWarning("almostmatt - normal Terminate."); if (!g_initialized) { LogWarning("GMA already shut down"); return; @@ -728,7 +717,6 @@ void Terminate() { env->DeleteGlobalRef(g_activity); g_activity = nullptr; - LogWarning("almostmatt - normal Terminate. releasing classes."); ReleaseClasses(env); util::Terminate(env); } @@ -854,7 +842,6 @@ AdValue::PrecisionType ConvertAndroidPrecisionTypeToCPPPrecisionType( static void JNICALL GmaInitializationHelper_initializationCompleteCallback( JNIEnv* env, jclass clazz, jobject j_initialization_status) { - LogWarning("almostmatt - called initializationCompleteCallback."); AdapterInitializationStatus adapter_status = PopulateAdapterInitializationStatus(j_initialization_status); { @@ -1297,7 +1284,6 @@ bool RegisterNatives() { JNIEnv* env = GetJNI(); - LogWarning("almostmatt - registering natives"); return ad_inspector_helper::RegisterNatives( env, kAdInspectorHelperMethods, FIREBASE_ARRAYSIZE(kAdInspectorHelperMethods)) && From 1bd61c9cd5fc5779be81df21bf0a788a7e32fb60 Mon Sep 17 00:00:00 2001 From: "almostmatt@google.com" Date: Tue, 1 Aug 2023 14:55:26 -0400 Subject: [PATCH 13/18] readme note and whitespace --- gma/integration_test/src/integration_test.cc | 7 +++++-- gma/src/android/gma_android.cc | 2 -- release_build_files/readme.md | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/gma/integration_test/src/integration_test.cc b/gma/integration_test/src/integration_test.cc index 311eb023f6..24dcb4a2cc 100644 --- a/gma/integration_test/src/integration_test.cc +++ b/gma/integration_test/src/integration_test.cc @@ -208,6 +208,7 @@ void InitializeGma(firebase::App* shared_app) { ::firebase::gma::Initialize(*app, &result); return result; }); + FirebaseGmaTest::WaitForCompletion(initializer.InitializeLastResult(), "Initialize"); @@ -234,8 +235,8 @@ void FirebaseGmaTest::SetUpTestSuite() { void FirebaseGmaTest::TearDownTestSuite() { // GMA does some of its initialization in the main thread, so if you terminate - // it before initialization has completed, it can cause issues. - // Wait for any pending initialization to be completed. + // it before initialization has completed, it can cause issues. So wait for + // any pending GMA initialization to be completed before calling terminate. auto initialize_future = firebase::gma::InitializeLastResult(); WaitForCompletion(initialize_future, "gma::Initialize"); LogDebug("Shutdown GMA."); @@ -347,6 +348,8 @@ void FirebaseGmaPreInitializationTests::SetUpTestSuite() { #endif // defined(ANDROID) } +// Test cases below. + TEST_F(FirebaseGmaMinimalTest, TestInitializeGmaWithoutFirebase) { // Don't initialize mediation in this test so that a later test can still // verify that Mediation has not been initialized. diff --git a/gma/src/android/gma_android.cc b/gma/src/android/gma_android.cc index 398941769e..8f6bbc2294 100644 --- a/gma/src/android/gma_android.cc +++ b/gma/src/android/gma_android.cc @@ -289,7 +289,6 @@ Future Initialize(JNIEnv* env, jobject activity, if (init_result_out) { *init_result_out = kInitResultFailedMissingDependency; } - // Need to return an invalid Future, because without GMA initialized, // there is no ReferenceCountedFutureImpl to hold an actual Future instance. return Future(); @@ -1283,7 +1282,6 @@ bool RegisterNatives() { }; JNIEnv* env = GetJNI(); - return ad_inspector_helper::RegisterNatives( env, kAdInspectorHelperMethods, FIREBASE_ARRAYSIZE(kAdInspectorHelperMethods)) && diff --git a/release_build_files/readme.md b/release_build_files/readme.md index 56af2582d0..056f256b90 100644 --- a/release_build_files/readme.md +++ b/release_build_files/readme.md @@ -631,6 +631,7 @@ code. - Changes - Firestore: Add support for disjunctions in queries (OR queries) ([#1335](https://github.com/firebase/firebase-cpp-sdk/pull/1335)). + - GMA (Android): Fixed a crash when Initializing GMA without a Firebase App. ### 11.3.0 - Changes @@ -678,7 +679,6 @@ code. - Storage (Desktop): Fixed a crash on Windows when uploading files from a path containing non-ANSI characters (Unicode above U+00FF). - Firestore: Added MultiDb support. ([#1321](https://github.com/firebase/firebase-cpp-sdk/pull/1321)). - - GMA (Android): Fixed a crash when Initializing GMA without a Firebase App. ### 11.0.1 - Changes From 9f4f56a4b37bea2d77a42f906f367fab58d58b0b Mon Sep 17 00:00:00 2001 From: "almostmatt@google.com" Date: Tue, 1 Aug 2023 15:04:05 -0400 Subject: [PATCH 14/18] capitalization --- gma/integration_test/src/integration_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gma/integration_test/src/integration_test.cc b/gma/integration_test/src/integration_test.cc index 24dcb4a2cc..76b994cf70 100644 --- a/gma/integration_test/src/integration_test.cc +++ b/gma/integration_test/src/integration_test.cc @@ -352,7 +352,7 @@ void FirebaseGmaPreInitializationTests::SetUpTestSuite() { TEST_F(FirebaseGmaMinimalTest, TestInitializeGmaWithoutFirebase) { // Don't initialize mediation in this test so that a later test can still - // verify that Mediation has not been initialized. + // verify that mediation has not been initialized. firebase::gma::DisableMediationInitialization(); LogDebug("Initializing GMA without a Firebase App."); firebase::InitResult result; From f3e25f0b746dc91cea3b5e25ebb7e81c6600cc61 Mon Sep 17 00:00:00 2001 From: "almostmatt@google.com" Date: Tue, 1 Aug 2023 15:07:30 -0400 Subject: [PATCH 15/18] capitalization --- gma/integration_test/src/integration_test.cc | 2 +- release_build_files/readme.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/gma/integration_test/src/integration_test.cc b/gma/integration_test/src/integration_test.cc index 76b994cf70..251513ad03 100644 --- a/gma/integration_test/src/integration_test.cc +++ b/gma/integration_test/src/integration_test.cc @@ -365,7 +365,7 @@ TEST_F(FirebaseGmaMinimalTest, TestInitializeGmaWithoutFirebase) { EXPECT_EQ(result, ::firebase::kInitResultSuccess); auto initialize_future = firebase::gma::InitializeLastResult(); WaitForCompletion(initialize_future, "gma::Initialize"); - ASSERT_EQ(initialize_future.error(), 0) << initialize_future.error_message(); + EXPECT_EQ(initialize_future.error(), 0) << initialize_future.error_message(); LogDebug("Successfully initialized GMA."); LogDebug("Shutdown GMA."); diff --git a/release_build_files/readme.md b/release_build_files/readme.md index f0c293ad27..24f7ac2017 100644 --- a/release_build_files/readme.md +++ b/release_build_files/readme.md @@ -629,7 +629,7 @@ code. ## Release Notes ### Upcoming Release - Changes - - GMA (Android): Fixed a crash when Initializing GMA without a Firebase App. + - GMA (Android): Fixed a crash when initializing GMA without a Firebase App. ### 11.3.0 - Changes From 6178f694cc120f7df5e57012641ec50060a1f4d3 Mon Sep 17 00:00:00 2001 From: "almostmatt@google.com" Date: Tue, 1 Aug 2023 15:18:57 -0400 Subject: [PATCH 16/18] also call util terminate if class is not found --- gma/src/android/gma_android.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/gma/src/android/gma_android.cc b/gma/src/android/gma_android.cc index 8f6bbc2294..bcc7e35949 100644 --- a/gma/src/android/gma_android.cc +++ b/gma/src/android/gma_android.cc @@ -299,6 +299,7 @@ Future Initialize(JNIEnv* env, jobject activity, if (!util::FindClass(env, "com/google/android/gms/ads/internal/ClientApi") && google_play_services::CheckAvailability(env, activity) != google_play_services::kAvailabilityAvailable) { + util::Terminate(env); if (init_result_out) { *init_result_out = kInitResultFailedMissingDependency; } From 4e3589729a10cbe96d4ae078e69f08b48c29016e Mon Sep 17 00:00:00 2001 From: "almostmatt@google.com" Date: Tue, 1 Aug 2023 15:35:27 -0400 Subject: [PATCH 17/18] minor change based on feedback --- gma/integration_test/src/integration_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gma/integration_test/src/integration_test.cc b/gma/integration_test/src/integration_test.cc index 251513ad03..ff70f5153e 100644 --- a/gma/integration_test/src/integration_test.cc +++ b/gma/integration_test/src/integration_test.cc @@ -237,8 +237,8 @@ void FirebaseGmaTest::TearDownTestSuite() { // GMA does some of its initialization in the main thread, so if you terminate // it before initialization has completed, it can cause issues. So wait for // any pending GMA initialization to be completed before calling terminate. - auto initialize_future = firebase::gma::InitializeLastResult(); - WaitForCompletion(initialize_future, "gma::Initialize"); + WaitForCompletion(firebase::gma::InitializeLastResult(), + "gma::InitializeLastResult"); LogDebug("Shutdown GMA."); firebase::gma::Terminate(); LogDebug("Shutdown Firebase App."); From 29f371b94854a20577031b6070ab59afb4996c93 Mon Sep 17 00:00:00 2001 From: "almostmatt@google.com" Date: Wed, 2 Aug 2023 00:05:35 -0400 Subject: [PATCH 18/18] remove error code check as waitforcompletion does it --- gma/integration_test/src/integration_test.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/gma/integration_test/src/integration_test.cc b/gma/integration_test/src/integration_test.cc index ff70f5153e..d44b0c869a 100644 --- a/gma/integration_test/src/integration_test.cc +++ b/gma/integration_test/src/integration_test.cc @@ -363,9 +363,7 @@ TEST_F(FirebaseGmaMinimalTest, TestInitializeGmaWithoutFirebase) { ::firebase::gma::Initialize(&result); #endif // defined(ANDROID) EXPECT_EQ(result, ::firebase::kInitResultSuccess); - auto initialize_future = firebase::gma::InitializeLastResult(); - WaitForCompletion(initialize_future, "gma::Initialize"); - EXPECT_EQ(initialize_future.error(), 0) << initialize_future.error_message(); + WaitForCompletion(firebase::gma::InitializeLastResult(), "gma::Initialize"); LogDebug("Successfully initialized GMA."); LogDebug("Shutdown GMA.");