Fix according to PR comments

Author: tom-andersen

packages/firestore/src/core/component_provider.ts

@@ -126,7 +126,7 @@ export class MemoryOfflineComponentProvider
       cfg,
       this.localStore
     );
-    this.indexBackfillerScheduler = this.createIndexBackfiller(
+    this.indexBackfillerScheduler = this.createIndexBackfillerScheduler(
       cfg,
       this.localStore
     );
@@ -139,7 +139,7 @@ export class MemoryOfflineComponentProvider
     return null;
   }
 
-  createIndexBackfiller(
+  createIndexBackfillerScheduler(
     cfg: ComponentConfiguration,
     localStore: LocalStore
   ): Scheduler | null {
@@ -236,7 +236,7 @@ export class IndexedDbOfflineComponentProvider extends MemoryOfflineComponentPro
     return new LruScheduler(garbageCollector, cfg.asyncQueue, localStore);
   }
 
-  createIndexBackfiller(
+  createIndexBackfillerScheduler(
     cfg: ComponentConfiguration,
     localStore: LocalStore
   ): Scheduler | null {

packages/firestore/src/local/index_backfiller.ts

@@ -42,6 +42,7 @@ const REGULAR_BACKFILL_DELAY_MS = 1;
 /** The maximum number of documents to process each time backfill() is called. */
 const MAX_DOCUMENTS_TO_PROCESS = 50;
 
+/** This class is responsible for the scheduling of Index Backfiller. */
 export class IndexBackfillerScheduler implements Scheduler {
   private task: DelayedOperation<void> | null;
 
@@ -102,11 +103,13 @@ export class IndexBackfillerScheduler implements Scheduler {
   }
 }
 
+/** Implements the steps for backfilling indexes. */
 export class IndexBackfiller {
   constructor(
     private readonly localStore: LocalStore,
     private readonly persistence: Persistence
   ) {}
+
   async backfill(
     maxDocumentsToProcess: number = MAX_DOCUMENTS_TO_PROCESS
   ): Promise<number> {
@@ -125,7 +128,7 @@ export class IndexBackfiller {
     const processedCollectionGroups = new Set<string>();
     let documentsRemaining = maxDocumentsToProcess;
     let continueLoop = true;
-    return PersistencePromise.loopUntil(
+    return PersistencePromise.doWhile(
       () => continueLoop === true && documentsRemaining > 0,
       () => {
         return this.localStore.indexManager

packages/firestore/src/local/index_manager.ts

@@ -165,6 +165,7 @@ export interface IndexManager {
     target: Target
   ): PersistencePromise<IndexOffset>;
 
+  /** Returns the minimum offset for the given collection group. */
   getMinOffsetFromCollectionGroup(
     transaction: PersistenceTransaction,
     collectionGroup: string

packages/firestore/src/local/indexeddb_index_manager.ts

@@ -51,7 +51,7 @@ import { FieldPath, ResourcePath } from '../model/path';
 import { TargetIndexMatcher } from '../model/target_index_matcher';
 import { isArray, refValue } from '../model/values';
 import { Value as ProtoValue } from '../protos/firestore_proto_api';
-import { debugAssert } from '../util/assert';
+import {debugAssert, hardAssert} from '../util/assert';
 import { logDebug } from '../util/log';
 import { immediateSuccessor, primitiveComparator } from '../util/misc';
 import { ObjectMap } from '../util/obj_map';
@@ -199,8 +199,6 @@ export class IndexedDbIndexManager implements IndexManager {
     if (index.indexState) {
       const states = indexStateStore(transaction);
       return result.next(indexId => {
-        if (index.indexState) {
-        }
         states.put(
           toDbIndexState(
             indexId,
@@ -972,12 +970,15 @@ export class IndexedDbIndexManager implements IndexManager {
     transaction: PersistenceTransaction,
     collectionGroup: string
   ): PersistencePromise<IndexOffset> {
-    return this.getFieldIndexes(transaction, collectionGroup).next(
-      this.getMinOffsetFromFieldIndexes
-    );
+    return this.getFieldIndexes(transaction, collectionGroup)
+      .next(fieldIndexes => this.getMinOffsetFromFieldIndexes(fieldIndexes));
   }
 
   getMinOffsetFromFieldIndexes(fieldIndexes: FieldIndex[]): IndexOffset {
+    hardAssert(
+      fieldIndexes.length !== 0,
+      "Found empty index group when looking for least recent index offset.");
+
     let minOffset: IndexOffset = fieldIndexes[0].indexState.offset;
     let maxBatchId: number = minOffset.largestBatchId;
     for (const fieldIndex of fieldIndexes) {
@@ -1003,8 +1004,8 @@ export class IndexedDbIndexManager implements IndexManager {
     let offset: IndexOffset | undefined;
     return PersistencePromise.forEach(
       this.getSubTargets(target),
-      (target: Target) => {
-        return this.getFieldIndex(transaction, target).next(index => {
+      (subTarget: Target) => {
+        return this.getFieldIndex(transaction, subTarget).next(index => {
           if (!index) {
             offset = IndexOffset.min();
           } else if (

packages/firestore/src/local/persistence.ts

@@ -253,7 +253,7 @@ export interface Persistence {
 }
 
 /**
- * Interface to control LRU and IndexBackfiller.
+ * Interface to schedule periodic tasks within SDK.
  */
 export interface Scheduler {
   readonly started: boolean;

packages/firestore/src/local/persistence_promise.ts

@@ -248,24 +248,21 @@ export class PersistencePromise<T> {
    * An alternative to recursive PersistencePromise calls, that avoids
    * potential memory problems from unbounded chains of promises.
    *
-   * The `action` will be called first, then `condition`. If `condition`
-   * return `true`, the process repeats, until `condition` returns false.
-   * @param condition
-   * @param action
+   * The `action` will be called repeatedly while `condition` is true.
    */
-  static loopUntil(
+  static doWhile(
     condition: () => boolean,
     action: () => PersistencePromise<void>
   ): PersistencePromise<void> {
     return new PersistencePromise<void>((resolve, reject) => {
-      const process = (): void => {
-        action().next(() => {
-          if (condition() === true) {
+      const process = () => {
+        if (condition() === true) {
+          action().next(() => {
             process();
-          } else {
-            resolve();
-          }
-        }, reject);
+          }, reject)
+        } else {
+          resolve();
+        }
       };
       process();
     });

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

@@ -160,7 +160,7 @@ export class MockMultiTabOfflineComponentProvider extends MultiTabOfflineCompone
     return null;
   }
 
-  createIndexBackfiller(
+  createIndexBackfillerScheduler(
     cfg: ComponentConfiguration,
     localStore: LocalStore
   ): Scheduler | null {