From 73ec1ab52956c658654a40c42f4c59806713b15e Mon Sep 17 00:00:00 2001 From: zachh Date: Tue, 9 Jan 2018 17:44:36 -0800 Subject: 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 --- .../dialer/calllog/RefreshAnnotatedCallLogWorker.java | 11 +++++++---- .../com/android/dialer/calllog/ui/NewCallLogFragment.java | 7 ++++++- .../android/dialer/common/concurrent/DialerFutures.java | 15 ++++++++++----- .../android/dialer/phonelookup/cp2/Cp2PhoneLookup.java | 14 +++++++++++--- 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. * - *

If an input fails, it is treated as if the predicate did not match. + *

If an input fails before a match is found, the returned future also fails. * *

Cancellation of the output future will cause cancellation of all input futures. * @@ -53,7 +53,6 @@ public class DialerFutures { Predicate predicate, T defaultValue) { AggregateFuture output = new AnyOfFuture<>(futures); - // Use an atomic reference to ensure that late listeners don't pin the FirstOfFuture in memory. final AtomicReference> ref = Atomics.newReference(output); final AtomicInteger pending = new AtomicInteger(output.futures.size()); for (final ListenableFuture future : output.futures) { @@ -65,14 +64,15 @@ public class DialerFutures { // cheaper than a CAS and atomicity is guaranteed by setFuture. AggregateFuture 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); @@ -107,6 +107,11 @@ public class DialerFutures { return super.set(t); } + @Override + protected boolean setException(Throwable throwable) { + return super.setException(throwable); + } + @Override protected boolean setFuture(ListenableFuture 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 { 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 { 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 { 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 { private ListenableFuture> queryPhoneLookupTableForContactIdsBasedOnRawNumber( String rawNumber) { + if (TextUtils.isEmpty(rawNumber)) { + return Futures.immediateFuture(new ArraySet<>()); + } return backgroundExecutorService.submit( () -> { Set 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", -- cgit v1.2.3