Skip to content

Commit 9aecf1e

Browse files
wilhuffa-maurice
authored andcommitted
Add Equals to the JNI Object proxy.
Also add GetClass to some JNI proxies. Remove Wrapper::EqualsJavaObject PiperOrigin-RevId: 334501941
1 parent d011534 commit 9aecf1e

13 files changed

+60
-59
lines changed

firestore/src/android/field_value_android.cc

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -613,17 +613,8 @@ void FieldValueInternal::Terminate(App* app) {
613613
}
614614

615615
bool operator==(const FieldValueInternal& lhs, const FieldValueInternal& rhs) {
616-
// Most likely only happens when comparing one with itself or both are Null.
617-
if (lhs.obj_ == rhs.obj_) {
618-
return true;
619-
}
620-
621-
// If only one of them is Null, then they cannot equal.
622-
if (lhs.obj_ == nullptr || rhs.obj_ == nullptr) {
623-
return false;
624-
}
625-
626-
return lhs.EqualsJavaObject(rhs);
616+
Env env = FirestoreInternal::GetEnv();
617+
return Object::Equals(env, lhs.ToJava(), rhs.ToJava());
627618
}
628619

629620
jobject FieldValueInternal::TryGetJobject(const FieldValue& value) {

firestore/src/android/firestore_android.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -177,12 +177,13 @@ bool FirestoreInternal::Initialize(App* app) {
177177
::firebase_firestore::firestore_resources_size);
178178
loader.CacheEmbeddedFiles();
179179

180-
if (!( // Call Initialize on each Firestore internal class.
181-
FieldValueInternal::Initialize(app) && Wrapper::Initialize(app))) {
180+
if (!FieldValueInternal::Initialize(app)) {
182181
ReleaseClasses(app);
183182
return false;
184183
}
185184

185+
jni::Object::Initialize(loader);
186+
186187
jni::ArrayList::Initialize(loader);
187188
jni::Boolean::Initialize(loader);
188189
jni::Collection::Initialize(loader);
@@ -239,7 +240,6 @@ void FirestoreInternal::ReleaseClasses(App* app) {
239240

240241
// Call Terminate on each Firestore internal class.
241242
FieldValueInternal::Terminate(app);
242-
Wrapper::Terminate(app);
243243
}
244244

245245
/* static */

firestore/src/android/query_android.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,8 @@ Local<Array<Object>> QueryInternal::ConvertFieldValues(
309309
}
310310

311311
bool operator==(const QueryInternal& lhs, const QueryInternal& rhs) {
312-
return lhs.EqualsJavaObject(rhs);
312+
Env env = FirestoreInternal::GetEnv();
313+
return Object::Equals(env, lhs.ToJava(), rhs.ToJava());
313314
}
314315

315316
} // namespace firestore

firestore/src/android/wrapper.cc

Lines changed: 0 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,6 @@ using jni::String;
2323

2424
} // namespace
2525

26-
// clang-format off
27-
#define OBJECT_METHOD(X) X(Equals, "equals", "(Ljava/lang/Object;)Z")
28-
// clang-format on
29-
30-
METHOD_LOOKUP_DECLARATION(object, OBJECT_METHOD)
31-
METHOD_LOOKUP_DEFINITION(object, "java/lang/Object", OBJECT_METHOD)
32-
3326
Wrapper::Wrapper(FirestoreInternal* firestore, jobject obj)
3427
: Wrapper(firestore, obj, AllowNullObject::Yes) {
3528
FIREBASE_ASSERT(obj != nullptr);
@@ -93,18 +86,6 @@ Wrapper::~Wrapper() {
9386
}
9487
}
9588

96-
bool Wrapper::EqualsJavaObject(const Wrapper& other) const {
97-
if (obj_ == other.obj_) {
98-
return true;
99-
}
100-
101-
JNIEnv* env = firestore_->app()->GetJNIEnv();
102-
jboolean result = env->CallBooleanMethod(
103-
obj_, object::GetMethodId(object::kEquals), other.obj_);
104-
CheckAndClearJniExceptions(env);
105-
return static_cast<bool>(result);
106-
}
107-
10889
Local<HashMap> Wrapper::MakeJavaMap(Env& env, const MapFieldValue& data) const {
10990
Local<HashMap> result = HashMap::Create(env);
11091
for (const auto& kv : data) {
@@ -140,20 +121,5 @@ Wrapper::UpdateFieldPathArgs Wrapper::MakeUpdateFieldPathArgs(
140121
return UpdateFieldPathArgs{Move(first_field), first_value, Move(varargs)};
141122
}
142123

143-
/* static */
144-
bool Wrapper::Initialize(App* app) {
145-
JNIEnv* env = app->GetJNIEnv();
146-
jobject activity = app->activity();
147-
bool result = object::CacheMethodIds(env, activity);
148-
util::CheckAndClearJniExceptions(env);
149-
return result;
150-
}
151-
152-
/* static */
153-
void Wrapper::Terminate(App* app) {
154-
JNIEnv* env = app->GetJNIEnv();
155-
object::ReleaseClass(env);
156-
util::CheckAndClearJniExceptions(env);
157-
}
158124
} // namespace firestore
159125
} // namespace firebase

firestore/src/android/wrapper.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,6 @@ class Wrapper {
5656
jobject java_object() const { return obj_; }
5757
jni::Object ToJava() const { return jni::Object(obj_); }
5858

59-
// Tests the equality of the wrapped Java Object.
60-
bool EqualsJavaObject(const Wrapper& other) const;
61-
6259
protected:
6360
enum class AllowNullObject { Yes };
6461
// Default constructor. Subclass is expected to set the obj_ a meaningful
@@ -129,9 +126,6 @@ class Wrapper {
129126

130127
private:
131128
friend class FirestoreInternal;
132-
133-
static bool Initialize(App* app);
134-
static void Terminate(App* app);
135129
};
136130

137131
} // namespace firestore

firestore/src/jni/class.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ std::string Class::GetName(Env& env) const {
2424
return env.Call(*this, kGetName).ToString(env);
2525
}
2626

27+
std::string Class::GetClassName(Env& env, const Object& object) {
28+
return util::JObjectClassName(env.get(), object.get());
29+
}
30+
2731
bool Class::IsArray(Env& env) const { return env.Call(*this, kIsArray); }
2832

2933
} // namespace jni

firestore/src/jni/class.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ class Class : public Object {
3030
*/
3131
std::string GetName(Env& env) const;
3232

33+
static std::string GetClassName(Env& env, const Object& object);
34+
3335
bool IsArray(Env& env) const;
3436

3537
private:

firestore/src/jni/list.cc

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,17 @@ namespace {
1111

1212
Method<Object> kGet("get", "(I)Ljava/lang/Object;");
1313
Method<Object> kSet("set", "(ILjava/lang/Object;)Ljava/lang/Object;");
14+
jclass g_clazz = nullptr;
1415

1516
} // namespace
1617

1718
void List::Initialize(Loader& loader) {
18-
loader.LoadFromExistingClass("java/util/List", util::list::GetClass(), kGet,
19-
kSet);
19+
g_clazz = util::list::GetClass();
20+
loader.LoadFromExistingClass("java/util/List", g_clazz, kGet, kSet);
2021
}
2122

23+
Class List::GetClass() { return Class(g_clazz); }
24+
2225
Local<Object> List::Get(Env& env, size_t i) const {
2326
return env.Call(*this, kGet, i);
2427
}

firestore/src/jni/list.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ class List : public Collection {
1515

1616
static void Initialize(Loader& loader);
1717

18+
static Class GetClass();
19+
1820
Local<Object> Get(Env& env, size_t i) const;
1921
Local<Object> Set(Env& env, size_t i, const Object& object);
2022
};

firestore/src/jni/map.cc

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,18 @@ Method<Object> kGet("get", "(Ljava/lang/Object;)Ljava/lang/Object;");
1515
Method<Object> kPut("put",
1616
"(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;");
1717
Method<Set> kKeySet("keySet", "()Ljava/util/Set;");
18+
jclass g_clazz = nullptr;
1819

1920
} // namespace
2021

2122
void Map::Initialize(Loader& loader) {
22-
loader.LoadFromExistingClass("java/util/Map", util::map::GetClass(), kSize,
23-
kGet, kPut, kKeySet);
23+
g_clazz = util::map::GetClass();
24+
loader.LoadFromExistingClass("java/util/Map", g_clazz, kSize, kGet, kPut,
25+
kKeySet);
2426
}
2527

28+
Class Map::GetClass() { return Class(g_clazz); }
29+
2630
size_t Map::Size(Env& env) const { return env.Call(*this, kSize); }
2731

2832
Local<Object> Map::Get(Env& env, const Object& key) {

firestore/src/jni/map.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ class Map : public Object {
1717

1818
static void Initialize(Loader& loader);
1919

20+
static Class GetClass();
21+
2022
size_t Size(Env& env) const;
2123
Local<Object> Get(Env& env, const Object& key);
2224
Local<Object> Put(Env& env, const Object& key, const Object& value);

firestore/src/jni/object.cc

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,22 @@
33
#include "app/src/util_android.h"
44
#include "firestore/src/jni/class.h"
55
#include "firestore/src/jni/env.h"
6+
#include "firestore/src/jni/loader.h"
67

78
namespace firebase {
89
namespace firestore {
910
namespace jni {
11+
namespace {
12+
13+
Method<bool> kEquals("equals", "(Ljava/lang/Object;)Z");
14+
jclass g_clazz = nullptr;
15+
16+
} // namespace
17+
18+
void Object::Initialize(Loader& loader) {
19+
g_clazz = util::object::GetClass();
20+
loader.LoadFromExistingClass("java/lang/Object", g_clazz, kEquals);
21+
}
1022

1123
Class Object::GetClass() { return Class(util::object::GetClass()); }
1224

@@ -16,6 +28,20 @@ std::string Object::ToString(JNIEnv* env) const {
1628

1729
std::string Object::ToString(Env& env) const { return ToString(env.get()); }
1830

31+
bool Object::Equals(Env& env, const Object& other) const {
32+
return env.Call(*this, kEquals, other);
33+
}
34+
35+
bool Object::Equals(Env& env, const Object& lhs, const Object& rhs) {
36+
// Most likely only happens when comparing one with itself or both are null.
37+
if (lhs.get() == rhs.get()) return true;
38+
39+
// If only one of them is nullptr, then they cannot equal.
40+
if (!lhs || !rhs) return false;
41+
42+
return lhs.Equals(env, rhs);
43+
}
44+
1945
} // namespace jni
2046
} // namespace firestore
2147
} // namespace firebase

firestore/src/jni/object.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ namespace jni {
1111

1212
class Class;
1313
class Env;
14+
class Loader;
1415

1516
/**
1617
* A wrapper for a JNI `jobject` that adds additional behavior.
@@ -29,6 +30,8 @@ class Object {
2930

3031
virtual jobject get() const { return object_; }
3132

33+
static void Initialize(Loader& loader);
34+
3235
static Class GetClass();
3336

3437
/**
@@ -38,6 +41,9 @@ class Object {
3841
std::string ToString(JNIEnv* env) const;
3942
std::string ToString(Env& env) const;
4043

44+
bool Equals(Env& env, const Object& other) const;
45+
static bool Equals(Env& env, const Object& lhs, const Object& rhs);
46+
4147
protected:
4248
jobject object_ = nullptr;
4349
};

0 commit comments

Comments
 (0)