Tweak naming / comments based on CR feedback.

Author: Michael Lehenbauer

packages/firestore/src/remote/online_state_tracker.ts

@@ -23,21 +23,22 @@ import { CancelablePromise } from '../util/promise';
 const LOG_TAG = 'OnlineStateTracker';
 
 // To deal with transient failures, we allow multiple stream attempts before
-// giving up and transitioning to Offline.
+// giving up and transitioning from OnlineState.Unknown to Offline.
 const MAX_WATCH_STREAM_FAILURES = 2;
 
 // To deal with stream attempts that don't succeed or fail in a timely manner,
-// we have a maximum timeout we'll wait for the stream to either succeed or fail
-// MAX_WATCH_STREAM_FAILURES times, else we revert to OnlineState.Offline.
-const MAX_WATCH_STREAM_TIMEOUT_MS = 10 * 1000;
+// we have a timeout for the stream to either succeed or fail
+// (MAX_WATCH_STREAM_FAILURES times). If the timeout is reached, we transition
+// from OnlineState.Unknown to Offline, rather than waiting indefinitely.
+const WATCH_STREAM_TIMEOUT_MS = 10 * 1000;
 
 /**
  * A component used by the RemoteStore to track the OnlineState (that is,
  * whether or not the client as a whole should be considered to be online or
  * offline), implementing the appropriate heuristics.
  *
  * In particular, when the client is trying to connect to the backend, we
- * allow up to MAX_WATCH_STREAM_FAILURES within MAX_WATCH_STREAM_TIMEOUT_MS for
+ * allow up to MAX_WATCH_STREAM_FAILURES within WATCH_STREAM_TIMEOUT_MS for
  * a connection to succeed. If we have too many failures or the timeout elapses,
  * then we set the OnlineState to Offline, and the client will behave as if
  * it is offline (get()s will return cached data, etc.).
@@ -54,11 +55,11 @@ export class OnlineStateTracker {
   private watchStreamFailures = 0;
 
   /**
-   * A timer that elapses after MAX_WATCH_STREAM_TIMEOUT_MS, at which point we
-   * revert to OnlineState.Offline without waiting for the stream to actually
-   * fail (MAX_WATCH_STREAM_FAILURES times).
+   * A timer that elapses after WATCH_STREAM_TIMEOUT_MS, at which point we
+   * transition from OnlineState.Unknown to OnlineState.Offline without waiting
+   * for the stream to actually fail (MAX_WATCH_STREAM_FAILURES times).
    */
-  private watchStreamTimerPromise: CancelablePromise<void> | null = null;
+  private watchStreamTimeout: CancelablePromise<void> | null = null;
 
   /**
    * Whether the client should log a warning message if it fails to connect to
@@ -75,26 +76,26 @@ export class OnlineStateTracker {
   /**
    * Called by RemoteStore when a watch stream is started.
    *
-   * It sets the OnlineState to Unknown and starts a MAX_WATCH_STREAM_TIMEOUT_MS
+   * It sets the OnlineState to Unknown and starts a WATCH_STREAM_TIMEOUT_MS
    * timer if necessary.
    */
   handleWatchStreamStart(): void {
     this.setAndBroadcast(OnlineState.Unknown);
 
-    if (this.watchStreamTimerPromise === null) {
-      this.watchStreamTimerPromise = this.asyncQueue.enqueueAfterDelay(
+    if (this.watchStreamTimeout === null) {
+      this.watchStreamTimeout = this.asyncQueue.enqueueAfterDelay(
         TimerId.OnlineStateTimeout,
-        MAX_WATCH_STREAM_TIMEOUT_MS,
+        WATCH_STREAM_TIMEOUT_MS,
         () => {
-          this.watchStreamTimerPromise = null;
+          this.watchStreamTimeout = null;
           assert(
             this.state === OnlineState.Unknown,
             'Timer should be canceled if we transitioned to a different state.'
           );
           log.debug(
             LOG_TAG,
             `Watch stream didn't reach online or offline within ` +
-              `${MAX_WATCH_STREAM_TIMEOUT_MS}ms. Considering client offline.`
+              `${WATCH_STREAM_TIMEOUT_MS}ms. Considering client offline.`
           );
           this.logClientOfflineWarningIfNecessary();
           this.setAndBroadcast(OnlineState.Offline);
@@ -158,9 +159,9 @@ export class OnlineStateTracker {
   }
 
   private clearWatchStreamTimer(): void {
-    if (this.watchStreamTimerPromise !== null) {
-      this.watchStreamTimerPromise.cancel();
-      this.watchStreamTimerPromise = null;
+    if (this.watchStreamTimeout !== null) {
+      this.watchStreamTimeout.cancel();
+      this.watchStreamTimeout = null;
     }
   }
 }

packages/firestore/src/util/async_queue.ts

@@ -31,12 +31,25 @@ type TimerHandle = any;
  * The string values are used when encoding these timer IDs in JSON spec tests.
  */
 export enum TimerId {
-  // All can be used with runDelayedOperationsEarly() to run all timers.
+  /** All can be used with runDelayedOperationsEarly() to run all timers. */
   All = 'all',
+
+  /**
+   * The following 4 timers are used in persistent_stream.ts for the listen and
+   * write streams. The "Idle" timer is used to close the stream due to
+   * inactivity. The "Connection" timer is used to start a stream once the
+   * appropriate backoff delay has elapsed.
+   */
   ListenStreamIdle = 'listen_stream_idle',
   ListenStreamConnection = 'listen_stream_connection',
   WriteStreamIdle = 'write_stream_idle',
   WriteStreamConnection = 'write_stream_connection',
+
+  /**
+   * A timer used in online_state_tracker.ts to transition from
+   * OnlineState.Unknown to Offline after a set timeout, rather than waiting
+   * indefinitely for success or failure.
+   */
   OnlineStateTimeout = 'online_state_timeout'
 }