addressing comments #1

Author: wu-hui

firebase-firestore/src/androidTest/java/com/google/firebase/firestore/FirestoreTest.java

@@ -1062,8 +1062,7 @@ public void testAppDeleteLeadsToFirestoreShutdown() {
   @Test
   public void testNewOperationThrowsAfterFirestoreShutdown() {
     FirebaseFirestore instance = testFirestore();
-    final DocumentReference reference = instance.document("abc/123");
-    DocumentReference ref = reference;
+    DocumentReference reference = instance.document("abc/123");
     waitFor(reference.set(Collections.singletonMap("Field", 100)));
 
     instance.shutdown();
@@ -1080,4 +1079,21 @@ public void testNewOperationThrowsAfterFirestoreShutdown() {
         () -> waitFor(instance.runTransaction(transaction -> transaction.get(reference))),
         expectedMessage);
   }
+
+  @Test
+  public void testShutdownCalledMultipleTimes() {
+    FirebaseFirestore instance = testFirestore();
+    DocumentReference reference = instance.document("abc/123");
+    waitFor(reference.set(Collections.singletonMap("Field", 100)));
+
+    instance.shutdown();
+
+    final String expectedMessage = "The client has already been shutdown";
+    expectError(() -> waitFor(reference.get()), expectedMessage);
+
+    // Calling a second time should go through and change nothing.
+    instance.shutdown();
+
+    expectError(() -> waitFor(reference.get()), expectedMessage);
+  }
 }

firebase-firestore/src/main/java/com/google/firebase/firestore/FirebaseFirestore.java

@@ -120,8 +120,8 @@ static FirebaseFirestore newInstance(
     FirebaseFirestore firestore =
         new FirebaseFirestore(context, databaseId, persistenceKey, provider, queue, app);
     app.addLifecycleEventListener(
-        /* onDeleted */ (firebaseAppName, options) -> {
-          firestore.shutdown(/* fromAppDeletion */ true);
+        (firebaseAppName, options) -> {
+          firestore.shutdown(/* fromAppDeletion= */ true);
         });
     return firestore;
   }
@@ -346,30 +346,29 @@ Task<Void> shutdown(boolean fromAppDeletion) {
   }
 
   /**
-   * Shuts down this Firestore SDK instance.
+   * Shuts down this FirebaseFirestore instance.
    *
-   * <p>IMPORTANT: If your application is designed to run with Firestore the entire life cycle, it
-   * is not recommended to call `shutdown` explicitly, the shutting down happens naturally when the
-   * application itself is shut down.
+   * <p>After shutdown only the {@link #clearPersistence()} method may be used. Any other method
+   * will throw an {@link IllegalStateException}.
    *
-   * <p>For applications designed to run without Firestore from some point on (typically to save
-   * resources as much as possible), you may call this method.
+   * <p>To restart after shutdown, simply create a new instance of FirebaseFirestore with {@link
+   * #getInstance()} or {@link #getInstance(FirebaseApp)}.
    *
-   * <p>Once `shutdown` is called, trying to issue new reads/writes through this instance will
-   * result in {@link IllegalStateException}. Already requested reads/writes might stop resolving.
-   * Be sure to handle this well in your application if proceeding with Firestore shutting down is
-   * desired in your case.
+   * <p>Shutdown does not cancel any pending writes and any tasks that are awaiting a response from
+   * the server will not be resolved. The next time you start this instance, it will resume
+   * attempting to send these writes to the server.
    *
-   * <p>Note that {@link #clearPersistence()} is an exception, it is designed to run when the
-   * Firestore SDK instance is shutdown.
+   * <p>Note: Under normal circumstances, calling <code>shutdown()</code> is not required. This
+   * method is useful only when you want to force this instance to release all of its resources or
+   * in combination with {@link #clearPersistence} to ensure that all local state is destroyed
+   * between test runs.
    *
-   * <p>To re-start a Firestore SDK instance, simply create a new instance via {@link
-   * #getInstance()} or {@link #getInstance(FirebaseApp)}.
+   * @return A <code>Task</code> that is resolved when the instance has been successfully shut down.
    */
   @VisibleForTesting
-  // TODO: Make this public and remove @VisibleForTesting
+  // TODO(b/135755126): Make this public and remove @VisibleForTesting
   Task<Void> shutdown() {
-    return shutdown(/* fromAppDeletion */ false);
+    return shutdown(/* fromAppDeletion= */ false);
   }
 
   @VisibleForTesting

firebase-firestore/src/main/java/com/google/firebase/firestore/FirestoreMultiDbComponent.java

@@ -43,7 +43,7 @@ class FirestoreMultiDbComponent {
     this.authProvider = authProvider;
   }
 
-  /** Provides instances of Firestore for given database names. */
+  /** Provides instances of Firestore for given database IDs. */
   @NonNull
   synchronized FirebaseFirestore get(@NonNull String databaseId) {
     FirebaseFirestore firestore = instances.get(databaseId);
@@ -55,7 +55,7 @@ synchronized FirebaseFirestore get(@NonNull String databaseId) {
   }
 
   /**
-   * Remove the instance of a given database name from this component, such that if {@link
+   * Remove the instance of a given database ID from this component, such that if {@link
    * FirestoreMultiDbComponent#get(String)} is called again with the same name, a new instance of
    * {@link FirebaseFirestore} is created.
    *

firebase-firestore/src/main/java/com/google/firebase/firestore/core/FirestoreClient.java

@@ -147,7 +147,7 @@ public Task<Void> shutdown() {
         });
   }
 
-  /** Has this client been shutdown. */
+  /** Returns true if this client has been shutdown. */
   public boolean isShutdown() {
     // Technically, the asyncQueue is still running, but only accepting tasks related to shutdown
     // or supposed to be run after shutdown. It is effectively shut down to the eyes of users.

firebase-firestore/src/main/java/com/google/firebase/firestore/util/AsyncQueue.java

@@ -178,14 +178,14 @@ public static <TResult> Task<TResult> callTask(Executor executor, Callable<Task<
   /**
    * A wrapper around a {@link ScheduledThreadPoolExecutor} class that provides:
    *
-   * <ul>
+   * <ol>
    *   <li>1. Synchronized task scheduling. This is different from function 3, which is about task
    *       execution in a single thread.
    *   <li>2. Ability to do soft-shutdown: only critical tasks related to shutting Firestore SDK
    *       down can be executed once the shutdown process initiated.
    *   <li>3. Single threaded execution service, no concurrent execution among the `Runnable`s
    *       scheduled in this Executor.
-   * </ul>
+   * </ol>
    */
   private class SynchronizedShutdownAwareExecutor implements Executor {
     /**