From 1c6f7c0f8b6a7f990691a61d99030d676b066d47 Mon Sep 17 00:00:00 2001
From: Michael Lehenbauer <mikelehen@google.com>
Date: Fri, 23 Mar 2018 16:26:57 -0700
Subject: [PATCH 1/2] Fix issue that could cause offline get()s to wait up to
 10 seconds.

OnlineStateTracker was reverting to OnlineState Unknown on every stream attempt
rather than remaining Offline once the offline heuristic had been met (i.e. 2
stream failures or 10 seconds). This means that get() requests made while
offline could be delayed up to 10 seconds each time (or until the next backoff
attempt failed).
---
 packages/firestore/CHANGELOG.md               |  5 +++
 .../src/remote/online_state_tracker.ts        | 20 ++++++---
 .../test/unit/specs/offline_spec.test.ts      | 43 +++++++++++++++++++
 3 files changed, 63 insertions(+), 5 deletions(-)

diff --git a/packages/firestore/CHANGELOG.md b/packages/firestore/CHANGELOG.md
index 24f75bf6ee2..529caa6d757 100644
--- a/packages/firestore/CHANGELOG.md
+++ b/packages/firestore/CHANGELOG.md
@@ -1,4 +1,9 @@
 # Unreleased
+- [fixed] Fixed a regression in the Firebase JS release 4.11.0 that could
+  cause get() requests made while offline to be delayed by up to 10
+  seconds (rather than returning from cache immediately).
+
+# 0.3.6
 - [fixed] Fixed a regression in the Firebase JS release 4.11.0 that could
   cause a crash if a user signs out while the client is offline, resulting in
   an error of "Attempted to schedule multiple operations with timer id
diff --git a/packages/firestore/src/remote/online_state_tracker.ts b/packages/firestore/src/remote/online_state_tracker.ts
index 147f8d49323..31c12d85014 100644
--- a/packages/firestore/src/remote/online_state_tracker.ts
+++ b/packages/firestore/src/remote/online_state_tracker.ts
@@ -74,15 +74,20 @@ export class OnlineStateTracker {
   ) {}
 
   /**
-   * Called by RemoteStore when a watch stream is started.
+   * Called by RemoteStore when a watch stream is started (including on each
+   * backoff attempt).
    *
-   * It sets the OnlineState to Unknown and starts the onlineStateTimer
-   * if necessary.
+   * It this is the first attempt, it sets the OnlineState to Unknown and starts
+   * the onlineStateTimer.
    */
   handleWatchStreamStart(): void {
-    this.setAndBroadcast(OnlineState.Unknown);
+    if (this.watchStreamFailures === 0) {
+      this.setAndBroadcast(OnlineState.Unknown);
 
-    if (this.onlineStateTimer === null) {
+      assert(
+        this.onlineStateTimer === null,
+        `onlineStateTimer shouldn't be started yet`
+      );
       this.onlineStateTimer = this.asyncQueue.enqueueAfterDelay(
         TimerId.OnlineStateTimeout,
         ONLINE_STATE_TIMEOUT_MS,
@@ -119,6 +124,11 @@ export class OnlineStateTracker {
   handleWatchStreamFailure(): void {
     if (this.state === OnlineState.Online) {
       this.setAndBroadcast(OnlineState.Unknown);
+
+      // To get to OnlineState.Online, set() must have been called which would
+      // have reset our heuristics.
+      assert(this.watchStreamFailures === 0, 'watchStreamFailures must be 0');
+      assert(this.onlineStateTimer === null, 'onlineStateTimer must be null');
     } else {
       this.watchStreamFailures++;
       if (this.watchStreamFailures >= MAX_WATCH_STREAM_FAILURES) {
diff --git a/packages/firestore/test/unit/specs/offline_spec.test.ts b/packages/firestore/test/unit/specs/offline_spec.test.ts
index af23a2ba614..2e6b913619f 100644
--- a/packages/firestore/test/unit/specs/offline_spec.test.ts
+++ b/packages/firestore/test/unit/specs/offline_spec.test.ts
@@ -181,4 +181,47 @@ describeSpec('Offline:', [], () => {
         })
     );
   });
+
+  specTest(
+    'New queries return immediately with fromCache=true when offline due to ' +
+      'stream failures.',
+    [],
+    () => {
+      const query1 = Query.atPath(path('collection'));
+      const query2 = Query.atPath(path('collection2'));
+      return (
+        spec()
+          .userListens(query1)
+          // 2 Failures should mark the client offline and trigger an empty
+          // fromCache event.
+          .watchStreamCloses(Code.UNAVAILABLE)
+          .watchStreamCloses(Code.UNAVAILABLE)
+          .expectEvents(query1, { fromCache: true })
+
+          // A new query should immediately return from cache.
+          .userListens(query2)
+          .expectEvents(query2, { fromCache: true })
+      );
+    }
+  );
+
+  specTest(
+    'New queries return immediately with fromCache=true when offline due to ' +
+      'OnlineState timeout.',
+    [],
+    () => {
+      const query1 = Query.atPath(path('collection'));
+      const query2 = Query.atPath(path('collection2'));
+      return (
+        spec()
+          .userListens(query1)
+          .runTimer(TimerId.OnlineStateTimeout)
+          .expectEvents(query1, { fromCache: true })
+
+          // A new query should immediately return from cache.
+          .userListens(query2)
+          .expectEvents(query2, { fromCache: true })
+      );
+    }
+  );
 });

From e50c10d54e9249eb7c6ccef8b23893def55dca34 Mon Sep 17 00:00:00 2001
From: Michael Lehenbauer <mikelehen@google.com>
Date: Fri, 23 Mar 2018 17:12:29 -0700
Subject: [PATCH 2/2] fix typo

---
 packages/firestore/src/remote/online_state_tracker.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/firestore/src/remote/online_state_tracker.ts b/packages/firestore/src/remote/online_state_tracker.ts
index 31c12d85014..881f9e9e265 100644
--- a/packages/firestore/src/remote/online_state_tracker.ts
+++ b/packages/firestore/src/remote/online_state_tracker.ts
@@ -77,7 +77,7 @@ export class OnlineStateTracker {
    * Called by RemoteStore when a watch stream is started (including on each
    * backoff attempt).
    *
-   * It this is the first attempt, it sets the OnlineState to Unknown and starts
+   * If this is the first attempt, it sets the OnlineState to Unknown and starts
    * the onlineStateTimer.
    */
   handleWatchStreamStart(): void {