Skip to content

Commit e6be822

Browse files
jonsimantovjonsimantov
authored
UMP iOS crash fix (#1463)
* Add debug log. * Add additional mutex locks. * Update UMP reference docs (#1456) * Updated some reference doc text. * Typo fix. * Add some debug logs for completion handler. * More debugging, and add an Fn slot for the OperationInProgress errors. * Add an instance tag to ensure that callbacks on iOS don't execute on the wrong instance. * Also verify instance before calling the native iOS method. * Format code. * Fix integration test structure. * Remove debug logging. * Fixes and added delay * Format code and comments. * Remove obsolete enum. * Format code. * Refactor OperationInProgress test into two tests. * Add a main queue flush to the GMA test class. * Format.
1 parent b1f6b59 commit e6be822

File tree

3 files changed

+260
-79
lines changed

3 files changed

+260
-79
lines changed

‎gma/integration_test/src/integration_test.cc

Lines changed: 166 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,7 @@ FirebaseGmaTest::~FirebaseGmaTest() {}
256256

257257
void FirebaseGmaTest::SetUp() {
258258
TEST_DOES_NOT_REQUIRE_USER_INTERACTION;
259+
ProcessEvents(100); // Flush main thread queue.
259260
FirebaseTest::SetUp();
260261

261262
// This example uses ad units that are specially configured to return test ads
@@ -2484,7 +2485,7 @@ class FirebaseGmaUmpTest : public FirebaseGmaTest {
24842485
enum ResetOption { kReset, kNoReset };
24852486

24862487
void InitializeUmp(ResetOption reset = kReset);
2487-
void TerminateUmp();
2488+
void TerminateUmp(ResetOption reset = kReset);
24882489

24892490
void SetUp() override;
24902491
void TearDown() override;
@@ -2506,9 +2507,11 @@ void FirebaseGmaUmpTest::InitializeUmp(ResetOption reset) {
25062507
}
25072508
}
25082509

2509-
void FirebaseGmaUmpTest::TerminateUmp() {
2510+
void FirebaseGmaUmpTest::TerminateUmp(ResetOption reset) {
25102511
if (consent_info_) {
2511-
consent_info_->Reset();
2512+
if (reset == kReset) {
2513+
consent_info_->Reset();
2514+
}
25122515
delete consent_info_;
25132516
consent_info_ = nullptr;
25142517
}
@@ -2901,47 +2904,121 @@ TEST_F(FirebaseGmaUmpTest, TestCanRequestAdsEEA) {
29012904
}
29022905

29032906
TEST_F(FirebaseGmaUmpTest, TestUmpCleanupWithDelay) {
2907+
// Ensure that if ConsentInfo is deleted after a delay, Futures are
2908+
// properly invalidated.
29042909
using firebase::gma::ump::ConsentFormStatus;
29052910
using firebase::gma::ump::ConsentRequestParameters;
29062911
using firebase::gma::ump::ConsentStatus;
29072912

29082913
ConsentRequestParameters params;
29092914
params.tag_for_under_age_of_consent = false;
2915+
params.debug_settings.debug_geography =
2916+
firebase::gma::ump::kConsentDebugGeographyNonEEA;
2917+
params.debug_settings.debug_device_ids = kTestDeviceIDs;
2918+
params.debug_settings.debug_device_ids.push_back(GetDebugDeviceId());
2919+
29102920
firebase::Future<void> future_request =
29112921
consent_info_->RequestConsentInfoUpdate(params);
29122922
firebase::Future<void> future_load = consent_info_->LoadConsentForm();
29132923
firebase::Future<void> future_show =
29142924
consent_info_->ShowConsentForm(app_framework::GetWindowController());
2925+
firebase::Future<void> future_load_and_show =
2926+
consent_info_->LoadAndShowConsentFormIfRequired(
2927+
app_framework::GetWindowController());
2928+
firebase::Future<void> future_privacy = consent_info_->ShowPrivacyOptionsForm(
2929+
app_framework::GetWindowController());
29152930

29162931
ProcessEvents(5000);
29172932

2918-
delete consent_info_;
2919-
consent_info_ = nullptr;
2933+
TerminateUmp(kNoReset);
29202934

29212935
EXPECT_EQ(future_request.status(), firebase::kFutureStatusInvalid);
29222936
EXPECT_EQ(future_load.status(), firebase::kFutureStatusInvalid);
29232937
EXPECT_EQ(future_show.status(), firebase::kFutureStatusInvalid);
2938+
EXPECT_EQ(future_load_and_show.status(), firebase::kFutureStatusInvalid);
2939+
EXPECT_EQ(future_privacy.status(), firebase::kFutureStatusInvalid);
29242940
}
29252941

29262942
TEST_F(FirebaseGmaUmpTest, TestUmpCleanupRaceCondition) {
2943+
// Ensure that if ConsentInfo is deleted immediately, operations
2944+
// (and their Futures) are properly invalidated.
2945+
29272946
using firebase::gma::ump::ConsentFormStatus;
29282947
using firebase::gma::ump::ConsentRequestParameters;
29292948
using firebase::gma::ump::ConsentStatus;
29302949

29312950
ConsentRequestParameters params;
29322951
params.tag_for_under_age_of_consent = false;
2952+
params.debug_settings.debug_geography =
2953+
firebase::gma::ump::kConsentDebugGeographyNonEEA;
2954+
params.debug_settings.debug_device_ids = kTestDeviceIDs;
2955+
params.debug_settings.debug_device_ids.push_back(GetDebugDeviceId());
2956+
29332957
firebase::Future<void> future_request =
29342958
consent_info_->RequestConsentInfoUpdate(params);
29352959
firebase::Future<void> future_load = consent_info_->LoadConsentForm();
29362960
firebase::Future<void> future_show =
29372961
consent_info_->ShowConsentForm(app_framework::GetWindowController());
2962+
firebase::Future<void> future_load_and_show =
2963+
consent_info_->LoadAndShowConsentFormIfRequired(
2964+
app_framework::GetWindowController());
2965+
firebase::Future<void> future_privacy = consent_info_->ShowPrivacyOptionsForm(
2966+
app_framework::GetWindowController());
29382967

2939-
delete consent_info_;
2940-
consent_info_ = nullptr;
2968+
TerminateUmp(kNoReset);
29412969

29422970
EXPECT_EQ(future_request.status(), firebase::kFutureStatusInvalid);
29432971
EXPECT_EQ(future_load.status(), firebase::kFutureStatusInvalid);
29442972
EXPECT_EQ(future_show.status(), firebase::kFutureStatusInvalid);
2973+
EXPECT_EQ(future_load_and_show.status(), firebase::kFutureStatusInvalid);
2974+
EXPECT_EQ(future_privacy.status(), firebase::kFutureStatusInvalid);
2975+
2976+
ProcessEvents(5000);
2977+
}
2978+
2979+
TEST_F(FirebaseGmaUmpTest, TestUmpCallbacksOnWrongInstance) {
2980+
// Ensure that if ConsentInfo is deleted and then recreated, stale
2981+
// callbacks don't call into the new instance.
2982+
using firebase::gma::ump::ConsentFormStatus;
2983+
using firebase::gma::ump::ConsentRequestParameters;
2984+
using firebase::gma::ump::ConsentStatus;
2985+
2986+
ConsentRequestParameters params;
2987+
params.tag_for_under_age_of_consent = false;
2988+
params.debug_settings.debug_geography =
2989+
firebase::gma::ump::kConsentDebugGeographyNonEEA;
2990+
params.debug_settings.debug_device_ids = kTestDeviceIDs;
2991+
params.debug_settings.debug_device_ids.push_back(GetDebugDeviceId());
2992+
2993+
firebase::Future<void> future_request =
2994+
consent_info_->RequestConsentInfoUpdate(params);
2995+
firebase::Future<void> future_load = consent_info_->LoadConsentForm();
2996+
firebase::Future<void> future_show =
2997+
consent_info_->ShowConsentForm(app_framework::GetWindowController());
2998+
firebase::Future<void> future_load_and_show =
2999+
consent_info_->LoadAndShowConsentFormIfRequired(
3000+
app_framework::GetWindowController());
3001+
firebase::Future<void> future_privacy = consent_info_->ShowPrivacyOptionsForm(
3002+
app_framework::GetWindowController());
3003+
3004+
TerminateUmp(kNoReset);
3005+
3006+
EXPECT_EQ(future_request.status(), firebase::kFutureStatusInvalid);
3007+
EXPECT_EQ(future_load.status(), firebase::kFutureStatusInvalid);
3008+
EXPECT_EQ(future_show.status(), firebase::kFutureStatusInvalid);
3009+
EXPECT_EQ(future_load_and_show.status(), firebase::kFutureStatusInvalid);
3010+
EXPECT_EQ(future_privacy.status(), firebase::kFutureStatusInvalid);
3011+
3012+
InitializeUmp(kNoReset);
3013+
3014+
EXPECT_EQ(future_request.status(), firebase::kFutureStatusInvalid);
3015+
EXPECT_EQ(future_load.status(), firebase::kFutureStatusInvalid);
3016+
EXPECT_EQ(future_show.status(), firebase::kFutureStatusInvalid);
3017+
EXPECT_EQ(future_load_and_show.status(), firebase::kFutureStatusInvalid);
3018+
EXPECT_EQ(future_privacy.status(), firebase::kFutureStatusInvalid);
3019+
3020+
// Give the operations time to complete.
3021+
ProcessEvents(5000);
29453022
}
29463023

29473024
TEST_F(FirebaseGmaUmpTest, TestUmpMethodsReturnOperationInProgress) {
@@ -2952,18 +3029,15 @@ TEST_F(FirebaseGmaUmpTest, TestUmpMethodsReturnOperationInProgress) {
29523029
using firebase::gma::ump::ConsentStatus;
29533030

29543031
// Check that all of the UMP operations properly return an OperationInProgress
2955-
// error if called more than once at the same time. Each step of this test is
2956-
// inherently flaky, so add flaky test blocks all over.
3032+
// error if called more than once at the same time.
29573033

29583034
ConsentRequestParameters params;
29593035
params.tag_for_under_age_of_consent = false;
29603036
params.debug_settings.debug_geography =
2961-
ShouldRunUITests() ? firebase::gma::ump::kConsentDebugGeographyEEA
2962-
: firebase::gma::ump::kConsentDebugGeographyNonEEA;
3037+
firebase::gma::ump::kConsentDebugGeographyNonEEA;
29633038
params.debug_settings.debug_device_ids = kTestDeviceIDs;
29643039
params.debug_settings.debug_device_ids.push_back(GetDebugDeviceId());
29653040

2966-
FLAKY_TEST_SECTION_BEGIN();
29673041
firebase::Future<void> future_request_1 =
29683042
consent_info_->RequestConsentInfoUpdate(params);
29693043
firebase::Future<void> future_request_2 =
@@ -2972,62 +3046,88 @@ TEST_F(FirebaseGmaUmpTest, TestUmpMethodsReturnOperationInProgress) {
29723046
future_request_2, "RequestConsentInfoUpdate second",
29733047
firebase::gma::ump::kConsentRequestErrorOperationInProgress);
29743048
WaitForCompletion(future_request_1, "RequestConsentInfoUpdate first");
2975-
FLAKY_TEST_SECTION_END();
29763049

2977-
if (ShouldRunUITests()) {
2978-
// The below should only be checked if UI tests are enabled, as they
2979-
// require some interaction.
2980-
FLAKY_TEST_SECTION_BEGIN();
2981-
firebase::Future<void> future_load_1 = consent_info_->LoadConsentForm();
2982-
firebase::Future<void> future_load_2 = consent_info_->LoadConsentForm();
2983-
WaitForCompletion(future_load_2, "LoadConsentForm second",
2984-
firebase::gma::ump::kConsentFormErrorOperationInProgress);
2985-
WaitForCompletion(future_load_1, "LoadConsentForm first");
2986-
FLAKY_TEST_SECTION_END();
2987-
2988-
FLAKY_TEST_SECTION_BEGIN();
2989-
firebase::Future<void> future_show_1 =
2990-
consent_info_->ShowConsentForm(app_framework::GetWindowController());
2991-
firebase::Future<void> future_show_2 =
2992-
consent_info_->ShowConsentForm(app_framework::GetWindowController());
2993-
WaitForCompletion(future_show_2, "ShowConsentForm second",
2994-
firebase::gma::ump::kConsentFormErrorOperationInProgress);
2995-
WaitForCompletion(future_show_1, "ShowConsentForm first");
2996-
FLAKY_TEST_SECTION_END();
2997-
2998-
FLAKY_TEST_SECTION_BEGIN();
2999-
firebase::Future<void> future_privacy_1 =
3000-
consent_info_->ShowPrivacyOptionsForm(
3001-
app_framework::GetWindowController());
3002-
firebase::Future<void> future_privacy_2 =
3003-
consent_info_->ShowPrivacyOptionsForm(
3004-
app_framework::GetWindowController());
3005-
WaitForCompletion(future_privacy_2, "ShowPrivacyOptionsForm second",
3006-
firebase::gma::ump::kConsentFormErrorOperationInProgress);
3007-
WaitForCompletion(future_privacy_1, "ShowPrivacyOptionsForm first");
3008-
FLAKY_TEST_SECTION_END();
3050+
firebase::Future<void> future_load_and_show_1 =
3051+
consent_info_->LoadAndShowConsentFormIfRequired(
3052+
app_framework::GetWindowController());
3053+
firebase::Future<void> future_load_and_show_2 =
3054+
consent_info_->LoadAndShowConsentFormIfRequired(
3055+
app_framework::GetWindowController());
3056+
WaitForCompletion(future_load_and_show_2,
3057+
"LoadAndShowConsentFormIfRequired second",
3058+
firebase::gma::ump::kConsentFormErrorOperationInProgress);
3059+
WaitForCompletion(future_load_and_show_1,
3060+
"LoadAndShowConsentFormIfRequired first");
3061+
}
30093062

3010-
consent_info_->Reset();
3011-
// Request again so we can test LoadAndShowConsentFormIfRequired.
3012-
WaitForCompletion(consent_info_->RequestConsentInfoUpdate(params),
3013-
"RequestConsentInfoUpdate");
3014-
3015-
FLAKY_TEST_SECTION_BEGIN();
3016-
firebase::Future<void> future_load_and_show_1 =
3017-
consent_info_->LoadAndShowConsentFormIfRequired(
3018-
app_framework::GetWindowController());
3019-
firebase::Future<void> future_load_and_show_2 =
3020-
consent_info_->LoadAndShowConsentFormIfRequired(
3021-
app_framework::GetWindowController());
3022-
WaitForCompletion(future_load_and_show_2,
3023-
"LoadAndShowConsentFormIfRequired second",
3024-
firebase::gma::ump::kConsentFormErrorOperationInProgress);
3025-
WaitForCompletion(future_load_and_show_1,
3026-
"LoadAndShowConsentFormIfRequired first");
3027-
FLAKY_TEST_SECTION_END();
3028-
} else {
3029-
LogInfo("Skipping methods that require user interaction.");
3030-
}
3063+
TEST_F(FirebaseGmaUmpTest, TestUmpMethodsReturnOperationInProgressWithUI) {
3064+
SKIP_TEST_ON_DESKTOP;
3065+
TEST_REQUIRES_USER_INTERACTION;
3066+
3067+
using firebase::gma::ump::ConsentFormStatus;
3068+
using firebase::gma::ump::ConsentRequestParameters;
3069+
using firebase::gma::ump::ConsentStatus;
3070+
3071+
// Check that all of the UMP operations properly return an OperationInProgress
3072+
// error if called more than once at the same time. This test include methods
3073+
// with UI interaction.
3074+
3075+
ConsentRequestParameters params;
3076+
params.tag_for_under_age_of_consent = false;
3077+
params.debug_settings.debug_geography =
3078+
firebase::gma::ump::kConsentDebugGeographyEEA;
3079+
params.debug_settings.debug_device_ids = kTestDeviceIDs;
3080+
params.debug_settings.debug_device_ids.push_back(GetDebugDeviceId());
3081+
3082+
firebase::Future<void> future_request_1 =
3083+
consent_info_->RequestConsentInfoUpdate(params);
3084+
firebase::Future<void> future_request_2 =
3085+
consent_info_->RequestConsentInfoUpdate(params);
3086+
WaitForCompletion(
3087+
future_request_2, "RequestConsentInfoUpdate second",
3088+
firebase::gma::ump::kConsentRequestErrorOperationInProgress);
3089+
WaitForCompletion(future_request_1, "RequestConsentInfoUpdate first");
3090+
3091+
firebase::Future<void> future_load_1 = consent_info_->LoadConsentForm();
3092+
firebase::Future<void> future_load_2 = consent_info_->LoadConsentForm();
3093+
WaitForCompletion(future_load_2, "LoadConsentForm second",
3094+
firebase::gma::ump::kConsentFormErrorOperationInProgress);
3095+
WaitForCompletion(future_load_1, "LoadConsentForm first");
3096+
3097+
firebase::Future<void> future_show_1 =
3098+
consent_info_->ShowConsentForm(app_framework::GetWindowController());
3099+
firebase::Future<void> future_show_2 =
3100+
consent_info_->ShowConsentForm(app_framework::GetWindowController());
3101+
WaitForCompletion(future_show_2, "ShowConsentForm second",
3102+
firebase::gma::ump::kConsentFormErrorOperationInProgress);
3103+
WaitForCompletion(future_show_1, "ShowConsentForm first");
3104+
3105+
firebase::Future<void> future_privacy_1 =
3106+
consent_info_->ShowPrivacyOptionsForm(
3107+
app_framework::GetWindowController());
3108+
firebase::Future<void> future_privacy_2 =
3109+
consent_info_->ShowPrivacyOptionsForm(
3110+
app_framework::GetWindowController());
3111+
WaitForCompletion(future_privacy_2, "ShowPrivacyOptionsForm second",
3112+
firebase::gma::ump::kConsentFormErrorOperationInProgress);
3113+
WaitForCompletion(future_privacy_1, "ShowPrivacyOptionsForm first");
3114+
3115+
consent_info_->Reset();
3116+
// Request again so we can test LoadAndShowConsentFormIfRequired.
3117+
WaitForCompletion(consent_info_->RequestConsentInfoUpdate(params),
3118+
"RequestConsentInfoUpdate");
3119+
3120+
firebase::Future<void> future_load_and_show_1 =
3121+
consent_info_->LoadAndShowConsentFormIfRequired(
3122+
app_framework::GetWindowController());
3123+
firebase::Future<void> future_load_and_show_2 =
3124+
consent_info_->LoadAndShowConsentFormIfRequired(
3125+
app_framework::GetWindowController());
3126+
WaitForCompletion(future_load_and_show_2,
3127+
"LoadAndShowConsentFormIfRequired second",
3128+
firebase::gma::ump::kConsentFormErrorOperationInProgress);
3129+
WaitForCompletion(future_load_and_show_1,
3130+
"LoadAndShowConsentFormIfRequired first");
30313131
}
30323132

30333133
} // namespace firebase_testapp_automated

‎gma/src/ios/ump/consent_info_internal_ios.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ class ConsentInfoInternalIos : public ConsentInfoInternal {
5252
private:
5353
static ConsentInfoInternalIos* s_instance;
5454
static firebase::Mutex s_instance_mutex;
55+
static unsigned int s_instance_tag;
5556

5657
void SetLoadedForm(UMPConsentForm *form) {
5758
loaded_form_ = form;

0 commit comments

Comments
 (0)