Skip to content

Handle jniresultcallback not loading due to lack of gms.Task. #953

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.

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 15 commits into from
May 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 11 additions & 10 deletions Android/firebase_dependencies.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -17,28 +17,24 @@ import org.gradle.util.ConfigureUtil;
// A map of library to the dependencies that need to be added for it.
def firebaseDependenciesMap = [
'app' : ['com.google.firebase:firebase-analytics:21.0.0'],
'play_services' : ['com.google.android.gms:play-services-base:18.0.1'],
'admob' : ['com.google.firebase:firebase-ads:19.8.0',
'com.google.firebase:firebase-analytics:21.0.0',
'com.google.android.gms:play-services-base:18.0.1'],
'analytics' : ['com.google.firebase:firebase-analytics:21.0.0',
'com.google.android.gms:play-services-base:18.0.1'],
'com.google.firebase:firebase-analytics:21.0.0'],
'analytics' : ['com.google.firebase:firebase-analytics:21.0.0'],
'auth' : ['com.google.firebase:firebase-auth:21.0.3'],
'database' : ['com.google.firebase:firebase-database:20.0.5'],
'dynamic_links' : ['com.google.firebase:firebase-dynamic-links:21.0.1'],
'firestore' : ['com.google.firebase:firebase-firestore:24.1.2'],
'functions' : ['com.google.firebase:firebase-functions:20.1.0'],
'installations' : ['com.google.firebase:firebase-installations:17.0.1',
'com.google.android.gms:play-services-base:18.0.1'],
'installations' : ['com.google.firebase:firebase-installations:17.0.1'],
'invites' : ['com.google.firebase:firebase-invites:17.0.0'],
// Messaging has an additional local dependency to include.
'messaging' : ['com.google.firebase:firebase-messaging:23.0.4',
'firebase_cpp_sdk.messaging:messaging_java',
'androidx.core:core:1.6.0-alpha03',
'com.google.flatbuffers:flatbuffers-java:1.12.0',
'com.google.android.gms:play-services-base:18.0.1'],
'com.google.flatbuffers:flatbuffers-java:1.12.0'],
'performance' : ['com.google.firebase:firebase-perf:20.0.6'],
'remote_config' : ['com.google.firebase:firebase-config:21.1.0',
'com.google.android.gms:play-services-base:18.0.1'],
'remote_config' : ['com.google.firebase:firebase-config:21.1.0'],
'storage' : ['com.google.firebase:firebase-storage:20.0.1'],
'testlab' : []
]
Expand Down Expand Up @@ -68,6 +64,10 @@ class Dependencies {

def getApp() {
libSet.add('app')
libSet.add('play_services')
}
def getAppWithoutPlayServices() {
libSet.add('app')
}
def getAdmob() {
libSet.add('admob')
Expand Down Expand Up @@ -146,6 +146,7 @@ project.afterEvaluate {
// App is required, so add it if it wasn't included.
if (!firebaseCpp.dependencies.libSet.contains('app')) {
firebaseCpp.dependencies.libSet.add('app')
firebaseCpp.dependencies.libSet.add('play_services')
}

for (String lib : firebaseCpp.dependencies.libSet) {
Expand Down
3 changes: 3 additions & 0 deletions admob/integration_test/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,10 @@ android {
}

apply from: "$gradle.firebase_cpp_sdk_dir/Android/firebase_dependencies.gradle"
// AdMob doesn't require play-services-base as a dependency, so include the
// version of App without it, to ensure the C++ SDK also doesn't require it.
firebaseCpp.dependencies {
appWithoutPlayServices
admob
}

Expand Down
12 changes: 9 additions & 3 deletions app/src/app_android.cc
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ METHOD_LOOKUP_DEFINITION(
namespace {

static int g_methods_cached_count = 0;
static bool g_initialized_google_play_services = false;

void ReleaseClasses(JNIEnv* env);

Expand All @@ -165,11 +166,13 @@ bool CacheMethods(JNIEnv* env, jobject activity) {
if (!(app::CacheMethodIds(env, activity) &&
options_builder::CacheMethodIds(env, activity) &&
options::CacheMethodIds(env, activity) &&
version_registrar::CacheMethodIds(env, activity) &&
google_play_services::Initialize(env, activity))) {
version_registrar::CacheMethodIds(env, activity))) {
ReleaseClasses(env);
return false;
}
if (google_play_services::Initialize(env, activity)) {
g_initialized_google_play_services = true;
}
}
return true;
}
Expand All @@ -182,7 +185,10 @@ void ReleaseClasses(JNIEnv* env) {
options_builder::ReleaseClass(env);
options::ReleaseClass(env);
version_registrar::ReleaseClass(env);
google_play_services::Terminate(env);
if (g_initialized_google_play_services) {
google_play_services::Terminate(env);
g_initialized_google_play_services = false;
}
util::Terminate(env);
}
}
Expand Down
69 changes: 44 additions & 25 deletions app/src/util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@
#include "app/src/include/firebase/internal/platform.h"

#if FIREBASE_PLATFORM_ANDROID
#include "app/src/google_play_services/availability_android.h"
#include "app/src/include/google_play_services/availability.h"
#include "app/src/util_android.h"
#endif // FIREBASE_PLATFORM_ANDROID
#include "app/src/include/firebase/internal/mutex.h"
#include "app/src/log.h"
Expand Down Expand Up @@ -77,32 +79,49 @@ static void PerformInitialize(ModuleInitializerData* data) {

#if FIREBASE_PLATFORM_ANDROID
if (init_result == kInitResultFailedMissingDependency) {
// On Android, we need to update or activate Google Play services
// before we can initialize this Firebase module.
LogWarning("Google Play services unavailable, trying to fix.");

Future<void> make_available = google_play_services::MakeAvailable(
data->app->GetJNIEnv(), data->app->activity());

make_available.OnCompletion(
[](const Future<void>& result, void* ptr) {
ModuleInitializerData* data =
reinterpret_cast<ModuleInitializerData*>(ptr);
if (result.status() == kFutureStatusComplete) {
if (result.error() == 0) {
LogInfo("Google Play services now available, continuing.");
PerformInitialize(data);
} else {
LogError("Google Play services still unavailable.");
int num_remaining = data->init_fns.size() - data->init_fn_idx;
data->future_impl.Complete(
data->future_handle_init, num_remaining,
"Unable to initialize due to missing Google Play services "
"dependency.");
// Note: If Initialize here succeeds, google_play_services::Terminate
// is called in the OnCompletion handler below. Note that these
// are reference-counted so it's safe to init/terminate an extra time..
if (google_play_services::Initialize(data->app->GetJNIEnv(),
data->app->activity())) {
// On Android, we need to update or activate Google Play services
// before we can initialize this Firebase module.
LogWarning("Google Play services unavailable, trying to fix.");

Future<void> make_available = google_play_services::MakeAvailable(
data->app->GetJNIEnv(), data->app->activity());

make_available.OnCompletion(
[](const Future<void>& result, void* ptr) {
ModuleInitializerData* data =
reinterpret_cast<ModuleInitializerData*>(ptr);
if (result.status() == kFutureStatusComplete) {
if (result.error() == 0) {
LogInfo("Google Play services now available, continuing.");
PerformInitialize(data);
google_play_services::Terminate(data->app->GetJNIEnv());
} else {
LogError("Google Play services still unavailable.");
int num_remaining = data->init_fns.size() - data->init_fn_idx;
data->future_impl.Complete(data->future_handle_init,
num_remaining,
"Unable to initialize due to "
"missing Google Play services "
"dependency.");
google_play_services::Terminate(util::GetJNIEnvFromApp());
}
}
}
},
data);
},
data);
} else {
int num_remaining = data->init_fns.size() - data->init_fn_idx;
data->future_impl.Complete(
data->future_handle_init, num_remaining,
"Could not run Google Play services update due to app "
"misconfiguration. Please add "
"com.google.android.gms:play-services-base as an Android "
"dependency to enable this functionality.");
}
}
#else // !FIREBASE_PLATFORM_ANDROID
// Outside of Android, we shouldn't get kInitResultFailedMissingDependency.
Expand Down
20 changes: 13 additions & 7 deletions app/src/util_android.cc
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,8 @@ static pthread_mutex_t g_task_callbacks_mutex;
// classes.
static std::vector<jobject>* g_class_loaders;

static bool g_jniresultcallback_loaded = false;

JNIEXPORT void JNICALL JniResultCallback_nativeOnResult(
JNIEnv* env, jobject clazz, jobject result, jboolean success,
jboolean cancelled, jstring status_message, jlong callback_fn_param,
Expand Down Expand Up @@ -393,7 +395,10 @@ static void ReleaseClasses(JNIEnv* env) {
uri::ReleaseClass(env);
object::ReleaseClass(env);
uribuilder::ReleaseClass(env);
jniresultcallback::ReleaseClass(env);
if (g_jniresultcallback_loaded) {
jniresultcallback::ReleaseClass(env);
g_jniresultcallback_loaded = false;
}
JavaThreadContext::Terminate(env);
#if defined(FIREBASE_ANDROID_FOR_DESKTOP)
java_uri::ReleaseClass(env);
Expand Down Expand Up @@ -526,12 +531,13 @@ bool Initialize(JNIEnv* env, jobject activity_object) {
return false;
}

if (!(jniresultcallback::CacheClassFromFiles(env, activity_object,
&embedded_files) &&
jniresultcallback::CacheMethodIds(env, activity_object) &&
jniresultcallback::RegisterNatives(env, &kJniCallbackMethod, 1))) {
return false;
}
// With a subset of dependencies, jniresultcallback can't be loaded due to
// lack of gms.Task. This is fine - just make sure not to try terminating it.
g_jniresultcallback_loaded =
(jniresultcallback::CacheClassFromFiles(env, activity_object,
&embedded_files) &&
jniresultcallback::CacheMethodIds(env, activity_object) &&
jniresultcallback::RegisterNatives(env, &kJniCallbackMethod, 1));

if (!JavaThreadContext::Initialize(env, activity_object, embedded_files)) {
return false;
Expand Down
21 changes: 11 additions & 10 deletions release_build_files/Android/firebase_dependencies.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -17,28 +17,24 @@ import org.gradle.util.ConfigureUtil;
// A map of library to the dependencies that need to be added for it.
def firebaseDependenciesMap = [
'app' : ['com.google.firebase:firebase-analytics:21.0.0'],
'play_services' : ['com.google.android.gms:play-services-base:18.0.1'],
'admob' : ['com.google.firebase:firebase-ads:19.8.0',
'com.google.firebase:firebase-analytics:21.0.0',
'com.google.android.gms:play-services-base:18.0.1'],
'analytics' : ['com.google.firebase:firebase-analytics:21.0.0',
'com.google.android.gms:play-services-base:18.0.1'],
'com.google.firebase:firebase-analytics:21.0.0'],
'analytics' : ['com.google.firebase:firebase-analytics:21.0.0'],
'auth' : ['com.google.firebase:firebase-auth:21.0.3'],
'database' : ['com.google.firebase:firebase-database:20.0.5'],
'dynamic_links' : ['com.google.firebase:firebase-dynamic-links:21.0.1'],
'firestore' : ['com.google.firebase:firebase-firestore:24.1.2'],
'functions' : ['com.google.firebase:firebase-functions:20.1.0'],
'installations' : ['com.google.firebase:firebase-installations:17.0.1',
'com.google.android.gms:play-services-base:18.0.1'],
'installations' : ['com.google.firebase:firebase-installations:17.0.1'],
'invites' : ['com.google.firebase:firebase-invites:17.0.0'],
// Messaging has an additional local dependency to include.
'messaging' : ['com.google.firebase:firebase-messaging:23.0.4',
'com.google.firebase.messaging.cpp:firebase_messaging_cpp@aar',
'androidx.core:core:1.6.0-alpha03',
'com.google.flatbuffers:flatbuffers-java:1.12.0',
'com.google.android.gms:play-services-base:18.0.1'],
'com.google.flatbuffers:flatbuffers-java:1.12.0'],
'performance' : ['com.google.firebase:firebase-perf:20.0.6'],
'remote_config' : ['com.google.firebase:firebase-config:21.1.0',
'com.google.android.gms:play-services-base:18.0.1'],
'remote_config' : ['com.google.firebase:firebase-config:21.1.0'],
'storage' : ['com.google.firebase:firebase-storage:20.0.1'],
'testlab' : []
]
Expand All @@ -49,6 +45,10 @@ class Dependencies {

def getApp() {
libSet.add('app')
libSet.add('play_services')
}
def getAppWithoutPlayServices() {
libSet.add('app')
}
def getAdmob() {
libSet.add('admob')
Expand Down Expand Up @@ -121,6 +121,7 @@ project.afterEvaluate {
// App is required, so add it if it wasn't included.
if (!firebaseCpp.dependencies.libSet.contains('app')) {
firebaseCpp.dependencies.libSet.add('app')
firebaseCpp.dependencies.libSet.add('play_services')
}

for (String lib : firebaseCpp.dependencies.libSet) {
Expand Down
32 changes: 32 additions & 0 deletions release_build_files/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,32 @@ Android SDK (20.x.x). Please ensure that you use firebase-ads version 19.8.0 in
conjunction with the latest firebase-analytics version to maintain
compatibility.

#### Gradle dependency file

Firebase C++ includes an `Android/firebase_dependencies.gradle` file
that helps you include the correct Android dependencies and native C++
libraries for each Firebase product. To use it, include the following
in your build.gradle file (you can omit any Firebase products you
aren't using):

```
apply from: "$gradle.firebase_cpp_sdk_dir/Android/firebase_dependencies.gradle"
firebaseCpp.dependencies {
app // Recommended for all apps using Firebase.
admob
analytics
auth
database
dynamicLinks
firestore
functions
installations
messaging
remoteConfig
storage
}
```

### iOS Dependencies

iOS users can include either xcframeworks or static libraries depending upon their
Expand Down Expand Up @@ -572,6 +598,12 @@ workflow use only during the development of your app, not for publicly shipping
code.

## Release Notes
### Upcoming release
- Changes
- General (Android): Fixed a bug that required Android apps to include
`com.google.android.gms:play-services-base` as an explicit dependency when
only using AdMob, Analytics, Remote Config, or Messaging.

### 9.0.0
- Changes
- General (iOS): Firebase C++ on iOS is now built using Xcode 13.3.1.
Expand Down
3 changes: 3 additions & 0 deletions remote_config/integration_test/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,10 @@ android {
}

apply from: "$gradle.firebase_cpp_sdk_dir/Android/firebase_dependencies.gradle"
// Remote Config doesn't require play-services-base as a dependency, so include
// the version of App without it, to ensure the C++ SDK also doesn't require it.
firebaseCpp.dependencies {
appWithoutPlayServices
remoteConfig
}

Expand Down