Merge remote-tracking branch 'origin/master' into ghm

Author: a-maurice

Android/firebase_dependencies.gradle

@@ -22,7 +22,7 @@ def firebaseDependenciesMap = [
   'analytics' : ['com.google.firebase:firebase-analytics:17.2.2'],
   'auth' : ['com.google.firebase:firebase-auth:19.2.0'],
   'database' : ['com.google.firebase:firebase-database:19.2.1'],
-  'dynamic_links' : ['com.google.firebase:firebase-dynamic-links:19.0.0'],
+  'dynamic_links' : ['com.google.firebase:firebase-dynamic-links:19.1.0'],
   'functions' : ['com.google.firebase:firebase-functions:19.0.2'],
   'instance_id' : ['com.google.firebase:firebase-iid:20.0.2'],
   'invites' : ['com.google.firebase:firebase-invites:17.0.0'],

app/invites_resources/build.gradle

@@ -46,7 +46,7 @@ android {
 
 dependencies {
   implementation 'com.google.firebase:firebase-analytics:17.2.2'
-  implementation 'com.google.firebase:firebase-dynamic-links:19.0.0'
+  implementation 'com.google.firebase:firebase-dynamic-links:19.1.0'
   implementation project(':app:app_resources')
 }
 

database/src/desktop/core/tree.h

@@ -535,7 +535,7 @@ class Tree {
   }
 
   // The key of this element in the Tree. This will the eqivalent to the
-  // std:;string stored in the children_ map in the parent Tree node.
+  // std::string stored in the children_ map in the parent Tree node.
   std::string key_;
 
   // The value stored at this location in the tree. A location in a tree is not

firestore/src/android/field_value_android.cc

@@ -112,8 +112,7 @@ FieldValueInternal::FieldValueInternal(const uint8_t* value, size_t size)
 }
 
 FieldValueInternal::FieldValueInternal(DocumentReference value)
-    : Wrapper(firebase::Move(*value.internal_)),
-      cached_type_(Type::kReference) {}
+    : Wrapper(value.internal_), cached_type_{Type::kReference} {}
 
 FieldValueInternal::FieldValueInternal(GeoPoint value)
     : cached_type_(Type::kGeoPoint) {
@@ -133,7 +132,8 @@ FieldValueInternal::FieldValueInternal(std::vector<FieldValue> value)
   jmethodID add_method = util::array_list::GetMethodId(util::array_list::kAdd);
   for (const FieldValue& element : value) {
     // ArrayList.Add() always returns true, which we have no use for.
-    env->CallBooleanMethod(obj_, add_method, element.internal_->obj_);
+    // TODO(b/150016438): don't conflate invalid `FieldValue`s and null.
+    env->CallBooleanMethod(obj_, add_method, TryGetJobject(element));
   }
   CheckAndClearJniExceptions(env);
 }
@@ -148,7 +148,8 @@ FieldValueInternal::FieldValueInternal(MapFieldValue value)
     jobject key = env->NewStringUTF(kv.first.c_str());
     // Map::Put() returns previously associated value or null, which we have no
     // use for.
-    env->CallObjectMethod(obj_, put_method, key, kv.second.internal_->obj_);
+    // TODO(b/150016438): don't conflate invalid `FieldValue`s and null.
+    env->CallObjectMethod(obj_, put_method, key, TryGetJobject(kv.second));
     env->DeleteLocalRef(key);
   }
   CheckAndClearJniExceptions(env);
@@ -345,6 +346,10 @@ DocumentReference FieldValueInternal::reference_value() const {
     FIREBASE_ASSERT(cached_type_ == Type::kReference);
   }
 
+  if (!obj_) {
+    // Reference may be invalid.
+    return DocumentReference{};
+  }
   return DocumentReference{new DocumentReferenceInternal(firestore_, obj_)};
 }
 
@@ -585,5 +590,9 @@ bool operator==(const FieldValueInternal& lhs, const FieldValueInternal& rhs) {
   return lhs.EqualsJavaObject(rhs);
 }
 
+jobject FieldValueInternal::TryGetJobject(const FieldValue& value) {
+  return value.internal_ ? value.internal_->obj_ : nullptr;
+}
+
 }  // namespace firestore
 }  // namespace firebase

firestore/src/android/field_value_android.h

@@ -70,6 +70,8 @@ class FieldValueInternal : public Wrapper {
   static bool Initialize(App* app);
   static void Terminate(App* app);
 
+  static jobject TryGetJobject(const FieldValue& value);
+
   static jobject delete_;
   static jobject server_timestamp_;
 

firestore/src/android/wrapper.cc

@@ -47,7 +47,7 @@ Wrapper::Wrapper(jclass clazz, jmethodID method_id, ...) {
 Wrapper::Wrapper(const Wrapper& wrapper)
     : Wrapper(wrapper.firestore_, wrapper.obj_, AllowNullObject::Yes) {}
 
-Wrapper::Wrapper(Wrapper&& wrapper)
+Wrapper::Wrapper(Wrapper&& wrapper) noexcept
     : firestore_(wrapper.firestore_), obj_(wrapper.obj_) {
   FIREBASE_ASSERT(firestore_ != nullptr);
   wrapper.obj_ = nullptr;
@@ -60,6 +60,14 @@ Wrapper::Wrapper() : obj_(nullptr) {
   FIREBASE_ASSERT(firestore_ != nullptr);
 }
 
+Wrapper::Wrapper(Wrapper* rhs) : Wrapper() {
+  if (rhs) {
+    firestore_ = rhs->firestore_;
+    FIREBASE_ASSERT(firestore_ != nullptr);
+    obj_ = firestore_->app()->GetJNIEnv()->NewGlobalRef(rhs->obj_);
+  }
+}
+
 Wrapper::Wrapper(FirestoreInternal* firestore, jobject obj, AllowNullObject)
     : firestore_(firestore),
       // NewGlobalRef() is supposed to accept Null Java object and return Null,

firestore/src/android/wrapper.h

@@ -37,7 +37,7 @@ class Wrapper {
 
   Wrapper(const Wrapper& wrapper);
 
-  Wrapper(Wrapper&& wrapper);
+  Wrapper(Wrapper&& wrapper) noexcept;
 
   virtual ~Wrapper();
 
@@ -62,6 +62,9 @@ class Wrapper {
   // Java object a.k.a. nullptr.
   Wrapper(FirestoreInternal* firestore, jobject obj, AllowNullObject);
 
+  // Similar to a copy constructor, but can handle the case where `rhs` is null.
+  explicit Wrapper(Wrapper* rhs);
+
   // Converts a java list of java type e.g. java.util.List<FirestoreJavaType> to
   // a C++ vector of equivalent type e.g. std::vector<FirestoreType>.
   template <typename FirestoreType, typename FirestoreTypeInternal>
@@ -100,8 +103,8 @@ class Wrapper {
       FirestoreInternal* firestore, MapFieldPathValue::const_iterator begin,
       MapFieldPathValue::const_iterator end);
 
-  FirestoreInternal* firestore_;  // not owning
-  jobject obj_;
+  FirestoreInternal* firestore_ = nullptr;  // not owning
+  jobject obj_ = nullptr;
 
  private:
   friend class FirestoreInternal;

firestore/src/include/firebase/firestore.h

@@ -51,6 +51,12 @@
 #include "firebase/firestore/timestamp.h"
 
 namespace firebase {
+/**
+ * @brief Cloud Firestore API.
+ *
+ * Cloud Firestore is a flexible, scalable database for mobile, web, and server
+ * development from Firebase and Google Cloud Platform.
+ */
 namespace firestore {
 
 class FirestoreInternal;

firestore/src/include/firebase/firestore/event_listener.h

@@ -22,7 +22,12 @@
 namespace firebase {
 namespace firestore {
 
-/** An interface for event listeners. */
+/**
+ * An interface for event listeners.
+ *
+ * @note This class should only be used when using the STLPort C++ runtime
+ * library.
+ */
 template <typename T>
 class EventListener {
  public:
@@ -40,13 +45,18 @@ class EventListener {
   virtual void OnEvent(const T& value, Error error) = 0;
 };
 
-/** Interface used for void EventListeners that don't produce a value. */
+/**
+ * Interface used for void EventListeners that don't produce a value.
+ *
+ * @note This class should only be used when using the STLPort C++ runtime
+ * library.
+ */
 template <>
 class EventListener<void> {
  public:
   virtual ~EventListener() = default;
 
-   /**
+  /**
    * @brief OnEvent will be called with the error if an error occurred.
    *
    * @param error The error if there was error. Error::Ok otherwise.

firestore/src/include/firebase/firestore/map_field_value.h

@@ -27,10 +27,14 @@ class FieldPath;
 class FieldValue;
 
 #ifdef STLPORT
+/** @brief A map of `FieldValue`s indexed by stringified field paths. */
 using MapFieldValue = std::tr1::unordered_map<std::string, FieldValue>;
+/** @brief A map of `FieldValue`s indexed by field paths. */
 using MapFieldPathValue = std::tr1::unordered_map<FieldPath, FieldValue>;
 #else
+/** @brief A map of `FieldValue`s indexed by stringified field paths. */
 using MapFieldValue = std::unordered_map<std::string, FieldValue>;
+/** @brief A map of `FieldValue`s indexed by field paths. */
 using MapFieldPathValue = std::unordered_map<FieldPath, FieldValue>;
 #endif