Skip to content

Commit 8cf3960

Browse files
DellaBittaDellaBitta
committed
merge feature/admob_2021_adresult
2 parents 633d167 + 051d3e8 commit 8cf3960

20 files changed

+198
-175
lines changed

‎admob/integration_test/src/integration_test.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -515,6 +515,7 @@ TEST_F(FirebaseAdMobTest, TestBannerView) {
515515
EXPECT_EQ(expected_num_bounding_box_changes,
516516
bounding_box_listener.bounding_box_changes_.size());
517517
const firebase::admob::LoadAdResult* result_ptr = load_ad_future.result();
518+
ASSERT_NE(result_ptr, nullptr);
518519
EXPECT_TRUE(result_ptr->is_successful());
519520
EXPECT_EQ(result_ptr->code(), firebase::admob::kAdMobErrorNone);
520521
EXPECT_TRUE(result_ptr->message().empty());
@@ -856,6 +857,7 @@ TEST_F(FirebaseAdMobTest, TestBannerViewErrorLoadInProgress) {
856857
WaitForCompletion(first_load_ad, "First LoadAd");
857858

858859
const firebase::admob::LoadAdResult* result_ptr = second_load_ad.result();
860+
ASSERT_NE(result_ptr, nullptr);
859861
EXPECT_FALSE(result_ptr->is_successful());
860862
EXPECT_EQ(result_ptr->code(), firebase::admob::kAdMobErrorLoadInProgress);
861863
EXPECT_EQ(result_ptr->message(), "Ad is currently loading.");
@@ -888,6 +890,7 @@ TEST_F(FirebaseAdMobTest, TestBannerViewErrorBadAdUnitId) {
888890
firebase::admob::kAdMobErrorInvalidRequest);
889891

890892
const firebase::admob::LoadAdResult* result_ptr = load_ad.result();
893+
ASSERT_NE(result_ptr, nullptr);
891894
EXPECT_FALSE(result_ptr->is_successful());
892895
EXPECT_EQ(result_ptr->code(), firebase::admob::kAdMobErrorInvalidRequest);
893896

@@ -1090,6 +1093,7 @@ TEST_F(FirebaseAdMobTest, TestInterstitialAdErrorLoadInProgress) {
10901093
WaitForCompletion(first_load_ad, "First LoadAd");
10911094

10921095
const firebase::admob::LoadAdResult* result_ptr = second_load_ad.result();
1096+
ASSERT_NE(result_ptr, nullptr);
10931097
EXPECT_FALSE(result_ptr->is_successful());
10941098
EXPECT_EQ(result_ptr->code(), firebase::admob::kAdMobErrorLoadInProgress);
10951099
EXPECT_EQ(result_ptr->message(), "Ad is currently loading.");
@@ -1117,6 +1121,7 @@ TEST_F(FirebaseAdMobTest, TestInterstitialAdErrorBadAdUnitId) {
11171121
firebase::admob::kAdMobErrorInvalidRequest);
11181122

11191123
const firebase::admob::LoadAdResult* result_ptr = load_ad.result();
1124+
ASSERT_NE(result_ptr, nullptr);
11201125
EXPECT_FALSE(result_ptr->is_successful());
11211126
EXPECT_EQ(result_ptr->code(), firebase::admob::kAdMobErrorInvalidRequest);
11221127
EXPECT_FALSE(result_ptr->message().empty());

‎admob/src/android/ad_result_android.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ METHOD_LOOKUP_DEFINITION(ad_error,
3434
"com/google/android/gms/ads/AdError",
3535
ADERROR_METHODS);
3636

37-
const char* AdResult::kUndefinedDomain = "undefined";
37+
const char* const AdResult::kUndefinedDomain = "undefined";
3838

3939
AdResult::AdResult() {
4040
// Default constructor is available for Future creation.

‎admob/src/android/admob_android.cc

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -477,33 +477,33 @@ void CompleteLoadAdCallback(FutureCallbackData<LoadAdResult>* callback_data,
477477
FIREBASE_ASSERT(callback_data);
478478

479479
std::string future_error_message;
480-
LoadAdResultInternal load_ad_result_internal;
481-
AdResultInternal* adr = &(load_ad_result_internal.ad_result_internal);
480+
AdResultInternal ad_result_internal;
482481

483-
adr->j_ad_error = j_load_ad_error;
484-
adr->is_successful = true; // assume until proven otherwise.
485-
adr->is_wrapper_error = false;
486-
adr->code = error_code;
482+
ad_result_internal.j_ad_error = j_load_ad_error;
483+
ad_result_internal.is_successful = true; // assume until proven otherwise.
484+
ad_result_internal.is_wrapper_error = false;
485+
ad_result_internal.code = error_code;
487486

488487
// Further result configuration is based on success/failure.
489488
if (j_load_ad_error != nullptr) {
490489
// The Android SDK returned an error. Use the j_ad_error object
491490
// to populate a LoadAdResult with the error specifics.
492-
adr->is_successful = false;
493-
} else if (adr->code != kAdMobErrorNone) {
491+
ad_result_internal.is_successful = false;
492+
} else if (ad_result_internal.code != kAdMobErrorNone) {
494493
// C++ SDK Android AdMob Wrapper encountered an error.
495-
adr->is_wrapper_error = true;
496-
adr->is_successful = false;
497-
adr->message = error_message;
498-
adr->domain = "SDK";
499-
adr->to_string = std::string("Internal error: ") + adr->message;
500-
future_error_message = adr->message;
494+
ad_result_internal.is_wrapper_error = true;
495+
ad_result_internal.is_successful = false;
496+
ad_result_internal.message = error_message;
497+
ad_result_internal.domain = "SDK";
498+
ad_result_internal.to_string =
499+
std::string("Internal error: ") + ad_result_internal.message;
500+
future_error_message = ad_result_internal.message;
501501
}
502502

503503
// Invoke a friend of LoadAdResult to have it invoke the LoadAdResult
504-
// protected constructor with the LoadAdResultInternal data.
505-
AdMobInternal::CompleteLoadAdFuture(
506-
callback_data, adr->code, future_error_message, load_ad_result_internal);
504+
// protected constructor with the AdResultInternal data.
505+
AdMobInternal::CompleteLoadAdFuture(callback_data, ad_result_internal.code,
506+
future_error_message, ad_result_internal);
507507
}
508508

509509
void CompleteLoadAdAndroidErrorResult(JNIEnv* env, jlong data_ptr,

‎admob/src/android/load_ad_result_android.cc

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,20 +40,19 @@ METHOD_LOOKUP_DEFINITION(load_ad_error,
4040

4141
LoadAdResult::LoadAdResult() : AdResult(), response_info_() {}
4242

43-
LoadAdResult::LoadAdResult(const LoadAdResultInternal& load_ad_result_internal)
44-
: AdResult(load_ad_result_internal.ad_result_internal) {
43+
LoadAdResult::LoadAdResult(const AdResultInternal& ad_result_internal)
44+
: AdResult(ad_result_internal) {
4545
JNIEnv* env = GetJNI();
4646
FIREBASE_ASSERT(env);
4747

48-
if (!load_ad_result_internal.ad_result_internal.is_successful &&
49-
!load_ad_result_internal.ad_result_internal.is_wrapper_error) {
50-
FIREBASE_ASSERT(load_ad_result_internal.ad_result_internal.j_ad_error);
48+
if (!ad_result_internal.is_successful &&
49+
!ad_result_internal.is_wrapper_error) {
50+
FIREBASE_ASSERT(ad_result_internal.j_ad_error);
5151

5252
// Marshalling the data of a LoadAdResult from the AdMob Android SDK is
5353
// delegated to the AdResult constructor (already invoked above) and
5454
// the ResponseInfo constructor below.
55-
jobject j_load_ad_error =
56-
load_ad_result_internal.ad_result_internal.j_ad_error;
55+
jobject j_load_ad_error = ad_result_internal.j_ad_error;
5756

5857
// Grab a reference to the ResponseInfo AdMob Android SDK object within
5958
// the AdError, and construct a C++ SDK analog of the ResponseInfo if the

‎admob/src/android/load_ad_result_android.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,6 @@ namespace admob {
3535

3636
METHOD_LOOKUP_DECLARATION(load_ad_error, LOADADERROR_METHODS);
3737

38-
struct LoadAdResultInternal {
39-
AdResultInternal ad_result_internal;
40-
};
41-
4238
} // namespace admob
4339
} // namespace firebase
4440

‎admob/src/common/admob_common.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,10 @@ void AdListener::OnAdOpened() {}
7272
void AdMobInternal::CompleteLoadAdFuture(
7373
FutureCallbackData<LoadAdResult>* callback_data, int error_code,
7474
const std::string& error_message,
75-
const LoadAdResultInternal& load_ad_result_internal) {
75+
const AdResultInternal& ad_result_internal) {
7676
callback_data->future_data->future_impl.CompleteWithResult(
7777
callback_data->future_handle, static_cast<int>(error_code),
78-
error_message.c_str(), LoadAdResult(load_ad_result_internal));
78+
error_message.c_str(), LoadAdResult(ad_result_internal));
7979
// This method is responsible for disposing of the callback data struct.
8080
delete callback_data;
8181
}

‎admob/src/common/admob_common.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,10 +111,13 @@ FutureCallbackData<LoadAdResult>* CreateLoadAdResultFutureCallbackData(
111111
// callbacks. This is achieved via friend relationships with those classes.
112112
class AdMobInternal {
113113
public:
114+
// Completes a LoadAdResult future given the AdResultInternal object.
114115
static void CompleteLoadAdFuture(
115116
FutureCallbackData<LoadAdResult>* callback_data, int error_code,
116117
const std::string& error_message,
117-
const LoadAdResultInternal& load_ad_result_internal);
118+
const AdResultInternal& ad_result_internal);
119+
120+
// Constructs and returns an AdResult object given an AdResultInteral object.
118121
static AdResult CreateAdResult(const AdResultInternal& ad_result_internal);
119122
};
120123

‎admob/src/common/banner_view_internal.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ void BannerViewInternal::NotifyListenerOfPaidEvent(const AdValue& ad_value) {
118118
}
119119

120120
Future<void> BannerViewInternal::GetLastResult(BannerViewFn fn) {
121+
FIREBASE_ASSERT(fn != kBannerViewFnLoadAd);
121122
return static_cast<const Future<void>&>(
122123
future_data_.future_impl.LastResult(fn));
123124
}

‎admob/src/include/firebase/admob/types.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ namespace admob {
4141
struct AdResultInternal;
4242
struct AdapterResponseInfoInternal;
4343
struct BoundingBox;
44-
struct LoadAdResultInternal;
4544
struct ResponseInfoInternal;
4645

4746
class AdViewBoundingBoxListener;
@@ -194,7 +193,7 @@ class AdResult {
194193
///
195194
/// The Admob SDK returns this domain for domain() method invocations when
196195
/// converting error information from legacy mediation adapter callbacks.
197-
static const char* kUndefinedDomain;
196+
static const char* const kUndefinedDomain;
198197

199198
protected:
200199
// Internal initialization of AdResult. Should only be used to create
@@ -711,7 +710,7 @@ class LoadAdResult : public AdResult {
711710
friend class AdMobInternal;
712711
friend class BannerView;
713712

714-
explicit LoadAdResult(const LoadAdResultInternal& load_ad_result_internal);
713+
explicit LoadAdResult(const AdResultInternal& ad_result_internal);
715714

716715
ResponseInfo response_info_;
717716
};

‎admob/src/ios/ad_result_ios.mm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
namespace firebase {
3131
namespace admob {
3232

33-
const char* AdResult::kUndefinedDomain = "undefined";
33+
const char* const AdResult::kUndefinedDomain = "undefined";
3434

3535
AdResult::AdResult() {
3636
// Default constructor is available for Future creation.

‎admob/src/ios/adapter_response_info_ios.mm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@
4646
latency_ = (int64_t)(internal.ad_network_response_info.latency) *
4747
(int64_t)1000;
4848

49-
NSString *to_string = [[NSString alloc]initWithFormat:@"AdapterResponseInfo"
49+
NSString *to_string = [NSString stringWithFormat:@"AdapterResponseInfo"
5050
"class name: %@, latency : %ld, ad result: %@",
5151
internal.ad_network_response_info.adNetworkClassName,
5252
internal.ad_network_response_info.latency,

‎admob/src/ios/admob_ios.mm

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
#include "admob/src/include/firebase/admob/types.h"
2828
#include "admob/src/ios/ad_result_ios.h"
2929
#include "admob/src/ios/adapter_response_info_ios.h"
30-
#include "admob/src/ios/load_ad_result_ios.h"
3130
#include "admob/src/ios/response_info_ios.h"
3231
#include "app/src/include/firebase/app.h"
3332
#include "app/src/log.h"
@@ -174,32 +173,32 @@ void CompleteLoadAdResult(FutureCallbackData<LoadAdResult>* callback_data,
174173
FIREBASE_ASSERT(error_message);
175174

176175
std::string future_error_message;
177-
LoadAdResultInternal load_ad_result_internal;
178-
AdResultInternal* adr = &(load_ad_result_internal.ad_result_internal);
176+
AdResultInternal ad_result_internal;
179177

180-
adr->ios_error = error;
181-
adr->is_successful = true; // assume until proven otherwise.
182-
adr->is_wrapper_error = false;
183-
adr->code = error_code;
178+
ad_result_internal.ios_error = error;
179+
ad_result_internal.is_successful = true; // assume until proven otherwise.
180+
ad_result_internal.is_wrapper_error = false;
181+
ad_result_internal.code = error_code;
184182

185183
// Futher result configuration is based on success/failure.
186184
if (error != nullptr) {
187185
// The iOS SDK returned an error. The NSError object
188186
// will be used by the LoadAdError implementation to populate
189187
// it's fields.
190-
adr->is_successful = false;
191-
} else if (adr->code != kAdMobErrorNone) {
188+
ad_result_internal.is_successful = false;
189+
} else if (ad_result_internal.code != kAdMobErrorNone) {
192190
// C++ SDK iOS AdMob Wrapper encountered an error.
193-
adr->is_wrapper_error = true;
194-
adr->is_successful = false;
195-
adr->message = std::string(error_message);
196-
adr->domain = "SDK";
197-
adr->to_string = std::string("Internal error: ") + adr->message;
198-
future_error_message = adr->message;
191+
ad_result_internal.is_wrapper_error = true;
192+
ad_result_internal.is_successful = false;
193+
ad_result_internal.message = std::string(error_message);
194+
ad_result_internal.domain = "SDK";
195+
ad_result_internal.to_string = std::string("Internal error: ") +
196+
ad_result_internal.message;
197+
future_error_message = ad_result_internal.message;
199198
}
200199

201200
AdMobInternal::CompleteLoadAdFuture(
202-
callback_data, adr->code, future_error_message, load_ad_result_internal);
201+
callback_data, ad_result_internal.code, future_error_message, ad_result_internal);
203202
}
204203

205204
void CompleteLoadAdInternalResult(
@@ -217,7 +216,7 @@ void CompleteLoadAdIOSResult(FutureCallbackData<LoadAdResult>* callback_data,
217216

218217
AdMobError error_code = MapADErrorCodeToCPPErrorCode((GADErrorCode)gad_error.code);
219218
CompleteLoadAdResult(callback_data, gad_error, error_code,
220-
/*error_message=*/"");
219+
util::NSStringToString(gad_error.localizedDescription).c_str());
221220
}
222221

223222
AdValue ConvertGADAdValueToCppAdValue(GADAdValue* gad_value) {

‎admob/src/ios/load_ad_result_ios.h

Lines changed: 0 additions & 36 deletions
This file was deleted.

‎admob/src/ios/load_ad_result_ios.mm

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

17-
#include "admob/src/ios/load_ad_result_ios.h"
18-
1917
#import <GoogleMobileAds/GoogleMobileAds.h>
2018

2119
#include "admob/src/include/firebase/admob.h"
20+
#include "admob/src/include/firebase/admob/types.h"
21+
#include "admob/src/ios/ad_result_ios.h"
2222
#include "admob/src/ios/response_info_ios.h"
2323
#include "app/src/util_ios.h"
2424

@@ -27,16 +27,16 @@
2727

2828
LoadAdResult::LoadAdResult() : AdResult(), response_info_() {}
2929

30-
LoadAdResult::LoadAdResult(const LoadAdResultInternal& load_ad_result_internal)
31-
: AdResult(load_ad_result_internal.ad_result_internal),
30+
LoadAdResult::LoadAdResult(const AdResultInternal& ad_result_internal)
31+
: AdResult(ad_result_internal),
3232
response_info_(ResponseInfoInternal( {nil} ) ) {
3333

34-
if (!load_ad_result_internal.ad_result_internal.is_successful &&
35-
!load_ad_result_internal.ad_result_internal.is_wrapper_error) {
34+
if (!ad_result_internal.is_successful &&
35+
!ad_result_internal.is_wrapper_error) {
3636

37-
FIREBASE_ASSERT(load_ad_result_internal.ad_result_internal.ios_error);
37+
FIREBASE_ASSERT(ad_result_internal.ios_error);
3838
ResponseInfoInternal response_info_internal = ResponseInfoInternal( {
39-
load_ad_result_internal.ad_result_internal.ios_error.userInfo[GADErrorUserInfoKeyResponseInfo]
39+
ad_result_internal.ios_error.userInfo[GADErrorUserInfoKeyResponseInfo]
4040
});
4141
response_info_ = ResponseInfo(response_info_internal);
4242
}

‎admob/src/stub/adapter_response_info_stub.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ struct AdapterResponseInfoInternal {
2626
int stub;
2727
};
2828

29-
const char* AdResult::kUndefinedDomain = "undefined";
29+
const char* const AdResult::kUndefinedDomain = "undefined";
3030

3131
AdapterResponseInfo::AdapterResponseInfo(
3232
const AdapterResponseInfoInternal& internal) {}

‎admob/src/stub/load_ad_result_stub.cc

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,9 @@
2222
namespace firebase {
2323
namespace admob {
2424

25-
struct LoadAdResultInternal {
26-
char stub;
27-
};
28-
2925
LoadAdResult::LoadAdResult() : AdResult(), response_info_() {}
3026

31-
LoadAdResult::LoadAdResult(const LoadAdResultInternal& load_ad_result_internal)
27+
LoadAdResult::LoadAdResult(const AdResultInternal& ad_result_internal)
3228
: AdResult() {}
3329

3430
LoadAdResult::LoadAdResult(const LoadAdResult& load_ad_result) : AdResult() {}

0 commit comments

Comments
 (0)