diff --git a/admob/integration_test/src/integration_test.cc b/admob/integration_test/src/integration_test.cc index 00899e57d7..86a7f1c410 100644 --- a/admob/integration_test/src/integration_test.cc +++ b/admob/integration_test/src/integration_test.cc @@ -795,6 +795,8 @@ TEST_F(FirebaseAdMobTest, TestBannerView) { // Set the listener. TestBoundingBoxListener bounding_box_listener; banner->SetBoundingBoxListener(&bounding_box_listener); + PauseForVisualInspectionAndCallbacks(); + int expected_num_bounding_box_changes = 0; EXPECT_EQ(expected_num_bounding_box_changes, bounding_box_listener.bounding_box_changes_.size()); diff --git a/admob/src/android/banner_view_internal_android.cc b/admob/src/android/banner_view_internal_android.cc index 81f06f0a23..4cd45fc99e 100644 --- a/admob/src/android/banner_view_internal_android.cc +++ b/admob/src/android/banner_view_internal_android.cc @@ -31,7 +31,6 @@ #include "admob/src/include/firebase/admob/banner_view.h" #include "admob/src/include/firebase/admob/types.h" #include "app/src/assert.h" -#include "app/src/semaphore.h" #include "app/src/util_android.h" namespace firebase { @@ -142,23 +141,17 @@ BannerViewInternalAndroid::BannerViewInternalAndroid(BannerView* base) env->DeleteLocalRef(helper_ref); } -void DestroyOnDeleteCallback(const Future& result, void* sem_data) { - if (sem_data != nullptr) { - Semaphore* semaphore = static_cast(sem_data); - semaphore->Post(); - } -} - BannerViewInternalAndroid::~BannerViewInternalAndroid() { firebase::MutexLock lock(mutex_); - - Semaphore semaphore(0); - InvokeNullary(kBannerViewFnDestroyOnDelete, banner_view_helper::kDestroy) - .OnCompletion(DestroyOnDeleteCallback, &semaphore); - semaphore.Wait(); - JNIEnv* env = ::firebase::admob::GetJNI(); + // The application should have invoked Destroy already, but + // invoke it now just in case they haven't in the hope that + // we can prevent leaking memory. + env->CallVoidMethod( + helper_, banner_view_helper::GetMethodId(banner_view_helper::kDestroy), + /*callbackDataPtr=*/nullptr, /*destructor_invocation=*/true); + env->DeleteGlobalRef(ad_view_); ad_view_ = nullptr; @@ -257,16 +250,19 @@ Future BannerViewInternalAndroid::Initialize(AdParent parent, if (initialized_) { const SafeFutureHandle future_handle = future_data_.future_impl.SafeAlloc(kBannerViewFnInitialize); + Future future = MakeFuture(&future_data_.future_impl, future_handle); CompleteFuture(kAdMobErrorAlreadyInitialized, kAdAlreadyInitializedErrorMessage, future_handle, &future_data_); - return MakeFuture(&future_data_.future_impl, future_handle); + return future; } initialized_ = true; FutureCallbackData* callback_data = CreateVoidFutureCallbackData(kBannerViewFnSetPosition, &future_data_); + Future future = + MakeFuture(&future_data_.future_impl, callback_data->future_handle); JNIEnv* env = ::firebase::admob::GetJNI(); FIREBASE_ASSERT(env); @@ -281,8 +277,7 @@ Future BannerViewInternalAndroid::Initialize(AdParent parent, call_data->callback_data = callback_data; util::RunOnMainThread(env, activity, InitializeBannerViewOnMainThread, call_data); - - return MakeFuture(&future_data_.future_impl, callback_data->future_handle); + return future; } // This function is run on the main thread and is called in the @@ -324,7 +319,8 @@ Future BannerViewInternalAndroid::LoadAd(const AdRequest& request) { FutureCallbackData* callback_data = CreateAdResultFutureCallbackData(kBannerViewFnLoadAd, &future_data_); - SafeFutureHandle future_handle = callback_data->future_handle; + Future future = + MakeFuture(&future_data_.future_impl, callback_data->future_handle); LoadAdOnMainThreadData* call_data = new LoadAdOnMainThreadData(); call_data->ad_request = request; @@ -334,7 +330,7 @@ Future BannerViewInternalAndroid::LoadAd(const AdRequest& request) { jobject activity = ::firebase::admob::GetActivity(); util::RunOnMainThread(env, activity, LoadAdOnMainThread, call_data); - return MakeFuture(&future_data_.future_impl, future_handle); + return future; } BoundingBox BannerViewInternalAndroid::bounding_box() const { @@ -390,7 +386,14 @@ Future BannerViewInternalAndroid::Resume() { Future BannerViewInternalAndroid::Destroy() { firebase::MutexLock lock(mutex_); - return InvokeNullary(kBannerViewFnDestroy, banner_view_helper::kDestroy); + FutureCallbackData* callback_data = + CreateVoidFutureCallbackData(kBannerViewFnDestroy, &future_data_); + Future future = + MakeFuture(&future_data_.future_impl, callback_data->future_handle); + ::firebase::admob::GetJNI()->CallVoidMethod( + helper_, banner_view_helper::GetMethodId(banner_view_helper::kDestroy), + reinterpret_cast(callback_data), /*destructor_invocation=*/false); + return future; } Future BannerViewInternalAndroid::SetPosition(int x, int y) { @@ -398,13 +401,14 @@ Future BannerViewInternalAndroid::SetPosition(int x, int y) { FutureCallbackData* callback_data = CreateVoidFutureCallbackData(kBannerViewFnSetPosition, &future_data_); - SafeFutureHandle future_handle = callback_data->future_handle; + Future future = + MakeFuture(&future_data_.future_impl, callback_data->future_handle); ::firebase::admob::GetJNI()->CallVoidMethod( helper_, banner_view_helper::GetMethodId(banner_view_helper::kMoveToXY), reinterpret_cast(callback_data), x, y); - return MakeFuture(&future_data_.future_impl, future_handle); + return future; } Future BannerViewInternalAndroid::SetPosition( @@ -413,14 +417,15 @@ Future BannerViewInternalAndroid::SetPosition( FutureCallbackData* callback_data = CreateVoidFutureCallbackData(kBannerViewFnSetPosition, &future_data_); - SafeFutureHandle future_handle = callback_data->future_handle; + Future future = + MakeFuture(&future_data_.future_impl, callback_data->future_handle); ::firebase::admob::GetJNI()->CallVoidMethod( helper_, banner_view_helper::GetMethodId(banner_view_helper::kMoveToPosition), reinterpret_cast(callback_data), static_cast(position)); - return MakeFuture(&future_data_.future_impl, future_handle); + return future; } // This function is run on the main thread and is called in the @@ -449,7 +454,8 @@ Future BannerViewInternalAndroid::InvokeNullary( FutureCallbackData* callback_data = CreateVoidFutureCallbackData(fn, &future_data_); - SafeFutureHandle future_handle = callback_data->future_handle; + Future future = + MakeFuture(&future_data_.future_impl, callback_data->future_handle); NulleryInvocationOnMainThreadData* call_data = new NulleryInvocationOnMainThreadData(); @@ -459,7 +465,7 @@ Future BannerViewInternalAndroid::InvokeNullary( util::RunOnMainThread(env, activity, InvokeNulleryOnMainThread, call_data); - return MakeFuture(&future_data_.future_impl, future_handle); + return future; } } // namespace internal diff --git a/admob/src/android/banner_view_internal_android.h b/admob/src/android/banner_view_internal_android.h index 5c63c21c26..4f5992c707 100644 --- a/admob/src/android/banner_view_internal_android.h +++ b/admob/src/android/banner_view_internal_android.h @@ -35,7 +35,7 @@ namespace admob { X(Show, "show", "(J)V"), \ X(Pause, "pause", "(J)V"), \ X(Resume, "resume", "(J)V"), \ - X(Destroy, "destroy", "(J)V"), \ + X(Destroy, "destroy", "(JZ)V"), \ X(MoveToPosition, "moveTo", "(JI)V"), \ X(MoveToXY, "moveTo", "(JII)V"), \ X(GetBoundingBox, "getBoundingBox", "()[I"), \ diff --git a/admob/src/android/interstitial_ad_internal_android.cc b/admob/src/android/interstitial_ad_internal_android.cc index ff3e6b8a71..5496a0caac 100644 --- a/admob/src/android/interstitial_ad_internal_android.cc +++ b/admob/src/android/interstitial_ad_internal_android.cc @@ -76,10 +76,11 @@ Future InterstitialAdInternalAndroid::Initialize(AdParent parent) { if (initialized_) { const SafeFutureHandle future_handle = future_data_.future_impl.SafeAlloc(kInterstitialAdFnInitialize); + Future future = MakeFuture(&future_data_.future_impl, future_handle); CompleteFuture(kAdMobErrorAlreadyInitialized, kAdAlreadyInitializedErrorMessage, future_handle, &future_data_); - return MakeFuture(&future_data_.future_impl, future_handle); + return future; } initialized_ = true; @@ -88,11 +89,13 @@ Future InterstitialAdInternalAndroid::Initialize(AdParent parent) { FutureCallbackData* callback_data = CreateVoidFutureCallbackData(kInterstitialAdFnInitialize, &future_data_); + Future future = + MakeFuture(&future_data_.future_impl, callback_data->future_handle); env->CallVoidMethod( helper_, interstitial_ad_helper::GetMethodId(interstitial_ad_helper::kInitialize), reinterpret_cast(callback_data), parent); - return MakeFuture(&future_data_.future_impl, callback_data->future_handle); + return future; } Future InterstitialAdInternalAndroid::LoadAd( @@ -100,11 +103,13 @@ Future InterstitialAdInternalAndroid::LoadAd( firebase::MutexLock lock(mutex_); if (!initialized_) { - SafeFutureHandle handle = + SafeFutureHandle future_handle = CreateFuture(kInterstitialAdFnLoadAd, &future_data_); + Future future = + MakeFuture(&future_data_.future_impl, future_handle); CompleteFuture(kAdMobErrorUninitialized, kAdUninitializedErrorMessage, - handle, &future_data_, AdResult()); - return MakeFuture(&future_data_.future_impl, handle); + future_handle, &future_data_, AdResult()); + return future; } admob::AdMobError error = kAdMobErrorNone; @@ -121,6 +126,8 @@ Future InterstitialAdInternalAndroid::LoadAd( FIREBASE_ASSERT(env); FutureCallbackData* callback_data = CreateAdResultFutureCallbackData(kInterstitialAdFnLoadAd, &future_data_); + Future future = + MakeFuture(&future_data_.future_impl, callback_data->future_handle); jstring j_ad_unit_str = env->NewStringUTF(ad_unit_id); ::firebase::admob::GetJNI()->CallVoidMethod( @@ -129,8 +136,7 @@ Future InterstitialAdInternalAndroid::LoadAd( reinterpret_cast(callback_data), j_ad_unit_str, j_request); env->DeleteLocalRef(j_ad_unit_str); env->DeleteLocalRef(j_request); - - return MakeFuture(&future_data_.future_impl, callback_data->future_handle); + return future; } Future InterstitialAdInternalAndroid::Show() { @@ -138,21 +144,23 @@ Future InterstitialAdInternalAndroid::Show() { if (!initialized_) { const SafeFutureHandle future_handle = future_data_.future_impl.SafeAlloc(kInterstitialAdFnShow); + Future future = MakeFuture(&future_data_.future_impl, future_handle); CompleteFuture(kAdMobErrorUninitialized, kAdUninitializedErrorMessage, future_handle, &future_data_); - return MakeFuture(&future_data_.future_impl, future_handle); + return future; } FutureCallbackData* callback_data = CreateVoidFutureCallbackData(kInterstitialAdFnShow, &future_data_); - SafeFutureHandle future_handle = callback_data->future_handle; + Future future = + MakeFuture(&future_data_.future_impl, callback_data->future_handle); ::firebase::admob::GetJNI()->CallVoidMethod( helper_, interstitial_ad_helper::GetMethodId(interstitial_ad_helper::kShow), reinterpret_cast(callback_data)); - return MakeFuture(&future_data_.future_impl, future_handle); + return future; } } // namespace internal diff --git a/admob/src/android/rewarded_ad_internal_android.cc b/admob/src/android/rewarded_ad_internal_android.cc index 085078fe68..644787fe40 100644 --- a/admob/src/android/rewarded_ad_internal_android.cc +++ b/admob/src/android/rewarded_ad_internal_android.cc @@ -79,15 +79,18 @@ Future RewardedAdInternalAndroid::Initialize(AdParent parent) { if (initialized_) { const SafeFutureHandle future_handle = future_data_.future_impl.SafeAlloc(kRewardedAdFnInitialize); + Future future = MakeFuture(&future_data_.future_impl, future_handle); CompleteFuture(kAdMobErrorAlreadyInitialized, kAdAlreadyInitializedErrorMessage, future_handle, &future_data_); - return MakeFuture(&future_data_.future_impl, future_handle); + return future; } initialized_ = true; FutureCallbackData* callback_data = CreateVoidFutureCallbackData(kRewardedAdFnInitialize, &future_data_); + Future future = + MakeFuture(&future_data_.future_impl, callback_data->future_handle); JNIEnv* env = ::firebase::admob::GetJNI(); FIREBASE_ASSERT(env); @@ -95,19 +98,21 @@ Future RewardedAdInternalAndroid::Initialize(AdParent parent) { helper_, rewarded_ad_helper::GetMethodId(rewarded_ad_helper::kInitialize), reinterpret_cast(callback_data), parent); util::CheckAndClearJniExceptions(env); - return MakeFuture(&future_data_.future_impl, callback_data->future_handle); + return future; } Future RewardedAdInternalAndroid::LoadAd(const char* ad_unit_id, const AdRequest& request) { firebase::MutexLock lock(mutex_); if (!initialized_) { - SafeFutureHandle handle = + SafeFutureHandle future_handle = future_data_.future_impl.SafeAlloc(kRewardedAdFnLoadAd, AdResult()); + Future future = + MakeFuture(&future_data_.future_impl, future_handle); CompleteFuture(kAdMobErrorUninitialized, kAdUninitializedErrorMessage, - handle, &future_data_, AdResult()); - return MakeFuture(&future_data_.future_impl, handle); + future_handle, &future_data_, AdResult()); + return future; } admob::AdMobError error = kAdMobErrorNone; @@ -122,6 +127,8 @@ Future RewardedAdInternalAndroid::LoadAd(const char* ad_unit_id, } FutureCallbackData* callback_data = CreateAdResultFutureCallbackData(kRewardedAdFnLoadAd, &future_data_); + Future future = + MakeFuture(&future_data_.future_impl, callback_data->future_handle); JNIEnv* env = GetJNI(); FIREBASE_ASSERT(env); @@ -133,7 +140,7 @@ Future RewardedAdInternalAndroid::LoadAd(const char* ad_unit_id, env->DeleteLocalRef(j_ad_unit_str); env->DeleteLocalRef(j_request); - return MakeFuture(&future_data_.future_impl, callback_data->future_handle); + return future; } Future RewardedAdInternalAndroid::Show( @@ -142,16 +149,18 @@ Future RewardedAdInternalAndroid::Show( if (!initialized_) { const SafeFutureHandle future_handle = future_data_.future_impl.SafeAlloc(kRewardedAdFnShow); + Future future = MakeFuture(&future_data_.future_impl, future_handle); CompleteFuture(kAdMobErrorUninitialized, kAdUninitializedErrorMessage, future_handle, &future_data_); - return MakeFuture(&future_data_.future_impl, future_handle); + return future; } user_earned_reward_listener_ = listener; FutureCallbackData* callback_data = CreateVoidFutureCallbackData(kRewardedAdFnShow, &future_data_); - SafeFutureHandle future_handle = callback_data->future_handle; + Future future = + MakeFuture(&future_data_.future_impl, callback_data->future_handle); JNIEnv* env = GetJNI(); FIREBASE_ASSERT(env); @@ -167,7 +176,7 @@ Future RewardedAdInternalAndroid::Show( env->DeleteLocalRef(j_verification_custom_data); env->DeleteLocalRef(j_verification_user_id); - return MakeFuture(&future_data_.future_impl, future_handle); + return future; } } // namespace internal diff --git a/admob/src/common/admob_common.cc b/admob/src/common/admob_common.cc index ff5194607c..28216976c2 100644 --- a/admob/src/common/admob_common.cc +++ b/admob/src/common/admob_common.cc @@ -227,7 +227,9 @@ const char* GetRequestAgentString() { // Mark a future as complete. void CompleteFuture(int error, const char* error_msg, SafeFutureHandle handle, FutureData* future_data) { - future_data->future_impl.Complete(handle, error, error_msg); + if (future_data->future_impl.ValidFuture(handle)) { + future_data->future_impl.Complete(handle, error, error_msg); + } } // For calls that aren't asynchronous, we can create and complete at the diff --git a/admob/src/ios/banner_view_internal_ios.mm b/admob/src/ios/banner_view_internal_ios.mm index 3c71f5b0ce..88e9811835 100644 --- a/admob/src/ios/banner_view_internal_ios.mm +++ b/admob/src/ios/banner_view_internal_ios.mm @@ -44,6 +44,7 @@ firebase::MutexLock lock(mutex_); const SafeFutureHandle future_handle = future_data_.future_impl.SafeAlloc(kBannerViewFnInitialize); + Future future = MakeFuture(&future_data_.future_impl, future_handle); if(initialized_) { CompleteFuture(kAdMobErrorAlreadyInitialized, @@ -58,7 +59,7 @@ CompleteFuture(kAdMobErrorNone, nullptr, future_handle, &future_data_); }); } - return MakeFuture(&future_data_.future_impl, future_handle); + return future; } Future BannerViewInternalIOS::LoadAd(const AdRequest& request) { @@ -66,12 +67,13 @@ FutureCallbackData* callback_data = CreateAdResultFutureCallbackData(kBannerViewFnLoadAd, &future_data_); - SafeFutureHandle future_handle = callback_data->future_handle; + Future future = MakeFuture(&future_data_.future_impl, + callback_data->future_handle); if (ad_load_callback_data_ != nil) { CompleteLoadAdInternalResult(callback_data, kAdMobErrorLoadInProgress, kAdLoadInProgressErrorMessage); - return MakeFuture(&future_data_.future_impl, future_handle); + return future; } // Persist a pointer to the callback data so that we may use it after the iOS // SDK returns the AdResult. @@ -97,13 +99,15 @@ [banner_view_ loadRequest:ad_request]; } }); - return MakeFuture(&future_data_.future_impl, future_handle); + return future; } Future BannerViewInternalIOS::SetPosition(int x, int y) { firebase::MutexLock lock(mutex_); - const firebase::SafeFutureHandle handle = + const firebase::SafeFutureHandle future_handle = future_data_.future_impl.SafeAlloc(kBannerViewFnSetPosition); + Future future = MakeFuture(&future_data_.future_impl, future_handle); + dispatch_async(dispatch_get_main_queue(), ^{ AdMobError error = kAdMobErrorUninitialized; const char* error_msg = kAdUninitializedErrorMessage; @@ -112,15 +116,17 @@ error = kAdMobErrorNone; error_msg = nullptr; } - CompleteFuture(error, error_msg, handle, &future_data_); + CompleteFuture(error, error_msg, future_handle, &future_data_); }); - return MakeFuture(&future_data_.future_impl, handle); + return future; } Future BannerViewInternalIOS::SetPosition(BannerView::Position position) { firebase::MutexLock lock(mutex_); - const firebase::SafeFutureHandle handle = + const firebase::SafeFutureHandle future_handle = future_data_.future_impl.SafeAlloc(kBannerViewFnSetPosition); + Future future = MakeFuture(&future_data_.future_impl, future_handle); + dispatch_async(dispatch_get_main_queue(), ^{ AdMobError error = kAdMobErrorUninitialized; const char* error_msg = kAdUninitializedErrorMessage; @@ -129,31 +135,35 @@ error = kAdMobErrorNone; error_msg = nullptr; } - CompleteFuture(error, error_msg, handle, &future_data_); + CompleteFuture(error, error_msg, future_handle, &future_data_); }); - return MakeFuture(&future_data_.future_impl, handle); + return future; } Future BannerViewInternalIOS::Hide() { firebase::MutexLock lock(mutex_); - const SafeFutureHandle handle = + const SafeFutureHandle future_handle = future_data_.future_impl.SafeAlloc(kBannerViewFnHide); + Future future = MakeFuture(&future_data_.future_impl, future_handle); + dispatch_async(dispatch_get_main_queue(), ^{ [banner_view_ hide]; - CompleteFuture(kAdMobErrorNone, nullptr, handle, &future_data_); + CompleteFuture(kAdMobErrorNone, nullptr, future_handle, &future_data_); }); - return MakeFuture(&future_data_.future_impl, handle); + return future; } Future BannerViewInternalIOS::Show() { firebase::MutexLock lock(mutex_); - const firebase::SafeFutureHandle handle = + const firebase::SafeFutureHandle future_handle = future_data_.future_impl.SafeAlloc(kBannerViewFnShow); + Future future = MakeFuture(&future_data_.future_impl, future_handle); + dispatch_async(dispatch_get_main_queue(), ^{ [banner_view_ show]; - CompleteFuture(kAdMobErrorNone, nullptr, handle, &future_data_); + CompleteFuture(kAdMobErrorNone, nullptr, future_handle, &future_data_); }); - return MakeFuture(&future_data_.future_impl, handle); + return future; } /// This method is part of the C++ interface and is used only on Android. @@ -175,8 +185,9 @@ /// Cleans up any resources created in BannerViewInternalIOS. Future BannerViewInternalIOS::Destroy() { firebase::MutexLock lock(mutex_); - const firebase::SafeFutureHandle handle = + const firebase::SafeFutureHandle future_handle = future_data_.future_impl.SafeAlloc(kBannerViewFnDestroy); + Future future = MakeFuture(&future_data_.future_impl, future_handle); destroy_mutex_.Acquire(); void (^destroyBlock)() = ^{ [banner_view_ destroy]; @@ -196,11 +207,11 @@ delete ad_load_callback_data_; ad_load_callback_data_ = nil; } - CompleteFuture(kAdMobErrorNone, nullptr, handle, &future_data_); + CompleteFuture(kAdMobErrorNone, nullptr, future_handle, &future_data_); destroy_mutex_.Release(); }; util::DispatchAsyncSafeMainQueue(destroyBlock); - return MakeFuture(&future_data_.future_impl, handle); + return future; } BoundingBox BannerViewInternalIOS::bounding_box() const { diff --git a/admob/src/ios/interstitial_ad_internal_ios.mm b/admob/src/ios/interstitial_ad_internal_ios.mm index 50f6ba0510..8e82943245 100644 --- a/admob/src/ios/interstitial_ad_internal_ios.mm +++ b/admob/src/ios/interstitial_ad_internal_ios.mm @@ -32,29 +32,20 @@ InterstitialAdInternalIOS::~InterstitialAdInternalIOS() { firebase::MutexLock lock(mutex_); - // Clean up any resources created in InterstitialAdInternalIOS. - Mutex mutex(Mutex::kModeNonRecursive); - __block Mutex *mutex_in_block = &mutex; - mutex.Acquire(); - void (^destroyBlock)() = ^{ - ((GADInterstitialAd*)interstitial_).fullScreenContentDelegate = nil; - interstitial_delegate_ = nil; - interstitial_ = nil; - if(ad_load_callback_data_ != nil) { - delete ad_load_callback_data_; - ad_load_callback_data_ = nil; - } - mutex_in_block->Release(); - }; - util::DispatchAsyncSafeMainQueue(destroyBlock); - mutex.Acquire(); - mutex.Release(); + ((GADInterstitialAd*)interstitial_).fullScreenContentDelegate = nil; + interstitial_delegate_ = nil; + interstitial_ = nil; + if(ad_load_callback_data_ != nil) { + delete ad_load_callback_data_; + ad_load_callback_data_ = nil; + } } Future InterstitialAdInternalIOS::Initialize(AdParent parent) { firebase::MutexLock lock(mutex_); const SafeFutureHandle future_handle = future_data_.future_impl.SafeAlloc(kInterstitialAdFnInitialize); + Future future = MakeFuture(&future_data_.future_impl, future_handle); if(initialized_) { CompleteFuture(kAdMobErrorAlreadyInitialized, @@ -64,7 +55,7 @@ parent_view_ = (UIView *)parent; CompleteFuture(kAdMobErrorNone, nullptr, future_handle, &future_data_); } - return MakeFuture(&future_data_.future_impl, future_handle); + return future; } Future InterstitialAdInternalIOS::LoadAd( @@ -73,12 +64,13 @@ FutureCallbackData* callback_data = CreateAdResultFutureCallbackData(kInterstitialAdFnLoadAd, &future_data_); - SafeFutureHandle future_handle = callback_data->future_handle; + Future future = MakeFuture(&future_data_.future_impl, + callback_data->future_handle); if (ad_load_callback_data_ != nil) { CompleteLoadAdInternalResult(callback_data, kAdMobErrorLoadInProgress, kAdLoadInProgressErrorMessage); - return MakeFuture(&future_data_.future_impl, future_handle); + return future; } // Persist a pointer to the callback data so that we may use it after the iOS @@ -117,13 +109,14 @@ } }); - return MakeFuture(&future_data_.future_impl, future_handle); + return future; } Future InterstitialAdInternalIOS::Show() { firebase::MutexLock lock(mutex_); - const firebase::SafeFutureHandle handle = + const firebase::SafeFutureHandle future_handle = future_data_.future_impl.SafeAlloc(kInterstitialAdFnShow); + Future future = MakeFuture(&future_data_.future_impl, future_handle); dispatch_async(dispatch_get_main_queue(), ^{ AdMobError error_code = kAdMobErrorLoadInProgress; const char* error_message = kAdLoadInProgressErrorMessage; @@ -136,9 +129,9 @@ error_code = kAdMobErrorNone; error_message = nullptr; } - CompleteFuture(error_code, error_message, handle, &future_data_); + CompleteFuture(error_code, error_message, future_handle, &future_data_); }); - return MakeFuture(&future_data_.future_impl, handle); + return future; } void InterstitialAdInternalIOS::InterstitialDidReceiveAd(GADInterstitialAd* ad) { diff --git a/admob/src/ios/rewarded_ad_internal_ios.mm b/admob/src/ios/rewarded_ad_internal_ios.mm index 592f4e6784..5e307ba2fb 100644 --- a/admob/src/ios/rewarded_ad_internal_ios.mm +++ b/admob/src/ios/rewarded_ad_internal_ios.mm @@ -33,22 +33,13 @@ RewardedAdInternalIOS::~RewardedAdInternalIOS() { firebase::MutexLock lock(mutex_); // Clean up any resources created in RewardedAdInternalIOS. - Mutex mutex(Mutex::kModeNonRecursive); - __block Mutex *mutex_in_block = &mutex; - mutex.Acquire(); - void (^destroyBlock)() = ^{ - ((GADRewardedAd*)rewarded_ad_).fullScreenContentDelegate = nil; - rewarded_ad_delegate_ = nil; - rewarded_ad_ = nil; - if(ad_load_callback_data_ != nil) { - delete ad_load_callback_data_; - ad_load_callback_data_ = nil; - } - mutex_in_block->Release(); - }; - util::DispatchAsyncSafeMainQueue(destroyBlock); - mutex.Acquire(); - mutex.Release(); + ((GADRewardedAd*)rewarded_ad_).fullScreenContentDelegate = nil; + rewarded_ad_delegate_ = nil; + rewarded_ad_ = nil; + if(ad_load_callback_data_ != nil) { + delete ad_load_callback_data_; + ad_load_callback_data_ = nil; + } } Future RewardedAdInternalIOS::Initialize(AdParent parent) { diff --git a/admob/src_java/com/google/firebase/admob/internal/cpp/BannerViewHelper.java b/admob/src_java/com/google/firebase/admob/internal/cpp/BannerViewHelper.java index 11226e5dc4..e1ec117c49 100644 --- a/admob/src_java/com/google/firebase/admob/internal/cpp/BannerViewHelper.java +++ b/admob/src_java/com/google/firebase/admob/internal/cpp/BannerViewHelper.java @@ -142,41 +142,57 @@ public void initialize(Activity activity) { /** * Destroy/deallocate the {@link PopupWindow} and {@link AdView}. */ - public void destroy(final long callbackDataPtr) { - // If the Activity isn't initialized, there is nothing to destroy. - if (mActivity == null) { - completeBannerViewFutureCallback(callbackDataPtr, - ConstantsHelper.CALLBACK_ERROR_NONE, - ConstantsHelper.CALLBACK_ERROR_MESSAGE_NONE); - return; - } - - // Stop any attempts to show the popup window. - synchronized (mPopUpLock) { - mPopUpRunnable = null; - } + public void destroy(final long callbackDataPtr, final boolean destructor_invocation) { + // If the Activity isn't initialized, or already Destroyed, then there's + // nothing to destroy. + if (mActivity != null) { + mActivity.runOnUiThread( + new Runnable() { + @Override + public void run() { + // Stop any attempts to show the popup window. + synchronized (mPopUpLock) { + mPopUpRunnable = null; + } - if (mAdView != null) { - mAdView.setAdListener(null); - mAdView.setOnPaidEventListener(null); - mAdView.destroy(); - mAdView = null; - } + if (mAdView != null) { + mAdView.setAdListener(null); + mAdView.setOnPaidEventListener(null); + mAdView.destroy(); + mAdView = null; + } - synchronized (mPopUpLock) { - if (mPopUp != null) { - mPopUp.dismiss(); - mPopUp = null; + synchronized (mPopUpLock) { + if (mPopUp != null) { + mPopUp.dismiss(); + mPopUp = null; + } + } + synchronized (mBannerViewLock) { + if (destructor_invocation == false) { + notifyBoundingBoxChanged(mBannerViewInternalPtr); + } + mBannerViewInternalPtr = CPP_NULLPTR; + } + mActivity = null; + + // BannerView's C++ destructor does not pass a future + // to callback and complete, since that would cause the destructor + // to block. + if (callbackDataPtr != CPP_NULLPTR) { + completeBannerViewFutureCallback(callbackDataPtr, + ConstantsHelper.CALLBACK_ERROR_NONE, + ConstantsHelper.CALLBACK_ERROR_MESSAGE_NONE); + } + } + }); + } else { + if (callbackDataPtr != CPP_NULLPTR) { + completeBannerViewFutureCallback(callbackDataPtr, + ConstantsHelper.CALLBACK_ERROR_NONE, + ConstantsHelper.CALLBACK_ERROR_MESSAGE_NONE); } } - - synchronized (mBannerViewLock) { - notifyBoundingBoxChanged(mBannerViewInternalPtr); - } - - completeBannerViewFutureCallback(callbackDataPtr, - ConstantsHelper.CALLBACK_ERROR_NONE, - ConstantsHelper.CALLBACK_ERROR_MESSAGE_NONE); } /** @@ -580,7 +596,6 @@ public boolean onPreDraw() { notifyBoundingBoxChanged(mBannerViewInternalPtr); } } - // Returning true tells Android to continue the draw as normal. return true; }