Skip to content

Fix user-agent register timing in C++ and Unity SDK #1217

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 10 commits into from
Mar 6, 2023
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
24 changes: 21 additions & 3 deletions app/src/app_android.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
* limitations under the License.
*/

#include "app/src/app_android.h"

#include <jni.h>
#include <string.h>

Expand Down Expand Up @@ -480,6 +482,9 @@ App* App::Create(const AppOptions& options, const char* name, JNIEnv* jni_env,
}
LogDebug("Creating Firebase App %s for %s", name, kFirebaseVersionString);
if (CacheMethods(jni_env, activity)) {
// Register C++ user-agents before creating Android app.
app_common::RegisterSdkUsage(jni_env);

// Try to get or create a new FirebaseApp object.
jobject platform_app =
CreateOrGetPlatformApp(jni_env, options, name, activity);
Expand Down Expand Up @@ -527,9 +532,14 @@ static void RegisterLibraryWithVersionRegistrar(JNIEnv* env,
env->DeleteLocalRef(registrar);
}

void App::RegisterLibrary(const char* library, const char* version) {
RegisterLibraryWithVersionRegistrar(util::GetJNIEnvFromApp(), library,
version);
void App::RegisterLibrary(const char* library, const char* version,
void* platform_resource) {
FIREBASE_ASSERT(platform_resource);

// Always relies on platform_resource to get reference to JNIEnv* to reduce
// complexity.
RegisterLibraryWithVersionRegistrar(
reinterpret_cast<JNIEnv*>(platform_resource), library, version);
app_common::RegisterLibrary(library, version);
}

Expand Down Expand Up @@ -570,4 +580,12 @@ JavaVM* App::java_vm() const { return internal_->java_vm(); }

jobject App::GetPlatformApp() const { return internal_->GetLocalRef(); }

void CallAfterEnsureMethodsCached(JNIEnv* env, jobject activity,
std::function<void()> callback) {
if (CacheMethods(env, activity)) {
callback();
ReleaseClasses(env);
}
}

} // namespace firebase
32 changes: 32 additions & 0 deletions app/src/app_android.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* Copyright 2023 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#ifndef FIREBASE_APP_SRC_APP_ANDROID_H_
#define FIREBASE_APP_SRC_APP_ANDROID_H_

#include <jni.h>

#include <functional>

namespace firebase {
// Make sure the Java classes and methods are cached before triggering the
// the callback. Can be slow if this is called BEFORE any Firebase App is
// created. This is currently used by Unity SDK.
void CallAfterEnsureMethodsCached(JNIEnv* env, jobject activity,
std::function<void()> callback);
} // namespace firebase

#endif // FIREBASE_APP_SRC_APP_ANDROID_H_
55 changes: 38 additions & 17 deletions app/src/app_common.cc
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ struct AppData {
// Tracks library registrations.
class LibraryRegistry {
private:
LibraryRegistry() {}
LibraryRegistry() : is_common_library_registered(false) {}

public:
// Register a library, returns true if the library version changed.
Expand Down Expand Up @@ -252,16 +252,30 @@ class LibraryRegistry {
}
}

static bool IsCommonLibraryRegistered() {
if (library_registry_) {
return library_registry_->is_common_library_registered;
}
return false;
}

static void SetCommonLibraryRegistered() {
if (library_registry_) {
library_registry_->is_common_library_registered = true;
}
}

private:
std::map<std::string, std::string> library_to_version_;
std::string user_agent_;
bool is_common_library_registered;

static LibraryRegistry* library_registry_;
};

// Guards g_apps and g_default_app.
static Mutex* g_app_mutex = new Mutex();
static std::map<std::string, UniquePtr<AppData>>* g_apps;
static std::map<std::string, UniquePtr<AppData>>* g_apps = nullptr;
static App* g_default_app = nullptr;
LibraryRegistry* LibraryRegistry::library_registry_ = nullptr;

Expand Down Expand Up @@ -294,21 +308,6 @@ App* AddApp(App* app, std::map<std::string, InitResult>* results) {
app_options.storage_bucket(), app_options.project_id(),
static_cast<int>(reinterpret_cast<intptr_t>(app)));
}
LibraryRegistry::Initialize();
if (created_first_app) {
// This calls the platform specific method to propagate the registration to
// any SDKs in use by this library.
App::RegisterLibrary(FIREBASE_CPP_USER_AGENT_PREFIX,
FIREBASE_VERSION_NUMBER_STRING);
App::RegisterLibrary(FIREBASE_CPP_USER_AGENT_PREFIX "-os",
kOperatingSystem);
App::RegisterLibrary(FIREBASE_CPP_USER_AGENT_PREFIX "-arch",
kCpuArchitecture);
App::RegisterLibrary(FIREBASE_CPP_USER_AGENT_PREFIX "-stl",
kCppRuntimeOrStl);
App::RegisterLibrary(FIREBASE_CPP_USER_AGENT_PREFIX "-buildsrc",
kBuildSource);
}
callback::Initialize();
AppCallback::NotifyAllAppCreated(app, results);
return app;
Expand Down Expand Up @@ -433,6 +432,28 @@ void RegisterLibrariesFromUserAgent(const char* user_agent) {
if (changed) registry->UpdateUserAgent();
}

void RegisterSdkUsage(void* platform_resource) {
MutexLock lock(*g_app_mutex);

// Only register libraries when no C++ apps was created before.
if (!LibraryRegistry::IsCommonLibraryRegistered()) {
LibraryRegistry::Initialize();
// This calls the platform specific method to propagate the registration to
// any SDKs in use by this library.
App::RegisterLibrary(FIREBASE_CPP_USER_AGENT_PREFIX,
FIREBASE_VERSION_NUMBER_STRING, platform_resource);
App::RegisterLibrary(FIREBASE_CPP_USER_AGENT_PREFIX "-os", kOperatingSystem,
platform_resource);
App::RegisterLibrary(FIREBASE_CPP_USER_AGENT_PREFIX "-arch",
kCpuArchitecture, platform_resource);
App::RegisterLibrary(FIREBASE_CPP_USER_AGENT_PREFIX "-stl",
kCppRuntimeOrStl, platform_resource);
App::RegisterLibrary(FIREBASE_CPP_USER_AGENT_PREFIX "-buildsrc",
kBuildSource, platform_resource);
LibraryRegistry::SetCommonLibraryRegistered();
}
}

const char* GetUserAgent() {
MutexLock lock(*g_app_mutex);
return LibraryRegistry::Initialize()->GetUserAgent();
Expand Down
4 changes: 4 additions & 0 deletions app/src/app_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@ void RegisterLibrary(const char* library, const char* version);
// This is useful when this SDK wraps other SDKs.
void RegisterLibrariesFromUserAgent(const char* user_agent);

// Register all user-agents related to C++ SDK usages.
// platform_resource currently is only used for passing JNIEnv on Android
void RegisterSdkUsage(void* platform_resource);

// Get the user agent string for all registered libraries.
// This is not thread safe w.r.t RegisterLibrary().
// NOTE: This is an internal method, use App::UserAgent() instead.
Expand Down
6 changes: 5 additions & 1 deletion app/src/app_desktop.cc
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,9 @@ App* App::Create(const AppOptions& options, const char* name) { // NOLINT

AppOptions options_with_defaults = options;
if (options_with_defaults.PopulateRequiredWithDefaults()) {
// Register C++/Unity user-agents
app_common::RegisterSdkUsage(nullptr);

app = new App();
app->name_ = name;
app->options_ = options_with_defaults;
Expand Down Expand Up @@ -168,7 +171,8 @@ internal::FunctionRegistry* App::function_registry() {
}
#endif // INTERNAL_EXPERIMENTAL

void App::RegisterLibrary(const char* library, const char* version) {
void App::RegisterLibrary(const char* library, const char* version,
void* /* platform_resource */) {
app_common::RegisterLibrary(library, version);
}

Expand Down
6 changes: 5 additions & 1 deletion app/src/app_ios.mm
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,10 @@ static void PlatformOptionsToAppOptions(FIROptions* platform_options, AppOptions
LogError("App %s already created, options will not be applied.", name);
return app;
}

// Register C++/Unity user-agents before creating iOS app.
app_common::RegisterSdkUsage(nullptr);

LogDebug("Creating Firebase App %s for %s", name, kFirebaseVersionString);
app = new App();
app->options_ = options;
Expand All @@ -277,7 +281,7 @@ static void PlatformOptionsToAppOptions(FIROptions* platform_options, AppOptions

App* App::GetInstance(const char* name) { return app_common::FindAppByName(name); }

void App::RegisterLibrary(const char* library, const char* version) {
void App::RegisterLibrary(const char* library, const char* version, void* /* platform_resource */) {
[FIRApp registerLibrary:@(library) withVersion:@(version)];
app_common::RegisterLibrariesFromUserAgent(
util::NSStringToString([FIRApp firebaseUserAgent]).c_str());
Expand Down
3 changes: 2 additions & 1 deletion app/src/app_stub.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ internal::FunctionRegistry* App::function_registry() {
}
#endif

void App::RegisterLibrary(const char* library, const char* version) {
void App::RegisterLibrary(const char* library, const char* version,
void* /* platform_resource */) {
app_common::RegisterLibrary(library, version);
}

Expand Down
5 changes: 4 additions & 1 deletion app/src/include/firebase/app.h
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,10 @@ class App {
/// @param library Name of the library to register as a user of the Firebase
/// C++ SDK.
/// @param version Version of the library being registered.
static void RegisterLibrary(const char* library, const char* version);
/// @param platform_resource Platform specific resource. Ex. for Android, this
/// is JNIEnv.
static void RegisterLibrary(const char* library, const char* version,
void* platform_resource);

// Internal method to retrieve the combined string of registered libraries.
static const char* GetUserAgent();
Expand Down
1 change: 0 additions & 1 deletion app/src/util_android.h
Original file line number Diff line number Diff line change
Expand Up @@ -1126,7 +1126,6 @@ jint AttachCurrentThread(JavaVM* java_vm, JNIEnv** env);
// firebase::App, either the default App (if it exists) or any valid
// App. If there is no instantiated App, returns nullptr.
JNIEnv* GetJNIEnvFromApp();

} // namespace util
// NOLINTNEXTLINE - allow namespace overridden
} // namespace firebase
Expand Down
7 changes: 6 additions & 1 deletion app/tests/app_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,12 @@ TEST_F(AppTest, TestRegisterLibrary) {
ContainsRegex("fire-cpp-arch/[^ ]+"));
EXPECT_THAT(std::string(App::GetUserAgent()),
ContainsRegex("fire-cpp-stl/[^ ]+"));
App::RegisterLibrary("fire-testing", "1.2.3");
#if FIREBASE_PLATFORM_ANDROID
App::RegisterLibrary("fire-testing", "1.2.3",
firebase_app_default->GetJNIEnv());
#else
App::RegisterLibrary("fire-testing", "1.2.3", nullptr);
#endif
EXPECT_THAT(std::string(App::GetUserAgent()),
HasSubstr("fire-testing/1.2.3"));
firebase_app_default.reset(nullptr);
Expand Down
6 changes: 5 additions & 1 deletion firestore/src/main/firestore_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,11 @@ FirestoreInternal::FirestoreInternal(
"com.google.firebase.firestore.transaction", /*threads=*/5))) {
ApplyDefaultSettings();

App::RegisterLibrary("fire-fst", kFirestoreVersionString);
#if FIREBASE_PLATFORM_ANDROID
App::RegisterLibrary("fire-fst", kFirestoreVersionString, app->GetJNIEnv());
#else
App::RegisterLibrary("fire-fst", kFirestoreVersionString, nullptr);
#endif
}

FirestoreInternal::~FirestoreInternal() {
Expand Down