Skip to content

Commit 70da363

Browse files
Googlera-maurice
authored andcommitted
Automated g4 rollback of changelist 279825051.
*** Reason for rollback *** Causes a compilation error on desktop integration tests. *** Original change description *** Removed mutex usage in Admob destructor to signal the completion of Destroy. The use of Mutex in this case was not thread-safe. Replaced the mutex with semaphores and added an integration test for iOS and Android. *** PiperOrigin-RevId: 280033445
1 parent dc1b73a commit 70da363

File tree

3 files changed

+18
-21
lines changed

3 files changed

+18
-21
lines changed

admob/src/android/banner_view_internal_android.cc

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@
1414
* limitations under the License.
1515
*/
1616

17-
#include "admob/src/android/banner_view_internal_android.h"
18-
1917
#include <assert.h>
2018
#include <jni.h>
2119

@@ -25,13 +23,13 @@
2523
#include "admob/admob_resources.h"
2624
#include "admob/src/android/ad_request_converter.h"
2725
#include "admob/src/android/admob_android.h"
26+
#include "admob/src/android/banner_view_internal_android.h"
2827
#include "admob/src/common/admob_common.h"
2928
#include "admob/src/include/firebase/admob.h"
3029
#include "admob/src/include/firebase/admob/banner_view.h"
3130
#include "admob/src/include/firebase/admob/types.h"
3231
#include "app/src/assert.h"
3332
#include "app/src/mutex.h"
34-
#include "app/src/semaphore.h"
3533
#include "app/src/util_android.h"
3634

3735
namespace firebase {
@@ -61,13 +59,21 @@ BannerViewInternalAndroid::BannerViewInternalAndroid(BannerView* base)
6159
BannerViewInternalAndroid::~BannerViewInternalAndroid() {
6260
JNIEnv* env = ::firebase::admob::GetJNI();
6361

64-
DestroyInternalData();
65-
66-
Semaphore semaphore(0);
67-
InvokeNullary(kBannerViewFnDestroyOnDelete, banner_view_helper::kDestroy)
68-
.OnCompletion([&semaphore](const Future<void>&) { semaphore.Post(); });
69-
semaphore.Wait();
70-
62+
// Destroy the banner view so all pending futures / callbacks complete.
63+
{
64+
Mutex mutex(Mutex::kModeNonRecursive);
65+
mutex.Acquire();
66+
Destroy().OnCompletion(
67+
[](const Future<void>&, void* mutex) {
68+
reinterpret_cast<Mutex*>(mutex)->Release();
69+
},
70+
&mutex);
71+
// Acquire a second Mutex lock to block until the Future for the last call
72+
// to Destroy() completes at which point the lambda function in OnCompletion
73+
// is called and the Mutex lock is released.
74+
mutex.Acquire();
75+
mutex.Release();
76+
}
7177
env->DeleteGlobalRef(helper_);
7278
helper_ = nullptr;
7379
}
@@ -125,7 +131,8 @@ Future<void> BannerViewInternalAndroid::Resume() {
125131
}
126132

127133
Future<void> BannerViewInternalAndroid::Destroy() {
128-
DestroyInternalData();
134+
// The bounding box is zeroed on destroy.
135+
bounding_box_ = {};
129136
return InvokeNullary(kBannerViewFnDestroy, banner_view_helper::kDestroy);
130137
}
131138

@@ -207,12 +214,6 @@ Future<void> BannerViewInternalAndroid::InvokeNullary(
207214
return GetLastResult(fn);
208215
}
209216

210-
void BannerViewInternalAndroid::DestroyInternalData() {
211-
// The bounding box is zeroed on destroy.
212-
bounding_box_ = {};
213-
}
214-
215-
216217
} // namespace internal
217218
} // namespace admob
218219
} // namespace firebase

admob/src/android/banner_view_internal_android.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,6 @@ class BannerViewInternalAndroid : public BannerViewInternal {
7878
// the future callback pointer.
7979
Future<void> InvokeNullary(BannerViewFn fn,
8080
banner_view_helper::Method method);
81-
82-
// Cleans up any C++ side data before invoking the Android SDK to do the same.
83-
void DestroyInternalData();
8481
};
8582

8683
} // namespace internal

admob/src/common/banner_view_internal.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ enum BannerViewFn {
3535
kBannerViewFnPause,
3636
kBannerViewFnResume,
3737
kBannerViewFnDestroy,
38-
kBannerViewFnDestroyOnDelete,
3938
kBannerViewFnMoveTo,
4039
kBannerViewFnCount
4140
};

0 commit comments

Comments
 (0)