Skip to content

Re-enable WrongInstance test with fixed logic and debug logs #1521

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jan 19, 2024
Merged

Re-enable WrongInstance test with fixed logic and debug logs #1521

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
55b48db
Re-enable WrongInstance test with fixed logic and debug logs
jonsimantov Jan 17, 2024
ef8d823
Add additional logging for when Futures complete.
jonsimantov Jan 17, 2024
575f712
Fix missing namespace.
jonsimantov Jan 17, 2024
13982a8
Merge branch 'main' into gma-ump-crash
jonsimantov Jan 18, 2024
8a092ce
Merge branch 'main' into gma-ump-crash
jonsimantov Jan 18, 2024
3ff582b
Add network error retry to RequestConsentInfoUpdate tests.
jonsimantov Jan 18, 2024
b6d3d10
Merge branch 'gma-ump-crash' of https://github.com/firebase/firebase-…
jonsimantov Jan 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 96 additions & 17 deletions gma/integration_test/src/integration_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2599,6 +2599,8 @@ TEST_F(FirebaseGmaUmpTest, TestUmpRequestConsentInfoUpdate) {
using firebase::gma::ump::ConsentRequestParameters;
using firebase::gma::ump::ConsentStatus;

FLAKY_TEST_SECTION_BEGIN();

ConsentRequestParameters params;
params.tag_for_under_age_of_consent = false;

Expand All @@ -2607,7 +2609,13 @@ TEST_F(FirebaseGmaUmpTest, TestUmpRequestConsentInfoUpdate) {

EXPECT_TRUE(future == consent_info_->RequestConsentInfoUpdateLastResult());

WaitForCompletion(future, "RequestConsentInfoUpdate");
WaitForCompletion(future, "RequestConsentInfoUpdate",
{firebase::gma::ump::kConsentRequestSuccess,
firebase::gma::ump::kConsentRequestErrorNetwork});
// Retry only network errors.
EXPECT_NE(future.error(), firebase::gma::ump::kConsentRequestErrorNetwork);

FLAKY_TEST_SECTION_END();

EXPECT_NE(consent_info_->GetConsentStatus(),
firebase::gma::ump::kConsentStatusUnknown);
Expand All @@ -2622,6 +2630,8 @@ TEST_F(FirebaseGmaUmpTest, TestUmpRequestConsentInfoUpdateDebugEEA) {
using firebase::gma::ump::ConsentRequestParameters;
using firebase::gma::ump::ConsentStatus;

FLAKY_TEST_SECTION_BEGIN();

ConsentRequestParameters params;
params.tag_for_under_age_of_consent = false;
params.debug_settings.debug_geography =
Expand All @@ -2632,7 +2642,13 @@ TEST_F(FirebaseGmaUmpTest, TestUmpRequestConsentInfoUpdateDebugEEA) {
firebase::Future<void> future =
consent_info_->RequestConsentInfoUpdate(params);

WaitForCompletion(future, "RequestConsentInfoUpdate");
WaitForCompletion(future, "RequestConsentInfoUpdate",
{firebase::gma::ump::kConsentRequestSuccess,
firebase::gma::ump::kConsentRequestErrorNetwork});
// Retry only network errors.
EXPECT_NE(future.error(), firebase::gma::ump::kConsentRequestErrorNetwork);

FLAKY_TEST_SECTION_END();

EXPECT_EQ(consent_info_->GetConsentStatus(),
firebase::gma::ump::kConsentStatusRequired);
Expand All @@ -2643,6 +2659,8 @@ TEST_F(FirebaseGmaUmpTest, TestUmpRequestConsentInfoUpdateDebugNonEEA) {
using firebase::gma::ump::ConsentRequestParameters;
using firebase::gma::ump::ConsentStatus;

FLAKY_TEST_SECTION_BEGIN();

ConsentRequestParameters params;
params.tag_for_under_age_of_consent = false;
params.debug_settings.debug_geography =
Expand All @@ -2653,7 +2671,13 @@ TEST_F(FirebaseGmaUmpTest, TestUmpRequestConsentInfoUpdateDebugNonEEA) {
firebase::Future<void> future =
consent_info_->RequestConsentInfoUpdate(params);

WaitForCompletion(future, "RequestConsentInfoUpdate");
WaitForCompletion(future, "RequestConsentInfoUpdate",
{firebase::gma::ump::kConsentRequestSuccess,
firebase::gma::ump::kConsentRequestErrorNetwork});
// Retry only network errors.
EXPECT_NE(future.error(), firebase::gma::ump::kConsentRequestErrorNetwork);

FLAKY_TEST_SECTION_END();

EXPECT_THAT(consent_info_->GetConsentStatus(),
AnyOf(Eq(firebase::gma::ump::kConsentStatusNotRequired),
Expand Down Expand Up @@ -2747,15 +2771,25 @@ TEST_F(FirebaseGmaUmpTest, TestUmpLoadFormUnderAgeOfConsent) {
using firebase::gma::ump::ConsentRequestParameters;
using firebase::gma::ump::ConsentStatus;

FLAKY_TEST_SECTION_BEGIN();

ConsentRequestParameters params;
params.tag_for_under_age_of_consent = true;
params.debug_settings.debug_geography =
firebase::gma::ump::kConsentDebugGeographyEEA;
params.debug_settings.debug_device_ids = kTestDeviceIDs;
params.debug_settings.debug_device_ids.push_back(GetDebugDeviceId());

WaitForCompletion(consent_info_->RequestConsentInfoUpdate(params),
"RequestConsentInfoUpdate");
firebase::Future<void> future =
consent_info_->RequestConsentInfoUpdate(params);

WaitForCompletion(future, "RequestConsentInfoUpdate",
{firebase::gma::ump::kConsentRequestSuccess,
firebase::gma::ump::kConsentRequestErrorNetwork});
// Retry only network errors.
EXPECT_NE(future.error(), firebase::gma::ump::kConsentRequestErrorNetwork);

FLAKY_TEST_SECTION_END();

firebase::Future<void> load_future = consent_info_->LoadConsentForm();
WaitForCompletion(load_future, "LoadConsentForm",
Expand All @@ -2770,15 +2804,25 @@ TEST_F(FirebaseGmaUmpTest, TestUmpLoadFormUnavailableDebugNonEEA) {
using firebase::gma::ump::ConsentRequestParameters;
using firebase::gma::ump::ConsentStatus;

FLAKY_TEST_SECTION_BEGIN();

ConsentRequestParameters params;
params.tag_for_under_age_of_consent = false;
params.debug_settings.debug_geography =
firebase::gma::ump::kConsentDebugGeographyNonEEA;
params.debug_settings.debug_device_ids = kTestDeviceIDs;
params.debug_settings.debug_device_ids.push_back(GetDebugDeviceId());

WaitForCompletion(consent_info_->RequestConsentInfoUpdate(params),
"RequestConsentInfoUpdate");
firebase::Future<void> future =
consent_info_->RequestConsentInfoUpdate(params);

WaitForCompletion(future, "RequestConsentInfoUpdate",
{firebase::gma::ump::kConsentRequestSuccess,
firebase::gma::ump::kConsentRequestErrorNetwork});
// Retry only network errors.
EXPECT_NE(future.error(), firebase::gma::ump::kConsentRequestErrorNetwork);

FLAKY_TEST_SECTION_END();

if (consent_info_->GetConsentStatus() !=
firebase::gma::ump::kConsentStatusRequired) {
Expand All @@ -2792,15 +2836,25 @@ TEST_F(FirebaseGmaUmpTest, TestUmpLoadAndShowIfRequiredDebugNonEEA) {
using firebase::gma::ump::ConsentRequestParameters;
using firebase::gma::ump::ConsentStatus;

FLAKY_TEST_SECTION_BEGIN();

ConsentRequestParameters params;
params.tag_for_under_age_of_consent = false;
params.debug_settings.debug_geography =
firebase::gma::ump::kConsentDebugGeographyNonEEA;
params.debug_settings.debug_device_ids = kTestDeviceIDs;
params.debug_settings.debug_device_ids.push_back(GetDebugDeviceId());

WaitForCompletion(consent_info_->RequestConsentInfoUpdate(params),
"RequestConsentInfoUpdate");
firebase::Future<void> future =
consent_info_->RequestConsentInfoUpdate(params);

WaitForCompletion(future, "RequestConsentInfoUpdate",
{firebase::gma::ump::kConsentRequestSuccess,
firebase::gma::ump::kConsentRequestErrorNetwork});
// Retry only network errors.
EXPECT_NE(future.error(), firebase::gma::ump::kConsentRequestErrorNetwork);

FLAKY_TEST_SECTION_END();

EXPECT_THAT(consent_info_->GetConsentStatus(),
AnyOf(Eq(firebase::gma::ump::kConsentStatusNotRequired),
Expand Down Expand Up @@ -3024,7 +3078,7 @@ TEST_F(FirebaseGmaUmpTest, TestUmpCleanupRaceCondition) {
ProcessEvents(5000);
}

TEST_F(FirebaseGmaUmpTest, TestUmpCallbacksOnWrongInstance_DISABLED) {
TEST_F(FirebaseGmaUmpTest, TestUmpCallbacksOnWrongInstance) {
// Ensure that if ConsentInfo is deleted and then recreated, stale
// callbacks don't call into the new instance and cause crashes.
using firebase::gma::ump::ConsentFormStatus;
Expand All @@ -3038,23 +3092,46 @@ TEST_F(FirebaseGmaUmpTest, TestUmpCallbacksOnWrongInstance_DISABLED) {
params.debug_settings.debug_device_ids = kTestDeviceIDs;
params.debug_settings.debug_device_ids.push_back(GetDebugDeviceId());

consent_info_->RequestConsentInfoUpdate(params);
consent_info_->LoadConsentForm();
LogDebug("RequestConsentInfoUpdate");
consent_info_->RequestConsentInfoUpdate(params).OnCompletion(
[](const firebase::Future<void>&) {
LogDebug("RequestConsentInfoUpdate done");
});
LogDebug("LoadConsentForm");
consent_info_->LoadConsentForm().OnCompletion(
[](const firebase::Future<void>&) { LogDebug("LoadConsentForm done"); });
// In automated tests, only check RequestConsentInfoUpdate and LoadConsentForm
// as the rest may show UI.
if (ShouldRunUITests()) {
consent_info_->ShowConsentForm(app_framework::GetWindowController());
consent_info_->LoadAndShowConsentFormIfRequired(
app_framework::GetWindowController());
consent_info_->ShowPrivacyOptionsForm(app_framework::GetWindowController());
LogDebug("ShowConsentForm");
consent_info_->ShowConsentForm(app_framework::GetWindowController())
.OnCompletion([](const firebase::Future<void>&) {
LogDebug("ShowConsentForm done");
});
LogDebug("LoadAndShowConsentFormIfRequired");
consent_info_
->LoadAndShowConsentFormIfRequired(app_framework::GetWindowController())
.OnCompletion([](const firebase::Future<void>&) {
LogDebug("LoadAndShowConsentFormIfRequired done");
});
LogDebug("ShowPrivacyOptionsForm");
consent_info_->ShowPrivacyOptionsForm(app_framework::GetWindowController())
.OnCompletion([](const firebase::Future<void>&) {
LogDebug("ShowPrivacyOptionsForm done");
});
}

LogDebug("Terminate");
TerminateUmp(kNoReset);

LogDebug("Initialize");
InitializeUmp(kNoReset);

// Give the operations time to complete.
LogDebug("Wait");
ProcessEvents(5000);

LogDebug("Done");
}

TEST_F(FirebaseGmaUmpTest, TestUmpMethodsReturnOperationInProgress) {
Expand Down Expand Up @@ -3086,7 +3163,9 @@ TEST_F(FirebaseGmaUmpTest, TestUmpMethodsReturnOperationInProgress) {
WaitForCompletion(
future_request_2, "RequestConsentInfoUpdate second",
firebase::gma::ump::kConsentRequestErrorOperationInProgress);
WaitForCompletion(future_request_1, "RequestConsentInfoUpdate first");
WaitForCompletion(future_request_1, "RequestConsentInfoUpdate first",
{firebase::gma::ump::kConsentRequestSuccess,
firebase::gma::ump::kConsentRequestErrorNetwork});

consent_info_->Reset();

Expand Down
110 changes: 41 additions & 69 deletions gma/src/ios/ump/consent_info_internal_ios.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,24 +125,19 @@ static ConsentFormError CppFormErrorFromIosFormError(NSInteger code) {
callback_instance_tag = s_instance_tag;

util::DispatchAsyncSafeMainQueue(^{
{
MutexLock lock(s_instance_mutex);
if (!s_instance || s_instance_tag != callback_instance_tag) {
// Instance changed or was invalidated, don't call the iOS method any more.
return;
}
MutexLock lock(s_instance_mutex);
if (!s_instance || s_instance_tag != callback_instance_tag) {
// Instance changed or was invalidated, don't call the iOS method any more.
return;
}
[UMPConsentInformation.sharedInstance
requestConsentInfoUpdateWithParameters:ios_parameters
completionHandler:^(NSError* _Nullable error) {
if (!error) {
MutexLock lock(s_instance_mutex);
if (s_instance && s_instance_tag == callback_instance_tag) {
MutexLock lock(s_instance_mutex);
if (s_instance && s_instance_tag == callback_instance_tag) {
if (!error) {
CompleteFuture(handle, kConsentRequestSuccess);
}
} else {
MutexLock lock(s_instance_mutex);
if (s_instance && s_instance_tag == callback_instance_tag) {
} else {
CompleteFuture(handle,
CppRequestErrorFromIosRequestError(error.code),
error.localizedDescription.UTF8String);
Expand Down Expand Up @@ -203,30 +198,22 @@ static ConsentFormError CppFormErrorFromIosFormError(NSInteger code) {
callback_instance_tag = s_instance_tag;

util::DispatchAsyncSafeMainQueue(^{
{
MutexLock lock(s_instance_mutex);
if (!s_instance || s_instance_tag != callback_instance_tag) {
// Instance changed or was invalidated, don't call the iOS method any more.
return;
}
MutexLock lock(s_instance_mutex);
if (!s_instance || s_instance_tag != callback_instance_tag) {
// Instance changed or was invalidated, don't call the iOS method any more.
return;
}
[UMPConsentForm
loadWithCompletionHandler:^(UMPConsentForm* _Nullable form, NSError* _Nullable error) {
if (form) {
MutexLock lock(s_instance_mutex);
if (s_instance && s_instance_tag == callback_instance_tag) {
MutexLock lock(s_instance_mutex);
if (s_instance && s_instance_tag == callback_instance_tag) {
if (form) {
SetLoadedForm(form);
CompleteFuture(handle, kConsentFormSuccess, "Success");
}
} else if (error) {
MutexLock lock(s_instance_mutex);
if (s_instance && s_instance_tag == callback_instance_tag) {
} else if (error) {
CompleteFuture(handle, CppFormErrorFromIosFormError(error.code),
error.localizedDescription.UTF8String);
}
} else {
MutexLock lock(s_instance_mutex);
if (s_instance && s_instance_tag == callback_instance_tag) {
} else {
CompleteFuture(handle, kConsentFormErrorUnknown, "An unknown error occurred.");
}
}
Expand Down Expand Up @@ -254,23 +241,18 @@ static ConsentFormError CppFormErrorFromIosFormError(NSInteger code) {
callback_instance_tag = s_instance_tag;

util::DispatchAsyncSafeMainQueue(^{
{
MutexLock lock(s_instance_mutex);
if (!s_instance || s_instance_tag != callback_instance_tag) {
// Instance changed or was invalidated, don't call the iOS method any more.
return;
}
MutexLock lock(s_instance_mutex);
if (!s_instance || s_instance_tag != callback_instance_tag) {
// Instance changed or was invalidated, don't call the iOS method any more.
return;
}
[loaded_form_ presentFromViewController:parent
completionHandler:^(NSError* _Nullable error) {
if (!error) {
MutexLock lock(s_instance_mutex);
if (s_instance && s_instance_tag == callback_instance_tag) {
MutexLock lock(s_instance_mutex);
if (s_instance && s_instance_tag == callback_instance_tag) {
if (!error) {
CompleteFuture(handle, kConsentRequestSuccess);
}
} else {
MutexLock lock(s_instance_mutex);
if (s_instance && s_instance_tag == callback_instance_tag) {
} else {
CompleteFuture(handle, CppFormErrorFromIosFormError(error.code),
error.localizedDescription.UTF8String);
}
Expand All @@ -297,24 +279,19 @@ static ConsentFormError CppFormErrorFromIosFormError(NSInteger code) {
callback_instance_tag = s_instance_tag;

util::DispatchAsyncSafeMainQueue(^{
{
MutexLock lock(s_instance_mutex);
if (!s_instance || s_instance_tag != callback_instance_tag) {
// Instance changed or was invalidated, don't call the iOS method any more.
return;
}
MutexLock lock(s_instance_mutex);
if (!s_instance || s_instance_tag != callback_instance_tag) {
// Instance changed or was invalidated, don't call the iOS method any more.
return;
}
[UMPConsentForm
loadAndPresentIfRequiredFromViewController:parent
completionHandler:^(NSError* _Nullable error) {
if (!error) {
MutexLock lock(s_instance_mutex);
if (s_instance && s_instance_tag == callback_instance_tag) {
MutexLock lock(s_instance_mutex);
if (s_instance && s_instance_tag == callback_instance_tag) {
if (!error) {
CompleteFuture(handle, kConsentRequestSuccess);
}
} else {
MutexLock lock(s_instance_mutex);
if (s_instance && s_instance_tag == callback_instance_tag) {
} else {
CompleteFuture(handle,
CppFormErrorFromIosFormError(error.code),
error.localizedDescription.UTF8String);
Expand Down Expand Up @@ -357,24 +334,19 @@ static ConsentFormError CppFormErrorFromIosFormError(NSInteger code) {
callback_instance_tag = s_instance_tag;

util::DispatchAsyncSafeMainQueue(^{
{
MutexLock lock(s_instance_mutex);
if (!s_instance || s_instance_tag != callback_instance_tag) {
// Instance changed or was invalidated, don't call the iOS method any more.
return;
}
MutexLock lock(s_instance_mutex);
if (!s_instance || s_instance_tag != callback_instance_tag) {
// Instance changed or was invalidated, don't call the iOS method any more.
return;
}
[UMPConsentForm
presentPrivacyOptionsFormFromViewController:parent
completionHandler:^(NSError* _Nullable error) {
if (!error) {
MutexLock lock(s_instance_mutex);
if (s_instance && s_instance_tag == callback_instance_tag) {
MutexLock lock(s_instance_mutex);
if (s_instance && s_instance_tag == callback_instance_tag) {
if (!error) {
CompleteFuture(handle, kConsentRequestSuccess);
}
} else {
MutexLock lock(s_instance_mutex);
if (s_instance && s_instance_tag == callback_instance_tag) {
} else {
CompleteFuture(handle,
CppFormErrorFromIosFormError(error.code),
error.localizedDescription.UTF8String);
Expand Down