fix: JS viewports must allow subscription to be changed while waiting (…

…#6473)

Fixes DH-18128

web/client-api/src/main/java/io/deephaven/web/client/api/subscription/AbstractTableSubscription.java

@@ -64,6 +64,8 @@ public abstract class AbstractTableSubscription extends HasEventHandling {
     protected enum Status {
         /** Waiting for some prerequisite before we can use it for the first time. */
         STARTING,
+        /** All prerequisites are met, waiting for the first snapshot to be returned. */
+        SUBSCRIPTION_REQUESTED,
         /** Successfully created, not waiting for any messages to be accurate. */
         ACTIVE,
         /** Waiting for an update to return from being active to being active again. */
@@ -117,7 +119,11 @@ protected void revive() {
             WebBarrageSubscription.ViewportChangedHandler viewportChangedHandler = this::onViewportChange;
             WebBarrageSubscription.DataChangedHandler dataChangedHandler = this::onDataChanged;
 
-            status = Status.ACTIVE;
+            status = Status.SUBSCRIPTION_REQUESTED;
+
+            // In order to create the subscription, we need to already have the table resolved, so we know if it
+            // is a blink table or not. In turn, we can't be prepared to handle any messages from the server until
+            // we know this, so we can't race messages with this design.
             this.barrageSubscription = WebBarrageSubscription.subscribe(
                     subscriptionType, state, viewportChangedHandler, dataChangedHandler);
 
@@ -164,7 +170,8 @@ protected static FlatBufferBuilder subscriptionRequest(byte[] tableTicket, BitSe
 
     protected abstract void sendFirstSubscriptionRequest();
 
-    protected void sendBarrageSubscriptionRequest(RangeSet viewport, JsArray<Column> columns, Double updateIntervalMs,
+    protected void sendBarrageSubscriptionRequest(@Nullable RangeSet viewport, JsArray<Column> columns,
+            Double updateIntervalMs,
             boolean isReverseViewport) {
         if (isClosed()) {
             if (failMsg == null) {
@@ -173,7 +180,9 @@ protected void sendBarrageSubscriptionRequest(RangeSet viewport, JsArray<Column>
                 throw new IllegalStateException("Can't change subscription, already failed: " + failMsg);
             }
         }
-        status = Status.PENDING_UPDATE;
+        if (status == Status.ACTIVE) {
+            status = Status.PENDING_UPDATE;
+        }
         this.columns = columns;
         this.viewportRowSet = viewport;
         this.columnBitSet = makeColumnBitset(columns);

web/client-api/src/main/java/io/deephaven/web/client/api/subscription/TableViewportSubscription.java

@@ -248,6 +248,7 @@ public void setViewport(double firstRow, double lastRow, @JsOptional @JsNullable
 
     public void setInternalViewport(double firstRow, double lastRow, Column[] columns, Double updateIntervalMs,
             Boolean isReverseViewport) {
+        // Until we've created the stream, we just cache the requested viewport
         if (status == Status.STARTING) {
             this.firstRow = firstRow;
             this.lastRow = lastRow;

web/client-api/src/test/java/io/deephaven/web/client/api/subscription/ViewportTestGwt.java

@@ -93,6 +93,35 @@ public void testViewportOnStaticTable() {
                 .then(this::finish).catch_(this::report);
     }
 
+    public void testChangePendingViewport() {
+        connect(tables)
+                .then(table("staticTable"))
+                .then(table -> {
+                    delayTestFinish(5001);
+
+                    table.setViewport(0, 25, null);
+                    assertEquals(100.0, table.getSize());
+                    return Promise.resolve(table);
+                })
+                .then(table -> {
+                    assertEquals(100.0, table.getSize());
+                    table.setViewport(5, 30, null);
+                    assertEquals(100.0, table.getSize());
+                    return assertUpdateReceived(table, viewport -> {
+                        assertEquals(100.0, table.getSize());
+
+                        assertEquals(5d, viewport.getOffset());
+                        assertEquals(26, viewport.getRows().length);
+                    }, 2500);
+                })
+                .then(table -> {
+                    assertEquals(100.0, table.getSize());
+                    table.close();
+                    return null;
+                })
+                .then(this::finish).catch_(this::report);
+    }
+
     // TODO: https://deephaven.atlassian.net/browse/DH-11196
     public void ignore_testViewportOnGrowingTable() {
         connect(tables)