Address comments.

Author: ehsannas

firestore/integration_test_internal/src/document_change_test.cc

@@ -116,26 +116,26 @@ TEST_F(DocumentChangeTest, Equality) {
   snapshot = listener.last_result();
 
   std::vector<DocumentChange> changes = snapshot.DocumentChanges();
-  EXPECT_EQ(changes.size(), 1);
-  // first_change: Added doc1.
-  auto first_change = changes[0];
-  EXPECT_TRUE(first_change == first_change);
-  EXPECT_TRUE(first_change != invalid_change_1);
-  EXPECT_FALSE(first_change != first_change);
-  EXPECT_FALSE(first_change == invalid_change_1);
+  ASSERT_EQ(changes.size(), 1);
+  // First change: Added doc1.
+  auto change1 = changes[0];
+  EXPECT_TRUE(change1 == change1);
+  EXPECT_TRUE(change1 != invalid_change_1);
+  EXPECT_FALSE(change1 != change1);
+  EXPECT_FALSE(change1 == invalid_change_1);
 
   WriteDocument(doc2, MapFieldValue{{"a", FieldValue::Integer(2)}});
   Await(listener, 2);
   snapshot = listener.last_result();
 
   changes = snapshot.DocumentChanges();
-  EXPECT_EQ(changes.size(), 1);
-  // second_change: Added doc2.
-  auto second_change = changes[0];
-  EXPECT_TRUE(second_change != first_change);
-  EXPECT_TRUE(second_change != invalid_change_1);
-  EXPECT_FALSE(second_change == first_change);
-  EXPECT_FALSE(second_change == invalid_change_1);
+  ASSERT_EQ(changes.size(), 1);
+  // Second change: Added doc2.
+  auto change2 = changes[0];
+  EXPECT_TRUE(change2 != change1);
+  EXPECT_TRUE(change2 != invalid_change_1);
+  EXPECT_FALSE(change2 == change1);
+  EXPECT_FALSE(change2 == invalid_change_1);
 
   // Make doc2 ordered before doc1.
   WriteDocument(doc2, MapFieldValue{{"a", FieldValue::Integer(0)}});
@@ -146,11 +146,11 @@ TEST_F(DocumentChangeTest, Equality) {
   EXPECT_EQ(changes.size(), 1);
   // third_change: Modified doc2.
   auto third_change = changes[0];
-  EXPECT_TRUE(third_change != first_change);
-  EXPECT_TRUE(third_change != second_change);
+  EXPECT_TRUE(third_change != change1);
+  EXPECT_TRUE(third_change != change2);
   EXPECT_TRUE(third_change != invalid_change_1);
-  EXPECT_FALSE(third_change == first_change);
-  EXPECT_FALSE(third_change == second_change);
+  EXPECT_FALSE(third_change == change1);
+  EXPECT_FALSE(third_change == change2);
   EXPECT_FALSE(third_change == invalid_change_1);
 }
 

firestore/src/android/document_change_android.cc

@@ -18,6 +18,7 @@
 
 #include "firestore/src/android/document_change_type_android.h"
 #include "firestore/src/android/document_snapshot_android.h"
+#include "firestore/src/jni/compare.h"
 #include "firestore/src/jni/env.h"
 #include "firestore/src/jni/loader.h"
 
@@ -71,8 +72,7 @@ std::size_t DocumentChangeInternal::new_index() const {
 
 bool operator==(const DocumentChangeInternal& lhs,
                 const DocumentChangeInternal& rhs) {
-  Env env = FirestoreInternal::GetEnv();
-  return Object::Equals(env, lhs.ToJava(), rhs.ToJava());
+  return jni::EqualityCompareJni(lhs, rhs);
 }
 
 }  // namespace firestore

firestore/src/common/document_change.cc

@@ -20,6 +20,7 @@
 
 #include "firestore/src/common/cleanup.h"
 #include "firestore/src/common/hard_assert_common.h"
+#include "firestore/src/common/util.h"
 #include "firestore/src/include/firebase/firestore/document_snapshot.h"
 #if defined(__ANDROID__)
 #include "firestore/src/android/document_change_android.h"
@@ -119,11 +120,7 @@ std::size_t DocumentChange::new_index() const {
 }
 
 bool operator==(const DocumentChange& lhs, const DocumentChange& rhs) {
-  if (!lhs.internal_ || !rhs.internal_) {
-    return lhs.internal_ == rhs.internal_;
-  }
-
-  return lhs.internal_ == rhs.internal_ || *lhs.internal_ == *rhs.internal_;
+  return EqualityCompare(lhs.internal_, rhs.internal_);
 }
 
 }  // namespace firestore