Updates to Admob Android to reduce usage of BannerViewHelper.java

admob/src/android/admob_android.cc

@@ -130,10 +130,13 @@ InitResult Initialize(JNIEnv* env, jobject activity) {
 
   if (!(mobile_ads::CacheMethodIds(env, activity) &&
         ad_request_builder::CacheMethodIds(env, activity) &&
+        ad_size::CacheMethodIds(env, activity) &&
+        ad_view::CacheMethodIds(env, activity) &&
         request_config_builder::CacheMethodIds(env, activity) &&
         banner_view_helper::CacheClassFromFiles(env, activity,
                                                 &embedded_files) != nullptr &&
         banner_view_helper::CacheMethodIds(env, activity) &&
+        banner_view_helper_ad_view_listener::CacheMethodIds(env, activity) &&
         interstitial_ad_helper::CacheClassFromFiles(
             env, activity, &embedded_files) != nullptr &&
         interstitial_ad_helper::CacheMethodIds(env, activity) &&
@@ -156,8 +159,11 @@ InitResult Initialize(JNIEnv* env, jobject activity) {
 void ReleaseClasses(JNIEnv* env) {
   mobile_ads::ReleaseClass(env);
   ad_request_builder::ReleaseClass(env);
+  ad_size::ReleaseClass(env);
+  ad_view::ReleaseClass(env);
   request_config_builder::ReleaseClass(env);
   banner_view_helper::ReleaseClass(env);
+  banner_view_helper_ad_view_listener::ReleaseClass(env);
   interstitial_ad_helper::ReleaseClass(env);
 }
 

admob/src/android/banner_view_internal_android.cc

@@ -21,6 +21,7 @@
 
 #include <cstdarg>
 #include <cstddef>
+#include <string>
 
 #include "admob/admob_resources.h"
 #include "admob/src/android/ad_request_converter.h"
@@ -42,19 +43,51 @@ METHOD_LOOKUP_DEFINITION(
     "com/google/firebase/admob/internal/cpp/BannerViewHelper",
     BANNERVIEWHELPER_METHODS);
 
+METHOD_LOOKUP_DEFINITION(
+    banner_view_helper_ad_view_listener,
+    "com/google/firebase/admob/internal/cpp/BannerViewHelper$AdViewListener",
+    BANNERVIEWHELPER_ADVIEWLISTENER_METHODS);
+
+METHOD_LOOKUP_DEFINITION(ad_view, "com/google/android/gms/ads/AdView",
+                         AD_VIEW_METHODS);
+
+METHOD_LOOKUP_DEFINITION(ad_size, "com/google/android/gms/ads/AdSize",
+                         AD_SIZE_METHODS);
+
 namespace internal {
 
 BannerViewInternalAndroid::BannerViewInternalAndroid(BannerView* base)
-    : BannerViewInternal(base), helper_(nullptr), bounding_box_() {
+    : BannerViewInternal(base),
+      helper_(nullptr),
+      initialized_(false),
+      bounding_box_() {
   JNIEnv* env = ::firebase::admob::GetJNI();
+  FIREBASE_ASSERT(env);
+
+  jobject activity = ::firebase::admob::GetActivity();
+  FIREBASE_ASSERT(activity);
+
+  jobject adview_ref =
+      env->NewObject(ad_view::GetClass(),
+                     ad_view::GetMethodId(ad_view::kConstructor), activity);
+
+  bool jni_exception = util::CheckAndClearJniExceptions(env);
+  FIREBASE_ASSERT(!jni_exception);
+  FIREBASE_ASSERT(adview_ref);
 
   jobject helper_ref = env->NewObject(
       banner_view_helper::GetClass(),
       banner_view_helper::GetMethodId(banner_view_helper::kConstructor),
-      reinterpret_cast<jlong>(this));
+      reinterpret_cast<jlong>(this), adview_ref);
+
+  jni_exception = util::CheckAndClearJniExceptions(env);
+  FIREBASE_ASSERT(!jni_exception);
   FIREBASE_ASSERT(helper_ref);
+
+  ad_view_ = env->NewGlobalRef(adview_ref);
+  env->DeleteLocalRef(adview_ref);
+
   helper_ = env->NewGlobalRef(helper_ref);
-  FIREBASE_ASSERT(helper_);
   env->DeleteLocalRef(helper_ref);
 }
 
@@ -66,37 +99,131 @@ void DestroyOnDeleteCallback(const Future<void>& result, void* sem_data) {
 }
 
 BannerViewInternalAndroid::~BannerViewInternalAndroid() {
-  JNIEnv* env = ::firebase::admob::GetJNI();
-
   DestroyInternalData();
 
   Semaphore semaphore(0);
   InvokeNullary(kBannerViewFnDestroyOnDelete, banner_view_helper::kDestroy)
       .OnCompletion(DestroyOnDeleteCallback, &semaphore);
-
   semaphore.Wait();
+
+  JNIEnv* env = ::firebase::admob::GetJNI();
+
+  env->DeleteGlobalRef(ad_view_);
+  ad_view_ = nullptr;
+
   env->DeleteGlobalRef(helper_);
   helper_ = nullptr;
 }
 
+struct BannerViewInternalInitializeData {
+  // Thread-safe call data.
+  BannerViewInternalInitializeData()
+      : activity_global(nullptr),
+        ad_view(nullptr),
+        banner_view_helper(nullptr) {}
+  ~BannerViewInternalInitializeData() {
+    JNIEnv* env = GetJNI();
+    env->DeleteGlobalRef(activity_global);
+    env->DeleteGlobalRef(ad_view);
+    env->DeleteGlobalRef(banner_view_helper);
+  }
+
+  jobject activity_global;
+  AdSize ad_size;
+  std::string ad_unit_id;
+  jobject ad_view;
+  jobject banner_view_helper;
+  FutureCallbackData* callback_data;
+};
+
+// This function is run on the main thread and is called in the
+// BannerViewInternalAndroid::Initialize() method.
+void InitializeBannerViewOnMainThread(void* data) {
+  BannerViewInternalInitializeData* call_data =
+      reinterpret_cast<BannerViewInternalInitializeData*>(data);
+  JNIEnv* env = GetJNI();
+  FIREBASE_ASSERT(env != nullptr);
+
+  jstring ad_unit_id_str = env->NewStringUTF(call_data->ad_unit_id.c_str());
+  env->CallVoidMethod(call_data->ad_view,
+                      ad_view::GetMethodId(ad_view::kSetAdUnitId),
+                      ad_unit_id_str);
+  env->DeleteLocalRef(ad_unit_id_str);
+
+  bool jni_exception = util::CheckAndClearJniExceptions(env);
+  FIREBASE_ASSERT(!jni_exception);
+
+  jobject ad_size = env->NewObject(
+      ad_size::GetClass(), ad_size::GetMethodId(ad_size::kConstructor),
+      call_data->ad_size.width, call_data->ad_size.height);
+
+  jni_exception = util::CheckAndClearJniExceptions(env);
+  FIREBASE_ASSERT(!jni_exception);
+
+  env->CallVoidMethod(call_data->ad_view,
+                      ad_view::GetMethodId(ad_view::kSetAdSize), ad_size);
+
+  jni_exception = util::CheckAndClearJniExceptions(env);
+  FIREBASE_ASSERT(!jni_exception);
+
+  env->CallVoidMethod(
+      call_data->banner_view_helper,
+      banner_view_helper::GetMethodId(banner_view_helper::kInitialize),
+      call_data->activity_global);
+
+  jni_exception = util::CheckAndClearJniExceptions(env);
+  FIREBASE_ASSERT(!jni_exception);
+
+  jobject ad_listener = nullptr;
+  ad_listener =
+      env->NewObject(banner_view_helper_ad_view_listener::GetClass(),
+                     banner_view_helper_ad_view_listener::GetMethodId(
+                         banner_view_helper_ad_view_listener::kConstructor),
+                     call_data->banner_view_helper);
+
+  jni_exception = util::CheckAndClearJniExceptions(env);
+  FIREBASE_ASSERT(!jni_exception);
+
+  env->CallVoidMethod(call_data->ad_view,
+                      ad_view::GetMethodId(ad_view::kSetAdListener),
+                      ad_listener);
+
+  jni_exception = util::CheckAndClearJniExceptions(env);
+  FIREBASE_ASSERT(!jni_exception);
+
+  env->DeleteLocalRef(ad_listener);
+
+  CompleteFuture(kAdMobErrorNone, "", call_data->callback_data->future_handle,
+                 call_data->callback_data->future_data);
+
+  delete call_data;
+}
+
 Future<void> BannerViewInternalAndroid::Initialize(AdParent parent,
                                                    const char* ad_unit_id,
                                                    AdSize size) {
   FutureCallbackData* callback_data =
       CreateFutureCallbackData(&future_data_, kBannerViewFnInitialize);
 
+  if (initialized_) {
+    CompleteFuture(kAdMobErrorAlreadyInitialized, "Ad is already initialized.",
+                   callback_data->future_handle, callback_data->future_data);
+    return GetLastResult(kBannerViewFnInitialize);
+  }
+
   JNIEnv* env = ::firebase::admob::GetJNI();
   jobject activity = ::firebase::admob::GetActivity();
 
-  jstring ad_unit_id_str = env->NewStringUTF(ad_unit_id);
-
-  env->CallVoidMethod(
-      helper_, banner_view_helper::GetMethodId(banner_view_helper::kInitialize),
-      reinterpret_cast<jlong>(callback_data), activity, ad_unit_id_str,
-      static_cast<jint>(size.ad_size_type), static_cast<jint>(size.width),
-      static_cast<jint>(size.height));
-
-  env->DeleteLocalRef(ad_unit_id_str);
+  BannerViewInternalInitializeData* call_data =
+      new BannerViewInternalInitializeData();
+  call_data->activity_global = env->NewGlobalRef(activity);
+  call_data->ad_size = size;
+  call_data->ad_unit_id = ad_unit_id;
+  call_data->ad_view = env->NewGlobalRef(ad_view_);
+  call_data->banner_view_helper = env->NewGlobalRef(helper_);
+  call_data->callback_data = callback_data;
+  util::RunOnMainThread(env, activity, InitializeBannerViewOnMainThread,
+                        call_data);
 
   return GetLastResult(kBannerViewFnInitialize);
 }
@@ -111,7 +238,6 @@ Future<void> BannerViewInternalAndroid::LoadAd(const AdRequest& request) {
   ::firebase::admob::GetJNI()->CallVoidMethod(
       helper_, banner_view_helper::GetMethodId(banner_view_helper::kLoadAd),
       reinterpret_cast<jlong>(callback_data), request_ref);
-
   return GetLastResult(kBannerViewFnLoadAd);
 }
 

admob/src/android/banner_view_internal_android.h

@@ -27,9 +27,8 @@ namespace admob {
 // time spent looking up methods by string.
 // clang-format off
 #define BANNERVIEWHELPER_METHODS(X)                                            \
-  X(Constructor, "<init>", "(J)V"),                                            \
-  X(Initialize, "initialize",                                                  \
-    "(JLandroid/app/Activity;Ljava/lang/String;III)V"),                        \
+  X(Constructor, "<init>", "(JLcom/google/android/gms/ads/AdView;)V"),         \
+  X(Initialize, "initialize", "(Landroid/app/Activity;)V"),                    \
   X(LoadAd, "loadAd", "(JLcom/google/android/gms/ads/AdRequest;)V"),           \
   X(Hide, "hide", "(J)V"),                                                     \
   X(Show, "show", "(J)V"),                                                     \
@@ -44,6 +43,31 @@ namespace admob {
 
 METHOD_LOOKUP_DECLARATION(banner_view_helper, BANNERVIEWHELPER_METHODS);
 
+#define BANNERVIEWHELPER_ADVIEWLISTENER_METHODS(X) \
+  X(Constructor, "<init>",                         \
+    "(Lcom/google/firebase/admob/internal/cpp/BannerViewHelper;)V")
+
+METHOD_LOOKUP_DECLARATION(banner_view_helper_ad_view_listener,
+                          BANNERVIEWHELPER_ADVIEWLISTENER_METHODS);
+
+// clang-format off
+#define AD_VIEW_METHODS(X)                                             \
+  X(Constructor, "<init>", "(Landroid/content/Context;)V"),            \
+  X(SetAdSize, "setAdSize", "(Lcom/google/android/gms/ads/AdSize;)V"), \
+  X(SetAdUnitId, "setAdUnitId", "(Ljava/lang/String;)V"),               \
+  X(SetAdListener, "setAdListener",                                    \
+    "(Lcom/google/android/gms/ads/AdListener;)V")
+// clang-format on
+
+METHOD_LOOKUP_DECLARATION(ad_view, AD_VIEW_METHODS);
+
+// clang-format off
+#define AD_SIZE_METHODS(X)                                            \
+  X(Constructor, "<init>", "(II)V")
+// clang-format on
+
+METHOD_LOOKUP_DECLARATION(ad_size, AD_SIZE_METHODS);
+
 namespace internal {
 
 class BannerViewInternalAndroid : public BannerViewInternal {
@@ -70,6 +94,11 @@ class BannerViewInternalAndroid : public BannerViewInternal {
   // SDK.
   jobject helper_;
 
+  // Reference to the Android AdView object used to display BannerView ads.
+  jobject ad_view_;
+
+  bool initialized_;
+
   // The banner view's current BoundingBox. This value is returned if the banner
   // view is hidden and the publisher calls GetBoundingBox().
   mutable BoundingBox bounding_box_;

admob/src_java/com/google/firebase/admob/internal/cpp/BannerViewHelper.java

@@ -49,11 +49,6 @@ public class BannerViewHelper implements ViewTreeObserver.OnPreDrawListener {
   // C++ nullptr for use with the callbacks.
   private static final long CPP_NULLPTR = 0;
 
-  // Ad Size Types (matches the AdSizeType enumeration in the public C++
-  // API). There is only one possible value right now, but this will likely
-  // increase.
-  public static final int ADSIZETYPE_STANDARD = 0;
-
   // The number of milliseconds to wait before attempting to create a PopUpWindow to hold an ad.
   private static final int WEBVIEW_DELAY_MILLISECONDS = 200;
 
@@ -120,8 +115,9 @@ public class BannerViewHelper implements ViewTreeObserver.OnPreDrawListener {
   private int mDesiredY;
 
   /** Constructor. */
-  public BannerViewHelper(long bannerViewInternalPtr) {
+  public BannerViewHelper(long bannerViewInternalPtr, AdView adView) {
     mBannerViewInternalPtr = bannerViewInternalPtr;
+    mAdView = adView;
     mCurrentPresentationState = ConstantsHelper.AD_VIEW_PRESENTATION_STATE_HIDDEN;
     mDesiredPosition = ConstantsHelper.AD_VIEW_POSITION_TOP_LEFT;
     mShouldUseXYForPosition = false;
@@ -137,47 +133,12 @@ public BannerViewHelper(long bannerViewInternalPtr) {
   }
 
   /**
-   * Initializes the {@link BannerView}. This creates the corresponding GMA SDK {@link AdView}
-   * object and sets it up.
+   * Initializes the {@link BannerView}. This stores the activity for use with
+   * callback and load operations.
    */
-  public void initialize(
-      final long callbackDataPtr,
-      Activity activity,
-      String adUnitID,
-      int adSizeType,
-      int width,
-      int height) {
-
-    // There is only one ad size type right now, which is why that parameter goes unused.
-    mAdSize = new AdSize(width, height);
-    mActivity = activity;
-    mAdUnitId = adUnitID;
-    synchronized (mPopUpLock) {
-      mPopUpRunnable = null;
-    }
-
-    // Create the AdView on the UI thread.
-    mActivity.runOnUiThread(
-        new Runnable() {
-          @Override
-          public void run() {
-            int errorCode;
-            String errorMessage;
-            if (mAdView == null) {
-              errorCode = ConstantsHelper.CALLBACK_ERROR_NONE;
-              errorMessage = ConstantsHelper.CALLBACK_ERROR_MESSAGE_NONE;
-              mAdView = new AdView(mActivity);
-              mAdView.setAdUnitId(mAdUnitId);
-              mAdView.setAdSize(mAdSize);
-              mAdView.setAdListener(new AdViewListener());
-            } else {
-              errorCode = ConstantsHelper.CALLBACK_ERROR_ALREADY_INITIALIZED;
-              errorMessage = ConstantsHelper.CALLBACK_ERROR_MESSAGE_ALREADY_INITIALIZED;
-            }
-
-            completeBannerViewFutureCallback(callbackDataPtr, errorCode, errorMessage);
-          }
-        });
+
+  public void initialize(Activity activity) {
+      mActivity = activity;
   }
 
   /** Destroy/deallocate the {@link PopupWindow} and {@link AdView}. */
@@ -239,7 +200,7 @@ public void loadAd(long callbackDataPtr, final AdRequest request) {
 
       mLoadAdCallbackDataPtr = callbackDataPtr;
     }
-
+    
     mActivity.runOnUiThread(
         new Runnable() {
           @Override
@@ -560,7 +521,7 @@ public void run() {
     return true;
   }
 
-  private class AdViewListener extends AdListener {
+  public class AdViewListener extends AdListener {
     @Override
     public void onAdClosed() {
       mCurrentPresentationState = ConstantsHelper.AD_VIEW_PRESENTATION_STATE_VISIBLE_WITH_AD;