VisibilityCache: Use jgit's ReachabilityChecker

The current reachability check is not using bitmaps when they are
available.

Replace the local walk with a ReachabilityChecker, and choose the
implementation depending on whether the repository has bitmaps.

Change-Id: I6a822d129b64a1bd002aba343e4b1ce3b7f3fd2e
diff --git a/java/com/google/gitiles/VisibilityChecker.java b/java/com/google/gitiles/VisibilityChecker.java
index ccd5cf1..da45248 100644
--- a/java/com/google/gitiles/VisibilityChecker.java
+++ b/java/com/google/gitiles/VisibilityChecker.java
@@ -42,6 +42,7 @@
  */
 package com.google.gitiles;
 
+import com.google.common.collect.ImmutableList;
 import java.io.IOException;
 import java.util.Collection;
 import org.eclipse.jgit.errors.IncorrectObjectTypeException;
@@ -49,7 +50,6 @@
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.RefDatabase;
 import org.eclipse.jgit.revwalk.RevCommit;
-import org.eclipse.jgit.revwalk.RevSort;
 import org.eclipse.jgit.revwalk.RevWalk;
 
 /**
@@ -59,6 +59,8 @@
  */
 public class VisibilityChecker {
 
+  // TODO(ifrade): Right now we are using always topoSort, but we should respect this parameter
+  // or delete it.
   private final boolean topoSort;
 
   /**
@@ -90,7 +92,7 @@
    * @param walk The walk to use for the reachability check
    * @param commit The starting commit. It *MUST* come from the walk in use
    * @param starters visible commits. Anything reachable from these commits is visible. Missing ids
-   *     or ids pointing to wrong kind of objects are ignored.
+   *     or ids referring to other kinds of objects are ignored.
    * @return true if we can get to {@code commit} from the {@code starters}
    * @throws IOException a pack file or loose object could not be read
    */
@@ -101,28 +103,29 @@
       return false;
     }
 
-    walk.reset();
-    if (topoSort) {
-      walk.sort(RevSort.TOPO);
+    ImmutableList<RevCommit> startCommits = objectIdsToCommits(walk, starters);
+    if (startCommits.isEmpty()) {
+      return false;
     }
 
-    walk.markStart(commit);
-    for (ObjectId id : starters) {
-      markUninteresting(walk, id);
-    }
-    // If the commit is reachable from any given tip, it will appear to be
-    // uninteresting to the RevWalk and no output will be produced.
-    return walk.next() == null;
+    return !walk.createReachabilityChecker()
+        .areAllReachable(ImmutableList.of(commit), startCommits)
+        .isPresent();
   }
 
-  private static void markUninteresting(RevWalk walk, ObjectId id) throws IOException {
-    if (id == null) {
-      return;
+  private static ImmutableList<RevCommit> objectIdsToCommits(RevWalk walk, Collection<ObjectId> ids)
+      throws IOException {
+    ImmutableList.Builder<RevCommit> commits = ImmutableList.builder();
+    for (ObjectId id : ids) {
+      try {
+        commits.add(walk.parseCommit(id));
+      } catch (MissingObjectException e) {
+        // TODO(ifrade): ResolveParser has already checked that the object exists in the repo.
+        // Report as AssertionError.
+      } catch (IncorrectObjectTypeException e) {
+        // Ignore, doesn't affect commit reachability
+      }
     }
-    try {
-      walk.markUninteresting(walk.parseCommit(id));
-    } catch (IncorrectObjectTypeException | MissingObjectException e) {
-      // Do nothing, doesn't affect reachability.
-    }
+    return commits.build();
   }
 }