Use SmartDialCache object for caching

Extract caching methods from SmartDialLoaderTask
and use a standalone SmartDialCache object instead. This
cache object handles caching failures as well as concurrent
multiple cache requests.

Bug: 6977981
Change-Id: I6df9e273191c7ac434d094e567d7a91814f8c030

3 files changed, 212 insertions, 162 deletions

diff --git a/src/com/android/dialer/dialpad/DialpadFragment.java b/src/com/android/dialer/dialpad/DialpadFragment.java
index f7a9056f5..2c4841685 100644
--- a/src/com/android/dialer/dialpad/DialpadFragment.java
+++ b/src/com/android/dialer/dialpad/DialpadFragment.java
@@ -170,7 +170,7 @@ public class DialpadFragment extends Fragment
     // Vibration (haptic feedback) for dialer key presses.
     private final HapticFeedback mHaptic = new HapticFeedback();
 
-    private boolean mNeedToCacheSmartDial = false;
+    private SmartDialCache mSmartDialCache;
     /** Identifier for the "Add Call" intent extra. */
     private static final String ADD_CALL_MODE_KEY = "add_call_mode";
 
@@ -279,7 +279,7 @@ public class DialpadFragment extends Fragment
 
         mContactsPrefs = new ContactsPreferences(getActivity());
         mCurrentCountryIso = GeoUtil.getCurrentCountryIso(getActivity());
-        mNeedToCacheSmartDial = true;
+        mSmartDialCache = new SmartDialCache(getActivity(), mContactsPrefs.getDisplayOrder());
         try {
             mHaptic.init(getActivity(),
                          getResources().getBoolean(R.bool.config_enable_dialer_key_vibration));
@@ -295,13 +295,6 @@ public class DialpadFragment extends Fragment
         if (state != null) {
             mDigitsFilledByIntent = state.getBoolean(PREF_DIGITS_FILLED_BY_INTENT);
         }
-
-        // Start caching contacts to use for smart dialling only if the dialpad fragment is visible
-        if (getUserVisibleHint()) {
-            SmartDialLoaderTask.startCacheContactsTaskIfNeeded(
-                    getActivity(), mContactsPrefs.getDisplayOrder());
-            mNeedToCacheSmartDial = false;
-        }
     }
 
     @Override
@@ -1679,10 +1672,8 @@ public class DialpadFragment extends Fragment
     @Override
     public void setUserVisibleHint(boolean isVisibleToUser) {
         super.setUserVisibleHint(isVisibleToUser);
-        if (isVisibleToUser && mNeedToCacheSmartDial) {
-            SmartDialLoaderTask.startCacheContactsTaskIfNeeded(
-                    getActivity(), mContactsPrefs.getDisplayOrder());
-            mNeedToCacheSmartDial = false;
+        if (isVisibleToUser) {
+            mSmartDialCache.cacheIfNeeded();
         }
     }
 
@@ -1704,7 +1695,7 @@ public class DialpadFragment extends Fragment
         if (digits.length() < 2) {
             mSmartDialAdapter.clear();
         } else {
-            final SmartDialLoaderTask task = new SmartDialLoaderTask(this, digits);
+            final SmartDialLoaderTask task = new SmartDialLoaderTask(this, digits, mSmartDialCache);
             // don't execute this in serial, otherwise we have to wait too long for results
             task.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR, new String[] {});
         }
diff --git a/src/com/android/dialer/dialpad/SmartDialCache.java b/src/com/android/dialer/dialpad/SmartDialCache.java
new file mode 100644
index 000000000..37746d27d
--- /dev/null
+++ b/src/com/android/dialer/dialpad/SmartDialCache.java
@@ -0,0 +1,187 @@
+/*
+ * Copyright (C) 2012 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.dialer.dialpad;
+
+import static com.android.dialer.dialpad.SmartDialAdapter.LOG_TAG;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
+
+import android.content.Context;
+import android.database.Cursor;
+import android.net.Uri;
+import android.provider.ContactsContract;
+import android.provider.ContactsContract.Contacts;
+import android.util.Log;
+
+import com.android.contacts.common.util.StopWatch;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Cache object used to cache Smart Dial contacts that handles various states of the cache:
+ * 1) Cache has been populated
+ * 2) Cache task is currently running
+ * 3) Cache task failed
+ */
+public class SmartDialCache {
+
+    public static class Contact {
+        public final String displayName;
+        public final String lookupKey;
+        public final long id;
+
+        public Contact(long id, String displayName, String lookupKey) {
+            this.displayName = displayName;
+            this.lookupKey = lookupKey;
+            this.id = id;
+        }
+    }
+
+    /** Query used for loadByContactName */
+    private interface ContactQuery {
+        Uri URI = Contacts.CONTENT_URI.buildUpon()
+                // Visible contact only
+                //.appendQueryParameter(ContactsContract.DIRECTORY_PARAM_KEY, "0")
+                .build();
+        String[] PROJECTION = new String[] {
+                Contacts._ID,
+                Contacts.DISPLAY_NAME,
+                Contacts.LOOKUP_KEY
+            };
+        String[] PROJECTION_ALTERNATIVE = new String[] {
+                Contacts._ID,
+                Contacts.DISPLAY_NAME_ALTERNATIVE,
+                Contacts.LOOKUP_KEY
+            };
+
+        int COLUMN_ID = 0;
+        int COLUMN_DISPLAY_NAME = 1;
+        int COLUMN_LOOKUP_KEY = 2;
+
+        String SELECTION =
+                Contacts.HAS_PHONE_NUMBER + "=1";
+
+        String ORDER_BY = Contacts.LAST_TIME_CONTACTED + " DESC";
+    }
+
+    // mContactsCache and mCachingStarted need to be volatile because we check for their status
+    // in cacheIfNeeded from the UI thread, to decided whether or not to fire up a caching thread.
+    private List<Contact> mContactsCache;
+    private volatile boolean mNeedsRecache = true;
+    private final int mNameDisplayOrder;
+    private final Context mContext;
+    private final Object mLock = new Object();
+
+    private static final boolean DEBUG = true; // STOPSHIP change to false.
+
+    public SmartDialCache(Context context, int nameDisplayOrder) {
+        mNameDisplayOrder = nameDisplayOrder;
+        Preconditions.checkNotNull(context, "Context must not be null");
+        mContext = context.getApplicationContext();
+    }
+
+    /**
+     * Performs a database query, iterates through the returned cursor and saves the retrieved
+     * contacts to a local cache.
+     */
+    private void cacheContacts(Context context) {
+        synchronized(mLock) {
+            // In extremely rare edge cases, getContacts() might be called and start caching
+            // between the time mCachingThread is added to the thread pool and it starts
+            // running. If so, at this point in time mContactsCache will no longer be null
+            // since it is populated by getContacts. We thus no longer have to perform any
+            // caching.
+            if (mContactsCache != null) {
+                if (DEBUG) {
+                    Log.d(LOG_TAG, "Contacts already cached");
+                }
+                return;
+            }
+            final StopWatch stopWatch = DEBUG ? StopWatch.start("SmartDial Cache") : null;
+            final Cursor c = context.getContentResolver().query(ContactQuery.URI,
+                    (mNameDisplayOrder == ContactsContract.Preferences.DISPLAY_ORDER_PRIMARY)
+                        ? ContactQuery.PROJECTION : ContactQuery.PROJECTION_ALTERNATIVE,
+                    ContactQuery.SELECTION, null,
+                    ContactQuery.ORDER_BY);
+            if (c == null) {
+                Log.w(LOG_TAG, "SmartDial query received null for cursor");
+                if (DEBUG) {
+                    stopWatch.stopAndLog("Query Failure", 0);
+                }
+                return;
+            }
+            try {
+                mContactsCache = Lists.newArrayListWithCapacity(c.getCount());
+                c.moveToPosition(-1);
+                while (c.moveToNext()) {
+                    final String displayName = c.getString(ContactQuery.COLUMN_DISPLAY_NAME);
+                    final long id = c.getLong(ContactQuery.COLUMN_ID);
+                    final String lookupKey = c.getString(ContactQuery.COLUMN_LOOKUP_KEY);
+                    mContactsCache.add(new Contact(id, displayName, lookupKey));
+                }
+            } finally {
+                c.close();
+                if (DEBUG) {
+                    stopWatch.stopAndLog("SmartDial Cache", 0);
+                }
+            }
+        }
+    }
+
+    /**
+     * Returns the list of cached contacts. If the caching task has not started or been completed,
+     * the method blocks till the caching process is complete before returning the full list of
+     * cached contacts. This means that this method should not be called from the UI thread.
+     *
+     * @return List of already cached contacts, or an empty list if the caching failed for any
+     * reason.
+     */
+    public List<Contact> getContacts() {
+        synchronized(mLock) {
+            if (mContactsCache == null) {
+                cacheContacts(mContext);
+                mNeedsRecache = false;
+                return (mContactsCache == null) ? new ArrayList<Contact>() : mContactsCache;
+            } else {
+                return mContactsCache;
+            }
+        }
+    }
+
+    /**
+     * Only start a new caching task if {@link #mContactsCache} is null and there is no caching
+     * task that is currently running
+     */
+    public void cacheIfNeeded() {
+        if (mNeedsRecache) {
+            mNeedsRecache = false;
+            startCachingThread();
+        }
+    }
+
+    private void startCachingThread() {
+        new Thread(new Runnable() {
+            @Override
+            public void run() {
+                cacheContacts(mContext);
+            }
+        }).start();
+    }
+
+}
diff --git a/src/com/android/dialer/dialpad/SmartDialLoaderTask.java b/src/com/android/dialer/dialpad/SmartDialLoaderTask.java
index 832183785..58b29d69e 100644
--- a/src/com/android/dialer/dialpad/SmartDialLoaderTask.java
+++ b/src/com/android/dialer/dialpad/SmartDialLoaderTask.java
@@ -31,35 +31,18 @@ import android.util.Log;
 
 import com.android.contacts.common.preference.ContactsPreferences;
 import com.android.contacts.common.util.StopWatch;
+import com.android.dialer.dialpad.SmartDialCache.Contact;
 
 import java.util.ArrayList;
 import java.util.List;
 
 /**
- * AsyncTask that performs one of two functions depending on which constructor is used.
- * If {@link #SmartDialLoaderTask(Context context, int nameDisplayOrder)} is used, the task
- * caches all contacts with a phone number into the static variable {@link #sContactsCache}.
- * If {@link #SmartDialLoaderTask(SmartDialLoaderCallback callback, String query)} is used, the
- * task searches through the cache to return the top 3 contacts(ranked by confidence) that match
- * the query, then passes it back to the {@link SmartDialLoaderCallback} through a callback
- * function.
+ * This task searches through the provided cache to return the top 3 contacts(ranked by confidence)
+ * that match the query, then passes it back to the {@link SmartDialLoaderCallback} through a
+ * callback function.
  */
-// TODO: Make the cache a singleton class and refactor to fix possible concurrency issues in the
-// future
 public class SmartDialLoaderTask extends AsyncTask<String, Integer, List<SmartDialEntry>> {
 
-    private class Contact {
-        final String mDisplayName;
-        final String mLookupKey;
-        final long mId;
-
-        public Contact(long id, String displayName, String lookupKey) {
-            mDisplayName = displayName;
-            mLookupKey = lookupKey;
-            mId = id;
-        }
-    }
-
     public interface SmartDialLoaderCallback {
         void setSmartDialAdapterEntries(List<SmartDialEntry> list);
     }
@@ -68,47 +51,26 @@ public class SmartDialLoaderTask extends AsyncTask<String, Integer, List<SmartDi
 
     private static final int MAX_ENTRIES = 3;
 
-    private static List<Contact> sContactsCache;
-
-    private final boolean mCacheOnly;
+    private final SmartDialCache mContactsCache;
 
     private final SmartDialLoaderCallback mCallback;
 
-    private final Context mContext;
     /**
      * See {@link ContactsPreferences#getDisplayOrder()}.
      * {@link ContactsContract.Preferences#DISPLAY_ORDER_PRIMARY} (first name first)
      * {@link ContactsContract.Preferences#DISPLAY_ORDER_ALTERNATIVE} (last name first)
      */
-    private final int mNameDisplayOrder;
-
     private final SmartDialNameMatcher mNameMatcher;
 
-    // cache only constructor
-    private SmartDialLoaderTask(Context context, int nameDisplayOrder) {
-        this.mNameDisplayOrder = nameDisplayOrder;
-        this.mContext = context;
-        // we're just caching contacts so no need to initialize a SmartDialNameMatcher or callback
-        this.mNameMatcher = null;
-        this.mCallback = null;
-        this.mCacheOnly = true;
-    }
-
-    public SmartDialLoaderTask(SmartDialLoaderCallback callback, String query) {
+    public SmartDialLoaderTask(SmartDialLoaderCallback callback, String query,
+            SmartDialCache cache) {
         this.mCallback = callback;
-        this.mContext = null;
-        this.mCacheOnly = false;
-        this.mNameDisplayOrder = 0;
         this.mNameMatcher = new SmartDialNameMatcher(PhoneNumberUtils.normalizeNumber(query));
+        this.mContactsCache = cache;
     }
 
     @Override
     protected List<SmartDialEntry> doInBackground(String... params) {
-        if (mCacheOnly) {
-            cacheContacts();
-            return Lists.newArrayList();
-        }
-
         return getContactMatches();
     }
 
@@ -119,116 +81,26 @@ public class SmartDialLoaderTask extends AsyncTask<String, Integer, List<SmartDi
         }
     }
 
-    /** Query used for loadByContactName */
-    private interface ContactQuery {
-        Uri URI = Contacts.CONTENT_URI.buildUpon()
-                // Visible contact only
-                //.appendQueryParameter(ContactsContract.DIRECTORY_PARAM_KEY, "0")
-                .build();
-        String[] PROJECTION = new String[] {
-                Contacts._ID,
-                Contacts.DISPLAY_NAME,
-                Contacts.LOOKUP_KEY
-            };
-        String[] PROJECTION_ALTERNATIVE = new String[] {
-                Contacts._ID,
-                Contacts.DISPLAY_NAME_ALTERNATIVE,
-                Contacts.LOOKUP_KEY
-            };
-
-        int COLUMN_ID = 0;
-        int COLUMN_DISPLAY_NAME = 1;
-        int COLUMN_LOOKUP_KEY = 2;
-
-        String SELECTION =
-                //Contacts.IN_VISIBLE_GROUP + "=1 and " +
-                Contacts.HAS_PHONE_NUMBER + "=1";
-
-        String ORDER_BY = Contacts.LAST_TIME_CONTACTED + " DESC";
-    }
-
-    public static void startCacheContactsTaskIfNeeded(Context context, int displayOrder) {
-        if (sContactsCache != null) {
-            // contacts have already been cached, just return
-            return;
-        }
-        final SmartDialLoaderTask task =
-                new SmartDialLoaderTask(context, displayOrder);
-        task.execute();
-    }
-
-    /**
-     * Caches the contacts into an in memory array list. This is called once at startup and should
-     * not be cancelled.
-     */
-    private void cacheContacts() {
-        final StopWatch stopWatch = DEBUG ? StopWatch.start("SmartDial Cache") : null;
-        if (sContactsCache != null) {
-            // contacts have already been cached, just return
-            stopWatch.stopAndLog("SmartDial Already Cached", 0);
-            return;
-        }
-        if (mContext == null) {
-            if (DEBUG) {
-                stopWatch.stopAndLog("Invalid context", 0);
-            }
-            return;
-        }
-        final Cursor c = mContext.getContentResolver().query(ContactQuery.URI,
-                (mNameDisplayOrder == ContactsContract.Preferences.DISPLAY_ORDER_PRIMARY)
-                    ? ContactQuery.PROJECTION : ContactQuery.PROJECTION_ALTERNATIVE,
-                ContactQuery.SELECTION, null,
-                ContactQuery.ORDER_BY);
-        if (c == null) {
-            if (DEBUG) {
-                stopWatch.stopAndLog("Query Failure", 0);
-            }
-            return;
-        }
-        sContactsCache = Lists.newArrayListWithCapacity(c.getCount());
-        try {
-            c.moveToPosition(-1);
-            while (c.moveToNext()) {
-                final String displayName = c.getString(ContactQuery.COLUMN_DISPLAY_NAME);
-                final long id = c.getLong(ContactQuery.COLUMN_ID);
-                final String lookupKey = c.getString(ContactQuery.COLUMN_LOOKUP_KEY);
-                sContactsCache.add(new Contact(id, displayName, lookupKey));
-            }
-        } finally {
-            c.close();
-            if (DEBUG) {
-                stopWatch.stopAndLog("SmartDial Cache", 0);
-            }
-        }
-    }
-
     /**
      * Loads all visible contacts with phone numbers and check if their display names match the
      * query.  Return at most {@link #MAX_ENTRIES} {@link SmartDialEntry}'s for the matching
      * contacts.
      */
     private ArrayList<SmartDialEntry> getContactMatches() {
-        final StopWatch stopWatch = DEBUG ? StopWatch.start(LOG_TAG + " Start Match") : null;
-        if (sContactsCache == null) {
-            // contacts should have been cached by this point in time, but in case they
-            // are not, we go ahead and cache them into memory.
-            if (DEBUG) {
-                Log.d(LOG_TAG, "empty cache");
-            }
-            cacheContacts();
-            // TODO: if sContactsCache is still null at this point we should try to recache
-        }
+        final List<Contact> cachedContactList = mContactsCache.getContacts();
+        // cachedContactList will never be null at this point
+
         if (DEBUG) {
-            Log.d(LOG_TAG, "Size of cache: " + sContactsCache.size());
+            Log.d(LOG_TAG, "Size of cache: " + cachedContactList.size());
         }
+
+        final StopWatch stopWatch = DEBUG ? StopWatch.start(LOG_TAG + " Start Match") : null;
         final ArrayList<SmartDialEntry> outList = Lists.newArrayList();
-        if (sContactsCache == null) {
-            return outList;
-        }
+
         int count = 0;
-        for (int i = 0; i < sContactsCache.size(); i++) {
-            final Contact contact = sContactsCache.get(i);
-            final String displayName = contact.mDisplayName;
+        for (int i = 0; i < cachedContactList.size(); i++) {
+            final Contact contact = cachedContactList.get(i);
+            final String displayName = contact.displayName;
 
             if (!mNameMatcher.matches(displayName)) {
                 continue;
@@ -236,8 +108,8 @@ public class SmartDialLoaderTask extends AsyncTask<String, Integer, List<SmartDi
             // Matched; create SmartDialEntry.
             @SuppressWarnings("unchecked")
             final SmartDialEntry entry = new SmartDialEntry(
-                     contact.mDisplayName,
-                     Contacts.getLookupUri(contact.mId, contact.mLookupKey),
+                     contact.displayName,
+                     Contacts.getLookupUri(contact.id, contact.lookupKey),
                      (ArrayList<SmartDialMatchPosition>) mNameMatcher.getMatchPositions().clone()
                      );
             outList.add(entry);