From ecadbe380ca1b7e2eeada45b82e53d47e05ec9b3 Mon Sep 17 00:00:00 2001
From: Mila <107142260+milaGGL@users.noreply.github.com>
Date: Fri, 28 Jun 2024 15:09:57 -0400
Subject: [PATCH] Fix multi-tab snapshot listener metadata sync issue (#8339)

* Update sync_engine_impl.ts

* add spec test

* update comment

* typo

* update spec test

* add changeset
---
 .changeset/flat-scissors-suffer.md            |  6 ++++
 .../firestore/src/core/sync_engine_impl.ts    |  8 ++++-
 .../test/unit/specs/listen_spec.test.ts       | 36 +++++++++++++++++++
 3 files changed, 49 insertions(+), 1 deletion(-)
 create mode 100644 .changeset/flat-scissors-suffer.md

diff --git a/.changeset/flat-scissors-suffer.md b/.changeset/flat-scissors-suffer.md
new file mode 100644
index 00000000000..65334be9720
--- /dev/null
+++ b/.changeset/flat-scissors-suffer.md
@@ -0,0 +1,6 @@
+---
+'@firebase/firestore': patch
+'firebase': patch
+---
+
+Fix persistence multi-tab snapshot listener metadata sync issue.
diff --git a/packages/firestore/src/core/sync_engine_impl.ts b/packages/firestore/src/core/sync_engine_impl.ts
index bba07f4f4bc..1f93db4f8bc 100644
--- a/packages/firestore/src/core/sync_engine_impl.ts
+++ b/packages/firestore/src/core/sync_engine_impl.ts
@@ -1095,7 +1095,13 @@ export async function syncEngineEmitNewSnapsAndNotifyLocalStore(
           // secondary clients to update query state.
           if (viewSnapshot || remoteEvent) {
             if (syncEngineImpl.isPrimaryClient) {
-              const isCurrent = viewSnapshot && !viewSnapshot.fromCache;
+              // Query state is set to `current` if:
+              // - There is a view change and it is up-to-date, or,
+              // - There is a global snapshot, the Target is current, and no changes to be resolved
+              const isCurrent = viewSnapshot
+                ? !viewSnapshot.fromCache
+                : remoteEvent?.targetChanges.get(queryView.targetId)?.current;
+
               syncEngineImpl.sharedClientState.updateQueryState(
                 queryView.targetId,
                 isCurrent ? 'current' : 'not-current'
diff --git a/packages/firestore/test/unit/specs/listen_spec.test.ts b/packages/firestore/test/unit/specs/listen_spec.test.ts
index df006fb33ff..3404c4b4472 100644
--- a/packages/firestore/test/unit/specs/listen_spec.test.ts
+++ b/packages/firestore/test/unit/specs/listen_spec.test.ts
@@ -1895,4 +1895,40 @@ describeSpec('Listens:', [], () => {
         .restoreListen(query1, 'resume-token-1000', /* expectedCount= */ 1);
     }
   );
+
+  specTest(
+    'Global snapshots would not alter query state if there is no changes',
+    ['multi-client'],
+    () => {
+      const query1 = query('collection');
+      const docA = doc('collection/a', 1000, { key: 'a' });
+      return (
+        client(0)
+          .becomeVisible()
+          .expectPrimaryState(true)
+          // Populate the cache first
+          .userListens(query1)
+          .watchAcksFull(query1, 1000, docA)
+          .expectEvents(query1, { added: [docA] })
+          .userUnlistens(query1)
+          .watchRemoves(query1)
+          // Listen to the query in the primary client
+          .userListens(query1, { resumeToken: 'resume-token-1000' })
+          .expectEvents(query1, {
+            added: [docA],
+            fromCache: true
+          })
+          .watchAcksFull(query1, 2000, docA)
+          .expectEvents(query1, { fromCache: false })
+          // Reproduces: https://github.com/firebase/firebase-js-sdk/issues/8314
+          // Watch could send a global snapshot from time to time. If there are no view changes,
+          // the query should not be marked as "not-current" as the Target is up to date.
+          .watchSnapshots(3000, [], 'resume-token-3000')
+          // Listen to the query in the secondary tab. The snapshot is up to date.
+          .client(1)
+          .userListens(query1)
+          .expectEvents(query1, { added: [docA], fromCache: false })
+      );
+    }
+  );
 });