From 24394c2156c1e68162b48741f68dfaf8f94f4708 Mon Sep 17 00:00:00 2001
From: cherylEnkidu <cheryllin@google.com>
Date: Wed, 16 Nov 2022 16:24:23 -0500
Subject: [PATCH 01/10] Implement ObjectArena and ArenaRef to solve JNI Global
 Reference Issue

---
 firestore/CMakeLists.txt                   |  2 +
 firestore/src/android/firestore_android.cc |  1 +
 firestore/src/jni/declaration.h            |  1 -
 firestore/src/jni/object_arena.cc          | 61 ++++++++++++++++
 firestore/src/jni/object_arena.h           | 48 ++++++++++++
 firestore/src/jni/ownership.h              | 85 +++++++++++++++++++++-
 6 files changed, 196 insertions(+), 2 deletions(-)
 create mode 100644 firestore/src/jni/object_arena.cc
 create mode 100644 firestore/src/jni/object_arena.h

diff --git a/firestore/CMakeLists.txt b/firestore/CMakeLists.txt
index 1d9c9c5f07..dd28a9035e 100644
--- a/firestore/CMakeLists.txt
+++ b/firestore/CMakeLists.txt
@@ -173,6 +173,8 @@ set(android_SRCS
     src/jni/map.h
     src/jni/object.cc
     src/jni/object.h
+    src/jni/object_arena.h
+    src/jni/object_arena.cc
     src/jni/ownership.h
     src/jni/set.h
     src/jni/string.cc
diff --git a/firestore/src/android/firestore_android.cc b/firestore/src/android/firestore_android.cc
index 62917accd8..024a9d0e32 100644
--- a/firestore/src/android/firestore_android.cc
+++ b/firestore/src/android/firestore_android.cc
@@ -302,6 +302,7 @@ bool FirestoreInternal::Initialize(App* app) {
     loader.CacheEmbeddedFiles();
 
     jni::Object::Initialize(loader);
+    jni::ObjectArena::Initialize(env, loader);
     jni::String::Initialize(env, loader);
     jni::ArrayList::Initialize(loader);
     jni::Boolean::Initialize(loader);
diff --git a/firestore/src/jni/declaration.h b/firestore/src/jni/declaration.h
index 23aa26d753..393a4d3f9a 100644
--- a/firestore/src/jni/declaration.h
+++ b/firestore/src/jni/declaration.h
@@ -24,7 +24,6 @@ namespace firestore {
 namespace jni {
 
 class Class;
-class Env;
 
 /**
  * A base class containing the details of a Java constructor. See
diff --git a/firestore/src/jni/object_arena.cc b/firestore/src/jni/object_arena.cc
new file mode 100644
index 0000000000..dea04ffa30
--- /dev/null
+++ b/firestore/src/jni/object_arena.cc
@@ -0,0 +1,61 @@
+/*
+* Copyright 2022 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.
+ */
+
+#include "firestore/src/jni/object_arena.h"
+
+#include "firestore/src/jni/env.h"
+#include "firestore/src/jni/loader.h"
+#include "firestore/src/jni/hash_map.h"
+#include "firestore/src/jni/long.h"
+
+namespace firebase {
+namespace firestore {
+namespace jni {
+namespace {
+
+Global<ObjectArena> kInstance;
+Global<HashMap> kHashMap;
+
+}  // namespace
+
+void ObjectArena::Initialize(Env& env, Loader& loader) {
+
+}
+
+ObjectArena& ObjectArena::GetInstance() { return kInstance; }
+
+Local<Object> ObjectArena::Get(Env& env, int64_t key) {
+    return kHashMap.Get(env, Long::Create(env, key));
+}
+
+int64_t ObjectArena::Put(Env& env, const Object& value) {
+  kHashMap.Put(env, Long::Create(env, next_key_), value);
+  return next_key_++;
+}
+
+void ObjectArena::Remove(Env& env, int64_t key) {
+  kHashMap.Remove(env, Long::Create(env, key));
+}
+
+int64_t ObjectArena::Dup(Env& env, int64_t key) {
+  kHashMap.Put(env, Long::Create(env, next_key_),
+               kHashMap.Get(env, Long::Create(env, key)));
+  return next_key_++;
+}
+
+}  // namespace jni
+}  // namespace firestore
+}  // namespace firebase
diff --git a/firestore/src/jni/object_arena.h b/firestore/src/jni/object_arena.h
new file mode 100644
index 0000000000..faa43bf983
--- /dev/null
+++ b/firestore/src/jni/object_arena.h
@@ -0,0 +1,48 @@
+/*
+* Copyright 2022 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_FIRESTORE_SRC_JNI_OBJECT_ARENA_H_
+#define FIREBASE_FIRESTORE_SRC_JNI_OBJECT_ARENA_H_
+
+#include "firestore/src/jni/object.h"
+
+namespace firebase {
+namespace firestore {
+namespace jni {
+
+/** `ObjectArena` serves as a local HashMap. */
+class ObjectArena : public Object {
+public:
+ using Object::Object;
+
+ static void Initialize(Env& env, Loader& loader);
+
+ static ObjectArena& GetInstance();
+
+ Local<Object> Get(Env& env, int64_t key);
+ int64_t Put(Env& env, const Object& value);
+ void Remove(Env& env, int64_t key);
+ int64_t Dup(Env& env, int64_t key);
+
+private:
+ int64_t next_key_ = 0;
+};
+
+}  // namespace jni
+}  // namespace firestore
+}  // namespace firebase
+
+#endif  // FIREBASE_FIRESTORE_SRC_JNI_OBJECT_ARENA_H_
diff --git a/firestore/src/jni/ownership.h b/firestore/src/jni/ownership.h
index 670fc72ae3..28f68660a2 100644
--- a/firestore/src/jni/ownership.h
+++ b/firestore/src/jni/ownership.h
@@ -21,7 +21,9 @@
 
 #include <string>
 
-#include "firestore/src/jni/jni_fwd.h"
+#include "absl/types/optional.h"
+#include "firestore/src/jni/env.h"
+#include "firestore/src/jni/object_arena.h"
 #include "firestore/src/jni/traits.h"
 
 namespace firebase {
@@ -283,6 +285,87 @@ class Global : public T {
   }
 };
 
+/**
+ * An RAII wrapper that uses ObjectArena to manage the reference. It automatically
+ * deletes the JNI local reference when it goes out of scope. Copies and moves
+ * are handled by creating additional references as required.
+ */
+class ArenaRef {
+ public:
+  ArenaRef() = default;
+
+  ArenaRef(const Object& ob) {
+    Env env(GetEnv());
+    id_ = ObjectArena::GetInstance().Put(env, ob);
+  }
+
+  ArenaRef(const ArenaRef& other) {
+    if (other.id_) {
+      Env env(GetEnv());
+      id_ = ObjectArena::GetInstance().Dup(env, other.id_.value());
+    }
+  }
+
+  ArenaRef& operator=(const ArenaRef& other) {
+    if (id_ != other.id_) {
+      Env env(GetEnv());
+      if (id_) {
+        ObjectArena::GetInstance().Remove(env, id_.value());
+      }
+      id_ = absl::nullopt;
+      if (other.id_) {
+        id_ = ObjectArena::GetInstance().Dup(other.id_.value());
+      }
+    }
+    return *this;
+  }
+
+  ArenaRef(ArenaRef&& other) noexcept : ArenaRef(other.release()) {}
+
+  ArenaRef& operator=(ArenaRef&& other) noexcept {
+    if (id_ != other.id_.value()) {
+      Env env(GetEnv());
+      if (id_) {
+        ObjectArena::GetInstance().Remove(env, id_.value());
+      }
+      id_ = other.release();
+    }
+    return *this;
+  }
+
+  ~ArenaRef() {
+    if (id_) {
+      Env env(GetEnv());
+      ObjectArena::GetInstance().Remove(env, id_.value());
+    }
+  }
+
+  Local<Object> get() const {
+    Env env(GetEnv());
+    return ObjectArena::GetInstance().Get(env, id_.value());
+  }
+
+  void clear() {
+    if (id_) {
+      Env env(GetEnv());
+      ObjectArena::GetInstance().Remove(env, id_.value());
+      id_ = absl::nullopt;
+    }
+  }
+
+ private:
+  ArenaRef(int64_t id): id_(id) {}
+
+  int64_t release() {
+    FIREBASE_DEV_ASSERT(id_);
+    int64_t id = id_.value();
+    id_ = absl::nullopt;
+    return id;
+  }
+
+  absl::optional<int64_t> id_;
+};
+
 }  // namespace jni
 }  // namespace firestore
 }  // namespace firebase

From 81d9dd6ef280fdde8160b76ad379c391b0b9ac40 Mon Sep 17 00:00:00 2001
From: cherylEnkidu <cheryllin@google.com>
Date: Wed, 16 Nov 2022 16:24:23 -0500
Subject: [PATCH 02/10] Implement ObjectArena and ArenaRef to solve JNI Global
 Reference Issue

---
 firestore/CMakeLists.txt          |  1 +
 firestore/src/jni/object_arena.cc | 34 +++++++++++-----------
 firestore/src/jni/object_arena.h  | 48 +++++++++++++++----------------
 firestore/src/jni/ownership.h     | 10 +++----
 4 files changed, 46 insertions(+), 47 deletions(-)

diff --git a/firestore/CMakeLists.txt b/firestore/CMakeLists.txt
index dd28a9035e..32b7887aaf 100644
--- a/firestore/CMakeLists.txt
+++ b/firestore/CMakeLists.txt
@@ -285,6 +285,7 @@ target_link_libraries(firebase_firestore
   PUBLIC
     firebase_app
     firebase_auth
+    absl_variant
 )
 
 if(FIRESTORE_USE_EXTERNAL_CMAKE_BUILD)
diff --git a/firestore/src/jni/object_arena.cc b/firestore/src/jni/object_arena.cc
index dea04ffa30..c4d6d863fc 100644
--- a/firestore/src/jni/object_arena.cc
+++ b/firestore/src/jni/object_arena.cc
@@ -1,24 +1,24 @@
 /*
-* Copyright 2022 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.
+ * Copyright 2022 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.
  */
 
 #include "firestore/src/jni/object_arena.h"
 
 #include "firestore/src/jni/env.h"
-#include "firestore/src/jni/loader.h"
 #include "firestore/src/jni/hash_map.h"
+#include "firestore/src/jni/loader.h"
 #include "firestore/src/jni/long.h"
 
 namespace firebase {
@@ -31,14 +31,12 @@ Global<HashMap> kHashMap;
 
 }  // namespace
 
-void ObjectArena::Initialize(Env& env, Loader& loader) {
-
-}
+void ObjectArena::Initialize(Env& env, Loader& loader) {}
 
 ObjectArena& ObjectArena::GetInstance() { return kInstance; }
 
 Local<Object> ObjectArena::Get(Env& env, int64_t key) {
-    return kHashMap.Get(env, Long::Create(env, key));
+  return kHashMap.Get(env, Long::Create(env, key));
 }
 
 int64_t ObjectArena::Put(Env& env, const Object& value) {
diff --git a/firestore/src/jni/object_arena.h b/firestore/src/jni/object_arena.h
index faa43bf983..3224901767 100644
--- a/firestore/src/jni/object_arena.h
+++ b/firestore/src/jni/object_arena.h
@@ -1,18 +1,18 @@
 /*
-* Copyright 2022 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.
-*/
+ * Copyright 2022 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_FIRESTORE_SRC_JNI_OBJECT_ARENA_H_
 #define FIREBASE_FIRESTORE_SRC_JNI_OBJECT_ARENA_H_
@@ -25,20 +25,20 @@ namespace jni {
 
 /** `ObjectArena` serves as a local HashMap. */
 class ObjectArena : public Object {
-public:
- using Object::Object;
+ public:
+  using Object::Object;
 
- static void Initialize(Env& env, Loader& loader);
+  static void Initialize(Env& env, Loader& loader);
 
- static ObjectArena& GetInstance();
+  static ObjectArena& GetInstance();
 
- Local<Object> Get(Env& env, int64_t key);
- int64_t Put(Env& env, const Object& value);
- void Remove(Env& env, int64_t key);
- int64_t Dup(Env& env, int64_t key);
+  Local<Object> Get(Env& env, int64_t key);
+  int64_t Put(Env& env, const Object& value);
+  void Remove(Env& env, int64_t key);
+  int64_t Dup(Env& env, int64_t key);
 
-private:
- int64_t next_key_ = 0;
+ private:
+  int64_t next_key_ = 0;
 };
 
 }  // namespace jni
diff --git a/firestore/src/jni/ownership.h b/firestore/src/jni/ownership.h
index 28f68660a2..994ae27820 100644
--- a/firestore/src/jni/ownership.h
+++ b/firestore/src/jni/ownership.h
@@ -286,15 +286,15 @@ class Global : public T {
 };
 
 /**
- * An RAII wrapper that uses ObjectArena to manage the reference. It automatically
- * deletes the JNI local reference when it goes out of scope. Copies and moves
- * are handled by creating additional references as required.
+ * An RAII wrapper that uses ObjectArena to manage the reference. It
+ * automatically deletes the JNI local reference when it goes out of scope.
+ * Copies and moves are handled by creating additional references as required.
  */
 class ArenaRef {
  public:
   ArenaRef() = default;
 
-  ArenaRef(const Object& ob) {
+  explicit ArenaRef(const Object& ob) {
     Env env(GetEnv());
     id_ = ObjectArena::GetInstance().Put(env, ob);
   }
@@ -354,7 +354,7 @@ class ArenaRef {
   }
 
  private:
-  ArenaRef(int64_t id): id_(id) {}
+  explicit ArenaRef(int64_t id) : id_(id) {}
 
   int64_t release() {
     FIREBASE_DEV_ASSERT(id_);

From de5a90d7a53a8fa5fd2064bde7d529d1b882f6d7 Mon Sep 17 00:00:00 2001
From: cherylEnkidu <cheryllin@google.com>
Date: Tue, 22 Nov 2022 11:51:29 -0500
Subject: [PATCH 03/10] Replace absl::optional implementation with
 std::pair<bool, int64_t>

---
 firestore/CMakeLists.txt      |  1 -
 firestore/src/jni/ownership.h | 55 ++++++++++++++++++++---------------
 2 files changed, 31 insertions(+), 25 deletions(-)

diff --git a/firestore/CMakeLists.txt b/firestore/CMakeLists.txt
index 32b7887aaf..dd28a9035e 100644
--- a/firestore/CMakeLists.txt
+++ b/firestore/CMakeLists.txt
@@ -285,7 +285,6 @@ target_link_libraries(firebase_firestore
   PUBLIC
     firebase_app
     firebase_auth
-    absl_variant
 )
 
 if(FIRESTORE_USE_EXTERNAL_CMAKE_BUILD)
diff --git a/firestore/src/jni/ownership.h b/firestore/src/jni/ownership.h
index 994ae27820..886fa1ab8c 100644
--- a/firestore/src/jni/ownership.h
+++ b/firestore/src/jni/ownership.h
@@ -20,8 +20,8 @@
 #include <jni.h>
 
 #include <string>
+#include <utility>
 
-#include "absl/types/optional.h"
 #include "firestore/src/jni/env.h"
 #include "firestore/src/jni/object_arena.h"
 #include "firestore/src/jni/traits.h"
@@ -296,25 +296,26 @@ class ArenaRef {
 
   explicit ArenaRef(const Object& ob) {
     Env env(GetEnv());
-    id_ = ObjectArena::GetInstance().Put(env, ob);
+    id_ = assignState(ObjectArena::GetInstance().Put(env, ob));
   }
 
   ArenaRef(const ArenaRef& other) {
-    if (other.id_) {
+    if (other.id_.first) {
       Env env(GetEnv());
-      id_ = ObjectArena::GetInstance().Dup(env, other.id_.value());
+      id_ = assignState(ObjectArena::GetInstance().Dup(env, other.id_.second));
     }
   }
 
   ArenaRef& operator=(const ArenaRef& other) {
     if (id_ != other.id_) {
       Env env(GetEnv());
-      if (id_) {
-        ObjectArena::GetInstance().Remove(env, id_.value());
+      if (id_.first) {
+        ObjectArena::GetInstance().Remove(env, id_.second);
       }
-      id_ = absl::nullopt;
-      if (other.id_) {
-        id_ = ObjectArena::GetInstance().Dup(other.id_.value());
+      reset();
+      if (other.id_.first) {
+        id_ =
+            assignState(ObjectArena::GetInstance().Dup(env, other.id_.second));
       }
     }
     return *this;
@@ -323,47 +324,53 @@ class ArenaRef {
   ArenaRef(ArenaRef&& other) noexcept : ArenaRef(other.release()) {}
 
   ArenaRef& operator=(ArenaRef&& other) noexcept {
-    if (id_ != other.id_.value()) {
+    if (id_.second != other.id_.second) {
       Env env(GetEnv());
-      if (id_) {
-        ObjectArena::GetInstance().Remove(env, id_.value());
+      if (id_.first) {
+        ObjectArena::GetInstance().Remove(env, id_.second);
       }
-      id_ = other.release();
+      id_ = assignState(other.release());
     }
     return *this;
   }
 
   ~ArenaRef() {
-    if (id_) {
+    if (id_.first) {
       Env env(GetEnv());
-      ObjectArena::GetInstance().Remove(env, id_.value());
+      ObjectArena::GetInstance().Remove(env, id_.second);
     }
   }
 
   Local<Object> get() const {
     Env env(GetEnv());
-    return ObjectArena::GetInstance().Get(env, id_.value());
+    return ObjectArena::GetInstance().Get(env, id_.second);
   }
 
   void clear() {
-    if (id_) {
+    if (id_.first) {
       Env env(GetEnv());
-      ObjectArena::GetInstance().Remove(env, id_.value());
-      id_ = absl::nullopt;
+      ObjectArena::GetInstance().Remove(env, id_.second);
+      reset();
     }
   }
 
  private:
-  explicit ArenaRef(int64_t id) : id_(id) {}
+  explicit ArenaRef(int64_t id) { assignState(id); }
 
   int64_t release() {
-    FIREBASE_DEV_ASSERT(id_);
-    int64_t id = id_.value();
-    id_ = absl::nullopt;
+    FIREBASE_DEV_ASSERT(id_.first);
+    int64_t id = id_.second;
+    reset();
     return id;
   }
 
-  absl::optional<int64_t> id_;
+  std::pair<bool, int64_t> assignState(int64_t id) { id_ = {true, id}; }
+
+  void reset() { id_ = {false, -1}; }
+
+  // A pair of variables which indicate the state of ArenaRef. If id_.first set
+  // to
+  std::pair<bool, int64_t> id_{false, -1};
 };
 
 }  // namespace jni

From 4391eb91c8b25385d0c45b1f7e69dafbdf602b87 Mon Sep 17 00:00:00 2001
From: cherylEnkidu <cheryllin@google.com>
Date: Tue, 22 Nov 2022 13:37:34 -0500
Subject: [PATCH 04/10] Fix Android build issue.

---
 firestore/CMakeLists.txt                   |   3 +-
 firestore/src/android/firestore_android.cc |   1 +
 firestore/src/jni/arena_ref.h              | 123 +++++++++++++++++++++
 firestore/src/jni/ownership.h              |  91 ---------------
 4 files changed, 126 insertions(+), 92 deletions(-)
 create mode 100644 firestore/src/jni/arena_ref.h

diff --git a/firestore/CMakeLists.txt b/firestore/CMakeLists.txt
index dd28a9035e..d4a8d7ce5b 100644
--- a/firestore/CMakeLists.txt
+++ b/firestore/CMakeLists.txt
@@ -138,6 +138,7 @@ set(android_SRCS
     src/android/wrapper.h
     src/android/write_batch_android.cc
     src/android/write_batch_android.h
+    src/jni/arena_ref.h
     src/jni/array.h
     src/jni/array_list.cc
     src/jni/array_list.h
@@ -173,8 +174,8 @@ set(android_SRCS
     src/jni/map.h
     src/jni/object.cc
     src/jni/object.h
-    src/jni/object_arena.h
     src/jni/object_arena.cc
+    src/jni/object_arena.h
     src/jni/ownership.h
     src/jni/set.h
     src/jni/string.cc
diff --git a/firestore/src/android/firestore_android.cc b/firestore/src/android/firestore_android.cc
index 024a9d0e32..29d1bda6be 100644
--- a/firestore/src/android/firestore_android.cc
+++ b/firestore/src/android/firestore_android.cc
@@ -73,6 +73,7 @@
 #include "firestore/src/jni/loader.h"
 #include "firestore/src/jni/long.h"
 #include "firestore/src/jni/map.h"
+#include "firestore/src/jni/object_arena.h"
 #include "firestore/src/jni/set.h"
 #include "firestore/src/jni/task.h"
 
diff --git a/firestore/src/jni/arena_ref.h b/firestore/src/jni/arena_ref.h
new file mode 100644
index 0000000000..7723a49dc4
--- /dev/null
+++ b/firestore/src/jni/arena_ref.h
@@ -0,0 +1,123 @@
+/*
+ * Copyright 2022 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_FIRESTORE_SRC_JNI_ARENA_REF_H_
+#define FIREBASE_FIRESTORE_SRC_JNI_ARENA_REF_H_
+
+#include <utility>
+
+#include "app/src/assert.h"
+#include "firestore/src/jni/env.h"
+#include "firestore/src/jni/object.h"
+#include "firestore/src/jni/object_arena.h"
+
+namespace firebase {
+namespace firestore {
+namespace jni {
+
+/**
+ * An RAII wrapper that uses ObjectArena to manage the reference. It
+ * automatically deletes the JNI local reference when it goes out of scope.
+ * Copies and moves are handled by creating additional references as required.
+ */
+class ArenaRef {
+ public:
+  ArenaRef() = default;
+
+  explicit ArenaRef(const Object& ob) {
+    Env env(GetEnv());
+    id_ = assignState(ObjectArena::GetInstance().Put(env, ob));
+  }
+
+  ArenaRef(const ArenaRef& other) {
+    if (other.id_.first) {
+      Env env(GetEnv());
+      id_ = assignState(ObjectArena::GetInstance().Dup(env, other.id_.second));
+    }
+  }
+
+  ArenaRef& operator=(const ArenaRef& other) {
+    if (id_ != other.id_) {
+      Env env(GetEnv());
+      if (id_.first) {
+        ObjectArena::GetInstance().Remove(env, id_.second);
+      }
+      reset();
+      if (other.id_.first) {
+        id_ =
+            assignState(ObjectArena::GetInstance().Dup(env, other.id_.second));
+      }
+    }
+    return *this;
+  }
+
+  ArenaRef(ArenaRef&& other) noexcept : ArenaRef(other.release()) {}
+
+  ArenaRef& operator=(ArenaRef&& other) noexcept {
+    if (id_.second != other.id_.second) {
+      Env env(GetEnv());
+      if (id_.first) {
+        ObjectArena::GetInstance().Remove(env, id_.second);
+      }
+      id_ = assignState(other.release());
+    }
+    return *this;
+  }
+
+  ~ArenaRef() {
+    if (id_.first) {
+      Env env(GetEnv());
+      ObjectArena::GetInstance().Remove(env, id_.second);
+    }
+  }
+
+  Local<Object> get() const {
+    Env env(GetEnv());
+    return ObjectArena::GetInstance().Get(env, id_.second);
+  }
+
+  void clear() {
+    if (id_.first) {
+      Env env(GetEnv());
+      ObjectArena::GetInstance().Remove(env, id_.second);
+      reset();
+    }
+  }
+
+ private:
+  explicit ArenaRef(int64_t id) { assignState(id); }
+
+  int64_t release() {
+    FIREBASE_DEV_ASSERT(id_.first);
+    int64_t id = id_.second;
+    reset();
+    return id;
+  }
+
+  std::pair<bool, int64_t> assignState(int64_t id) { id_ = {true, id}; }
+
+  void reset() { id_ = {false, -1}; }
+
+  // A pair of variables which indicate the state of ArenaRef. If id_.first set
+  // to
+  std::pair<bool, int64_t> id_{false, -1};
+};
+
+}  // namespace jni
+}  // namespace firestore
+}  // namespace firebase
+
+#endif  // FIREBASE_FIRESTORE_SRC_JNI_ARENA_REF_H_
diff --git a/firestore/src/jni/ownership.h b/firestore/src/jni/ownership.h
index 886fa1ab8c..aed6ab4174 100644
--- a/firestore/src/jni/ownership.h
+++ b/firestore/src/jni/ownership.h
@@ -20,10 +20,7 @@
 #include <jni.h>
 
 #include <string>
-#include <utility>
 
-#include "firestore/src/jni/env.h"
-#include "firestore/src/jni/object_arena.h"
 #include "firestore/src/jni/traits.h"
 
 namespace firebase {
@@ -285,94 +282,6 @@ class Global : public T {
   }
 };
 
-/**
- * An RAII wrapper that uses ObjectArena to manage the reference. It
- * automatically deletes the JNI local reference when it goes out of scope.
- * Copies and moves are handled by creating additional references as required.
- */
-class ArenaRef {
- public:
-  ArenaRef() = default;
-
-  explicit ArenaRef(const Object& ob) {
-    Env env(GetEnv());
-    id_ = assignState(ObjectArena::GetInstance().Put(env, ob));
-  }
-
-  ArenaRef(const ArenaRef& other) {
-    if (other.id_.first) {
-      Env env(GetEnv());
-      id_ = assignState(ObjectArena::GetInstance().Dup(env, other.id_.second));
-    }
-  }
-
-  ArenaRef& operator=(const ArenaRef& other) {
-    if (id_ != other.id_) {
-      Env env(GetEnv());
-      if (id_.first) {
-        ObjectArena::GetInstance().Remove(env, id_.second);
-      }
-      reset();
-      if (other.id_.first) {
-        id_ =
-            assignState(ObjectArena::GetInstance().Dup(env, other.id_.second));
-      }
-    }
-    return *this;
-  }
-
-  ArenaRef(ArenaRef&& other) noexcept : ArenaRef(other.release()) {}
-
-  ArenaRef& operator=(ArenaRef&& other) noexcept {
-    if (id_.second != other.id_.second) {
-      Env env(GetEnv());
-      if (id_.first) {
-        ObjectArena::GetInstance().Remove(env, id_.second);
-      }
-      id_ = assignState(other.release());
-    }
-    return *this;
-  }
-
-  ~ArenaRef() {
-    if (id_.first) {
-      Env env(GetEnv());
-      ObjectArena::GetInstance().Remove(env, id_.second);
-    }
-  }
-
-  Local<Object> get() const {
-    Env env(GetEnv());
-    return ObjectArena::GetInstance().Get(env, id_.second);
-  }
-
-  void clear() {
-    if (id_.first) {
-      Env env(GetEnv());
-      ObjectArena::GetInstance().Remove(env, id_.second);
-      reset();
-    }
-  }
-
- private:
-  explicit ArenaRef(int64_t id) { assignState(id); }
-
-  int64_t release() {
-    FIREBASE_DEV_ASSERT(id_.first);
-    int64_t id = id_.second;
-    reset();
-    return id;
-  }
-
-  std::pair<bool, int64_t> assignState(int64_t id) { id_ = {true, id}; }
-
-  void reset() { id_ = {false, -1}; }
-
-  // A pair of variables which indicate the state of ArenaRef. If id_.first set
-  // to
-  std::pair<bool, int64_t> id_{false, -1};
-};
-
 }  // namespace jni
 }  // namespace firestore
 }  // namespace firebase

From edbf8cd9a6817a7035311e419a514999526c5838 Mon Sep 17 00:00:00 2001
From: cherylEnkidu <cheryllin@google.com>
Date: Fri, 2 Dec 2022 14:18:37 -0500
Subject: [PATCH 05/10] Replace the usage of Global Reference in
 Android::FieldValue

---
 firestore/src/android/field_value_android.cc | 57 +++++++++++---------
 firestore/src/android/field_value_android.h  |  5 +-
 firestore/src/jni/arena_ref.h                | 14 ++---
 3 files changed, 43 insertions(+), 33 deletions(-)

diff --git a/firestore/src/android/field_value_android.cc b/firestore/src/android/field_value_android.cc
index 071188a726..d5bb404e12 100644
--- a/firestore/src/android/field_value_android.cc
+++ b/firestore/src/android/field_value_android.cc
@@ -106,31 +106,36 @@ FieldValueInternal::FieldValueInternal(Type type, const Object& object)
 FieldValueInternal::FieldValueInternal(bool value)
     : cached_type_(Type::kBoolean) {
   Env env = GetEnv();
-  object_ = Boolean::Create(env, value);
+  auto boxed_ptr = Boolean::Create(env, value);
+  object_ = jni::ArenaRef(boxed_ptr);
 }
 
 FieldValueInternal::FieldValueInternal(int64_t value)
     : cached_type_(Type::kInteger) {
   Env env = GetEnv();
-  object_ = Long::Create(env, value);
+  auto boxed_ptr = Long::Create(env, value);
+  object_ = jni::ArenaRef(boxed_ptr);
 }
 
 FieldValueInternal::FieldValueInternal(double value)
     : cached_type_(Type::kDouble) {
   Env env = GetEnv();
-  object_ = Double::Create(env, value);
+  auto boxed_ptr = Double::Create(env, value);
+  object_ = jni::ArenaRef(boxed_ptr);
 }
 
 FieldValueInternal::FieldValueInternal(Timestamp value)
     : cached_type_(Type::kTimestamp) {
   Env env = GetEnv();
-  object_ = TimestampInternal::Create(env, value);
+  auto boxed_ptr = TimestampInternal::Create(env, value);
+  object_ = jni::ArenaRef(boxed_ptr);
 }
 
 FieldValueInternal::FieldValueInternal(std::string value)
     : cached_type_(Type::kString) {
   Env env = GetEnv();
-  object_ = env.NewStringUtf(value);
+  auto boxed_ptr = env.NewStringUtf(value);
+  object_ = jni::ArenaRef(boxed_ptr);
 }
 
 // We do not initialize cached_blob_ with value here as the instance constructed
@@ -140,20 +145,22 @@ FieldValueInternal::FieldValueInternal(std::string value)
 FieldValueInternal::FieldValueInternal(const uint8_t* value, size_t size)
     : cached_type_(Type::kBlob) {
   Env env = GetEnv();
-  object_ = BlobInternal::Create(env, value, size);
+  auto boxed_ptr = BlobInternal::Create(env, value, size);
+  object_ = jni::ArenaRef(boxed_ptr);
 }
 
 FieldValueInternal::FieldValueInternal(DocumentReference value)
     : cached_type_{Type::kReference} {
   if (value.internal_ != nullptr) {
-    object_ = value.internal_->ToJava();
+    object_ = jni::ArenaRef(value.internal_->ToJava());
   }
 }
 
 FieldValueInternal::FieldValueInternal(GeoPoint value)
     : cached_type_(Type::kGeoPoint) {
   Env env = GetEnv();
-  object_ = GeoPointInternal::Create(env, value);
+  auto boxed_ptr = GeoPointInternal::Create(env, value);
+  object_ = jni::ArenaRef(boxed_ptr);
 }
 
 FieldValueInternal::FieldValueInternal(const std::vector<FieldValue>& value)
@@ -164,7 +171,7 @@ FieldValueInternal::FieldValueInternal(const std::vector<FieldValue>& value)
     // TODO(b/150016438): don't conflate invalid `FieldValue`s and null.
     list.Add(env, ToJava(element));
   }
-  object_ = list;
+  object_ = jni::ArenaRef(list);
 }
 
 FieldValueInternal::FieldValueInternal(const MapFieldValue& value)
@@ -176,63 +183,63 @@ FieldValueInternal::FieldValueInternal(const MapFieldValue& value)
     Local<String> key = env.NewStringUtf(kv.first);
     map.Put(env, key, ToJava(kv.second));
   }
-  object_ = map;
+  object_ = jni::ArenaRef(map);
 }
 
 Type FieldValueInternal::type() const {
   if (cached_type_ != Type::kNull) {
     return cached_type_;
   }
-  if (!object_) {
+  if (!object_.has_value()) {
     return Type::kNull;
   }
 
   // We do not have any knowledge on the type yet. Check the runtime type with
   // each known type.
   Env env = GetEnv();
-  if (env.IsInstanceOf(object_, Boolean::GetClass())) {
+  if (env.IsInstanceOf(object_.get(), Boolean::GetClass())) {
     cached_type_ = Type::kBoolean;
     return Type::kBoolean;
   }
-  if (env.IsInstanceOf(object_, Long::GetClass())) {
+  if (env.IsInstanceOf(object_.get(), Long::GetClass())) {
     cached_type_ = Type::kInteger;
     return Type::kInteger;
   }
-  if (env.IsInstanceOf(object_, Double::GetClass())) {
+  if (env.IsInstanceOf(object_.get(), Double::GetClass())) {
     cached_type_ = Type::kDouble;
     return Type::kDouble;
   }
-  if (env.IsInstanceOf(object_, TimestampInternal::GetClass())) {
+  if (env.IsInstanceOf(object_.get(), TimestampInternal::GetClass())) {
     cached_type_ = Type::kTimestamp;
     return Type::kTimestamp;
   }
-  if (env.IsInstanceOf(object_, String::GetClass())) {
+  if (env.IsInstanceOf(object_.get(), String::GetClass())) {
     cached_type_ = Type::kString;
     return Type::kString;
   }
-  if (env.IsInstanceOf(object_, BlobInternal::GetClass())) {
+  if (env.IsInstanceOf(object_.get(), BlobInternal::GetClass())) {
     cached_type_ = Type::kBlob;
     return Type::kBlob;
   }
-  if (env.IsInstanceOf(object_, DocumentReferenceInternal::GetClass())) {
+  if (env.IsInstanceOf(object_.get(), DocumentReferenceInternal::GetClass())) {
     cached_type_ = Type::kReference;
     return Type::kReference;
   }
-  if (env.IsInstanceOf(object_, GeoPointInternal::GetClass())) {
+  if (env.IsInstanceOf(object_.get(), GeoPointInternal::GetClass())) {
     cached_type_ = Type::kGeoPoint;
     return Type::kGeoPoint;
   }
-  if (env.IsInstanceOf(object_, List::GetClass())) {
+  if (env.IsInstanceOf(object_.get(), List::GetClass())) {
     cached_type_ = Type::kArray;
     return Type::kArray;
   }
-  if (env.IsInstanceOf(object_, Map::GetClass())) {
+  if (env.IsInstanceOf(object_.get(), Map::GetClass())) {
     cached_type_ = Type::kMap;
     return Type::kMap;
   }
 
   FIREBASE_ASSERT_MESSAGE(false, "Unsupported FieldValue type: %s",
-                          Class::GetClassName(env, object_).c_str());
+                          Class::GetClassName(env, object_.get()).c_str());
   return Type::kNull;
 }
 
@@ -391,12 +398,12 @@ bool operator==(const FieldValueInternal& lhs, const FieldValueInternal& rhs) {
 template <typename T>
 T FieldValueInternal::Cast(jni::Env& env, Type type) const {
   if (cached_type_ == Type::kNull) {
-    FIREBASE_ASSERT(env.IsInstanceOf(object_, T::GetClass()));
+    FIREBASE_ASSERT(env.IsInstanceOf(object_.get(), T::GetClass()));
     cached_type_ = type;
   } else {
     FIREBASE_ASSERT(cached_type_ == type);
   }
-  auto typed_value = static_cast<jni::JniType<T>>(object_.get());
+  auto typed_value = static_cast<jni::JniType<T>>(object_.get().get());
   return T(typed_value);
 }
 
@@ -412,7 +419,7 @@ Local<Array<Object>> FieldValueInternal::MakeArray(
 Env FieldValueInternal::GetEnv() { return FirestoreInternal::GetEnv(); }
 
 Object FieldValueInternal::ToJava(const FieldValue& value) {
-  return value.internal_ ? value.internal_->object_ : Object();
+  return value.internal_ ? value.internal_->object_.get() : Object();
 }
 
 }  // namespace firestore
diff --git a/firestore/src/android/field_value_android.h b/firestore/src/android/field_value_android.h
index ceb49d23da..236b1f5188 100644
--- a/firestore/src/android/field_value_android.h
+++ b/firestore/src/android/field_value_android.h
@@ -25,6 +25,7 @@
 #include "firebase/firestore/timestamp.h"
 #include "firestore/src/include/firebase/firestore/document_reference.h"
 #include "firestore/src/include/firebase/firestore/field_value.h"
+#include "firestore/src/jni/arena_ref.h"
 #include "firestore/src/jni/jni_fwd.h"
 #include "firestore/src/jni/object.h"
 #include "firestore/src/jni/ownership.h"
@@ -87,7 +88,7 @@ class FieldValueInternal {
   std::vector<FieldValue> array_value() const;
   MapFieldValue map_value() const;
 
-  const jni::Global<jni::Object>& ToJava() const { return object_; }
+  jni::Local<jni::Object> ToJava() const { return object_.get(); }
 
   static FieldValue Delete();
   static FieldValue ServerTimestamp();
@@ -116,7 +117,7 @@ class FieldValueInternal {
 
   static jni::Env GetEnv();
 
-  jni::Global<jni::Object> object_;
+  jni::ArenaRef object_;
 
   // Below are cached type information. It is costly to get type info from
   // jobject of Object type. So we cache it if we have already known.
diff --git a/firestore/src/jni/arena_ref.h b/firestore/src/jni/arena_ref.h
index 7723a49dc4..09afd27bfe 100644
--- a/firestore/src/jni/arena_ref.h
+++ b/firestore/src/jni/arena_ref.h
@@ -39,13 +39,13 @@ class ArenaRef {
 
   explicit ArenaRef(const Object& ob) {
     Env env(GetEnv());
-    id_ = assignState(ObjectArena::GetInstance().Put(env, ob));
+    assignState(ObjectArena::GetInstance().Put(env, ob));
   }
 
   ArenaRef(const ArenaRef& other) {
     if (other.id_.first) {
       Env env(GetEnv());
-      id_ = assignState(ObjectArena::GetInstance().Dup(env, other.id_.second));
+      assignState(ObjectArena::GetInstance().Dup(env, other.id_.second));
     }
   }
 
@@ -57,8 +57,7 @@ class ArenaRef {
       }
       reset();
       if (other.id_.first) {
-        id_ =
-            assignState(ObjectArena::GetInstance().Dup(env, other.id_.second));
+        assignState(ObjectArena::GetInstance().Dup(env, other.id_.second));
       }
     }
     return *this;
@@ -72,7 +71,7 @@ class ArenaRef {
       if (id_.first) {
         ObjectArena::GetInstance().Remove(env, id_.second);
       }
-      id_ = assignState(other.release());
+      assignState(other.release());
     }
     return *this;
   }
@@ -84,7 +83,10 @@ class ArenaRef {
     }
   }
 
+  bool has_value() const { return id_.first; }
+
   Local<Object> get() const {
+    FIREBASE_DEV_ASSERT(id_.first);
     Env env(GetEnv());
     return ObjectArena::GetInstance().Get(env, id_.second);
   }
@@ -107,7 +109,7 @@ class ArenaRef {
     return id;
   }
 
-  std::pair<bool, int64_t> assignState(int64_t id) { id_ = {true, id}; }
+  void assignState(int64_t id) { id_ = {true, id}; }
 
   void reset() { id_ = {false, -1}; }
 

From a005e027d8b70906dce91809560b7f3b5941dcd1 Mon Sep 17 00:00:00 2001
From: cherylEnkidu <cheryllin@google.com>
Date: Fri, 9 Dec 2022 14:34:08 -0500
Subject: [PATCH 06/10] Address feedback, reimplement ArenaRef

---
 .../integration_test_internal/CMakeLists.txt  |  1 +
 .../src/jni/arena_ref_test.cc                 | 36 ++++++++
 firestore/src/android/field_value_android.cc  | 35 ++++---
 firestore/src/android/field_value_android.h   |  2 +-
 firestore/src/jni/arena_ref.h                 | 92 +++++++++++--------
 firestore/src/jni/object_arena.cc             | 11 ++-
 firestore/src/jni/object_arena.h              |  5 +
 7 files changed, 126 insertions(+), 56 deletions(-)
 create mode 100644 firestore/integration_test_internal/src/jni/arena_ref_test.cc

diff --git a/firestore/integration_test_internal/CMakeLists.txt b/firestore/integration_test_internal/CMakeLists.txt
index 75667cb60d..0a970bd1c1 100644
--- a/firestore/integration_test_internal/CMakeLists.txt
+++ b/firestore/integration_test_internal/CMakeLists.txt
@@ -118,6 +118,7 @@ set(FIREBASE_INTEGRATION_TEST_ANDROID_TEST_SRCS
     src/android/snapshot_metadata_android_test.cc
     src/android/timestamp_android_test.cc
     src/android/transaction_options_android_test.cc
+    src/jni/arena_ref_test.cc
     src/jni/declaration_test.cc
     src/jni/env_test.cc
     src/jni/object_test.cc
diff --git a/firestore/integration_test_internal/src/jni/arena_ref_test.cc b/firestore/integration_test_internal/src/jni/arena_ref_test.cc
new file mode 100644
index 0000000000..fb94e11b13
--- /dev/null
+++ b/firestore/integration_test_internal/src/jni/arena_ref_test.cc
@@ -0,0 +1,36 @@
+/*
+ * Copyright 2022 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.
+ */
+
+#include "firestore/src/jni/arena_ref.h"
+#include "firestore_integration_test.h"
+
+namespace firebase {
+namespace firestore {
+namespace jni {
+namespace {
+
+using ArenaRefTest = FirestoreIntegrationTest;
+
+TEST_F(ArenaRefTest, ConstructorTest) {
+  Env env(GetEnv());
+  ArenaRef ref(ObjectArena::Create(env));
+  EXPECT_TRUE(ref.is_valid());
+}
+
+}  // namespace
+}  // namespace jni
+}  // namespace firestore
+}  // namespace firebase
diff --git a/firestore/src/android/field_value_android.cc b/firestore/src/android/field_value_android.cc
index d5bb404e12..73e6f71c67 100644
--- a/firestore/src/android/field_value_android.cc
+++ b/firestore/src/android/field_value_android.cc
@@ -197,49 +197,50 @@ Type FieldValueInternal::type() const {
   // We do not have any knowledge on the type yet. Check the runtime type with
   // each known type.
   Env env = GetEnv();
-  if (env.IsInstanceOf(object_.get(), Boolean::GetClass())) {
+  if (env.IsInstanceOf(object_.get(env), Boolean::GetClass())) {
     cached_type_ = Type::kBoolean;
     return Type::kBoolean;
   }
-  if (env.IsInstanceOf(object_.get(), Long::GetClass())) {
+  if (env.IsInstanceOf(object_.get(env), Long::GetClass())) {
     cached_type_ = Type::kInteger;
     return Type::kInteger;
   }
-  if (env.IsInstanceOf(object_.get(), Double::GetClass())) {
+  if (env.IsInstanceOf(object_.get(env), Double::GetClass())) {
     cached_type_ = Type::kDouble;
     return Type::kDouble;
   }
-  if (env.IsInstanceOf(object_.get(), TimestampInternal::GetClass())) {
+  if (env.IsInstanceOf(object_.get(env), TimestampInternal::GetClass())) {
     cached_type_ = Type::kTimestamp;
     return Type::kTimestamp;
   }
-  if (env.IsInstanceOf(object_.get(), String::GetClass())) {
+  if (env.IsInstanceOf(object_.get(env), String::GetClass())) {
     cached_type_ = Type::kString;
     return Type::kString;
   }
-  if (env.IsInstanceOf(object_.get(), BlobInternal::GetClass())) {
+  if (env.IsInstanceOf(object_.get(env), BlobInternal::GetClass())) {
     cached_type_ = Type::kBlob;
     return Type::kBlob;
   }
-  if (env.IsInstanceOf(object_.get(), DocumentReferenceInternal::GetClass())) {
+  if (env.IsInstanceOf(object_.get(env),
+                       DocumentReferenceInternal::GetClass())) {
     cached_type_ = Type::kReference;
     return Type::kReference;
   }
-  if (env.IsInstanceOf(object_.get(), GeoPointInternal::GetClass())) {
+  if (env.IsInstanceOf(object_.get(env), GeoPointInternal::GetClass())) {
     cached_type_ = Type::kGeoPoint;
     return Type::kGeoPoint;
   }
-  if (env.IsInstanceOf(object_.get(), List::GetClass())) {
+  if (env.IsInstanceOf(object_.get(env), List::GetClass())) {
     cached_type_ = Type::kArray;
     return Type::kArray;
   }
-  if (env.IsInstanceOf(object_.get(), Map::GetClass())) {
+  if (env.IsInstanceOf(object_.get(env), Map::GetClass())) {
     cached_type_ = Type::kMap;
     return Type::kMap;
   }
 
   FIREBASE_ASSERT_MESSAGE(false, "Unsupported FieldValue type: %s",
-                          Class::GetClassName(env, object_.get()).c_str());
+                          Class::GetClassName(env, object_.get(env)).c_str());
   return Type::kNull;
 }
 
@@ -398,12 +399,12 @@ bool operator==(const FieldValueInternal& lhs, const FieldValueInternal& rhs) {
 template <typename T>
 T FieldValueInternal::Cast(jni::Env& env, Type type) const {
   if (cached_type_ == Type::kNull) {
-    FIREBASE_ASSERT(env.IsInstanceOf(object_.get(), T::GetClass()));
+    FIREBASE_ASSERT(env.IsInstanceOf(object_.get(env), T::GetClass()));
     cached_type_ = type;
   } else {
     FIREBASE_ASSERT(cached_type_ == type);
   }
-  auto typed_value = static_cast<jni::JniType<T>>(object_.get().get());
+  auto typed_value = static_cast<jni::JniType<T>>(object_.get(env).get());
   return T(typed_value);
 }
 
@@ -418,8 +419,14 @@ Local<Array<Object>> FieldValueInternal::MakeArray(
 
 Env FieldValueInternal::GetEnv() { return FirestoreInternal::GetEnv(); }
 
+jni::Local<jni::Object> FieldValueInternal::ToJava() const {
+  Env env = GetEnv();
+  return object_.get(env);
+}
+
 Object FieldValueInternal::ToJava(const FieldValue& value) {
-  return value.internal_ ? value.internal_->object_.get() : Object();
+  Env env = GetEnv();
+  return value.internal_ ? value.internal_->object_.get(env) : Object();
 }
 
 }  // namespace firestore
diff --git a/firestore/src/android/field_value_android.h b/firestore/src/android/field_value_android.h
index 236b1f5188..c64912016d 100644
--- a/firestore/src/android/field_value_android.h
+++ b/firestore/src/android/field_value_android.h
@@ -88,7 +88,7 @@ class FieldValueInternal {
   std::vector<FieldValue> array_value() const;
   MapFieldValue map_value() const;
 
-  jni::Local<jni::Object> ToJava() const { return object_.get(); }
+  jni::Local<jni::Object> ToJava() const;
 
   static FieldValue Delete();
   static FieldValue ServerTimestamp();
diff --git a/firestore/src/jni/arena_ref.h b/firestore/src/jni/arena_ref.h
index 09afd27bfe..d9ea12271c 100644
--- a/firestore/src/jni/arena_ref.h
+++ b/firestore/src/jni/arena_ref.h
@@ -39,83 +39,97 @@ class ArenaRef {
 
   explicit ArenaRef(const Object& ob) {
     Env env(GetEnv());
-    assignState(ObjectArena::GetInstance().Put(env, ob));
+    int64_t id = ObjectArena::GetInstance().Put(env, ob);
+    if (env.ok()) {
+      assignState(id);
+    }
   }
 
-  ArenaRef(const ArenaRef& other) {
-    if (other.id_.first) {
+  explicit ArenaRef(const ArenaRef& other) {
+    if (other.valid_) {
       Env env(GetEnv());
-      assignState(ObjectArena::GetInstance().Dup(env, other.id_.second));
+      int64_t id = ObjectArena::GetInstance().Dup(env, other.id_);
+      if (env.ok()) {
+        assignState(id);
+      }
     }
   }
 
   ArenaRef& operator=(const ArenaRef& other) {
-    if (id_ != other.id_) {
-      Env env(GetEnv());
-      if (id_.first) {
-        ObjectArena::GetInstance().Remove(env, id_.second);
-      }
-      reset();
-      if (other.id_.first) {
-        assignState(ObjectArena::GetInstance().Dup(env, other.id_.second));
+    Env env(GetEnv());
+    clear(env);
+    if (other.valid_ && id_ != other.id_) {
+      if (other.valid_) {
+        int64_t id = ObjectArena::GetInstance().Dup(env, other.id_);
+        if (env.ok()) {
+          assignState(id);
+        }
       }
     }
     return *this;
   }
 
-  ArenaRef(ArenaRef&& other) noexcept : ArenaRef(other.release()) {}
+  explicit ArenaRef(ArenaRef&& other) {
+    if (other.valid_) {
+      assignState(other.release());
+    }
+  }
 
-  ArenaRef& operator=(ArenaRef&& other) noexcept {
-    if (id_.second != other.id_.second) {
-      Env env(GetEnv());
-      if (id_.first) {
-        ObjectArena::GetInstance().Remove(env, id_.second);
+  ArenaRef& operator=(ArenaRef&& other) {
+    Env env(GetEnv());
+    clear(env);
+    if (other.valid_ && id_ != other.id_) {
+      if (other.valid_) {
+        assignState(other.release());
       }
-      assignState(other.release());
     }
     return *this;
   }
 
   ~ArenaRef() {
-    if (id_.first) {
+    if (valid_) {
       Env env(GetEnv());
-      ObjectArena::GetInstance().Remove(env, id_.second);
+      ObjectArena::GetInstance().Remove(env, id_);
     }
   }
 
-  bool has_value() const { return id_.first; }
+  bool has_value() const { return valid_; }
 
-  Local<Object> get() const {
-    FIREBASE_DEV_ASSERT(id_.first);
-    Env env(GetEnv());
-    return ObjectArena::GetInstance().Get(env, id_.second);
+  Local<Object> get(Env& env) const {
+    FIREBASE_DEV_ASSERT(valid_);
+    return ObjectArena::GetInstance().Get(env, id_);
   }
 
-  void clear() {
-    if (id_.first) {
-      Env env(GetEnv());
-      ObjectArena::GetInstance().Remove(env, id_.second);
+  void clear(Env& env) {
+    if (valid_) {
+      ObjectArena::GetInstance().Remove(env, id_);
       reset();
     }
   }
 
- private:
-  explicit ArenaRef(int64_t id) { assignState(id); }
+  bool is_valid() { return valid_ && id_ >= 0; }
 
+ private:
   int64_t release() {
-    FIREBASE_DEV_ASSERT(id_.first);
-    int64_t id = id_.second;
+    FIREBASE_DEV_ASSERT(valid_);
+    int64_t id = id_;
     reset();
     return id;
   }
 
-  void assignState(int64_t id) { id_ = {true, id}; }
+  void assignState(int64_t id) {
+    valid_ = true;
+    id_ = id;
+  }
 
-  void reset() { id_ = {false, -1}; }
+  void reset() {
+    valid_ = false;
+    id_ = -1;
+  }
 
-  // A pair of variables which indicate the state of ArenaRef. If id_.first set
-  // to
-  std::pair<bool, int64_t> id_{false, -1};
+  // valid_ indicate the state of ArenaRef
+  bool valid_ = false;
+  int64_t id_ = -1;
 };
 
 }  // namespace jni
diff --git a/firestore/src/jni/object_arena.cc b/firestore/src/jni/object_arena.cc
index c4d6d863fc..19649c3a33 100644
--- a/firestore/src/jni/object_arena.cc
+++ b/firestore/src/jni/object_arena.cc
@@ -27,11 +27,18 @@ namespace jni {
 namespace {
 
 Global<ObjectArena> kInstance;
-Global<HashMap> kHashMap;
 
 }  // namespace
 
-void ObjectArena::Initialize(Env& env, Loader& loader) {}
+void ObjectArena::Initialize(Env& env, Loader& loader) {
+  kInstance = ObjectArena::Create(env);
+}
+
+Global<ObjectArena> ObjectArena::Create(Env& env) {
+  ObjectArena ob;
+  ob.kHashMap = HashMap::Create(env);
+  return ob;
+}
 
 ObjectArena& ObjectArena::GetInstance() { return kInstance; }
 
diff --git a/firestore/src/jni/object_arena.h b/firestore/src/jni/object_arena.h
index 3224901767..a01e308c9e 100644
--- a/firestore/src/jni/object_arena.h
+++ b/firestore/src/jni/object_arena.h
@@ -17,7 +17,9 @@
 #ifndef FIREBASE_FIRESTORE_SRC_JNI_OBJECT_ARENA_H_
 #define FIREBASE_FIRESTORE_SRC_JNI_OBJECT_ARENA_H_
 
+#include "firestore/src/jni/hash_map.h"
 #include "firestore/src/jni/object.h"
+#include "firestore/src/jni/ownership.h"
 
 namespace firebase {
 namespace firestore {
@@ -32,6 +34,8 @@ class ObjectArena : public Object {
 
   static ObjectArena& GetInstance();
 
+  static Global<ObjectArena> Create(Env& env);
+
   Local<Object> Get(Env& env, int64_t key);
   int64_t Put(Env& env, const Object& value);
   void Remove(Env& env, int64_t key);
@@ -39,6 +43,7 @@ class ObjectArena : public Object {
 
  private:
   int64_t next_key_ = 0;
+  Global<HashMap> kHashMap;
 };
 
 }  // namespace jni

From a48d72da7ee167fd84215e8fb0ac652042c28404 Mon Sep 17 00:00:00 2001
From: cherylEnkidu <cheryllin@google.com>
Date: Mon, 12 Dec 2022 23:08:28 -0500
Subject: [PATCH 07/10] Bug fix, adding tests

---
 .../src/jni/arena_ref_test.cc                 | 30 +++++++++++++++++--
 firestore/src/android/firestore_android.cc    |  2 +-
 firestore/src/jni/object_arena.cc             | 18 +++++------
 firestore/src/jni/object_arena.h              |  4 +--
 4 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/firestore/integration_test_internal/src/jni/arena_ref_test.cc b/firestore/integration_test_internal/src/jni/arena_ref_test.cc
index fb94e11b13..b3b2766bc9 100644
--- a/firestore/integration_test_internal/src/jni/arena_ref_test.cc
+++ b/firestore/integration_test_internal/src/jni/arena_ref_test.cc
@@ -15,6 +15,8 @@
  */
 
 #include "firestore/src/jni/arena_ref.h"
+#include "firestore/src/jni/boolean.h"
+
 #include "firestore_integration_test.h"
 
 namespace firebase {
@@ -26,8 +28,32 @@ using ArenaRefTest = FirestoreIntegrationTest;
 
 TEST_F(ArenaRefTest, ConstructorTest) {
   Env env(GetEnv());
-  ArenaRef ref(ObjectArena::Create(env));
-  EXPECT_TRUE(ref.is_valid());
+  auto boxed_ptr = Boolean::Create(env, true);
+  ArenaRef ref1(boxed_ptr);
+  EXPECT_TRUE(ref1.is_valid());
+
+  ArenaRef ref2(ref1);
+  EXPECT_TRUE(ref1.is_valid());
+  EXPECT_TRUE(ref2.is_valid());
+
+  ArenaRef ref3(std::move(ref2));
+  EXPECT_TRUE(ref3.is_valid());
+}
+
+TEST_F(ArenaRefTest, AssignmentOperatorTest) {
+  Env env(GetEnv());
+  auto boxed_ptr = Boolean::Create(env, true);
+  ArenaRef ref1(boxed_ptr);
+  EXPECT_TRUE(ref1.is_valid());
+
+  ArenaRef ref2, ref3;
+
+  ref2 = ref1;
+  EXPECT_TRUE(ref1.is_valid());
+  EXPECT_TRUE(ref2.is_valid());
+
+  ref3 = std::move(ref2);
+  EXPECT_TRUE(ref3.is_valid());
 }
 
 }  // namespace
diff --git a/firestore/src/android/firestore_android.cc b/firestore/src/android/firestore_android.cc
index 29d1bda6be..1fb8eb3546 100644
--- a/firestore/src/android/firestore_android.cc
+++ b/firestore/src/android/firestore_android.cc
@@ -303,7 +303,6 @@ bool FirestoreInternal::Initialize(App* app) {
     loader.CacheEmbeddedFiles();
 
     jni::Object::Initialize(loader);
-    jni::ObjectArena::Initialize(env, loader);
     jni::String::Initialize(env, loader);
     jni::ArrayList::Initialize(loader);
     jni::Boolean::Initialize(loader);
@@ -315,6 +314,7 @@ bool FirestoreInternal::Initialize(App* app) {
     jni::List::Initialize(loader);
     jni::Long::Initialize(loader);
     jni::Map::Initialize(loader);
+    jni::ObjectArena::Initialize(env, loader);
 
     InitializeFirestore(loader);
     InitializeFirestoreTasks(loader);
diff --git a/firestore/src/jni/object_arena.cc b/firestore/src/jni/object_arena.cc
index 19649c3a33..ef5fa43e75 100644
--- a/firestore/src/jni/object_arena.cc
+++ b/firestore/src/jni/object_arena.cc
@@ -31,33 +31,29 @@ Global<ObjectArena> kInstance;
 }  // namespace
 
 void ObjectArena::Initialize(Env& env, Loader& loader) {
-  kInstance = ObjectArena::Create(env);
+  kInstance = ObjectArena(env);
 }
 
-Global<ObjectArena> ObjectArena::Create(Env& env) {
-  ObjectArena ob;
-  ob.kHashMap = HashMap::Create(env);
-  return ob;
-}
+ObjectArena::ObjectArena(Env& env) : hash_map_(HashMap::Create(env)) {}
 
 ObjectArena& ObjectArena::GetInstance() { return kInstance; }
 
 Local<Object> ObjectArena::Get(Env& env, int64_t key) {
-  return kHashMap.Get(env, Long::Create(env, key));
+  return hash_map_.Get(env, Long::Create(env, key));
 }
 
 int64_t ObjectArena::Put(Env& env, const Object& value) {
-  kHashMap.Put(env, Long::Create(env, next_key_), value);
+  hash_map_.Put(env, Long::Create(env, next_key_), value);
   return next_key_++;
 }
 
 void ObjectArena::Remove(Env& env, int64_t key) {
-  kHashMap.Remove(env, Long::Create(env, key));
+  hash_map_.Remove(env, Long::Create(env, key));
 }
 
 int64_t ObjectArena::Dup(Env& env, int64_t key) {
-  kHashMap.Put(env, Long::Create(env, next_key_),
-               kHashMap.Get(env, Long::Create(env, key)));
+  hash_map_.Put(env, Long::Create(env, next_key_),
+                hash_map_.Get(env, Long::Create(env, key)));
   return next_key_++;
 }
 
diff --git a/firestore/src/jni/object_arena.h b/firestore/src/jni/object_arena.h
index a01e308c9e..d6eab9434a 100644
--- a/firestore/src/jni/object_arena.h
+++ b/firestore/src/jni/object_arena.h
@@ -34,7 +34,7 @@ class ObjectArena : public Object {
 
   static ObjectArena& GetInstance();
 
-  static Global<ObjectArena> Create(Env& env);
+  explicit ObjectArena(Env& env);
 
   Local<Object> Get(Env& env, int64_t key);
   int64_t Put(Env& env, const Object& value);
@@ -43,7 +43,7 @@ class ObjectArena : public Object {
 
  private:
   int64_t next_key_ = 0;
-  Global<HashMap> kHashMap;
+  Global<HashMap> hash_map_;
 };
 
 }  // namespace jni

From 73bc880c56105a0fa53c21070e843f2bbfbb61a6 Mon Sep 17 00:00:00 2001
From: Denver Coneybeare <dconeybe@google.com>
Date: Wed, 14 Dec 2022 14:34:01 -0500
Subject: [PATCH 08/10] object_arena.cc: add an assertion into the constructor
 to crash if creating the HashMap fails.

---
 firestore/src/jni/object_arena.cc | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/firestore/src/jni/object_arena.cc b/firestore/src/jni/object_arena.cc
index ef5fa43e75..ba898c4221 100644
--- a/firestore/src/jni/object_arena.cc
+++ b/firestore/src/jni/object_arena.cc
@@ -16,6 +16,7 @@
 
 #include "firestore/src/jni/object_arena.h"
 
+#include "firestore/src/common/hard_assert_common.h"
 #include "firestore/src/jni/env.h"
 #include "firestore/src/jni/hash_map.h"
 #include "firestore/src/jni/loader.h"
@@ -34,7 +35,12 @@ void ObjectArena::Initialize(Env& env, Loader& loader) {
   kInstance = ObjectArena(env);
 }
 
-ObjectArena::ObjectArena(Env& env) : hash_map_(HashMap::Create(env)) {}
+ObjectArena::ObjectArena(Env& env) : hash_map_(HashMap::Create(env)) {
+    if (! env.ok()) {
+      env.get()->ExceptionDescribe();
+    }
+    SIMPLE_HARD_ASSERT(hash_map_, "zzyzx hash_map_ is null");
+}
 
 ObjectArena& ObjectArena::GetInstance() { return kInstance; }
 

From 73a78034f1d61e6867676c1e95c19ae84d63fbf3 Mon Sep 17 00:00:00 2001
From: Denver Coneybeare <dconeybe@google.com>
Date: Thu, 15 Dec 2022 16:04:51 -0500
Subject: [PATCH 09/10] wip to decouple ObjectArena from
 ObjectArena::Initialize()

---
 .../src/field_value_test.cc                   |  5 +++++
 firestore/src/jni/object_arena.cc             | 19 +++++++------------
 firestore/src/jni/object_arena.h              | 15 +++++++--------
 3 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/firestore/integration_test_internal/src/field_value_test.cc b/firestore/integration_test_internal/src/field_value_test.cc
index bd82c1aa90..97d90861d2 100644
--- a/firestore/integration_test_internal/src/field_value_test.cc
+++ b/firestore/integration_test_internal/src/field_value_test.cc
@@ -34,6 +34,11 @@ namespace firestore {
 using Type = FieldValue::Type;
 using FieldValueTest = FirestoreIntegrationTest;
 
+TEST(DenverTest, zzyzx) {
+  auto field_value = FieldValue::String("hello");
+  ASSERT_EQ(field_value.ToString(), "zzyzx");
+}
+
 // Sanity test for stubs
 TEST_F(FieldValueTest, TestFieldValueTypes) {
   FieldValue::Null();
diff --git a/firestore/src/jni/object_arena.cc b/firestore/src/jni/object_arena.cc
index ba898c4221..f91c28fac9 100644
--- a/firestore/src/jni/object_arena.cc
+++ b/firestore/src/jni/object_arena.cc
@@ -27,24 +27,19 @@ namespace firestore {
 namespace jni {
 namespace {
 
-Global<ObjectArena> kInstance;
-
 }  // namespace
 
-void ObjectArena::Initialize(Env& env, Loader& loader) {
-  kInstance = ObjectArena(env);
+ObjectArena::ObjectArena(Env& env) {
+  hash_map_class_ = env.get()->FindClass("java/lang/HashMap");
+  hash_map_put_ = env.get()->GetMethodID(hash_map_class_, "put", "(Ljava/lang/object");
 }
 
-ObjectArena::ObjectArena(Env& env) : hash_map_(HashMap::Create(env)) {
-    if (! env.ok()) {
-      env.get()->ExceptionDescribe();
-    }
-    SIMPLE_HARD_ASSERT(hash_map_, "zzyzx hash_map_ is null");
+ObjectArena& ObjectArena::GetInstance(Env& env) {
+  static auto* instance = new ObjectArena(env);
+  return *instance;
 }
 
-ObjectArena& ObjectArena::GetInstance() { return kInstance; }
-
-Local<Object> ObjectArena::Get(Env& env, int64_t key) {
+jobject ObjectArena::Get(Env& env, int64_t key) {
   return hash_map_.Get(env, Long::Create(env, key));
 }
 
diff --git a/firestore/src/jni/object_arena.h b/firestore/src/jni/object_arena.h
index d6eab9434a..dd875afefe 100644
--- a/firestore/src/jni/object_arena.h
+++ b/firestore/src/jni/object_arena.h
@@ -26,15 +26,11 @@ namespace firestore {
 namespace jni {
 
 /** `ObjectArena` serves as a local HashMap. */
-class ObjectArena : public Object {
+class ObjectArena {
  public:
-  using Object::Object;
+  ~ObjectArena() = delete;
 
-  static void Initialize(Env& env, Loader& loader);
-
-  static ObjectArena& GetInstance();
-
-  explicit ObjectArena(Env& env);
+  static ObjectArena& GetInstance(Env& env);
 
   Local<Object> Get(Env& env, int64_t key);
   int64_t Put(Env& env, const Object& value);
@@ -42,8 +38,11 @@ class ObjectArena : public Object {
   int64_t Dup(Env& env, int64_t key);
 
  private:
+  explicit ObjectArena(Env& env);
+
   int64_t next_key_ = 0;
-  Global<HashMap> hash_map_;
+  jclass hash_map_class_ = nullptr;
+  jmethodID hash_map_put_ = nullptr;
 };
 
 }  // namespace jni

From 6b6caabefc44f981e969b496577c1b5e0680c3ad Mon Sep 17 00:00:00 2001
From: cherylEnkidu <cheryllin@google.com>
Date: Wed, 21 Dec 2022 10:42:33 -0500
Subject: [PATCH 10/10] Change the impl of object_arena.

---
 firestore/src/android/firestore_android.cc |  1 -
 firestore/src/jni/arena_ref.h              | 20 ++++-----
 firestore/src/jni/env.h                    | 21 +++++-----
 firestore/src/jni/object_arena.cc          | 49 +++++++++++++++++-----
 firestore/src/jni/object_arena.h           | 12 +++++-
 5 files changed, 70 insertions(+), 33 deletions(-)

diff --git a/firestore/src/android/firestore_android.cc b/firestore/src/android/firestore_android.cc
index 1fb8eb3546..dff12baeac 100644
--- a/firestore/src/android/firestore_android.cc
+++ b/firestore/src/android/firestore_android.cc
@@ -314,7 +314,6 @@ bool FirestoreInternal::Initialize(App* app) {
     jni::List::Initialize(loader);
     jni::Long::Initialize(loader);
     jni::Map::Initialize(loader);
-    jni::ObjectArena::Initialize(env, loader);
 
     InitializeFirestore(loader);
     InitializeFirestoreTasks(loader);
diff --git a/firestore/src/jni/arena_ref.h b/firestore/src/jni/arena_ref.h
index d9ea12271c..7d505f9ac4 100644
--- a/firestore/src/jni/arena_ref.h
+++ b/firestore/src/jni/arena_ref.h
@@ -38,8 +38,8 @@ class ArenaRef {
   ArenaRef() = default;
 
   explicit ArenaRef(const Object& ob) {
-    Env env(GetEnv());
-    int64_t id = ObjectArena::GetInstance().Put(env, ob);
+    Env env;
+    int64_t id = ObjectArena::GetInstance(env).Put(env, ob.get());
     if (env.ok()) {
       assignState(id);
     }
@@ -47,8 +47,8 @@ class ArenaRef {
 
   explicit ArenaRef(const ArenaRef& other) {
     if (other.valid_) {
-      Env env(GetEnv());
-      int64_t id = ObjectArena::GetInstance().Dup(env, other.id_);
+      Env env;
+      int64_t id = ObjectArena::GetInstance(env).Dup(env, other.id_);
       if (env.ok()) {
         assignState(id);
       }
@@ -56,11 +56,11 @@ class ArenaRef {
   }
 
   ArenaRef& operator=(const ArenaRef& other) {
-    Env env(GetEnv());
+    Env env;
     clear(env);
     if (other.valid_ && id_ != other.id_) {
       if (other.valid_) {
-        int64_t id = ObjectArena::GetInstance().Dup(env, other.id_);
+        int64_t id = ObjectArena::GetInstance(env).Dup(env, other.id_);
         if (env.ok()) {
           assignState(id);
         }
@@ -88,8 +88,8 @@ class ArenaRef {
 
   ~ArenaRef() {
     if (valid_) {
-      Env env(GetEnv());
-      ObjectArena::GetInstance().Remove(env, id_);
+      Env env;
+      ObjectArena::GetInstance(env).Remove(env, id_);
     }
   }
 
@@ -97,12 +97,12 @@ class ArenaRef {
 
   Local<Object> get(Env& env) const {
     FIREBASE_DEV_ASSERT(valid_);
-    return ObjectArena::GetInstance().Get(env, id_);
+    return env.MakeResult<Object>(ObjectArena::GetInstance(env).Get(env, id_));
   }
 
   void clear(Env& env) {
     if (valid_) {
-      ObjectArena::GetInstance().Remove(env, id_);
+      ObjectArena::GetInstance(env).Remove(env, id_);
       reset();
     }
   }
diff --git a/firestore/src/jni/env.h b/firestore/src/jni/env.h
index a7c3354d68..498c7a0607 100644
--- a/firestore/src/jni/env.h
+++ b/firestore/src/jni/env.h
@@ -474,6 +474,17 @@ class Env {
     RecordException();
   }
 
+  template <typename T>
+  EnableForReference<T, Local<T>> MakeResult(jobject object) {
+    // JNI object method results are always jobject, even when the actual type
+    // is jstring or jclass. Cast to the correct type here so that Local<T>
+    // doesn't have to account for this.
+    auto typed_object = static_cast<JniType<T>>(object);
+    return Local<T>(env_, typed_object);
+  }
+
+  void RecordException();
+
  private:
   /**
    * Invokes the JNI instance method using the given method reference on JNIEnv.
@@ -513,7 +524,6 @@ class Env {
     RecordException();
   }
 
-  void RecordException();
   std::string ErrorDescription(const Object& object);
   const char* ErrorName(jint error);
 
@@ -522,15 +532,6 @@ class Env {
     return static_cast<T>(value);
   }
 
-  template <typename T>
-  EnableForReference<T, Local<T>> MakeResult(jobject object) {
-    // JNI object method results are always jobject, even when the actual type
-    // is jstring or jclass. Cast to the correct type here so that Local<T>
-    // doesn't have to account for this.
-    auto typed_object = static_cast<JniType<T>>(object);
-    return Local<T>(env_, typed_object);
-  }
-
   JNIEnv* env_ = nullptr;
 
   UnhandledExceptionHandler exception_handler_ = nullptr;
diff --git a/firestore/src/jni/object_arena.cc b/firestore/src/jni/object_arena.cc
index f91c28fac9..4c4a73d348 100644
--- a/firestore/src/jni/object_arena.cc
+++ b/firestore/src/jni/object_arena.cc
@@ -18,9 +18,7 @@
 
 #include "firestore/src/common/hard_assert_common.h"
 #include "firestore/src/jni/env.h"
-#include "firestore/src/jni/hash_map.h"
 #include "firestore/src/jni/loader.h"
-#include "firestore/src/jni/long.h"
 
 namespace firebase {
 namespace firestore {
@@ -30,8 +28,20 @@ namespace {
 }  // namespace
 
 ObjectArena::ObjectArena(Env& env) {
-  hash_map_class_ = env.get()->FindClass("java/lang/HashMap");
-  hash_map_put_ = env.get()->GetMethodID(hash_map_class_, "put", "(Ljava/lang/object");
+  hash_map_class_ = env.get()->FindClass("java/util/HashMap");
+  hash_map_ctor_ = env.get()->GetMethodID(hash_map_class_, "<init>", "()V");
+  hash_map_ = env.get()->NewGlobalRef(
+          env.get()->NewObject(hash_map_class_, hash_map_ctor_));
+
+  hash_map_get_ = env.get()->GetMethodID(hash_map_class_, "get",
+                                         "(Ljava/lang/Object;)Ljava/lang/Object;");
+  hash_map_put_ = env.get()->GetMethodID(hash_map_class_, "put",
+                                         "(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;");
+  hash_map_remove_ = env.get()->GetMethodID(hash_map_class_, "remove",
+                                         "(Ljava/lang/Object;)Ljava/lang/Object;");
+
+  long_class_ = env.get()->FindClass("java/lang/Long");
+  long_ctor_ = env.get()->GetMethodID(long_class_, "<init>", "(J)V");
 }
 
 ObjectArena& ObjectArena::GetInstance(Env& env) {
@@ -40,23 +50,42 @@ ObjectArena& ObjectArena::GetInstance(Env& env) {
 }
 
 jobject ObjectArena::Get(Env& env, int64_t key) {
-  return hash_map_.Get(env, Long::Create(env, key));
+  jobject long_key = MakeKey(env, key);
+  jobject result = env.get()->CallObjectMethod(hash_map_, hash_map_get_, long_key);
+  SIMPLE_HARD_ASSERT(env.ok(), "Get() fails");
+  return result;
 }
 
-int64_t ObjectArena::Put(Env& env, const Object& value) {
-  hash_map_.Put(env, Long::Create(env, next_key_), value);
+int64_t ObjectArena::Put(Env& env, jobject value) {
+  SIMPLE_HARD_ASSERT(reinterpret_cast<jobject>(0x1) != value, "value is null");
+  SIMPLE_HARD_ASSERT(reinterpret_cast<jobject>(0x1) != hash_map_, "hash_map_ is null");
+  env.RecordException();
+  env.get()->CallObjectMethod(hash_map_, hash_map_put_, next_key_, value);
+  env.RecordException();
+  SIMPLE_HARD_ASSERT(env.ok(), "Put() fails");
   return next_key_++;
 }
 
 void ObjectArena::Remove(Env& env, int64_t key) {
-  hash_map_.Remove(env, Long::Create(env, key));
+  jobject long_key = MakeKey(env, key);
+  env.get()->CallObjectMethod(hash_map_, hash_map_remove_, long_key);
+  SIMPLE_HARD_ASSERT(env.ok(), "Remove() fails");
 }
 
 int64_t ObjectArena::Dup(Env& env, int64_t key) {
-  hash_map_.Put(env, Long::Create(env, next_key_),
-                hash_map_.Get(env, Long::Create(env, key)));
+  jobject long_key = MakeKey(env, key);
+  jobject old_key = env.get()->CallObjectMethod(hash_map_, hash_map_get_, long_key);
+  env.get()->CallObjectMethod(hash_map_, hash_map_put_,
+                              MakeKey(env, next_key_),
+                              old_key);
+  SIMPLE_HARD_ASSERT(env.ok(), "Dup() fails");
   return next_key_++;
 }
+jobject ObjectArena::MakeKey(Env& env, int64_t key) {
+  jobject result = env.get()->NewObject(long_class_, long_ctor_, key);
+  SIMPLE_HARD_ASSERT(env.ok(), "MakeKey() fails");
+  return result;
+}
 
 }  // namespace jni
 }  // namespace firestore
diff --git a/firestore/src/jni/object_arena.h b/firestore/src/jni/object_arena.h
index dd875afefe..8e9a1341d0 100644
--- a/firestore/src/jni/object_arena.h
+++ b/firestore/src/jni/object_arena.h
@@ -32,17 +32,25 @@ class ObjectArena {
 
   static ObjectArena& GetInstance(Env& env);
 
-  Local<Object> Get(Env& env, int64_t key);
-  int64_t Put(Env& env, const Object& value);
+  jobject Get(Env& env, int64_t key);
+  int64_t Put(Env& env, jobject value);
   void Remove(Env& env, int64_t key);
   int64_t Dup(Env& env, int64_t key);
 
  private:
   explicit ObjectArena(Env& env);
+  jobject MakeKey(Env& env, int64_t key);
 
   int64_t next_key_ = 0;
   jclass hash_map_class_ = nullptr;
+  jmethodID hash_map_ctor_ = nullptr;
+  jobject hash_map_ = nullptr;
+  jmethodID hash_map_get_ = nullptr;
   jmethodID hash_map_put_ = nullptr;
+  jmethodID hash_map_remove_ = nullptr;
+
+  jclass long_class_ = nullptr;
+  jmethodID long_ctor_ = nullptr;
 };
 
 }  // namespace jni