Remove LoadAdResult from new Admob.

admob/CMakeLists.txt

@@ -39,7 +39,6 @@ set(android_SRCS
     src/android/admob_android.cc
     src/android/banner_view_internal_android.cc
     src/android/interstitial_ad_internal_android.cc
-    src/android/load_ad_result_android.cc
     src/android/response_info_android.cc)
 
 # Source files used by the iOS implementation.
@@ -53,15 +52,13 @@ set(ios_SRCS
     src/ios/admob_ios.mm
     src/ios/banner_view_internal_ios.mm
     src/ios/interstitial_ad_internal_ios.mm
-    src/ios/load_ad_result_ios.mm
     src/ios/response_info_ios.mm)
 
 # Source files used by the stub implementation.
 set(stub_SRCS
     src/stub/ad_result_stub.cc
     src/stub/adapter_response_info_stub.cc
     src/stub/admob_stub.cc
-    src/stub/load_ad_result_stub.cc
     src/stub/response_info_stub.cc)
 
 if(ANDROID)

admob/integration_test/src/integration_test.cc

@@ -509,12 +509,12 @@ TEST_F(FirebaseAdMobTest, TestBannerView) {
 
   // Load the banner ad.
   firebase::admob::AdRequest request = GetAdRequest();
-  firebase::Future<firebase::admob::LoadAdResult> load_ad_future =
+  firebase::Future<firebase::admob::AdResult> load_ad_future =
       banner->LoadAd(request);
   WaitForCompletion(load_ad_future, "LoadAd");
   EXPECT_EQ(expected_num_bounding_box_changes,
             bounding_box_listener.bounding_box_changes_.size());
-  const firebase::admob::LoadAdResult* result_ptr = load_ad_future.result();
+  const firebase::admob::AdResult* result_ptr = load_ad_future.result();
   ASSERT_NE(result_ptr, nullptr);
   EXPECT_TRUE(result_ptr->is_successful());
   EXPECT_EQ(result_ptr->code(), firebase::admob::kAdMobErrorNone);
@@ -636,9 +636,6 @@ TEST_F(FirebaseAdMobTest, TestBannerView) {
   app_framework::ProcessEvents(2000);
 
   WaitForCompletion(banner->Destroy(), "Destroy BannerView");
-
-  LogDebug("And again to ensure destruction callbacks are recorded.");
-  app_framework::ProcessEvents(2000);
   banner->SetBoundingBoxListener(nullptr);
   delete banner;
 
@@ -718,7 +715,7 @@ TEST_F(FirebaseAdMobTest, TestBannerViewAdOpenedAdClosed) {
 
   // Load the banner ad.
   firebase::admob::AdRequest request = GetAdRequest();
-  firebase::Future<firebase::admob::LoadAdResult> load_ad_future =
+  firebase::Future<firebase::admob::AdResult> load_ad_future =
       banner->LoadAd(request);
   WaitForCompletion(load_ad_future, "LoadAd");
   WaitForCompletion(banner->Show(), "Show 0");
@@ -737,9 +734,6 @@ TEST_F(FirebaseAdMobTest, TestBannerViewAdOpenedAdClosed) {
     app_framework::ProcessEvents(1000);
   }
 
-  LogDebug("Waiting for a moment to ensure all callbacks are recorded.");
-  app_framework::ProcessEvents(2000);
-
   // Ensure all of the expected events were triggered on Android.
   EXPECT_EQ(ad_listener.num_on_ad_clicked_, 1);
   EXPECT_EQ(ad_listener.num_on_ad_impression_, 1);
@@ -853,16 +847,16 @@ TEST_F(FirebaseAdMobTest, TestBannerViewErrorLoadInProgress) {
   // successful ad loads instead of the expected
   // kAdMobErrorLoadInProgress error.
   firebase::admob::AdRequest request = GetAdRequest();
-  firebase::Future<firebase::admob::LoadAdResult> first_load_ad =
+  firebase::Future<firebase::admob::AdResult> first_load_ad =
       banner->LoadAd(request);
-  firebase::Future<firebase::admob::LoadAdResult> second_load_ad =
+  firebase::Future<firebase::admob::AdResult> second_load_ad =
       banner->LoadAd(request);
 
   WaitForCompletion(second_load_ad, "Second LoadAd",
                     firebase::admob::kAdMobErrorLoadInProgress);
   WaitForCompletion(first_load_ad, "First LoadAd");
 
-  const firebase::admob::LoadAdResult* result_ptr = second_load_ad.result();
+  const firebase::admob::AdResult* result_ptr = second_load_ad.result();
   ASSERT_NE(result_ptr, nullptr);
   EXPECT_FALSE(result_ptr->is_successful());
   EXPECT_EQ(result_ptr->code(), firebase::admob::kAdMobErrorLoadInProgress);
@@ -890,12 +884,11 @@ TEST_F(FirebaseAdMobTest, TestBannerViewErrorBadAdUnitId) {
 
   // Load the banner ad.
   firebase::admob::AdRequest request = GetAdRequest();
-  firebase::Future<firebase::admob::LoadAdResult> load_ad =
-      banner->LoadAd(request);
+  firebase::Future<firebase::admob::AdResult> load_ad = banner->LoadAd(request);
   WaitForCompletion(load_ad, "LoadAd",
                     firebase::admob::kAdMobErrorInvalidRequest);
 
-  const firebase::admob::LoadAdResult* result_ptr = load_ad.result();
+  const firebase::admob::AdResult* result_ptr = load_ad.result();
   ASSERT_NE(result_ptr, nullptr);
   EXPECT_FALSE(result_ptr->is_successful());
   EXPECT_EQ(result_ptr->code(), firebase::admob::kAdMobErrorInvalidRequest);
@@ -1088,16 +1081,16 @@ TEST_F(FirebaseAdMobTest, TestInterstitialAdErrorLoadInProgress) {
   // successful ad loads instead of the expected
   // kAdMobErrorLoadInProgress error.
   firebase::admob::AdRequest request = GetAdRequest();
-  firebase::Future<firebase::admob::LoadAdResult> first_load_ad =
+  firebase::Future<firebase::admob::AdResult> first_load_ad =
       interstitial_ad->LoadAd(kInterstitialAdUnit, request);
-  firebase::Future<firebase::admob::LoadAdResult> second_load_ad =
+  firebase::Future<firebase::admob::AdResult> second_load_ad =
       interstitial_ad->LoadAd(kInterstitialAdUnit, request);
 
   WaitForCompletion(second_load_ad, "Second LoadAd",
                     firebase::admob::kAdMobErrorLoadInProgress);
   WaitForCompletion(first_load_ad, "First LoadAd");
 
-  const firebase::admob::LoadAdResult* result_ptr = second_load_ad.result();
+  const firebase::admob::AdResult* result_ptr = second_load_ad.result();
   ASSERT_NE(result_ptr, nullptr);
   EXPECT_FALSE(result_ptr->is_successful());
   EXPECT_EQ(result_ptr->code(), firebase::admob::kAdMobErrorLoadInProgress);
@@ -1120,12 +1113,12 @@ TEST_F(FirebaseAdMobTest, TestInterstitialAdErrorBadAdUnitId) {
 
   // Load the interstitial ad.
   firebase::admob::AdRequest request = GetAdRequest();
-  firebase::Future<firebase::admob::LoadAdResult> load_ad =
+  firebase::Future<firebase::admob::AdResult> load_ad =
       interstitial_ad->LoadAd(kBadAdUnit, request);
   WaitForCompletion(load_ad, "LoadAd",
                     firebase::admob::kAdMobErrorInvalidRequest);
 
-  const firebase::admob::LoadAdResult* result_ptr = load_ad.result();
+  const firebase::admob::AdResult* result_ptr = load_ad.result();
   ASSERT_NE(result_ptr, nullptr);
   EXPECT_FALSE(result_ptr->is_successful());
   EXPECT_EQ(result_ptr->code(), firebase::admob::kAdMobErrorInvalidRequest);

admob/src/android/ad_result_android.cc

@@ -23,6 +23,7 @@
 
 #include "admob/src/android/ad_request_converter.h"
 #include "admob/src/android/admob_android.h"
+#include "admob/src/android/response_info_android.h"
 #include "admob/src/common/admob_common.h"
 #include "admob/src/include/firebase/admob.h"
 
@@ -34,6 +35,11 @@ METHOD_LOOKUP_DEFINITION(ad_error,
                          "com/google/android/gms/ads/AdError",
                          ADERROR_METHODS);
 
+METHOD_LOOKUP_DEFINITION(load_ad_error,
+                         PROGUARD_KEEP_CLASS
+                         "com/google/android/gms/ads/LoadAdError",
+                         LOADADERROR_METHODS);
+
 const char* const AdResult::kUndefinedDomain = "undefined";
 
 AdResult::AdResult() {
@@ -43,11 +49,17 @@ AdResult::AdResult() {
   internal_ = new AdResultInternal();
   internal_->is_successful = false;
   internal_->is_wrapper_error = true;
+  internal_->is_load_ad_error = false;
   internal_->code = kAdMobErrorUninitialized;
   internal_->domain = "SDK";
   internal_->message = "This AdResult has not be initialized.";
   internal_->to_string = internal_->message;
   internal_->j_ad_error = nullptr;
+
+  // While most data is passed into this object through the AdResultInternal
+  // structure (above), the response_info_ is constructed when parsing
+  // the j_ad_error itself.
+  response_info_ = new ResponseInfo();
 }
 
 AdResult::AdResult(const AdResultInternal& ad_result_internal) {
@@ -57,7 +69,9 @@ AdResult::AdResult(const AdResultInternal& ad_result_internal) {
   internal_ = new AdResultInternal();
   internal_->is_successful = ad_result_internal.is_successful;
   internal_->is_wrapper_error = ad_result_internal.is_wrapper_error;
+  internal_->is_load_ad_error = ad_result_internal.is_load_ad_error;
   internal_->j_ad_error = nullptr;
+  response_info_ = new ResponseInfo();
 
   // AdResults can be returned on success, or for errors encountered in the C++
   // SDK wrapper, or in the Android AdMob SDK.  The structure is populated
@@ -104,17 +118,44 @@ AdResult::AdResult(const AdResultInternal& ad_result_internal) {
     internal_->message = util::JStringToString(env, j_message);
     env->DeleteLocalRef(j_message);
 
-    // To string.
-    jobject j_to_string = env->CallObjectMethod(
-        internal_->j_ad_error, ad_error::GetMethodId(ad_error::kToString));
-    FIREBASE_ASSERT(j_to_string);
-    internal_->to_string = util::JStringToString(env, j_to_string);
-    env->DeleteLocalRef(j_to_string);
+    // Differentiate between a com.google.android.gms.ads.AdError or its
+    // com.google.android.gms.ads.LoadAdError subclass.
+    if (!internal_->is_load_ad_error) {
+      // AdError.
+      jobject j_to_string = env->CallObjectMethod(
+          internal_->j_ad_error, ad_error::GetMethodId(ad_error::kToString));
+      FIREBASE_ASSERT(j_to_string);
+      internal_->to_string = util::JStringToString(env, j_to_string);
+      env->DeleteLocalRef(j_to_string);
+    } else {
+      // LoadAdError.
+      jobject j_response_info = env->CallObjectMethod(
+          internal_->j_ad_error,
+          load_ad_error::GetMethodId(load_ad_error::kGetResponseInfo));
+
+      if (j_response_info != nullptr) {
+        ResponseInfoInternal response_info_internal;
+        response_info_internal.j_response_info = j_response_info;
+        *response_info_ = ResponseInfo(response_info_internal);
+        env->DeleteLocalRef(j_response_info);
+      }
+
+      // A to_string value of this LoadAdError.  Invoke the set_to_string
+      // protected method of the AdResult parent class to overwrite whatever
+      // it parsed.
+      jobject j_to_string = env->CallObjectMethod(
+          internal_->j_ad_error,
+          load_ad_error::GetMethodId(load_ad_error::kToString));
+      internal_->to_string = util::JStringToString(env, j_to_string);
+      env->DeleteLocalRef(j_to_string);
+    }
   }
 }
 
 AdResult::AdResult(const AdResult& ad_result) : AdResult() {
+  FIREBASE_ASSERT(ad_result.response_info_ != nullptr);
   // Reuse the assignment operator.
+  this->response_info_ = new ResponseInfo();
   *this = ad_result;
 }
 
@@ -127,46 +168,57 @@ AdResult& AdResult::operator=(const AdResult& ad_result) {
   FIREBASE_ASSERT(env);
   FIREBASE_ASSERT(internal_);
   FIREBASE_ASSERT(ad_result.internal_);
+  FIREBASE_ASSERT(response_info_);
+  FIREBASE_ASSERT(ad_result.response_info_);
 
   AdResultInternal* preexisting_internal = internal_;
   {
     // Lock the parties so they're not deleted while the copying takes place.
-    MutexLock(ad_result.internal_->mutex);
-    MutexLock(internal_->mutex);
+    MutexLock ad_result_lock(ad_result.internal_->mutex);
+    MutexLock lock(internal_->mutex);
     internal_ = new AdResultInternal();
 
     internal_->is_successful = ad_result.internal_->is_successful;
     internal_->is_wrapper_error = ad_result.internal_->is_wrapper_error;
+    internal_->is_load_ad_error = ad_result.internal_->is_load_ad_error;
     internal_->code = ad_result.internal_->code;
     internal_->domain = ad_result.internal_->domain;
     internal_->message = ad_result.internal_->message;
     if (ad_result.internal_->j_ad_error != nullptr) {
       internal_->j_ad_error =
           env->NewGlobalRef(ad_result.internal_->j_ad_error);
     }
-
     if (preexisting_internal->j_ad_error) {
       env->DeleteGlobalRef(preexisting_internal->j_ad_error);
       preexisting_internal->j_ad_error = nullptr;
     }
+
+    *response_info_ = *ad_result.response_info_;
   }
 
   // Deleting the internal_ deletes the mutex within it, so we wait for
   // complete deletion until after the mutex lock leaves scope.
   delete preexisting_internal;
+
   return *this;
 }
 
 AdResult::~AdResult() {
   FIREBASE_ASSERT(internal_);
+  FIREBASE_ASSERT(response_info_);
+
   if (internal_->j_ad_error != nullptr) {
     JNIEnv* env = GetJNI();
     FIREBASE_ASSERT(env);
     env->DeleteGlobalRef(internal_->j_ad_error);
     internal_->j_ad_error = nullptr;
   }
+
   delete internal_;
   internal_ = nullptr;
+
+  delete response_info_;
+  response_info_ = nullptr;
 }
 
 bool AdResult::is_successful() const {
@@ -193,7 +245,9 @@ std::unique_ptr<AdResult> AdResult::GetCause() const {
 
     AdResultInternal ad_result_internal;
     ad_result_internal.is_wrapper_error = false;
+    ad_result_internal.is_load_ad_error = false;
     ad_result_internal.j_ad_error = j_ad_error;
+
     std::unique_ptr<AdResult> ad_result =
         std::unique_ptr<AdResult>(new AdResult(ad_result_internal));
     env->DeleteLocalRef(j_ad_error);
@@ -219,17 +273,16 @@ const std::string& AdResult::message() const {
   return internal_->message;
 }
 
+const ResponseInfo& AdResult::response_info() const {
+  FIREBASE_ASSERT(response_info_ != nullptr);
+  return *response_info_;
+}
+
 /// Returns a log friendly string version of this object.
 const std::string& AdResult::ToString() const {
   FIREBASE_ASSERT(internal_);
   return internal_->to_string;
 }
 
-/// Protected method used by LoadAdResult, a subclass which constructs its
-/// to_string representation programatically after the AdResult construction.
-void AdResult::set_to_string(std::string to_string) {
-  FIREBASE_ASSERT(internal_);
-  internal_->to_string = to_string;
-}
 }  // namespace admob
 }  // namespace firebase

admob/src/android/ad_result_android.h

@@ -38,7 +38,14 @@ namespace admob {
   X(ToString, "toString", "()Ljava/lang/String;")
 // clang-format on
 
+#define LOADADERROR_METHODS(X)                      \
+  X(GetResponseInfo, "getResponseInfo",             \
+    "()Lcom/google/android/gms/ads/ResponseInfo;"), \
+      X(ToString, "toString", "()Ljava/lang/String;")
+// clang-format on
+
 METHOD_LOOKUP_DECLARATION(ad_error, ADERROR_METHODS);
+METHOD_LOOKUP_DECLARATION(load_ad_error, LOADADERROR_METHODS);
 
 struct AdResultInternal {
   // True if the result contains an error originating from C++/Java wrapper
@@ -48,13 +55,16 @@ struct AdResultInternal {
   // True if this was a successful result.
   bool is_successful;
 
-  // An error code
+  // True if this error data represents a result from a LoadAd request.
+  bool is_load_ad_error;
+
+  // An error code.
   AdMobError code;
 
-  // A cached value of com.google.android.gms.ads.AdError.domain
+  // A cached value of com.google.android.gms.ads.AdError.domain.
   std::string domain;
 
-  // A cached value of com.google.android.gms.ads.AdError.message
+  // A cached value of com.google.android.gms.ads.AdError.message.
   std::string message;
 
   // A cached result from invoking com.google.android.gms.ads.AdError.ToString.

admob/src/android/adapter_response_info_android.cc

@@ -44,6 +44,7 @@ AdapterResponseInfo::AdapterResponseInfo(
 
   // Construct an AdResultInternal from the AdError in the AdapterResponseInfo.
   AdResultInternal ad_result_internal;
+  ad_result_internal.is_load_ad_error = false;
   ad_result_internal.j_ad_error = env->CallObjectMethod(
       j_adapter_response_info,
       adapter_response_info::GetMethodId(adapter_response_info::kGetAdError));