Fix b/72502745: OnlineState changes cause limbo document crash.

packages/firestore/src/core/view.ts

@@ -244,7 +244,8 @@ export class View {
       );
     });
 
-    const limboChanges = this.applyTargetChange(targetChange);
+    this.applyTargetChange(targetChange);
+    const limboChanges = this.updateLimboDocuments();
     const synced = this.limboDocuments.size === 0 && this.current;
     const newSyncState = synced ? SyncState.Synced : SyncState.Local;
     const syncStateChanged = newSyncState !== this.syncState;
@@ -320,9 +321,7 @@ export class View {
    * Updates syncedDocuments, current, and limbo docs based on the given change.
    * Returns the list of changes to which docs are in limbo.
    */
-  private applyTargetChange(
-    targetChange?: TargetChange
-  ): LimboDocumentChange[] {
+  private applyTargetChange(targetChange?: TargetChange): void {
     if (targetChange) {
       const targetMapping = targetChange.mapping;
       if (targetMapping instanceof ResetMapping) {
@@ -348,19 +347,23 @@ export class View {
           );
       }
     }
+  }
+
+  private updateLimboDocuments(): LimboDocumentChange[] {
+    // We can only determine limbo documents when we're in-sync with the server.
+    if (!this.current) {
+      return [];
+    }
 
-    // Recompute the set of limbo docs.
     // TODO(klimt): Do this incrementally so that it's not quadratic when
     // updating many documents.
     const oldLimboDocuments = this.limboDocuments;
     this.limboDocuments = documentKeySet();
-    if (this.current) {
-      this.documentSet.forEach(doc => {
-        if (this.shouldBeInLimbo(doc.key)) {
-          this.limboDocuments = this.limboDocuments.add(doc.key);
-        }
-      });
-    }
+    this.documentSet.forEach(doc => {
+      if (this.shouldBeInLimbo(doc.key)) {
+        this.limboDocuments = this.limboDocuments.add(doc.key);
+      }
+    });
 
     // Diff the new limbo docs with the old limbo docs.
     const changes: LimboDocumentChange[] = [];

packages/firestore/test/unit/specs/offline_spec.test.ts

@@ -115,4 +115,35 @@ describeSpec('Offline:', [], () => {
         .expectEvents(query, { fromCache: false })
     );
   });
+
+  specTest('Queries with limbo documents handle going offline.', [], () => {
+    const query = Query.atPath(path('collection'));
+    const docA = doc('collection/a', 1000, { key: 'a' });
+    const docB = doc('collection/b', 1005, { key: 'b' });
+    const limboQuery = Query.atPath(docA.key.path);
+    return (
+      spec()
+        .userListens(query)
+        .watchAcksFull(query, 1000, docA)
+        .expectEvents(query, { added: [docA] })
+        .watchResets(query)
+        // No more documents
+        .watchCurrents(query, 'resume-token-1001')
+        .watchSnapshots(1001)
+        // docA will now be in limbo (triggering fromCache=true)
+        .expectLimboDocs(docA.key)
+        .expectEvents(query, { fromCache: true })
+        // first error triggers unknown state
+        .watchStreamCloses(Code.UNAVAILABLE)
+        .restoreListen(query, 'resume-token-1001')
+        // getting two more errors triggers offline state.
+        .watchStreamCloses(Code.UNAVAILABLE)
+        .watchStreamCloses(Code.UNAVAILABLE)
+        .watchAcksFull(query, 1001)
+        .watchAcksFull(limboQuery, 1001)
+        // Limbo document is resolved. No longer from cache.
+        .expectEvents(query, { removed: [docA], fromCache: false })
+        .expectLimboDocs()
+    );
+  });
 });