Merge changes I47f1e487,I715a97ba,Idb424831

* changes:
  Added logging for popping in new call log.
  Move update color code from CallCardPresenter to InCallPresenter.
  Log the number of "invalid" CP2 rows.

9 files changed, 146 insertions, 64 deletions

diff --git a/java/com/android/dialer/calllog/ui/NewCallLogAdapter.java b/java/com/android/dialer/calllog/ui/NewCallLogAdapter.java
index 05a339978..839ba332f 100644
--- a/java/com/android/dialer/calllog/ui/NewCallLogAdapter.java
+++ b/java/com/android/dialer/calllog/ui/NewCallLogAdapter.java
@@ -25,6 +25,7 @@ import android.view.LayoutInflater;
 import android.view.ViewGroup;
 import com.android.dialer.calllogutils.CallLogDates;
 import com.android.dialer.common.Assert;
+import com.android.dialer.logging.Logger;
 import com.android.dialer.time.Clock;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
@@ -53,6 +54,7 @@ final class NewCallLogAdapter extends RecyclerView.Adapter<ViewHolder> {
 
   private final Clock clock;
   private final RealtimeRowProcessor realtimeRowProcessor;
+  private final PopCounts popCounts = new PopCounts();
 
   private Cursor cursor;
 
@@ -76,6 +78,7 @@ final class NewCallLogAdapter extends RecyclerView.Adapter<ViewHolder> {
   void updateCursor(Cursor updatedCursor) {
     this.cursor = updatedCursor;
     this.realtimeRowProcessor.clearCache();
+    this.popCounts.reset();
 
     setHeaderPositions();
     notifyDataSetChanged();
@@ -85,6 +88,10 @@ final class NewCallLogAdapter extends RecyclerView.Adapter<ViewHolder> {
     this.realtimeRowProcessor.clearCache();
   }
 
+  void logMetrics(Context context) {
+    Logger.get(context).logAnnotatedCallLogMetrics(popCounts.popped, popCounts.didNotPop);
+  }
+
   private void setHeaderPositions() {
     // If there are no rows to display, set all header positions to null.
     if (!cursor.moveToFirst()) {
@@ -138,7 +145,8 @@ final class NewCallLogAdapter extends RecyclerView.Adapter<ViewHolder> {
             LayoutInflater.from(viewGroup.getContext())
                 .inflate(R.layout.new_call_log_entry, viewGroup, false),
             clock,
-            realtimeRowProcessor);
+            realtimeRowProcessor,
+            popCounts);
       default:
         throw Assert.createUnsupportedOperationFailException("Unsupported view type: " + viewType);
     }
@@ -207,4 +215,14 @@ final class NewCallLogAdapter extends RecyclerView.Adapter<ViewHolder> {
     }
     return cursor.getCount() + numberOfHeaders;
   }
+
+  static class PopCounts {
+    int popped;
+    int didNotPop;
+
+    private void reset() {
+      popped = 0;
+      didNotPop = 0;
+    }
+  }
 }
diff --git a/java/com/android/dialer/calllog/ui/NewCallLogFragment.java b/java/com/android/dialer/calllog/ui/NewCallLogFragment.java
index bb1a7303e..0f1c2510a 100644
--- a/java/com/android/dialer/calllog/ui/NewCallLogFragment.java
+++ b/java/com/android/dialer/calllog/ui/NewCallLogFragment.java
@@ -89,6 +89,15 @@ public final class NewCallLogFragment extends Fragment implements LoaderCallback
   }
 
   @Override
+  public void onStop() {
+    super.onStop();
+
+    if (recyclerView.getAdapter() != null) {
+      ((NewCallLogAdapter) recyclerView.getAdapter()).logMetrics(getContext());
+    }
+  }
+
+  @Override
   public void onPause() {
     super.onPause();
     LogUtil.enterBlock("NewCallLogFragment.onPause");
diff --git a/java/com/android/dialer/calllog/ui/NewCallLogViewHolder.java b/java/com/android/dialer/calllog/ui/NewCallLogViewHolder.java
index f322b562b..1f84ebfdf 100644
--- a/java/com/android/dialer/calllog/ui/NewCallLogViewHolder.java
+++ b/java/com/android/dialer/calllog/ui/NewCallLogViewHolder.java
@@ -27,6 +27,7 @@ import android.view.View;
 import android.widget.ImageView;
 import android.widget.TextView;
 import com.android.dialer.calllog.model.CoalescedRow;
+import com.android.dialer.calllog.ui.NewCallLogAdapter.PopCounts;
 import com.android.dialer.calllog.ui.menu.NewCallLogMenu;
 import com.android.dialer.calllogutils.CallLogEntryText;
 import com.android.dialer.calllogutils.CallLogIntents;
@@ -60,10 +61,12 @@ final class NewCallLogViewHolder extends RecyclerView.ViewHolder {
   private final Clock clock;
   private final RealtimeRowProcessor realtimeRowProcessor;
   private final ExecutorService uiExecutorService;
+  private final PopCounts popCounts;
 
   private long currentRowId;
 
-  NewCallLogViewHolder(View view, Clock clock, RealtimeRowProcessor realtimeRowProcessor) {
+  NewCallLogViewHolder(
+      View view, Clock clock, RealtimeRowProcessor realtimeRowProcessor, PopCounts popCounts) {
     super(view);
     this.context = view.getContext();
     contactPhotoView = view.findViewById(R.id.contact_photo_view);
@@ -79,6 +82,7 @@ final class NewCallLogViewHolder extends RecyclerView.ViewHolder {
 
     this.clock = clock;
     this.realtimeRowProcessor = realtimeRowProcessor;
+    this.popCounts = popCounts;
     uiExecutorService = DialerExecutorComponent.get(context).uiExecutor();
   }
 
@@ -258,13 +262,17 @@ final class NewCallLogViewHolder extends RecyclerView.ViewHolder {
       // If the user scrolled then this ViewHolder may not correspond to the completed task and
       // there's nothing to do.
       if (originalRow.getId() != currentRowId) {
+        popCounts.didNotPop++;
         return;
       }
       // Only update the UI if the updated row differs from the original row (which has already
       // been displayed).
       if (!updatedRow.equals(originalRow)) {
         displayRow(updatedRow);
+        popCounts.popped++;
+        return;
       }
+      popCounts.didNotPop++;
     }
 
     @Override
diff --git a/java/com/android/dialer/calllog/ui/RealtimeRowProcessor.java b/java/com/android/dialer/calllog/ui/RealtimeRowProcessor.java
index b955e029b..c5148d93e 100644
--- a/java/com/android/dialer/calllog/ui/RealtimeRowProcessor.java
+++ b/java/com/android/dialer/calllog/ui/RealtimeRowProcessor.java
@@ -198,8 +198,14 @@ public final class RealtimeRowProcessor {
 
   private CoalescedRow applyPhoneLookupInfoToRow(
       PhoneLookupInfo phoneLookupInfo, CoalescedRow row) {
+    // Force the "cp2_info_incomplete" value to the original value so that it is not used when
+    // comparing the original row to the updated row.
+    // TODO(linyuh): Improve the comparison instead.
     return row.toBuilder()
-        .setNumberAttributes(NumberAttributesConverter.fromPhoneLookupInfo(phoneLookupInfo).build())
+        .setNumberAttributes(
+            NumberAttributesConverter.fromPhoneLookupInfo(phoneLookupInfo)
+                .setIsCp2InfoIncomplete(row.getNumberAttributes().getIsCp2InfoIncomplete())
+                .build())
         .build();
   }
 }
diff --git a/java/com/android/dialer/logging/LoggingBindings.java b/java/com/android/dialer/logging/LoggingBindings.java
index a6795ed1e..7c580cb77 100644
--- a/java/com/android/dialer/logging/LoggingBindings.java
+++ b/java/com/android/dialer/logging/LoggingBindings.java
@@ -90,4 +90,10 @@ public interface LoggingBindings {
 
   /** Logs a call auto-blocked in call screening. */
   void logAutoBlockedCall(String phoneNumber);
+
+  /** Logs annotated call log metrics. */
+  void logAnnotatedCallLogMetrics(int invalidNumbersInCallLog);
+
+  /** Logs annotated call log metrics. */
+  void logAnnotatedCallLogMetrics(int numberRowsThatDidPop, int numberRowsThatDidNotPop);
 }
diff --git a/java/com/android/dialer/logging/LoggingBindingsStub.java b/java/com/android/dialer/logging/LoggingBindingsStub.java
index de08f4497..65ebd1a52 100644
--- a/java/com/android/dialer/logging/LoggingBindingsStub.java
+++ b/java/com/android/dialer/logging/LoggingBindingsStub.java
@@ -64,4 +64,10 @@ public class LoggingBindingsStub implements LoggingBindings {
 
   @Override
   public void logAutoBlockedCall(String phoneNumber) {}
+
+  @Override
+  public void logAnnotatedCallLogMetrics(int invalidNumbersInCallLog) {}
+
+  @Override
+  public void logAnnotatedCallLogMetrics(int numberRowsThatDidPop, int numberRowsThatDidNotPop) {}
 }
diff --git a/java/com/android/dialer/phonelookup/cp2/Cp2DefaultDirectoryPhoneLookup.java b/java/com/android/dialer/phonelookup/cp2/Cp2DefaultDirectoryPhoneLookup.java
index 902a2fbe3..c5d4e53f2 100644
--- a/java/com/android/dialer/phonelookup/cp2/Cp2DefaultDirectoryPhoneLookup.java
+++ b/java/com/android/dialer/phonelookup/cp2/Cp2DefaultDirectoryPhoneLookup.java
@@ -33,7 +33,9 @@ import com.android.dialer.common.Assert;
 import com.android.dialer.common.LogUtil;
 import com.android.dialer.common.concurrent.Annotations.BackgroundExecutor;
 import com.android.dialer.common.concurrent.Annotations.LightweightExecutor;
+import com.android.dialer.configprovider.ConfigProvider;
 import com.android.dialer.inject.ApplicationContext;
+import com.android.dialer.logging.Logger;
 import com.android.dialer.phonelookup.PhoneLookup;
 import com.android.dialer.phonelookup.PhoneLookupInfo;
 import com.android.dialer.phonelookup.PhoneLookupInfo.Cp2Info;
@@ -64,15 +66,11 @@ public final class Cp2DefaultDirectoryPhoneLookup implements PhoneLookup<Cp2Info
   private static final String PREF_LAST_TIMESTAMP_PROCESSED =
       "cp2DefaultDirectoryPhoneLookupLastTimestampProcessed";
 
-  // We cannot efficiently process invalid numbers because batch queries cannot be constructed which
-  // accomplish the necessary loose matching. We'll attempt to process a limited number of them,
-  // but if there are too many we fall back to querying CP2 at render time.
-  private static final int MAX_SUPPORTED_INVALID_NUMBERS = 5;
-
   private final Context appContext;
   private final SharedPreferences sharedPreferences;
   private final ListeningExecutorService backgroundExecutorService;
   private final ListeningExecutorService lightweightExecutorService;
+  private final ConfigProvider configProvider;
 
   @Nullable private Long currentLastTimestampProcessed;
 
@@ -81,11 +79,13 @@ public final class Cp2DefaultDirectoryPhoneLookup implements PhoneLookup<Cp2Info
       @ApplicationContext Context appContext,
       @Unencrypted SharedPreferences sharedPreferences,
       @BackgroundExecutor ListeningExecutorService backgroundExecutorService,
-      @LightweightExecutor ListeningExecutorService lightweightExecutorService) {
+      @LightweightExecutor ListeningExecutorService lightweightExecutorService,
+      ConfigProvider configProvider) {
     this.appContext = appContext;
     this.sharedPreferences = sharedPreferences;
     this.backgroundExecutorService = backgroundExecutorService;
     this.lightweightExecutorService = lightweightExecutorService;
+    this.configProvider = configProvider;
   }
 
   @Override
@@ -138,7 +138,7 @@ public final class Cp2DefaultDirectoryPhoneLookup implements PhoneLookup<Cp2Info
   @Override
   public ListenableFuture<Boolean> isDirty(ImmutableSet<DialerPhoneNumber> phoneNumbers) {
     PartitionedNumbers partitionedNumbers = new PartitionedNumbers(phoneNumbers);
-    if (partitionedNumbers.invalidNumbers().size() > MAX_SUPPORTED_INVALID_NUMBERS) {
+    if (partitionedNumbers.invalidNumbers().size() > getMaxSupportedInvalidNumbers()) {
       // If there are N invalid numbers, we can't determine determine dirtiness without running N
       // queries; since running this many queries is not feasible for the (lightweight) isDirty
       // check, simply return true. The expectation is that this should rarely be the case as the
@@ -234,7 +234,8 @@ public final class Cp2DefaultDirectoryPhoneLookup implements PhoneLookup<Cp2Info
 
     // Then run a separate query for each invalid number. Separate queries are done to accomplish
     // loose matching which couldn't be accomplished with a batch query.
-    Assert.checkState(partitionedNumbers.invalidNumbers().size() <= MAX_SUPPORTED_INVALID_NUMBERS);
+    Assert.checkState(
+        partitionedNumbers.invalidNumbers().size() <= getMaxSupportedInvalidNumbers());
     for (String invalidNumber : partitionedNumbers.invalidNumbers()) {
       queryFutures.add(queryPhoneLookupTableForContactIdsBasedOnRawNumber(invalidNumber));
     }
@@ -529,7 +530,11 @@ public final class Cp2DefaultDirectoryPhoneLookup implements PhoneLookup<Cp2Info
       ImmutableMap<DialerPhoneNumber, Cp2Info> existingInfoMap) {
     ArraySet<DialerPhoneNumber> unprocessableNumbers = new ArraySet<>();
     PartitionedNumbers partitionedNumbers = new PartitionedNumbers(existingInfoMap.keySet());
-    if (partitionedNumbers.invalidNumbers().size() > MAX_SUPPORTED_INVALID_NUMBERS) {
+
+    int invalidNumberCount = partitionedNumbers.invalidNumbers().size();
+    Logger.get(appContext).logAnnotatedCallLogMetrics(invalidNumberCount);
+
+    if (invalidNumberCount > getMaxSupportedInvalidNumbers()) {
       for (String invalidNumber : partitionedNumbers.invalidNumbers()) {
         unprocessableNumbers.addAll(partitionedNumbers.dialerPhoneNumbersForInvalid(invalidNumber));
       }
@@ -928,4 +933,13 @@ public final class Cp2DefaultDirectoryPhoneLookup implements PhoneLookup<Cp2Info
     }
     return where.toString();
   }
+
+  /**
+   * We cannot efficiently process invalid numbers because batch queries cannot be constructed which
+   * accomplish the necessary loose matching. We'll attempt to process a limited number of them, but
+   * if there are too many we fall back to querying CP2 at render time.
+   */
+  private long getMaxSupportedInvalidNumbers() {
+    return configProvider.getLong("cp2_phone_lookup_max_invalid_numbers", 5);
+  }
 }
diff --git a/java/com/android/incallui/CallCardPresenter.java b/java/com/android/incallui/CallCardPresenter.java
index 9c5e0623e..2f88c8836 100644
--- a/java/com/android/incallui/CallCardPresenter.java
+++ b/java/com/android/incallui/CallCardPresenter.java
@@ -271,10 +271,10 @@ public class CallCardPresenter
 
       // getCallToDisplay doesn't go through outgoing or incoming calls. It will return the
       // highest priority call to display as the secondary call.
-      secondary = getCallToDisplay(callList, null, true);
+      secondary = InCallPresenter.getCallToDisplay(callList, null, true);
     } else if (newState == InCallState.INCALL) {
-      primary = getCallToDisplay(callList, null, false);
-      secondary = getCallToDisplay(callList, primary, true);
+      primary = InCallPresenter.getCallToDisplay(callList, null, false);
+      secondary = InCallPresenter.getCallToDisplay(callList, primary, true);
     }
 
     LogUtil.v("CallCardPresenter.onStateChange", "primary call: " + primary);
@@ -302,7 +302,6 @@ public class CallCardPresenter
     this.primaryNumber = primaryNumber;
 
     if (this.primary != null) {
-      InCallPresenter.getInstance().onForegroundCallChanged(this.primary);
       inCallScreen.updateInCallScreenColors();
     }
 
@@ -636,54 +635,6 @@ public class CallCardPresenter
     }
   }
 
-  /**
-   * Get the highest priority call to display. Goes through the calls and chooses which to return
-   * based on priority of which type of call to display to the user. Callers can use the "ignore"
-   * feature to get the second best call by passing a previously found primary call as ignore.
-   *
-   * @param ignore A call to ignore if found.
-   */
-  private DialerCall getCallToDisplay(
-      CallList callList, DialerCall ignore, boolean skipDisconnected) {
-    // Active calls come second.  An active call always gets precedent.
-    DialerCall retval = callList.getActiveCall();
-    if (retval != null && retval != ignore) {
-      return retval;
-    }
-
-    // Sometimes there is intemediate state that two calls are in active even one is about
-    // to be on hold.
-    retval = callList.getSecondActiveCall();
-    if (retval != null && retval != ignore) {
-      return retval;
-    }
-
-    // Disconnected calls get primary position if there are no active calls
-    // to let user know quickly what call has disconnected. Disconnected
-    // calls are very short lived.
-    if (!skipDisconnected) {
-      retval = callList.getDisconnectingCall();
-      if (retval != null && retval != ignore) {
-        return retval;
-      }
-      retval = callList.getDisconnectedCall();
-      if (retval != null && retval != ignore) {
-        return retval;
-      }
-    }
-
-    // Then we go to background call (calls on hold)
-    retval = callList.getBackgroundCall();
-    if (retval != null && retval != ignore) {
-      return retval;
-    }
-
-    // Lastly, we go to a second background call.
-    retval = callList.getSecondBackgroundCall();
-
-    return retval;
-  }
-
   private void updatePrimaryDisplayInfo() {
     if (inCallScreen == null) {
       // TODO: May also occur if search result comes back after ui is destroyed. Look into
diff --git a/java/com/android/incallui/InCallPresenter.java b/java/com/android/incallui/InCallPresenter.java
index 2e98a969d..e11b376c1 100644
--- a/java/com/android/incallui/InCallPresenter.java
+++ b/java/com/android/incallui/InCallPresenter.java
@@ -771,6 +771,22 @@ public class InCallPresenter implements CallList.Listener, AudioModeProvider.Aud
         "Phone switching state: " + oldState + " -> " + newState);
     inCallState = newState;
 
+    // Foreground call changed
+    DialerCall primary = null;
+    if (newState == InCallState.INCOMING) {
+      primary = callList.getIncomingCall();
+    } else if (newState == InCallState.PENDING_OUTGOING || newState == InCallState.OUTGOING) {
+      primary = callList.getOutgoingCall();
+      if (primary == null) {
+        primary = callList.getPendingOutgoingCall();
+      }
+    } else if (newState == InCallState.INCALL) {
+      primary = getCallToDisplay(callList, null, false);
+    }
+    if (primary != null) {
+      onForegroundCallChanged(primary);
+    }
+
     // notify listeners of new state
     for (InCallStateListener listener : listeners) {
       LogUtil.d(
@@ -787,6 +803,54 @@ public class InCallPresenter implements CallList.Listener, AudioModeProvider.Aud
     Trace.endSection();
   }
 
+  /**
+   * Get the highest priority call to display. Goes through the calls and chooses which to return
+   * based on priority of which type of call to display to the user. Callers can use the "ignore"
+   * feature to get the second best call by passing a previously found primary call as ignore.
+   *
+   * @param ignore A call to ignore if found.
+   */
+  static DialerCall getCallToDisplay(
+      CallList callList, DialerCall ignore, boolean skipDisconnected) {
+    // Active calls come second.  An active call always gets precedent.
+    DialerCall retval = callList.getActiveCall();
+    if (retval != null && retval != ignore) {
+      return retval;
+    }
+
+    // Sometimes there is intemediate state that two calls are in active even one is about
+    // to be on hold.
+    retval = callList.getSecondActiveCall();
+    if (retval != null && retval != ignore) {
+      return retval;
+    }
+
+    // Disconnected calls get primary position if there are no active calls
+    // to let user know quickly what call has disconnected. Disconnected
+    // calls are very short lived.
+    if (!skipDisconnected) {
+      retval = callList.getDisconnectingCall();
+      if (retval != null && retval != ignore) {
+        return retval;
+      }
+      retval = callList.getDisconnectedCall();
+      if (retval != null && retval != ignore) {
+        return retval;
+      }
+    }
+
+    // Then we go to background call (calls on hold)
+    retval = callList.getBackgroundCall();
+    if (retval != null && retval != ignore) {
+      return retval;
+    }
+
+    // Lastly, we go to a second background call.
+    retval = callList.getSecondBackgroundCall();
+
+    return retval;
+  }
+
   /** Called when there is a new incoming call. */
   @Override
   public void onIncomingCall(DialerCall call) {