Allow user to stop listening after shutdown + other minor fixes. (#701)

* Allow user to stop listening after shutdown + other minor fixes.

* fix comment as well

* add @hide to InstanceRegistry.

Author: wu-hui

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

@@ -1099,6 +1099,22 @@ public void testShutdownCalledMultipleTimes() {
     expectError(() -> waitFor(reference.get()), expectedMessage);
   }
 
+  @Test
+  public void testCanStopListeningAfterShutdown() {
+    FirebaseFirestore instance = testFirestore();
+    DocumentReference reference = instance.document("abc/123");
+    EventAccumulator<DocumentSnapshot> eventAccumulator = new EventAccumulator<>();
+    ListenerRegistration registration = reference.addSnapshotListener(eventAccumulator.listener());
+    eventAccumulator.await();
+
+    waitFor(instance.shutdown());
+
+    // This should proceed without error.
+    registration.remove();
+    // Multiple calls should proceed as an effectively no-op.
+    registration.remove();
+  }
+
   @Test
   public void testWaitForPendingWritesResolves() {
     DocumentReference documentReference = testCollection("abc").document("123");

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

@@ -49,7 +49,11 @@
  */
 public class FirebaseFirestore {
 
-  /** Provides a registry management interface for {@code FirebaseFirestore} instances. */
+  /**
+   * Provides a registry management interface for {@code FirebaseFirestore} instances.
+   *
+   * @hide
+   */
   public interface InstanceRegistry {
     /** Removes the Cloud Firestore instance with given name from registry. */
     void remove(@NonNull String databaseId);

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

@@ -93,23 +93,20 @@ public int addQueryListener(QueryListener queryListener) {
     return queryInfo.targetId;
   }
 
-  /** Removes a previously added listener and returns true if the listener was found. */
-  public boolean removeQueryListener(QueryListener listener) {
+  /** Removes a previously added listener. It's a no-op if the listener is not found. */
+  public void removeQueryListener(QueryListener listener) {
     Query query = listener.getQuery();
     QueryListenersInfo queryInfo = queries.get(query);
     boolean lastListen = false;
-    boolean found = false;
     if (queryInfo != null) {
-      found = queryInfo.listeners.remove(listener);
+      queryInfo.listeners.remove(listener);
       lastListen = queryInfo.listeners.isEmpty();
     }
 
     if (lastListen) {
       queries.remove(query);
       syncEngine.stopListening(query);
     }
-
-    return found;
   }
 
   @Override

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

@@ -165,7 +165,6 @@ public QueryListener listen(
 
   /** Stops listening to a query previously listened to. */
   public void stopListening(QueryListener listener) {
-    this.verifyNotShutdown();
     asyncQueue.enqueueAndForget(() -> eventManager.removeQueryListener(listener));
   }
 

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

@@ -336,6 +336,7 @@ public void handleRemoteEvent(RemoteEvent event) {
   /** Applies an OnlineState change to the sync engine and notifies any views of the change. */
   @Override
   public void handleOnlineStateChange(OnlineState onlineState) {
+    assertCallback("handleOnlineStateChange");
     ArrayList<ViewSnapshot> newViewSnapshots = new ArrayList<>();
     for (Map.Entry<Query, QueryView> entry : queryViewsByQuery.entrySet()) {
       View view = entry.getValue().getView();