Changed behavior of failures in DialerFutures#firstMatching.

DialerFutures#firstMatching now fails if one of its provided futures fails before a match is found.

This is to accomodate the case where no source returns true for isDirty, but one or more fail; we want to be notified of the failure in that case and not silently treat that case as not dirty.

Also fixed a bug in NewCallLogFragment where the failed future wasn't causing the application to crash.

Also improved some related logging in RefreshAnnotatedCallLogWorker and Cp2PhoneLookup, and fixed a bug where empty numbers were not being handled correctly.

Bug: 71504246
Test: unit
PiperOrigin-RevId: 181401710
Change-Id: I23e207ac334ff80ac95b08a8f4f775a528a8c511

4 files changed, 34 insertions, 13 deletions

diff --git a/java/com/android/dialer/calllog/RefreshAnnotatedCallLogWorker.java b/java/com/android/dialer/calllog/RefreshAnnotatedCallLogWorker.java
index e5cc3eb89..4c5904ef1 100644
--- a/java/com/android/dialer/calllog/RefreshAnnotatedCallLogWorker.java
+++ b/java/com/android/dialer/calllog/RefreshAnnotatedCallLogWorker.java
@@ -117,10 +117,13 @@ public class RefreshAnnotatedCallLogWorker {
     // After determining isDirty, conditionally call rebuild.
     return Futures.transformAsync(
         isDirtyFuture,
-        isDirty ->
-            Preconditions.checkNotNull(isDirty)
-                ? rebuild(appContext)
-                : Futures.immediateFuture(null),
+        isDirty -> {
+          LogUtil.v(
+              "RefreshAnnotatedCallLogWorker.checkDirtyAndRebuildIfNecessary",
+              "isDirty: %b",
+              Preconditions.checkNotNull(isDirty));
+          return isDirty ? rebuild(appContext) : Futures.immediateFuture(null);
+        },
         lightweightExecutorService);
   }
 
diff --git a/java/com/android/dialer/calllog/ui/NewCallLogFragment.java b/java/com/android/dialer/calllog/ui/NewCallLogFragment.java
index e422b5f83..d0656a433 100644
--- a/java/com/android/dialer/calllog/ui/NewCallLogFragment.java
+++ b/java/com/android/dialer/calllog/ui/NewCallLogFragment.java
@@ -142,7 +142,12 @@ public final class NewCallLogFragment extends Fragment
                   ? refreshAnnotatedCallLogWorker.refreshWithDirtyCheck()
                   : refreshAnnotatedCallLogWorker.refreshWithoutDirtyCheck();
           refreshAnnotatedCallLogListener.listen(
-              getContext(), future, unused -> {}, RuntimeException::new);
+              getContext(),
+              future,
+              unused -> {},
+              throwable -> {
+                throw new RuntimeException(throwable);
+              });
         };
     ThreadUtil.getUiThreadHandler().postDelayed(refreshAnnotatedCallLogRunnable, WAIT_MILLIS);
   }
diff --git a/java/com/android/dialer/common/concurrent/DialerFutures.java b/java/com/android/dialer/common/concurrent/DialerFutures.java
index 98299823b..ac88e6aa1 100644
--- a/java/com/android/dialer/common/concurrent/DialerFutures.java
+++ b/java/com/android/dialer/common/concurrent/DialerFutures.java
@@ -35,7 +35,7 @@ public class DialerFutures {
    * predicate, cancelling all inputs upon completion. If none match, {@code defaultValue} is
    * returned.
    *
-   * <p>If an input fails, it is treated as if the predicate did not match.
+   * <p>If an input fails before a match is found, the returned future also fails.
    *
    * <p>Cancellation of the output future will cause cancellation of all input futures.
    *
@@ -53,7 +53,6 @@ public class DialerFutures {
       Predicate<T> predicate,
       T defaultValue) {
     AggregateFuture<T> output = new AnyOfFuture<>(futures);
-    // Use an atomic reference to ensure that late listeners don't pin the FirstOfFuture in memory.
     final AtomicReference<AggregateFuture<T>> ref = Atomics.newReference(output);
     final AtomicInteger pending = new AtomicInteger(output.futures.size());
     for (final ListenableFuture<? extends T> future : output.futures) {
@@ -65,14 +64,15 @@ public class DialerFutures {
               // cheaper than a CAS and atomicity is guaranteed by setFuture.
               AggregateFuture<T> output = ref.get();
               if (output != null) {
-                boolean threw = false;
                 T value = null;
                 try {
                   value = Futures.getDone(future);
                 } catch (ExecutionException e) {
-                  threw = true;
+                  ref.set(null); // unpin
+                  output.setException(e);
+                  return;
                 }
-                if (threw || !predicate.apply(value)) {
+                if (!predicate.apply(value)) {
                   if (pending.decrementAndGet() == 0) {
                     // we are the last future (and every other future hasn't matched or failed).
                     output.set(defaultValue);
@@ -108,6 +108,11 @@ public class DialerFutures {
     }
 
     @Override
+    protected boolean setException(Throwable throwable) {
+      return super.setException(throwable);
+    }
+
+    @Override
     protected boolean setFuture(ListenableFuture<? extends T> t) {
       return super.setFuture(t);
     }
diff --git a/java/com/android/dialer/phonelookup/cp2/Cp2PhoneLookup.java b/java/com/android/dialer/phonelookup/cp2/Cp2PhoneLookup.java
index 5ae0fb68a..dd440708c 100644
--- a/java/com/android/dialer/phonelookup/cp2/Cp2PhoneLookup.java
+++ b/java/com/android/dialer/phonelookup/cp2/Cp2PhoneLookup.java
@@ -211,6 +211,7 @@ public final class Cp2PhoneLookup implements PhoneLookup<Cp2Info> {
               anyContactsDeletedFuture,
               anyContactsDeleted -> {
                 if (anyContactsDeleted) {
+                  LogUtil.v("Cp2PhoneLookup.isDirty", "returning true because contacts deleted");
                   return Futures.immediateFuture(true);
                 }
                 // Hopefully the most common case is there are no contacts updated; we can detect
@@ -221,6 +222,9 @@ public final class Cp2PhoneLookup implements PhoneLookup<Cp2Info> {
                     noContactsModifiedSinceFuture,
                     noContactsModifiedSince -> {
                       if (noContactsModifiedSince) {
+                        LogUtil.v(
+                            "Cp2PhoneLookup.isDirty",
+                            "returning false because no contacts modified since last run");
                         return Futures.immediateFuture(false);
                       }
                       // This method is more expensive but is probably the most likely scenario; we
@@ -236,6 +240,9 @@ public final class Cp2PhoneLookup implements PhoneLookup<Cp2Info> {
                           contactsUpdatedFuture,
                           contactsUpdated -> {
                             if (contactsUpdated) {
+                              LogUtil.v(
+                                  "Cp2PhoneLookup.isDirty",
+                                  "returning true because a previously called contact was updated");
                               return Futures.immediateFuture(true);
                             }
                             // This is the most expensive method so do it last; the scenario is that
@@ -357,13 +364,14 @@ public final class Cp2PhoneLookup implements PhoneLookup<Cp2Info> {
 
   private ListenableFuture<Set<Long>> queryPhoneLookupTableForContactIdsBasedOnRawNumber(
       String rawNumber) {
+    if (TextUtils.isEmpty(rawNumber)) {
+      return Futures.immediateFuture(new ArraySet<>());
+    }
     return backgroundExecutorService.submit(
         () -> {
           Set<Long> contactIds = new ArraySet<>();
           try (Cursor cursor =
-              queryPhoneLookup(
-                  new String[] {android.provider.ContactsContract.PhoneLookup.CONTACT_ID},
-                  rawNumber)) {
+              queryPhoneLookup(new String[] {ContactsContract.PhoneLookup.CONTACT_ID}, rawNumber)) {
             if (cursor == null) {
               LogUtil.w(
                   "Cp2PhoneLookup.queryPhoneLookupTableForContactIdsBasedOnRawNumber",