Review feedback

Author: wilhuff

Firestore/Example/Tests/SpecTests/FSTSyncEngineTestDriver.mm

@@ -413,7 +413,7 @@ - (void)receiveWatchStreamError:(int)errorCode userInfo:(NSDictionary<NSString *
   _workerQueue->EnqueueBlocking([&] {
     _datastore->FailWatchStream(error);
     // Unlike web, stream should re-open synchronously (if we have any listeners)
-    if (_queryListeners.size() > 0) {
+    if (!_queryListeners.empty()) {
       HARD_ASSERT(_datastore->IsWatchStreamOpen(), "Watch stream is open");
     }
   });

Firestore/Source/Core/FSTEventManager.mm

@@ -56,7 +56,6 @@
 
   void Erase(const std::shared_ptr<QueryListener> &listener) {
     auto found = absl::c_find(listeners, listener);
-    HARD_ASSERT(found != listeners.end(), "Shouldn't remove listeners that don't exist");
     if (found != listeners.end()) {
       listeners.erase(found);
     }
@@ -71,7 +70,8 @@ void set_view_snapshot(const absl::optional<ViewSnapshot> &snapshot) {
   }
 
  private:
-  // Enforce constness of this ViewSnapshot
+  // Other members are public in this struct, ensure that any reads are
+  // copies by requiring reads to go through a const getter.
   absl::optional<ViewSnapshot> snapshot_;
 };
 
@@ -107,7 +107,7 @@ - (instancetype)initWithSyncEngine:(FSTSyncEngine *)syncEngine {
 - (TargetId)addListener:(std::shared_ptr<QueryListener>)listener {
   FSTQuery *query = listener->query();
 
-  auto inserted = _queries.insert({query, {}});
+  auto inserted = _queries.emplace(query, QueryListenersInfo{});
   bool first_listen = inserted.second;
   QueryListenersInfo &query_info = inserted.first->second;
 
@@ -127,13 +127,13 @@ - (TargetId)addListener:(std::shared_ptr<QueryListener>)listener {
 
 - (void)removeListener:(const std::shared_ptr<QueryListener> &)listener {
   FSTQuery *query = listener->query();
-  bool last_listen = NO;
+  bool last_listen = false;
 
   auto found_iter = _queries.find(query);
   if (found_iter != _queries.end()) {
     QueryListenersInfo &query_info = found_iter->second;
     query_info.Erase(listener);
-    last_listen = (query_info.listeners.empty());
+    last_listen = query_info.listeners.empty();
   }
 
   if (last_listen) {

Firestore/core/src/firebase/firestore/core/query_listener.h

@@ -1,5 +1,5 @@
 /*
- * Copyright 2017 Google
+ * Copyright 2019 Google
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -88,7 +88,7 @@ class QueryListener {
   bool ShouldRaiseEvent(const ViewSnapshot& snapshot) const;
   void RaiseInitialEvent(const ViewSnapshot& snapshot);
 
-  FSTQuery* query_;
+  FSTQuery* query_ = nil;
   ListenOptions options_;
 
   /** The ViewSnapshotHandler associated with this query listener. */

Firestore/core/src/firebase/firestore/core/query_listener.mm

@@ -1,5 +1,5 @@
 /*
- * Copyright 2017 Google
+ * Copyright 2019 Google
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -93,7 +93,7 @@
     return true;
   }
 
-  // NOTE: We consider OnlineState.Unknown as online (it should become Offline
+  // NOTE: We consider OnlineState::Unknown as online (it should become Offline
   // or Online if we wait long enough).
   bool maybe_online = online_state != OnlineState::Offline;
 
@@ -110,7 +110,7 @@
 }
 
 bool QueryListener::ShouldRaiseEvent(const ViewSnapshot& snapshot) const {
-  // We don't need to handle includeDocumentMetadataChanges here because the
+  // We don't need to handle include_document_metadata_changes() here because the
   // Metadata only changes have already been stripped out if needed. At this
   // point the only changes we will see are the ones we should propagate.
   if (!snapshot.document_changes().empty()) {
@@ -132,6 +132,7 @@
 void QueryListener::RaiseInitialEvent(const ViewSnapshot& snapshot) {
   HARD_ASSERT(!raised_initial_event_,
               "Trying to raise initial events for second time");
+
   ViewSnapshot modified_snapshot = ViewSnapshot::FromInitialDocuments(
       snapshot.query(), snapshot.documents(), snapshot.mutated_keys(),
       snapshot.from_cache(), snapshot.excludes_metadata_changes());