Offline get() improvements.

[Port of firebase/firebase-js-sdk#1133]

1. Changed MAX_WATCH_STREAM_FAILURES to 1 per conversation with @wilhuff and
   @jdimond.
2. Fixed a "race" where when a get() triggered a watch stream error, we'd
   restart the stream, then get() would remove its listener, and so we had no
   listener left once the watch stream was "opened". Without a listen to send
   we'd be stuck in Offline state (even though we should be Unknown whenever
   we have no listens to send), resulting in the next get() returning data
   from cache without even attempting to reach the backend.

Author: Michael Lehenbauer

Firestore/Example/Tests/SpecTests/json/offline_spec_test.json

@@ -30,15 +30,6 @@
           }
         }
       },
-      {
-        "watchStreamClose": {
-          "error": {
-            "code": 14,
-            "message": "Simulated Backend Error"
-          },
-          "runBackoffTimer": true
-        }
-      },
       {
         "watchStreamClose": {
           "error": {
@@ -125,15 +116,6 @@
           "runBackoffTimer": true
         }
       },
-      {
-        "watchStreamClose": {
-          "error": {
-            "code": 14,
-            "message": "Simulated Backend Error"
-          },
-          "runBackoffTimer": true
-        }
-      },
       {
         "watchStreamClose": {
           "error": {
@@ -209,15 +191,6 @@
           }
         }
       },
-      {
-        "watchStreamClose": {
-          "error": {
-            "code": 14,
-            "message": "Simulated Backend Error"
-          },
-          "runBackoffTimer": true
-        }
-      },
       {
         "watchStreamClose": {
           "error": {
@@ -283,15 +256,6 @@
           }
         }
       },
-      {
-        "watchStreamClose": {
-          "error": {
-            "code": 14,
-            "message": "Simulated Backend Error"
-          },
-          "runBackoffTimer": true
-        }
-      },
       {
         "watchStreamClose": {
           "error": {
@@ -422,15 +386,6 @@
           }
         }
       },
-      {
-        "watchStreamClose": {
-          "error": {
-            "code": 14,
-            "message": "Simulated Backend Error"
-          },
-          "runBackoffTimer": true
-        }
-      },
       {
         "watchStreamClose": {
           "error": {
@@ -669,15 +624,6 @@
           "runBackoffTimer": true
         }
       },
-      {
-        "watchStreamClose": {
-          "error": {
-            "code": 14,
-            "message": "Simulated Backend Error"
-          },
-          "runBackoffTimer": true
-        }
-      },
       {
         "watchAck": [
           2
@@ -957,15 +903,6 @@
           }
         }
       },
-      {
-        "watchStreamClose": {
-          "error": {
-            "code": 14,
-            "message": "Simulated Backend Error"
-          },
-          "runBackoffTimer": true
-        }
-      },
       {
         "watchStreamClose": {
           "error": {

Firestore/Example/Tests/SpecTests/json/remote_store_spec_test.json

@@ -517,7 +517,19 @@
             "message": "Simulated Backend Error"
           },
           "runBackoffTimer": true
-        }
+        },
+        "expect": [
+          {
+            "query": {
+              "path": "collection",
+              "filters": [],
+              "orderBys": []
+            },
+            "errorCode": 0,
+            "fromCache": true,
+            "hasPendingWrites": false
+          }
+        ]
       },
       {
         "watchAck": [
@@ -618,6 +630,18 @@
           },
           "runBackoffTimer": false
         },
+        "expect": [
+          {
+            "query": {
+              "path": "collection",
+              "filters": [],
+              "orderBys": []
+            },
+            "errorCode": 0,
+            "fromCache": true,
+            "hasPendingWrites": false
+          }
+        ],
         "stateExpect": {
           "activeTargets": {}
         }

Firestore/Source/Remote/FSTOnlineStateTracker.mm

@@ -27,7 +27,10 @@
 
 // To deal with transient failures, we allow multiple stream attempts before giving up and
 // transitioning from OnlineState Unknown to Offline.
-static const int kMaxWatchStreamFailures = 2;
+// TODO(mikelehen): This used to be set to 2 as a mitigation for b/66228394. @jdimond thinks that
+// bug is sufficiently fixed so that we can set this back to 1. If that works okay, we could
+// potentially remove this logic entirely.
+static const int kMaxWatchStreamFailures = 1;
 
 // To deal with stream attempts that don't succeed or fail in a timely manner, we have a
 // timeout for OnlineState to reach Online or Offline. If the timeout is reached, we transition

Firestore/Source/Remote/FSTRemoteStore.mm

@@ -252,13 +252,20 @@ - (void)sendWatchRequestWithQueryData:(FSTQueryData *)queryData {
 - (void)stopListeningToTargetID:(TargetId)targetID {
   FSTBoxedTargetID *targetKey = @(targetID);
   FSTQueryData *queryData = self.listenTargets[targetKey];
-  HARD_ASSERT(queryData, "unlistenToTarget: target not currently watched: %s", targetKey);
+  HARD_ASSERT(queryData, "stopListeningToTargetID: target not currently watched: %s", targetKey);
 
   [self.listenTargets removeObjectForKey:targetKey];
   if ([self isNetworkEnabled] && [self.watchStream isOpen]) {
     [self sendUnwatchRequestForTargetID:targetKey];
-    if ([self.listenTargets count] == 0) {
+  }
+  if ([self.listenTargets count] == 0) {
+    if ([self isNetworkEnabled] && [self.watchStream isOpen]) {
       [self.watchStream markIdle];
+    } else {
+      // Revert to OnlineState::Unknown if the watch stream is not open and we have no listeners,
+      // since without any listens to send we cannot confirm if the stream is healthy and upgrade
+      // to OnlineState::Online.
+      [self.onlineStateTracker updateState:OnlineState::Unknown];
     }
   }
 }