Convert GeoPoint to the new JNI framework

PiperOrigin-RevId: 333116446

Author: wilhuff

firestore/src/android/field_value_android.cc

@@ -131,10 +131,9 @@ FieldValueInternal::FieldValueInternal(DocumentReference value)
 
 FieldValueInternal::FieldValueInternal(GeoPoint value)
     : cached_type_(Type::kGeoPoint) {
-  JNIEnv* env = firestore_->app()->GetJNIEnv();
-  jobject obj = GeoPointInternal::GeoPointToJavaGeoPoint(env, value);
-  obj_ = env->NewGlobalRef(obj);
-  env->DeleteLocalRef(obj);
+  Env env = GetEnv();
+  Local<GeoPointInternal> obj = GeoPointInternal::Create(env, value);
+  obj_ = env.get()->NewGlobalRef(obj.get());
 }
 
 FieldValueInternal::FieldValueInternal(std::vector<FieldValue> value)
@@ -209,7 +208,7 @@ Type FieldValueInternal::type() const {
     cached_type_ = Type::kReference;
     return Type::kReference;
   }
-  if (env->IsInstanceOf(obj_, GeoPointInternal::GetClass())) {
+  if (env->IsInstanceOf(obj_, GeoPointInternal::GetClass().get())) {
     cached_type_ = Type::kGeoPoint;
     return Type::kGeoPoint;
   }
@@ -371,17 +370,17 @@ DocumentReference FieldValueInternal::reference_value() const {
 }
 
 GeoPoint FieldValueInternal::geo_point_value() const {
-  JNIEnv* env = firestore_->app()->GetJNIEnv();
+  Env env = GetEnv();
 
   // Make sure this instance is of correct type.
   if (cached_type_ == Type::kNull) {
-    FIREBASE_ASSERT(env->IsInstanceOf(obj_, GeoPointInternal::GetClass()));
+    FIREBASE_ASSERT(env.IsInstanceOf(obj_, GeoPointInternal::GetClass()));
     cached_type_ = Type::kGeoPoint;
   } else {
     FIREBASE_ASSERT(cached_type_ == Type::kGeoPoint);
   }
 
-  return GeoPointInternal::JavaGeoPointToGeoPoint(env, obj_);
+  return GeoPointInternal(obj_).ToPublic(env);
 }
 
 std::vector<FieldValue> FieldValueInternal::array_value() const {

firestore/src/android/firestore_android.cc

@@ -166,7 +166,6 @@ bool FirestoreInternal::Initialize(App* app) {
           // Call Initialize on each Firestore internal class.
           FieldValueInternal::Initialize(app) &&
           FirebaseFirestoreExceptionInternal::Initialize(app) &&
-          GeoPointInternal::Initialize(app) &&
           ListenerRegistrationInternal::Initialize(app) &&
           QueryInternal::Initialize(app) &&
           TransactionInternal::Initialize(app) && Wrapper::Initialize(app) &&
@@ -196,6 +195,7 @@ bool FirestoreInternal::Initialize(App* app) {
     DocumentReferenceInternal::Initialize(loader);
     DocumentSnapshotInternal::Initialize(loader);
     FieldPathConverter::Initialize(loader);
+    GeoPointInternal::Initialize(loader);
     MetadataChangesInternal::Initialize(loader);
     QuerySnapshotInternal::Initialize(loader);
     ServerTimestampBehaviorInternal::Initialize(loader);
@@ -254,7 +254,6 @@ void FirestoreInternal::ReleaseClasses(App* app) {
   EventListenerInternal::Terminate(app);
   FieldValueInternal::Terminate(app);
   FirebaseFirestoreExceptionInternal::Terminate(app);
-  GeoPointInternal::Terminate(app);
   ListenerRegistrationInternal::Terminate(app);
   QueryInternal::Terminate(app);
   TransactionInternal::Terminate(app);

firestore/src/android/geo_point_android.cc

@@ -1,65 +1,44 @@
 #include "firestore/src/android/geo_point_android.h"
 
-#include <stdint.h>
-
-#include "app/src/util_android.h"
-#include "firestore/src/android/util_android.h"
+#include "firestore/src/jni/env.h"
+#include "firestore/src/jni/loader.h"
 
 namespace firebase {
 namespace firestore {
+namespace {
 
-// clang-format off
-#define GEO_POINT_METHODS(X)                                    \
-  X(Constructor, "<init>", "(DD)V", util::kMethodTypeInstance), \
-  X(GetLatitude, "getLatitude", "()D"),                         \
-  X(GetLongitude, "getLongitude", "()D")
-// clang-format on
+using jni::Class;
+using jni::Constructor;
+using jni::Env;
+using jni::Local;
+using jni::Method;
 
-METHOD_LOOKUP_DECLARATION(geo_point, GEO_POINT_METHODS)
-METHOD_LOOKUP_DEFINITION(geo_point,
-                         PROGUARD_KEEP_CLASS
-                         "com/google/firebase/firestore/GeoPoint",
-                         GEO_POINT_METHODS)
+constexpr char kClassName[] =
+    PROGUARD_KEEP_CLASS "com/google/firebase/firestore/GeoPoint";
+Constructor<GeoPointInternal> kConstructor("(DD)V");
+Method<double> kGetLatitude("getLatitude", "()D");
+Method<double> kGetLongitude("getLongitude", "()D");
 
-/* static */
-jobject GeoPointInternal::GeoPointToJavaGeoPoint(JNIEnv* env,
-                                                 const GeoPoint& point) {
-  jobject result = env->NewObject(
-      geo_point::GetClass(), geo_point::GetMethodId(geo_point::kConstructor),
-      static_cast<jdouble>(point.latitude()),
-      static_cast<jdouble>(point.longitude()));
-  CheckAndClearJniExceptions(env);
-  return result;
-}
+jclass g_clazz = nullptr;
 
-/* static */
-GeoPoint GeoPointInternal::JavaGeoPointToGeoPoint(JNIEnv* env, jobject obj) {
-  jdouble latitude = env->CallDoubleMethod(
-      obj, geo_point::GetMethodId(geo_point::kGetLatitude));
-  jdouble longitude = env->CallDoubleMethod(
-      obj, geo_point::GetMethodId(geo_point::kGetLongitude));
-  CheckAndClearJniExceptions(env);
-  return GeoPoint{static_cast<double>(latitude),
-                  static_cast<double>(longitude)};
+}  // namespace
+
+void GeoPointInternal::Initialize(jni::Loader& loader) {
+  g_clazz =
+      loader.LoadClass(kClassName, kConstructor, kGetLatitude, kGetLongitude);
 }
 
-/* static */
-jclass GeoPointInternal::GetClass() { return geo_point::GetClass(); }
+Class GeoPointInternal::GetClass() { return Class(g_clazz); }
 
-/* static */
-bool GeoPointInternal::Initialize(App* app) {
-  JNIEnv* env = app->GetJNIEnv();
-  jobject activity = app->activity();
-  bool result = geo_point::CacheMethodIds(env, activity);
-  util::CheckAndClearJniExceptions(env);
-  return result;
+Local<GeoPointInternal> GeoPointInternal::Create(Env& env,
+                                                 const GeoPoint& point) {
+  return env.New(kConstructor, point.latitude(), point.longitude());
 }
 
-/* static */
-void GeoPointInternal::Terminate(App* app) {
-  JNIEnv* env = app->GetJNIEnv();
-  geo_point::ReleaseClass(env);
-  util::CheckAndClearJniExceptions(env);
+GeoPoint GeoPointInternal::ToPublic(Env& env) const {
+  double latitude = env.Call(*this, kGetLatitude);
+  double longitude = env.Call(*this, kGetLongitude);
+  return GeoPoint(latitude, longitude);
 }
 
 }  // namespace firestore

firestore/src/android/geo_point_android.h

@@ -1,37 +1,30 @@
 #ifndef FIREBASE_FIRESTORE_CLIENT_CPP_SRC_ANDROID_GEO_POINT_ANDROID_H_
 #define FIREBASE_FIRESTORE_CLIENT_CPP_SRC_ANDROID_GEO_POINT_ANDROID_H_
 
-#include <jni.h>
-
-#include "app/src/include/firebase/app.h"
+#include "firestore/src/jni/jni_fwd.h"
+#include "firestore/src/jni/object.h"
 #include "firebase/firestore/geo_point.h"
 
 namespace firebase {
 namespace firestore {
 
-// This is the non-wrapper Android implementation of GeoPoint. Since GeoPoint
-// has most methods inlined, we use it directly instead of wrapping around a
-// Java GeoPoint object. We still need the helper functions to convert between
-// the two types. In addition, we also need proper initializer and terminator
-// for the Java class cache/uncache.
-class GeoPointInternal {
+/** A C++ proxy for a Java `GeoPoint`. */
+class GeoPointInternal : public jni::Object {
  public:
   using ApiType = GeoPoint;
 
-  // Convert a C++ GeoPoint into a Java GeoPoint.
-  static jobject GeoPointToJavaGeoPoint(JNIEnv* env, const GeoPoint& point);
+  using jni::Object::Object;
 
-  // Convert a Java GeoPoint into a C++ GeoPoint.
-  static GeoPoint JavaGeoPointToGeoPoint(JNIEnv* env, jobject obj);
+  static void Initialize(jni::Loader& loader);
 
-  // Gets the class object of Java GeoPoint class.
-  static jclass GetClass();
+  static jni::Class GetClass();
 
- private:
-  friend class FirestoreInternal;
+  /** Creates a C++ proxy for a Java `GeoPoint` object. */
+  static jni::Local<GeoPointInternal> Create(jni::Env& env,
+                                             const GeoPoint& point);
 
-  static bool Initialize(App* app);
-  static void Terminate(App* app);
+  /** Converts a Java GeoPoint to a public C++ GeoPoint. */
+  GeoPoint ToPublic(jni::Env& env) const;
 };
 
 }  // namespace firestore

firestore/src/tests/android/geo_point_android_test.cc

@@ -1,24 +1,26 @@
 #include "firestore/src/android/geo_point_android.h"
 
-#include <jni.h>
-
+#include "firestore/src/jni/env.h"
 #include "firestore/src/tests/firestore_integration_test.h"
-#include "testing/base/public/gmock.h"
 #include "gtest/gtest.h"
 #include "firebase/firestore/geo_point.h"
 
 namespace firebase {
 namespace firestore {
+namespace {
+
+using jni::Env;
 
-TEST_F(FirestoreIntegrationTest, Converter) {
-  JNIEnv* env = app()->GetJNIEnv();
+using GeoPointTest = FirestoreIntegrationTest;
 
-  const GeoPoint point{12.0, 34.0};
-  jobject java_point = GeoPointInternal::GeoPointToJavaGeoPoint(env, point);
-  EXPECT_EQ(point, GeoPointInternal::JavaGeoPointToGeoPoint(env, java_point));
+TEST_F(GeoPointTest, Converts) {
+  Env env;
 
-  env->DeleteLocalRef(java_point);
+  GeoPoint point{12.0, 34.0};
+  auto java_point = GeoPointInternal::Create(env, point);
+  EXPECT_EQ(point, java_point.ToPublic(env));
 }
 
+}  // namespace
 }  // namespace firestore
 }  // namespace firebase