From 809592f398e7c6377a04a0daa564362c3f6d040a Mon Sep 17 00:00:00 2001 From: "almostmatt@google.com" Date: Thu, 10 Feb 2022 13:34:39 -0500 Subject: [PATCH 1/7] When GetAuth is called, guarantee that the underlying native getter for Auth is also called so that any usage logging related to the native SDK is triggered. --- auth/src/android/auth_android.cc | 8 ++++++++ auth/src/auth.cc | 1 + auth/src/common.h | 3 +++ auth/src/desktop/auth_desktop.cc | 4 ++++ auth/src/ios/auth_ios.mm | 5 +++++ 5 files changed, 21 insertions(+) diff --git a/auth/src/android/auth_android.cc b/auth/src/android/auth_android.cc index 35eedf36e5..e435023325 100644 --- a/auth/src/android/auth_android.cc +++ b/auth/src/android/auth_android.cc @@ -193,6 +193,14 @@ static void ReleaseClasses(JNIEnv* env) { ReleaseCommonClasses(env); } +void LogHeartbeat(Auth* auth) { + // Calling the native getter is sufficient to cause a Heartbeat to be logged. + JNIEnv* env = Env(auth->auth_data); + jobject platform_app = auth->app()->GetPlatformApp(); + env->CallStaticObjectMethod( + auth::GetClass(), auth::GetMethodId(auth::kGetInstance), platform_app); +} + void* CreatePlatformAuth(App* app) { // Grab varous java objects from the app. JNIEnv* env = app->GetJNIEnv(); diff --git a/auth/src/auth.cc b/auth/src/auth.cc index 8e1195d0dc..826cb57f20 100644 --- a/auth/src/auth.cc +++ b/auth/src/auth.cc @@ -68,6 +68,7 @@ Auth* Auth::GetAuth(App* app, InitResult* init_result_out) { Auth* existing_auth = FindAuth(app); if (existing_auth) { if (init_result_out != nullptr) *init_result_out = kInitResultSuccess; + LogHeartbeat(existing_auth); return existing_auth; } diff --git a/auth/src/common.h b/auth/src/common.h index 43721735a2..7d42337594 100644 --- a/auth/src/common.h +++ b/auth/src/common.h @@ -33,6 +33,9 @@ enum CredentialApiFunction { // Platform-specific method to create the wrapped Auth class. void* CreatePlatformAuth(App* app); +// Platform-specific method that causes a heartbeat to be logged. +void LogHeartbeat(Auth* auth); + // Platform-specific method to initialize AuthData. void InitPlatformAuth(AuthData* auth_data); diff --git a/auth/src/desktop/auth_desktop.cc b/auth/src/desktop/auth_desktop.cc index a4674977de..e551f599b3 100644 --- a/auth/src/desktop/auth_desktop.cc +++ b/auth/src/desktop/auth_desktop.cc @@ -82,6 +82,10 @@ Future DoSignInWithCredential(Promise promise, } // namespace +void LogHeartbeat(Auth* const auth) { + // This is a stub until desktop implementation supports heartbeat logging. +} + void* CreatePlatformAuth(App* const app) { FIREBASE_ASSERT_RETURN(nullptr, app); diff --git a/auth/src/ios/auth_ios.mm b/auth/src/ios/auth_ios.mm index e53e5a6eda..37c995d3a4 100644 --- a/auth/src/ios/auth_ios.mm +++ b/auth/src/ios/auth_ios.mm @@ -135,6 +135,11 @@ explicit ListenerHandleHolder(T handle) : handle(handle) {} T handle; }; +void LogHeartbeat(Auth* auth) { + // Calling the native getter is sufficient to cause a Heartbeat to be logged. + [FIRAuth authWithApp:auth->app()->GetPlatformApp()]; +} + // Platform-specific method to create the wrapped Auth class. void *CreatePlatformAuth(App *app) { // Grab the auth for our app. From 17f17c3a298ee00f0c333e22e5ab951d66bb0f83 Mon Sep 17 00:00:00 2001 From: "almostmatt@google.com" Date: Thu, 10 Feb 2022 13:37:59 -0500 Subject: [PATCH 2/7] minor - move LogHearthbeat method after the Create/Init/DestroyPlatformAuth methods. --- auth/src/android/auth_android.cc | 16 ++++++++-------- auth/src/common.h | 6 +++--- auth/src/desktop/auth_desktop.cc | 7 +++---- auth/src/ios/auth_ios.mm | 10 +++++----- 4 files changed, 19 insertions(+), 20 deletions(-) diff --git a/auth/src/android/auth_android.cc b/auth/src/android/auth_android.cc index e435023325..b671b4ae1f 100644 --- a/auth/src/android/auth_android.cc +++ b/auth/src/android/auth_android.cc @@ -193,14 +193,6 @@ static void ReleaseClasses(JNIEnv* env) { ReleaseCommonClasses(env); } -void LogHeartbeat(Auth* auth) { - // Calling the native getter is sufficient to cause a Heartbeat to be logged. - JNIEnv* env = Env(auth->auth_data); - jobject platform_app = auth->app()->GetPlatformApp(); - env->CallStaticObjectMethod( - auth::GetClass(), auth::GetMethodId(auth::kGetInstance), platform_app); -} - void* CreatePlatformAuth(App* app) { // Grab varous java objects from the app. JNIEnv* env = app->GetJNIEnv(); @@ -317,6 +309,14 @@ void Auth::DestroyPlatformAuth(AuthData* auth_data) { } } +void LogHeartbeat(Auth* auth) { + // Calling the native getter is sufficient to cause a Heartbeat to be logged. + JNIEnv* env = Env(auth->auth_data); + jobject platform_app = auth->app()->GetPlatformApp(); + env->CallStaticObjectMethod( + auth::GetClass(), auth::GetMethodId(auth::kGetInstance), platform_app); +} + JNIEXPORT void JNICALL JniAuthStateListener_nativeOnAuthStateChanged( JNIEnv* env, jobject clazz, jlong callback_data) { AuthData* auth_data = reinterpret_cast(callback_data); diff --git a/auth/src/common.h b/auth/src/common.h index 7d42337594..e997cc1379 100644 --- a/auth/src/common.h +++ b/auth/src/common.h @@ -33,15 +33,15 @@ enum CredentialApiFunction { // Platform-specific method to create the wrapped Auth class. void* CreatePlatformAuth(App* app); -// Platform-specific method that causes a heartbeat to be logged. -void LogHeartbeat(Auth* auth); - // Platform-specific method to initialize AuthData. void InitPlatformAuth(AuthData* auth_data); // Platform-specific method to destroy the wrapped Auth class. void DestroyPlatformAuth(AuthData* auth_data); +// Platform-specific method that causes a heartbeat to be logged. +void LogHeartbeat(Auth* auth); + // All the result functions are similar. // Just return the local Future, cast to the proper result type. #define AUTH_RESULT_FN(class_name, fn_name, result_type) \ diff --git a/auth/src/desktop/auth_desktop.cc b/auth/src/desktop/auth_desktop.cc index e551f599b3..98771b2691 100644 --- a/auth/src/desktop/auth_desktop.cc +++ b/auth/src/desktop/auth_desktop.cc @@ -82,10 +82,6 @@ Future DoSignInWithCredential(Promise promise, } // namespace -void LogHeartbeat(Auth* const auth) { - // This is a stub until desktop implementation supports heartbeat logging. -} - void* CreatePlatformAuth(App* const app) { FIREBASE_ASSERT_RETURN(nullptr, app); @@ -98,6 +94,9 @@ void* CreatePlatformAuth(App* const app) { void InitializeFunctionRegistryListener(AuthData* auth_data); void DestroyFunctionRegistryListener(AuthData* auth_data); +// This is a stub until desktop implementation supports heartbeat logging. +void LogHeartbeat(Auth* const auth) {} + IdTokenRefreshListener::IdTokenRefreshListener() : token_timestamp_(0) {} IdTokenRefreshListener::~IdTokenRefreshListener() {} diff --git a/auth/src/ios/auth_ios.mm b/auth/src/ios/auth_ios.mm index 37c995d3a4..10fb248de8 100644 --- a/auth/src/ios/auth_ios.mm +++ b/auth/src/ios/auth_ios.mm @@ -135,11 +135,6 @@ explicit ListenerHandleHolder(T handle) : handle(handle) {} T handle; }; -void LogHeartbeat(Auth* auth) { - // Calling the native getter is sufficient to cause a Heartbeat to be logged. - [FIRAuth authWithApp:auth->app()->GetPlatformApp()]; -} - // Platform-specific method to create the wrapped Auth class. void *CreatePlatformAuth(App *app) { // Grab the auth for our app. @@ -228,6 +223,11 @@ void UpdateCurrentUser(AuthData *auth_data) { auth_data->auth_impl = nullptr; } +void LogHeartbeat(Auth* auth) { + // Calling the native getter is sufficient to cause a Heartbeat to be logged. + [FIRAuth authWithApp:auth->app()->GetPlatformApp()]; +} + Future Auth::FetchProvidersForEmail(const char *email) { // Create data structure to hold asynchronous results. FetchProvidersResult initial_data; From dc2661a2c71e9467a336644d1e82645829b9963d Mon Sep 17 00:00:00 2001 From: "almostmatt@google.com" Date: Thu, 10 Feb 2022 15:51:43 -0500 Subject: [PATCH 3/7] Call DeleteLocalRef to free variables allocated in android LogHeartbeat method --- auth/src/android/auth_android.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/auth/src/android/auth_android.cc b/auth/src/android/auth_android.cc index b671b4ae1f..799608ebb6 100644 --- a/auth/src/android/auth_android.cc +++ b/auth/src/android/auth_android.cc @@ -313,8 +313,11 @@ void LogHeartbeat(Auth* auth) { // Calling the native getter is sufficient to cause a Heartbeat to be logged. JNIEnv* env = Env(auth->auth_data); jobject platform_app = auth->app()->GetPlatformApp(); - env->CallStaticObjectMethod( + jobject j_auth_impl = env->CallStaticObjectMethod( auth::GetClass(), auth::GetMethodId(auth::kGetInstance), platform_app); + util::CheckAndClearJniExceptions(env); + env->DeleteLocalRef(platform_app); + env->DeleteLocalRef(j_auth_impl); } JNIEXPORT void JNICALL JniAuthStateListener_nativeOnAuthStateChanged( From 932a0ead441104b34cd9a77851ee5754206cea61 Mon Sep 17 00:00:00 2001 From: "almostmatt@google.com" Date: Thu, 10 Feb 2022 15:59:37 -0500 Subject: [PATCH 4/7] formatting - added space --- auth/src/ios/auth_ios.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/auth/src/ios/auth_ios.mm b/auth/src/ios/auth_ios.mm index 10fb248de8..02d0e0c18a 100644 --- a/auth/src/ios/auth_ios.mm +++ b/auth/src/ios/auth_ios.mm @@ -223,7 +223,7 @@ void UpdateCurrentUser(AuthData *auth_data) { auth_data->auth_impl = nullptr; } -void LogHeartbeat(Auth* auth) { +void LogHeartbeat(Auth *auth) { // Calling the native getter is sufficient to cause a Heartbeat to be logged. [FIRAuth authWithApp:auth->app()->GetPlatformApp()]; } From 6793a5dac55f2c5507726b3f2122f994ce5b8467 Mon Sep 17 00:00:00 2001 From: "almostmatt@google.com" Date: Mon, 14 Feb 2022 14:00:06 -0500 Subject: [PATCH 5/7] Minor changes based on review feedback --- auth/src/android/auth_android.cc | 2 +- auth/src/auth.cc | 2 ++ auth/src/common.h | 1 + auth/src/desktop/auth_desktop.cc | 3 ++- 4 files changed, 6 insertions(+), 2 deletions(-) diff --git a/auth/src/android/auth_android.cc b/auth/src/android/auth_android.cc index 799608ebb6..4b662ff76d 100644 --- a/auth/src/android/auth_android.cc +++ b/auth/src/android/auth_android.cc @@ -316,8 +316,8 @@ void LogHeartbeat(Auth* auth) { jobject j_auth_impl = env->CallStaticObjectMethod( auth::GetClass(), auth::GetMethodId(auth::kGetInstance), platform_app); util::CheckAndClearJniExceptions(env); - env->DeleteLocalRef(platform_app); env->DeleteLocalRef(j_auth_impl); + env->DeleteLocalRef(platform_app); } JNIEXPORT void JNICALL JniAuthStateListener_nativeOnAuthStateChanged( diff --git a/auth/src/auth.cc b/auth/src/auth.cc index 826cb57f20..aee9cd1c33 100644 --- a/auth/src/auth.cc +++ b/auth/src/auth.cc @@ -68,6 +68,8 @@ Auth* Auth::GetAuth(App* app, InitResult* init_result_out) { Auth* existing_auth = FindAuth(app); if (existing_auth) { if (init_result_out != nullptr) *init_result_out = kInitResultSuccess; + // Log heartbeat data when instance getters are called. + // See go/firebase-platform-logging-design for more information. LogHeartbeat(existing_auth); return existing_auth; } diff --git a/auth/src/common.h b/auth/src/common.h index e997cc1379..f92acb301f 100644 --- a/auth/src/common.h +++ b/auth/src/common.h @@ -40,6 +40,7 @@ void InitPlatformAuth(AuthData* auth_data); void DestroyPlatformAuth(AuthData* auth_data); // Platform-specific method that causes a heartbeat to be logged. +// See go/firebase-platform-logging-design for more information. void LogHeartbeat(Auth* auth); // All the result functions are similar. diff --git a/auth/src/desktop/auth_desktop.cc b/auth/src/desktop/auth_desktop.cc index 98771b2691..5126049bac 100644 --- a/auth/src/desktop/auth_desktop.cc +++ b/auth/src/desktop/auth_desktop.cc @@ -94,7 +94,8 @@ void* CreatePlatformAuth(App* const app) { void InitializeFunctionRegistryListener(AuthData* auth_data); void DestroyFunctionRegistryListener(AuthData* auth_data); -// This is a stub until desktop implementation supports heartbeat logging. +// TODO(b/211006737): This is a stub until desktop implementation supports +// heartbeat logging. void LogHeartbeat(Auth* const auth) {} IdTokenRefreshListener::IdTokenRefreshListener() : token_timestamp_(0) {} From cef65f1844fe721bd17d9694fc04f2921ce53470 Mon Sep 17 00:00:00 2001 From: "almostmatt@google.com" Date: Thu, 17 Feb 2022 12:39:47 -0500 Subject: [PATCH 6/7] Minor syntax fixes for android and ios LogHeartbeat --- auth/src/android/auth_android.cc | 4 ++-- auth/src/ios/auth_ios.mm | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/auth/src/android/auth_android.cc b/auth/src/android/auth_android.cc index 4b662ff76d..3f71e4a60e 100644 --- a/auth/src/android/auth_android.cc +++ b/auth/src/android/auth_android.cc @@ -311,8 +311,8 @@ void Auth::DestroyPlatformAuth(AuthData* auth_data) { void LogHeartbeat(Auth* auth) { // Calling the native getter is sufficient to cause a Heartbeat to be logged. - JNIEnv* env = Env(auth->auth_data); - jobject platform_app = auth->app()->GetPlatformApp(); + JNIEnv* env = Env(auth->auth_data_); + jobject platform_app = auth->app().GetPlatformApp(); jobject j_auth_impl = env->CallStaticObjectMethod( auth::GetClass(), auth::GetMethodId(auth::kGetInstance), platform_app); util::CheckAndClearJniExceptions(env); diff --git a/auth/src/ios/auth_ios.mm b/auth/src/ios/auth_ios.mm index 02d0e0c18a..01ab3d160f 100644 --- a/auth/src/ios/auth_ios.mm +++ b/auth/src/ios/auth_ios.mm @@ -225,7 +225,7 @@ void UpdateCurrentUser(AuthData *auth_data) { void LogHeartbeat(Auth *auth) { // Calling the native getter is sufficient to cause a Heartbeat to be logged. - [FIRAuth authWithApp:auth->app()->GetPlatformApp()]; + [FIRAuth authWithApp:auth->app().GetPlatformApp()]; } Future Auth::FetchProvidersForEmail(const char *email) { From 2b3ef8abf4475166b03cb02df8a4396721bedaf0 Mon Sep 17 00:00:00 2001 From: "almostmatt@google.com" Date: Thu, 17 Feb 2022 16:27:09 -0500 Subject: [PATCH 7/7] Fix related to visibility of auth_data_ in LogHeartbeat --- auth/src/include/firebase/auth.h | 1 + 1 file changed, 1 insertion(+) diff --git a/auth/src/include/firebase/auth.h b/auth/src/include/firebase/auth.h index 77d46da3cd..93c38f518f 100644 --- a/auth/src/include/firebase/auth.h +++ b/auth/src/include/firebase/auth.h @@ -532,6 +532,7 @@ class Auth { friend void EnableTokenAutoRefresh(AuthData* authData); friend void DisableTokenAutoRefresh(AuthData* authData); friend void ResetTokenRefreshCounter(AuthData* authData); + friend void LogHeartbeat(Auth* auth); /// @endcond // Find Auth instance using App. Return null if the instance does not exist.