NewSearchFragment contact photos now properly open quick contact cards.

There was an issue where businesses' and remote contacts' contact photos
wouldn't open contact cards correctly. The issue was rooted in the incorrect
contact uri being assigned to the quick contact badge.

from the bugbash:
16. Tap on business icon from search results says “ no application found” instead of opening the business info
17. Same as #16 but with contact from Directory Google.com - “The contact doesn’t exist” when tapping contact icon

Bug: 64902476
Test: existing
PiperOrigin-RevId: 167111016
Change-Id: I4b6f7ca812d2fc4dc220951e8c05db2c8b8d6114

11 files changed, 156 insertions, 36 deletions

diff --git a/java/com/android/dialer/searchfragment/common/SearchCursor.java b/java/com/android/dialer/searchfragment/common/SearchCursor.java
index 368ee09d6..7ad19aabd 100644
--- a/java/com/android/dialer/searchfragment/common/SearchCursor.java
+++ b/java/com/android/dialer/searchfragment/common/SearchCursor.java
@@ -35,4 +35,11 @@ public interface SearchCursor extends Cursor {
    * @return true if the data set has changed.
    */
   boolean updateQuery(@NonNull String query);
+
+  /**
+   * Returns an ID unique to the directory this cursor reads from. Generally this value will be
+   * related to {@link android.provider.ContactsContract.Directory} but could differ depending on
+   * the implementation.
+   */
+  long getDirectoryId();
 }
diff --git a/java/com/android/dialer/searchfragment/cp2/SearchContactsCursor.java b/java/com/android/dialer/searchfragment/cp2/SearchContactsCursor.java
index 18c9ecd7f..508ca7f57 100644
--- a/java/com/android/dialer/searchfragment/cp2/SearchContactsCursor.java
+++ b/java/com/android/dialer/searchfragment/cp2/SearchContactsCursor.java
@@ -20,6 +20,7 @@ import android.content.Context;
 import android.database.Cursor;
 import android.database.MatrixCursor;
 import android.database.MergeCursor;
+import android.provider.ContactsContract.Directory;
 import android.support.annotation.Nullable;
 import com.android.dialer.searchfragment.common.SearchCursor;
 
@@ -32,7 +33,7 @@ final class SearchContactsCursor extends MergeCursor implements SearchCursor {
 
   private final ContactFilterCursor contactFilterCursor;
 
-  public static SearchContactsCursor newInstnace(
+  static SearchContactsCursor newInstance(
       Context context, ContactFilterCursor contactFilterCursor) {
     MatrixCursor headerCursor = new MatrixCursor(HEADER_PROJECTION);
     headerCursor.addRow(new String[] {context.getString(R.string.all_contacts)});
@@ -56,6 +57,11 @@ final class SearchContactsCursor extends MergeCursor implements SearchCursor {
   }
 
   @Override
+  public long getDirectoryId() {
+    return Directory.DEFAULT;
+  }
+
+  @Override
   public int getCount() {
     // If we don't have any contents, we don't want to show the header
     int count = contactFilterCursor.getCount();
diff --git a/java/com/android/dialer/searchfragment/cp2/SearchContactsCursorLoader.java b/java/com/android/dialer/searchfragment/cp2/SearchContactsCursorLoader.java
index 84fd64ae5..b7fc9b5c5 100644
--- a/java/com/android/dialer/searchfragment/cp2/SearchContactsCursorLoader.java
+++ b/java/com/android/dialer/searchfragment/cp2/SearchContactsCursorLoader.java
@@ -47,6 +47,6 @@ public final class SearchContactsCursorLoader extends CursorLoader {
     // Filtering logic
     ContactFilterCursor contactFilterCursor = new ContactFilterCursor(cursor, query);
     // Header logic
-    return SearchContactsCursor.newInstnace(getContext(), contactFilterCursor);
+    return SearchContactsCursor.newInstance(getContext(), contactFilterCursor);
   }
 }
diff --git a/java/com/android/dialer/searchfragment/list/NewSearchFragment.java b/java/com/android/dialer/searchfragment/list/NewSearchFragment.java
index 7fee9699a..910e454f8 100644
--- a/java/com/android/dialer/searchfragment/list/NewSearchFragment.java
+++ b/java/com/android/dialer/searchfragment/list/NewSearchFragment.java
@@ -120,7 +120,6 @@ public final class NewSearchFragment extends Fragment
 
   private void initLoaders() {
     getLoaderManager().initLoader(CONTACTS_LOADER_ID, null, this);
-    loadNearbyPlacesCursor();
     loadRemoteDirectoriesCursor();
   }
 
@@ -129,7 +128,14 @@ public final class NewSearchFragment extends Fragment
     if (id == CONTACTS_LOADER_ID) {
       return new SearchContactsCursorLoader(getContext(), query);
     } else if (id == NEARBY_PLACES_LOADER_ID) {
-      return new NearbyPlacesCursorLoader(getContext(), query);
+      // Directories represent contact data sources on the device, but since nearby places aren't
+      // stored on the device, they don't have a directory ID. We pass the list of all existing IDs
+      // so that we can find one that doesn't collide.
+      List<Integer> directoryIds = new ArrayList<>();
+      for (Directory directory : directories) {
+        directoryIds.add(directory.getId());
+      }
+      return new NearbyPlacesCursorLoader(getContext(), query, directoryIds);
     } else if (id == REMOTE_DIRECTORIES_LOADER_ID) {
       return new RemoteDirectoriesCursorLoader(getContext());
     } else if (id == REMOTE_CONTACTS_LOADER_ID) {
@@ -162,6 +168,7 @@ public final class NewSearchFragment extends Fragment
       while (cursor.moveToNext()) {
         directories.add(RemoteDirectoriesCursorLoader.readDirectory(cursor));
       }
+      loadNearbyPlacesCursor();
       loadRemoteContactsCursors();
 
     } else {
@@ -212,18 +219,6 @@ public final class NewSearchFragment extends Fragment
     ThreadUtil.getUiThreadHandler().removeCallbacks(capabilitiesUpdatedRunnable);
   }
 
-  private void loadNearbyPlacesCursor() {
-    // Cancel existing load if one exists.
-    ThreadUtil.getUiThreadHandler().removeCallbacks(loadNearbyPlacesRunnable);
-
-    // If nearby places is not enabled, do not try to load them.
-    if (!PhoneDirectoryExtenderAccessor.get(getContext()).isEnabled(getContext())) {
-      return;
-    }
-    ThreadUtil.getUiThreadHandler()
-        .postDelayed(loadNearbyPlacesRunnable, NETWORK_SEARCH_DELAY_MILLIS);
-  }
-
   @Override
   public void onRequestPermissionsResult(
       int requestCode, @NonNull String[] permissions, @NonNull int[] grantResults) {
@@ -250,12 +245,14 @@ public final class NewSearchFragment extends Fragment
     }
   }
 
+  // Loads remote directories.
   private void loadRemoteDirectoriesCursor() {
     if (!remoteDirectoriesDisabledForTesting) {
       getLoaderManager().initLoader(REMOTE_DIRECTORIES_LOADER_ID, null, this);
     }
   }
 
+  // Should not be called before remote directories have finished loading.
   private void loadRemoteContactsCursors() {
     if (remoteDirectoriesDisabledForTesting) {
       return;
@@ -267,6 +264,19 @@ public final class NewSearchFragment extends Fragment
         .postDelayed(loadRemoteContactsRunnable, NETWORK_SEARCH_DELAY_MILLIS);
   }
 
+  // Should not be called before remote directories (not contacts) have finished loading.
+  private void loadNearbyPlacesCursor() {
+    // Cancel existing load if one exists.
+    ThreadUtil.getUiThreadHandler().removeCallbacks(loadNearbyPlacesRunnable);
+
+    // If nearby places is not enabled, do not try to load them.
+    if (!PhoneDirectoryExtenderAccessor.get(getContext()).isEnabled(getContext())) {
+      return;
+    }
+    ThreadUtil.getUiThreadHandler()
+        .postDelayed(loadNearbyPlacesRunnable, NETWORK_SEARCH_DELAY_MILLIS);
+  }
+
   @Override
   public void onResume() {
     super.onResume();
diff --git a/java/com/android/dialer/searchfragment/nearbyplaces/NearbyPlaceViewHolder.java b/java/com/android/dialer/searchfragment/nearbyplaces/NearbyPlaceViewHolder.java
index 575582e07..fa0782623 100644
--- a/java/com/android/dialer/searchfragment/nearbyplaces/NearbyPlaceViewHolder.java
+++ b/java/com/android/dialer/searchfragment/nearbyplaces/NearbyPlaceViewHolder.java
@@ -17,13 +17,14 @@
 package com.android.dialer.searchfragment.nearbyplaces;
 
 import android.content.Context;
-import android.database.Cursor;
 import android.net.Uri;
 import android.provider.ContactsContract;
+import android.provider.ContactsContract.Contacts;
 import android.support.v7.widget.RecyclerView;
 import android.view.View;
 import android.widget.QuickContactBadge;
 import android.widget.TextView;
+import com.android.contacts.common.util.Constants;
 import com.android.dialer.callintent.CallInitiationType;
 import com.android.dialer.callintent.CallIntentBuilder;
 import com.android.dialer.contactphoto.ContactPhotoManager;
@@ -31,6 +32,7 @@ import com.android.dialer.lettertile.LetterTileDrawable;
 import com.android.dialer.searchfragment.common.Projections;
 import com.android.dialer.searchfragment.common.QueryBoldingUtil;
 import com.android.dialer.searchfragment.common.R;
+import com.android.dialer.searchfragment.common.SearchCursor;
 import com.android.dialer.telecom.TelecomUtil;
 
 /** ViewHolder for a nearby place row. */
@@ -57,14 +59,13 @@ public final class NearbyPlaceViewHolder extends RecyclerView.ViewHolder
    * Binds the ViewHolder with a cursor from {@link NearbyPlacesCursorLoader} with the data found at
    * the cursors set position.
    */
-  public void bind(Cursor cursor, String query) {
+  public void bind(SearchCursor cursor, String query) {
     number = cursor.getString(Projections.PHONE_NUMBER);
     String name = cursor.getString(Projections.PHONE_DISPLAY_NAME);
     String address = cursor.getString(Projections.PHONE_LABEL);
 
     placeName.setText(QueryBoldingUtil.getNameWithQueryBolded(query, name));
     placeAddress.setText(QueryBoldingUtil.getNameWithQueryBolded(query, address));
-
     String photoUri = cursor.getString(Projections.PHONE_PHOTO_URI);
     ContactPhotoManager.getInstance(context)
         .loadDialerThumbnailOrPhoto(
@@ -73,13 +74,21 @@ public final class NearbyPlaceViewHolder extends RecyclerView.ViewHolder
             cursor.getLong(Projections.PHONE_PHOTO_ID),
             photoUri == null ? null : Uri.parse(photoUri),
             name,
-            LetterTileDrawable.TYPE_DEFAULT);
+            LetterTileDrawable.TYPE_BUSINESS);
   }
 
-  private static Uri getContactUri(Cursor cursor) {
-    long contactId = cursor.getLong(Projections.PHONE_ID);
-    String lookupKey = cursor.getString(Projections.PHONE_LOOKUP_KEY);
-    return ContactsContract.Contacts.getLookupUri(contactId, lookupKey);
+  private static Uri getContactUri(SearchCursor cursor) {
+    // Since the lookup key for Nearby Places is actually a JSON representation of the information,
+    // we need to pass it in as an encoded fragment in our contact uri.
+    // It includes information like display name, photo uri, phone number, ect.
+    String businessInfoJson = cursor.getString(Projections.PHONE_LOOKUP_KEY);
+    return Contacts.CONTENT_LOOKUP_URI
+        .buildUpon()
+        .appendPath(Constants.LOOKUP_URI_ENCODED)
+        .appendQueryParameter(
+            ContactsContract.DIRECTORY_PARAM_KEY, String.valueOf(cursor.getDirectoryId()))
+        .encodedFragment(businessInfoJson)
+        .build();
   }
 
   @Override
diff --git a/java/com/android/dialer/searchfragment/nearbyplaces/NearbyPlacesCursor.java b/java/com/android/dialer/searchfragment/nearbyplaces/NearbyPlacesCursor.java
index a4142a41d..3be59b672 100644
--- a/java/com/android/dialer/searchfragment/nearbyplaces/NearbyPlacesCursor.java
+++ b/java/com/android/dialer/searchfragment/nearbyplaces/NearbyPlacesCursor.java
@@ -27,16 +27,23 @@ import com.android.dialer.searchfragment.common.SearchCursor;
 final class NearbyPlacesCursor extends MergeCursor implements SearchCursor {
 
   private final Cursor nearbyPlacesCursor;
+  private final long directoryId;
 
-  public static NearbyPlacesCursor newInstnace(Context context, Cursor nearbyPlacesCursor) {
+  /**
+   * @param directoryId unique directory id that doesn't collide with other remote/local
+   *     directories. directoryIds are needed to load the correct quick contact card.
+   */
+  static NearbyPlacesCursor newInstance(
+      Context context, Cursor nearbyPlacesCursor, long directoryId) {
     MatrixCursor headerCursor = new MatrixCursor(HEADER_PROJECTION);
     headerCursor.addRow(new String[] {context.getString(R.string.nearby_places)});
-    return new NearbyPlacesCursor(new Cursor[] {headerCursor, nearbyPlacesCursor});
+    return new NearbyPlacesCursor(new Cursor[] {headerCursor, nearbyPlacesCursor}, directoryId);
   }
 
-  private NearbyPlacesCursor(Cursor[] cursors) {
+  private NearbyPlacesCursor(Cursor[] cursors, long directoryId) {
     super(cursors);
     nearbyPlacesCursor = cursors[1];
+    this.directoryId = directoryId;
   }
 
   @Override
@@ -61,4 +68,9 @@ final class NearbyPlacesCursor extends MergeCursor implements SearchCursor {
     int count = nearbyPlacesCursor.getCount();
     return count == 0 ? 0 : count + 1;
   }
+
+  @Override
+  public long getDirectoryId() {
+    return directoryId;
+  }
 }
diff --git a/java/com/android/dialer/searchfragment/nearbyplaces/NearbyPlacesCursorLoader.java b/java/com/android/dialer/searchfragment/nearbyplaces/NearbyPlacesCursorLoader.java
index 6807a6e6b..c8bb36a73 100644
--- a/java/com/android/dialer/searchfragment/nearbyplaces/NearbyPlacesCursorLoader.java
+++ b/java/com/android/dialer/searchfragment/nearbyplaces/NearbyPlacesCursorLoader.java
@@ -21,21 +21,37 @@ import android.content.CursorLoader;
 import android.database.Cursor;
 import android.net.Uri;
 import android.provider.ContactsContract;
+import android.support.annotation.NonNull;
 import com.android.contacts.common.extensions.PhoneDirectoryExtenderAccessor;
+import com.android.dialer.common.LogUtil;
 import com.android.dialer.searchfragment.common.Projections;
+import java.util.List;
 
 /** Cursor loader for nearby places search results. */
 public final class NearbyPlacesCursorLoader extends CursorLoader {
 
   private static final String MAX_RESULTS = "3";
+  private static final long INVALID_DIRECTORY_ID = Long.MAX_VALUE;
+  private final long directoryId;
 
-  public NearbyPlacesCursorLoader(Context context, String query) {
+  /**
+   * @param directoryIds List of directoryIds associated with all directories on device. Required in
+   *     order to find a directory ID for the nearby places cursor that doesn't collide with
+   *     existing directories.
+   */
+  public NearbyPlacesCursorLoader(
+      Context context, String query, @NonNull List<Integer> directoryIds) {
     super(context, getContentUri(context, query), Projections.PHONE_PROJECTION, null, null, null);
+    this.directoryId = getDirectoryId(directoryIds);
   }
 
   @Override
   public Cursor loadInBackground() {
-    return NearbyPlacesCursor.newInstnace(getContext(), super.loadInBackground());
+    if (directoryId == INVALID_DIRECTORY_ID) {
+      LogUtil.i("NearbyPlacesCursorLoader.loadInBackground", "directory id not set.");
+      return null;
+    }
+    return NearbyPlacesCursor.newInstance(getContext(), super.loadInBackground(), directoryId);
   }
 
   private static Uri getContentUri(Context context, String query) {
@@ -46,4 +62,22 @@ public final class NearbyPlacesCursorLoader extends CursorLoader {
         .appendQueryParameter(ContactsContract.LIMIT_PARAM_KEY, MAX_RESULTS)
         .build();
   }
+
+  private static long getDirectoryId(List<Integer> directoryIds) {
+    if (directoryIds.isEmpty()) {
+      return INVALID_DIRECTORY_ID;
+    }
+
+    // The Directory.LOCAL_INVISIBLE might not be a directory we use, but we can't reuse it's
+    // "special" ID.
+    long maxId = ContactsContract.Directory.LOCAL_INVISIBLE;
+    for (int i = 0, n = directoryIds.size(); i < n; i++) {
+      long id = directoryIds.get(i);
+      if (id > maxId) {
+        maxId = id;
+      }
+    }
+    // Add one so that the nearby places ID doesn't collide with extended directory IDs.
+    return maxId + 1;
+  }
 }
diff --git a/java/com/android/dialer/searchfragment/remote/RemoteContactViewHolder.java b/java/com/android/dialer/searchfragment/remote/RemoteContactViewHolder.java
index 5fb12d349..df3eacc5b 100644
--- a/java/com/android/dialer/searchfragment/remote/RemoteContactViewHolder.java
+++ b/java/com/android/dialer/searchfragment/remote/RemoteContactViewHolder.java
@@ -22,6 +22,7 @@ import android.database.Cursor;
 import android.net.Uri;
 import android.provider.ContactsContract;
 import android.provider.ContactsContract.CommonDataKinds.Phone;
+import android.provider.ContactsContract.Contacts;
 import android.support.v7.widget.RecyclerView;
 import android.text.TextUtils;
 import android.view.View;
@@ -119,10 +120,14 @@ public final class RemoteContactViewHolder extends RecyclerView.ViewHolder
     return (String) Phone.getTypeLabel(resources, numberType, numberLabel);
   }
 
-  private static Uri getContactUri(Cursor cursor) {
+  private static Uri getContactUri(SearchCursor cursor) {
     long contactId = cursor.getLong(Projections.PHONE_ID);
     String lookupKey = cursor.getString(Projections.PHONE_LOOKUP_KEY);
-    return ContactsContract.Contacts.getLookupUri(contactId, lookupKey);
+    return Contacts.getLookupUri(contactId, lookupKey)
+        .buildUpon()
+        .appendQueryParameter(
+            ContactsContract.DIRECTORY_PARAM_KEY, String.valueOf(cursor.getDirectoryId()))
+        .build();
   }
 
   @Override
diff --git a/java/com/android/dialer/searchfragment/remote/RemoteContactsCursor.java b/java/com/android/dialer/searchfragment/remote/RemoteContactsCursor.java
index d7c4f3805..e6f3c2607 100644
--- a/java/com/android/dialer/searchfragment/remote/RemoteContactsCursor.java
+++ b/java/com/android/dialer/searchfragment/remote/RemoteContactsCursor.java
@@ -26,6 +26,7 @@ import com.android.dialer.common.Assert;
 import com.android.dialer.searchfragment.common.SearchCursor;
 import com.android.dialer.searchfragment.remote.RemoteDirectoriesCursorLoader.Directory;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 
 /**
@@ -41,6 +42,16 @@ import java.util.List;
 public final class RemoteContactsCursor extends MergeCursor implements SearchCursor {
 
   /**
+   * {@link SearchCursor#HEADER_PROJECTION} with {@link #COLUMN_DIRECTORY_ID} appended on the end.
+   *
+   * <p>This is needed to get the directoryId associated with each contact. directoryIds are needed
+   * to load the correct quick contact card.
+   */
+  private static final String[] PROJECTION = buildProjection();
+
+  private static final String COLUMN_DIRECTORY_ID = "directory_id";
+
+  /**
    * Returns a single cursor with headers inserted between each non-empty cursor. If all cursors are
    * empty, null or closed, this method returns null.
    */
@@ -78,18 +89,24 @@ public final class RemoteContactsCursor extends MergeCursor implements SearchCur
         continue;
       }
 
-      cursorList.add(createHeaderCursor(context, directory.getDisplayName()));
+      cursorList.add(createHeaderCursor(context, directory.getDisplayName(), directory.getId()));
       cursorList.add(cursor);
     }
     return cursorList.toArray(new Cursor[cursorList.size()]);
   }
 
-  private static MatrixCursor createHeaderCursor(Context context, String name) {
-    MatrixCursor headerCursor = new MatrixCursor(HEADER_PROJECTION, 1);
-    headerCursor.addRow(new String[] {context.getString(R.string.directory, name)});
+  private static MatrixCursor createHeaderCursor(Context context, String name, int id) {
+    MatrixCursor headerCursor = new MatrixCursor(PROJECTION, 1);
+    headerCursor.addRow(new Object[] {context.getString(R.string.directory, name), id});
     return headerCursor;
   }
 
+  private static String[] buildProjection() {
+    String[] projection = Arrays.copyOf(HEADER_PROJECTION, HEADER_PROJECTION.length + 1);
+    projection[projection.length - 1] = COLUMN_DIRECTORY_ID;
+    return projection;
+  }
+
   /** Returns true if the current position is a header row. */
   @Override
   public boolean isHeader() {
@@ -97,6 +114,21 @@ public final class RemoteContactsCursor extends MergeCursor implements SearchCur
   }
 
   @Override
+  public long getDirectoryId() {
+    int position = getPosition();
+    // proceed backwards until we reach the header row, which contains the directory ID.
+    while (moveToPrevious()) {
+      int id = getInt(getColumnIndex(COLUMN_DIRECTORY_ID));
+      if (id != -1) {
+        // return the cursor to it's original position/state
+        moveToPosition(position);
+        return id;
+      }
+    }
+    throw Assert.createIllegalStateFailException("No directory id for contact at: " + position);
+  }
+
+  @Override
   public boolean updateQuery(@Nullable String query) {
     // When the query changes, a new network request is made for nearby places. Meaning this cursor
     // will be closed and another created, so return false.
diff --git a/java/com/android/dialer/searchfragment/remote/RemoteDirectoriesCursorLoader.java b/java/com/android/dialer/searchfragment/remote/RemoteDirectoriesCursorLoader.java
index 327a62c7b..de71025cd 100644
--- a/java/com/android/dialer/searchfragment/remote/RemoteDirectoriesCursorLoader.java
+++ b/java/com/android/dialer/searchfragment/remote/RemoteDirectoriesCursorLoader.java
@@ -67,7 +67,7 @@ public final class RemoteDirectoriesCursorLoader extends CursorLoader {
       return new AutoValue_RemoteDirectoriesCursorLoader_Directory(id, displayName, supportsPhotos);
     }
 
-    abstract int getId();
+    public abstract int getId();
 
     /** Returns a user facing display name of the directory. Null if none exists. */
     abstract @Nullable String getDisplayName();
diff --git a/java/com/android/dialer/searchfragment/testing/TestSearchCursor.java b/java/com/android/dialer/searchfragment/testing/TestSearchCursor.java
index 9a0b95789..7e6299eac 100644
--- a/java/com/android/dialer/searchfragment/testing/TestSearchCursor.java
+++ b/java/com/android/dialer/searchfragment/testing/TestSearchCursor.java
@@ -44,4 +44,9 @@ public final class TestSearchCursor extends MergeCursor implements SearchCursor
   public boolean updateQuery(@Nullable String query) {
     return false;
   }
+
+  @Override
+  public long getDirectoryId() {
+    return 0;
+  }
 }