Remove strict mode violation with number formatting.
The call log is currently performing formatting on phone numbers in the
main thread (during bind view).
At the same time, however, we are always doing a background request to
look-up the contacts. Move the formatting call to the background thread.
In order for the right information to be shown as soon as the call log
is opened, we also need to store this information in our cached values
in the database. This means that the number will show unformatted the
very first time, and then updated once the background require is
completed.
Bug: 5316982
Change-Id: I20d1971948afa33c7825f0bd38a9520021f75378
diff --git a/src/com/android/contacts/calllog/CallLogAdapter.java b/src/com/android/contacts/calllog/CallLogAdapter.java
index d54a47e..17209b4 100644
--- a/src/com/android/contacts/calllog/CallLogAdapter.java
+++ b/src/com/android/contacts/calllog/CallLogAdapter.java
@@ -39,7 +39,6 @@
import android.provider.ContactsContract.PhoneLookup;
import android.telephony.PhoneNumberUtils;
import android.text.TextUtils;
-import android.util.Pair;
import android.view.LayoutInflater;
import android.view.View;
import android.view.ViewGroup;
@@ -47,6 +46,8 @@
import java.util.LinkedList;
+import libcore.util.Objects;
+
/**
* Adapter class to fill in data for the Call Log.
*/
@@ -76,6 +77,49 @@
private ExpirableCache<String, ContactInfo> mContactInfoCache;
/**
+ * A request for contact details for the given number.
+ */
+ private static final class ContactInfoRequest {
+ /** The number to look-up. */
+ public final String number;
+ /** The country in which a call to or from this number was placed or received. */
+ public final String countryIso;
+ /** The cached contact information stored in the call log. */
+ public final ContactInfo callLogInfo;
+
+ public ContactInfoRequest(String number, String countryIso, ContactInfo callLogInfo) {
+ this.number = number;
+ this.countryIso = countryIso;
+ this.callLogInfo = callLogInfo;
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) return true;
+ if (obj == null) return false;
+ if (!(obj instanceof ContactInfoRequest)) return false;
+
+ ContactInfoRequest other = (ContactInfoRequest) obj;
+
+ if (!TextUtils.equals(number, other.number)) return false;
+ if (!TextUtils.equals(countryIso, other.countryIso)) return false;
+ if (!Objects.equal(callLogInfo, other.callLogInfo)) return false;
+
+ return true;
+ }
+
+ @Override
+ public int hashCode() {
+ final int prime = 31;
+ int result = 1;
+ result = prime * result + ((callLogInfo == null) ? 0 : callLogInfo.hashCode());
+ result = prime * result + ((countryIso == null) ? 0 : countryIso.hashCode());
+ result = prime * result + ((number == null) ? 0 : number.hashCode());
+ return result;
+ }
+ }
+
+ /**
* List of requests to update contact details.
* <p>
* Each request is made of a phone number to look up, and the contact info currently stored in
@@ -84,7 +128,7 @@
* The requests are added when displaying the contacts and are processed by a background
* thread.
*/
- private final LinkedList<Pair<String, ContactInfo>> mRequests;
+ private final LinkedList<ContactInfoRequest> mRequests;
private volatile boolean mDone;
private boolean mLoading = true;
@@ -162,7 +206,7 @@
mCallFetcher = callFetcher;
mContactInfoCache = ExpirableCache.create(CONTACT_INFO_CACHE_SIZE);
- mRequests = new LinkedList<Pair<String,ContactInfo>>();
+ mRequests = new LinkedList<ContactInfoRequest>();
mPreDrawListener = null;
Resources resources = mContext.getResources();
@@ -245,8 +289,9 @@
* started with a delay. See {@link #START_PROCESSING_REQUESTS_DELAY_MILLIS}.
*/
@VisibleForTesting
- void enqueueRequest(String number, ContactInfo callLogInfo, boolean immediate) {
- Pair<String, ContactInfo> request = new Pair<String, ContactInfo>(number, callLogInfo);
+ void enqueueRequest(String number, String countryIso, ContactInfo callLogInfo,
+ boolean immediate) {
+ ContactInfoRequest request = new ContactInfoRequest(number, countryIso, callLogInfo);
synchronized (mRequests) {
if (!mRequests.contains(request)) {
mRequests.add(request);
@@ -330,6 +375,7 @@
info.normalizedNumber = null; // meaningless for SIP addresses
info.photoId = dataTableCursor.getLong(
dataTableCursor.getColumnIndex(Data.PHOTO_ID));
+ info.formattedNumber = null; // meaningless for SIP addresses
} else {
info = ContactInfo.EMPTY;
}
@@ -350,7 +396,7 @@
* <p>
* If the lookup fails for some other reason, it returns null.
*/
- private ContactInfo queryContactInfoForPhoneNumber(String number) {
+ private ContactInfo queryContactInfoForPhoneNumber(String number, String countryIso) {
final ContactInfo info;
// "number" is a regular phone number, so use the
@@ -372,6 +418,9 @@
info.number = phonesCursor.getString(PhoneQuery.MATCHED_NUMBER);
info.normalizedNumber = phonesCursor.getString(PhoneQuery.NORMALIZED_NUMBER);
info.photoId = phonesCursor.getLong(PhoneQuery.PHOTO_ID);
+ info.formattedNumber = formatPhoneNumber(info.number, info.formattedNumber,
+ countryIso);
+
} else {
info = ContactInfo.EMPTY;
}
@@ -394,7 +443,7 @@
* It returns true if it updated the content of the cache and we should therefore tell the
* view to update its content.
*/
- private boolean queryContactInfo(String number, ContactInfo callLogInfo) {
+ private boolean queryContactInfo(String number, String countryIso, ContactInfo callLogInfo) {
final ContactInfo info;
// Determine the contact info.
@@ -405,12 +454,12 @@
// Check whether the username is actually a phone number of contact.
String username = number.substring(0, number.indexOf('@'));
if (PhoneNumberUtils.isGlobalPhoneNumber(username)) {
- sipInfo = queryContactInfoForPhoneNumber(username);
+ sipInfo = queryContactInfoForPhoneNumber(username, countryIso);
}
}
info = sipInfo;
} else {
- info = queryContactInfoForPhoneNumber(number);
+ info = queryContactInfoForPhoneNumber(number, countryIso);
}
if (info == null) {
@@ -418,21 +467,27 @@
return false;
}
+ final ContactInfo updatedInfo;
+ // If we did not find a matching contact, generate an empty contact info for the number.
+ if (info == ContactInfo.EMPTY) {
+ // Did not find a matching contact.
+ updatedInfo = new ContactInfo();
+ updatedInfo.number = number;
+ updatedInfo.formattedNumber = formatPhoneNumber(number, null, countryIso);
+ } else {
+ updatedInfo = info;
+ }
+
// Check the existing entry in the cache: only if it has changed we should update the
// view.
ContactInfo existingInfo = mContactInfoCache.getPossiblyExpired(number);
- boolean updated = !info.equals(existingInfo);
- if (updated) {
- // The formattedNumber is computed by the UI thread when needed. Since we updated
- // the details of the contact, set this value to null for now.
- info.formattedNumber = null;
- }
+ boolean updated = !updatedInfo.equals(existingInfo);
// Store the data in the cache so that the UI thread can use to display it. Store it
// even if it has not changed so that it is marked as not expired.
- mContactInfoCache.put(number, info);
+ mContactInfoCache.put(number, updatedInfo);
// Update the call log even if the cache it is up-to-date: it is possible that the cache
// contains the value from a different call log entry.
- updateCallLogContactInfoCache(number, info, callLogInfo);
+ updateCallLogContactInfoCache(number, updatedInfo, callLogInfo);
return updated;
}
@@ -444,13 +499,10 @@
public void run() {
boolean needNotify = false;
while (!mDone) {
- String number = null;
- ContactInfo callLogInfo = null;
+ ContactInfoRequest request = null;
synchronized (mRequests) {
if (!mRequests.isEmpty()) {
- Pair<String, ContactInfo> request = mRequests.removeFirst();
- number = request.first;
- callLogInfo = request.second;
+ request = mRequests.removeFirst();
} else {
if (needNotify) {
needNotify = false;
@@ -464,7 +516,8 @@
}
}
}
- if (!mDone && number != null && queryContactInfo(number, callLogInfo)) {
+ if (!mDone && request != null
+ && queryContactInfo(request.number, request.countryIso, request.callLogInfo)) {
needNotify = true;
}
}
@@ -565,7 +618,6 @@
final long date = c.getLong(CallLogQuery.DATE);
final long duration = c.getLong(CallLogQuery.DURATION);
final int callType = c.getInt(CallLogQuery.CALL_TYPE);
- final String formattedNumber;
final String countryIso = c.getString(CallLogQuery.COUNTRY_ISO);
final ContactInfo cachedContactInfo = getContactInfoFromCallLog(c);
@@ -597,43 +649,30 @@
// If this is a number that cannot be dialed, there is no point in looking up a contact
// for it.
info = ContactInfo.EMPTY;
- formattedNumber = null;
} else if (cachedInfo == null) {
mContactInfoCache.put(number, ContactInfo.EMPTY);
// Use the cached contact info from the call log.
info = cachedContactInfo;
// The db request should happen on a non-UI thread.
// Request the contact details immediately since they are currently missing.
- enqueueRequest(number, cachedContactInfo, true);
- // Format the phone number in the call log as best as we can.
- formattedNumber = formatPhoneNumber(info.number, info.normalizedNumber, countryIso);
- info.formattedNumber = formattedNumber;
+ enqueueRequest(number, countryIso, cachedContactInfo, true);
+ // We will format the phone number when we make the background request.
} else {
if (cachedInfo.isExpired()) {
// The contact info is no longer up to date, we should request it. However, we
// do not need to request them immediately.
- enqueueRequest(number, cachedContactInfo, false);
+ enqueueRequest(number, countryIso, cachedContactInfo, false);
} else if (!callLogInfoMatches(cachedContactInfo, info)) {
// The call log information does not match the one we have, look it up again.
// We could simply update the call log directly, but that needs to be done in a
// background thread, so it is easier to simply request a new lookup, which will, as
// a side-effect, update the call log.
- enqueueRequest(number, cachedContactInfo, false);
+ enqueueRequest(number, countryIso, cachedContactInfo, false);
}
- if (info != ContactInfo.EMPTY) {
- // Format and cache phone number for found contact.
- if (info.formattedNumber == null) {
- info.formattedNumber =
- formatPhoneNumber(info.number, info.normalizedNumber, countryIso);
- }
- formattedNumber = info.formattedNumber;
- } else {
+ if (info == ContactInfo.EMPTY) {
// Use the cached contact info from the call log.
info = cachedContactInfo;
- // Format the phone number in the call log as best as we can.
- formattedNumber = formatPhoneNumber(info.number, info.normalizedNumber, countryIso);
- info.formattedNumber = formattedNumber;
}
}
@@ -642,6 +681,7 @@
final int ntype = info.type;
final String label = info.label;
final long photoId = info.photoId;
+ CharSequence formattedNumber = info.formattedNumber;
final int[] callTypes = getCallTypes(c, count);
final String geocode = c.getString(CallLogQuery.GEOCODED_LOCATION);
final PhoneCallDetails details;
@@ -724,6 +764,10 @@
values.put(Calls.CACHED_PHOTO_ID, updatedInfo.photoId);
needsUpdate = true;
}
+ if (!TextUtils.equals(updatedInfo.formattedNumber, callLogInfo.formattedNumber)) {
+ values.put(Calls.CACHED_FORMATTED_NUMBER, updatedInfo.formattedNumber);
+ needsUpdate = true;
+ }
} else {
// No previous values, store all of them.
values.put(Calls.CACHED_NAME, updatedInfo.name);
@@ -733,6 +777,7 @@
values.put(Calls.CACHED_MATCHED_NUMBER, updatedInfo.number);
values.put(Calls.CACHED_NORMALIZED_NUMBER, updatedInfo.normalizedNumber);
values.put(Calls.CACHED_PHOTO_ID, updatedInfo.photoId);
+ values.put(Calls.CACHED_FORMATTED_NUMBER, updatedInfo.formattedNumber);
needsUpdate = true;
}
@@ -759,8 +804,7 @@
info.number = matchedNumber == null ? c.getString(CallLogQuery.NUMBER) : matchedNumber;
info.normalizedNumber = c.getString(CallLogQuery.CACHED_NORMALIZED_NUMBER);
info.photoId = c.getLong(CallLogQuery.CACHED_PHOTO_ID);
- // TODO: This could be added to the call log cached values as well.
- info.formattedNumber = null; // Computed on demand.
+ info.formattedNumber = c.getString(CallLogQuery.CACHED_FORMATTED_NUMBER);
return info;
}
diff --git a/src/com/android/contacts/calllog/CallLogQuery.java b/src/com/android/contacts/calllog/CallLogQuery.java
index 960097d..e622b3d 100644
--- a/src/com/android/contacts/calllog/CallLogQuery.java
+++ b/src/com/android/contacts/calllog/CallLogQuery.java
@@ -41,6 +41,7 @@
Calls.CACHED_MATCHED_NUMBER, // 12
Calls.CACHED_NORMALIZED_NUMBER, // 13
Calls.CACHED_PHOTO_ID, // 14
+ Calls.CACHED_FORMATTED_NUMBER, // 15
};
public static final int ID = 0;
@@ -58,8 +59,9 @@
public static final int CACHED_MATCHED_NUMBER = 12;
public static final int CACHED_NORMALIZED_NUMBER = 13;
public static final int CACHED_PHOTO_ID = 14;
+ public static final int CACHED_FORMATTED_NUMBER = 15;
/** The index of the synthetic "section" column in the extended projection. */
- public static final int SECTION = 15;
+ public static final int SECTION = 16;
/**
* The name of the synthetic "section" column.
diff --git a/src/com/android/contacts/calllog/CallLogQueryHandler.java b/src/com/android/contacts/calllog/CallLogQueryHandler.java
index f12f37e..2a11cc3 100644
--- a/src/com/android/contacts/calllog/CallLogQueryHandler.java
+++ b/src/com/android/contacts/calllog/CallLogQueryHandler.java
@@ -107,7 +107,7 @@
// The values in this row correspond to default values for _PROJECTION from CallLogQuery
// plus the section value.
matrixCursor.addRow(new Object[]{
- 0L, "", 0L, 0L, 0, "", "", "", null, 0, null, null, null, null, 0L, section
+ 0L, "", 0L, 0L, 0, "", "", "", null, 0, null, null, null, null, 0L, null, section
});
return matrixCursor;
}
diff --git a/src/com/android/contacts/calllog/ContactInfo.java b/src/com/android/contacts/calllog/ContactInfo.java
index 4dc5512..cf33d45 100644
--- a/src/com/android/contacts/calllog/ContactInfo.java
+++ b/src/com/android/contacts/calllog/ContactInfo.java
@@ -60,7 +60,7 @@
if (type != other.type) return false;
if (!TextUtils.equals(label, other.label)) return false;
if (!TextUtils.equals(number, other.number)) return false;
- // Ignore formatted number.
+ if (!TextUtils.equals(formattedNumber, other.formattedNumber)) return false;
if (!TextUtils.equals(normalizedNumber, other.normalizedNumber)) return false;
if (photoId != other.photoId) return false;
return true;
diff --git a/tests/src/com/android/contacts/activities/CallLogActivityTests.java b/tests/src/com/android/contacts/activities/CallLogActivityTests.java
index c1f31ae..ac314b6 100644
--- a/tests/src/com/android/contacts/activities/CallLogActivityTests.java
+++ b/tests/src/com/android/contacts/activities/CallLogActivityTests.java
@@ -184,13 +184,26 @@
}
@MediumTest
- public void testBindView_NumberOnly() {
+ public void testBindView_NumberOnlyNoCache() {
mCursor.moveToFirst();
insert(TEST_NUMBER, NOW, 0, Calls.INCOMING_TYPE);
View view = mAdapter.newStandAloneView(getActivity(), mParentView);
mAdapter.bindStandAloneView(view, getActivity(), mCursor);
CallLogListItemViews views = (CallLogListItemViews) view.getTag();
+ assertNameIs(views, TEST_NUMBER);
+ }
+
+ @MediumTest
+ public void testBindView_NumberOnlyDbCachedFormattedNumber() {
+ mCursor.moveToFirst();
+ Object[] values = getValuesToInsert(TEST_NUMBER, NOW, 0, Calls.INCOMING_TYPE);
+ values[CallLogQuery.CACHED_FORMATTED_NUMBER] = TEST_FORMATTED_NUMBER;
+ insertValues(values);
+ View view = mAdapter.newStandAloneView(getActivity(), mParentView);
+ mAdapter.bindStandAloneView(view, getActivity(), mCursor);
+
+ CallLogListItemViews views = (CallLogListItemViews) view.getTag();
assertNameIs(views, TEST_FORMATTED_NUMBER);
}
@@ -473,6 +486,25 @@
* @param type Either Call.OUTGOING_TYPE or Call.INCOMING_TYPE or Call.MISSED_TYPE.
*/
private void insert(String number, long date, int duration, int type) {
+ insertValues(getValuesToInsert(number, date, duration, type));
+ }
+
+ /** Inserts the given values in the cursor. */
+ private void insertValues(Object[] values) {
+ mCursor.addRow(values);
+ ++mIndex;
+ }
+
+ /**
+ * Returns the values for a new call entry.
+ *
+ * @param number The phone number. For unknown and private numbers,
+ * use CallerInfo.UNKNOWN_NUMBER or CallerInfo.PRIVATE_NUMBER.
+ * @param date In millisec since epoch. Use NOW to use the current time.
+ * @param duration In seconds of the call. Use RAND_DURATION to pick a random one.
+ * @param type Either Call.OUTGOING_TYPE or Call.INCOMING_TYPE or Call.MISSED_TYPE.
+ */
+ private Object[] getValuesToInsert(String number, long date, int duration, int type) {
Object[] values = CallLogQueryTestUtils.createTestExtendedValues();
values[CallLogQuery.ID] = mIndex;
values[CallLogQuery.NUMBER] = number;
@@ -484,8 +516,7 @@
values[CallLogQuery.CALL_TYPE] = type;
values[CallLogQuery.COUNTRY_ISO] = TEST_COUNTRY_ISO;
values[CallLogQuery.SECTION] = CallLogQuery.SECTION_OLD_ITEM;
- mCursor.addRow(values);
- ++mIndex;
+ return values;
}
/**
@@ -496,19 +527,11 @@
* @param duration In seconds of the call. Use RAND_DURATION to pick a random one.
*/
private void insertVoicemail(String number, long date, int duration) {
- Object[] values = CallLogQueryTestUtils.createTestExtendedValues();
- values[CallLogQuery.ID] = mIndex;
- values[CallLogQuery.NUMBER] = number;
- values[CallLogQuery.DATE] = date == NOW ? new Date().getTime() : date;
- values[CallLogQuery.DURATION] = duration < 0 ? mRnd.nextInt(10 * 60) : duration;
- values[CallLogQuery.CALL_TYPE] = Calls.VOICEMAIL_TYPE;
- values[CallLogQuery.COUNTRY_ISO] = TEST_COUNTRY_ISO;
+ Object[] values = getValuesToInsert(number, date, duration, Calls.VOICEMAIL_TYPE);
// Must have the same index as the row.
values[CallLogQuery.VOICEMAIL_URI] =
ContentUris.withAppendedId(VoicemailContract.Voicemails.CONTENT_URI, mIndex);
- values[CallLogQuery.SECTION] = CallLogQuery.SECTION_OLD_ITEM;
- mCursor.addRow(values);
- ++mIndex;
+ insertValues(values);
}
/**
diff --git a/tests/src/com/android/contacts/calllog/CallLogAdapterTest.java b/tests/src/com/android/contacts/calllog/CallLogAdapterTest.java
index 25a0ae3..b159f61 100644
--- a/tests/src/com/android/contacts/calllog/CallLogAdapterTest.java
+++ b/tests/src/com/android/contacts/calllog/CallLogAdapterTest.java
@@ -83,6 +83,8 @@
TestCallLogAdapter.Request request = mAdapter.requests.get(0);
// It is for the number we need to show.
assertEquals(TEST_NUMBER, request.number);
+ // It has the right country.
+ assertEquals(TEST_COUNTRY_ISO, request.countryIso);
// Since there is nothing in the cache, it is an immediate request.
assertTrue("should be immediate", request.immediate);
}
@@ -163,6 +165,7 @@
private Object[] createCallLogEntry() {
Object[] values = CallLogQueryTestUtils.createTestExtendedValues();
values[CallLogQuery.NUMBER] = TEST_NUMBER;
+ values[CallLogQuery.COUNTRY_ISO] = TEST_COUNTRY_ISO;
return values;
}
@@ -183,11 +186,14 @@
private static final class TestCallLogAdapter extends CallLogAdapter {
public static class Request {
public final String number;
+ public final String countryIso;
public final ContactInfo callLogInfo;
public final boolean immediate;
- public Request(String number, ContactInfo callLogInfo, boolean immediate) {
+ public Request(String number, String countryIso, ContactInfo callLogInfo,
+ boolean immediate) {
this.number = number;
+ this.countryIso = countryIso;
this.callLogInfo = callLogInfo;
this.immediate = immediate;
}
@@ -201,8 +207,9 @@
}
@Override
- void enqueueRequest(String number, ContactInfo callLogInfo, boolean immediate) {
- requests.add(new Request(number, callLogInfo, immediate));
+ void enqueueRequest(String number, String countryIso, ContactInfo callLogInfo,
+ boolean immediate) {
+ requests.add(new Request(number, countryIso, callLogInfo, immediate));
}
}
}
diff --git a/tests/src/com/android/contacts/calllog/CallLogQueryTestUtils.java b/tests/src/com/android/contacts/calllog/CallLogQueryTestUtils.java
index ff18477..331d388 100644
--- a/tests/src/com/android/contacts/calllog/CallLogQueryTestUtils.java
+++ b/tests/src/com/android/contacts/calllog/CallLogQueryTestUtils.java
@@ -28,7 +28,8 @@
public class CallLogQueryTestUtils {
public static Object[] createTestValues() {
Object[] values = new Object[]{
- 0L, "", 0L, 0L, Calls.INCOMING_TYPE, "", "", "", null, 0, null, null, null, null, 0L
+ 0L, "", 0L, 0L, Calls.INCOMING_TYPE, "", "", "", null, 0, null, null, null, null,
+ 0L, null
};
assertEquals(CallLogQuery._PROJECTION.length, values.length);
return values;
@@ -37,7 +38,7 @@
public static Object[] createTestExtendedValues() {
Object[] values = new Object[]{
0L, "", 0L, 0L, Calls.INCOMING_TYPE, "", "", "", null, 0, null, null, null, null,
- 0L, CallLogQuery.SECTION_OLD_ITEM
+ 0L, null, CallLogQuery.SECTION_OLD_ITEM
};
Assert.assertEquals(CallLogQuery.EXTENDED_PROJECTION.length, values.length);
return values;