From 7028a4c1672f4db5286167c0d68e0136ba7c912c Mon Sep 17 00:00:00 2001 From: cketti Date: Sat, 9 Oct 2010 00:24:43 +0000 Subject: [PATCH] Fixes issue 2144 - Removed the contact names cache (memory leak) - Changed Contacts.searchByAddress() to Contacts.getNameForAddress(). Cursor is now immediately closed. - Only try to resolve contact names when "Global settings" -> "Show contact name" is checked - Never resolve contact names if number of recipients exceeds a threshold --- src/com/fsck/k9/activity/MessageView.java | 9 +- src/com/fsck/k9/helper/Contacts.java | 9 +- src/com/fsck/k9/helper/ContactsSdk3_4.java | 33 +++--- src/com/fsck/k9/helper/ContactsSdk5.java | 28 ++--- src/com/fsck/k9/helper/MessageHelper.java | 2 +- src/com/fsck/k9/mail/Address.java | 115 ++++++++------------- 6 files changed, 83 insertions(+), 113 deletions(-) diff --git a/src/com/fsck/k9/activity/MessageView.java b/src/com/fsck/k9/activity/MessageView.java index 4fae5910f..1ba481cab 100644 --- a/src/com/fsck/k9/activity/MessageView.java +++ b/src/com/fsck/k9/activity/MessageView.java @@ -1455,8 +1455,6 @@ public class MessageView extends K9Activity implements OnClickListener { final Address senderEmail = mMessage.getFrom()[0]; mContacts.createContact(this, senderEmail); - - Address.clearContactsNameCache(); } catch (Exception e) { @@ -2170,11 +2168,12 @@ public class MessageView extends K9Activity implements OnClickListener final Message message) throws MessagingException { String subjectText = message.getSubject(); - CharSequence fromText = Address.toFriendly(message.getFrom(), mContacts); + final Contacts contacts = K9.showContactName() ? mContacts : null; + CharSequence fromText = Address.toFriendly(message.getFrom(), contacts); String dateText = getDateFormat().format(message.getSentDate()); String timeText = getTimeFormat().format(message.getSentDate()); - CharSequence toText = Address.toFriendly(message.getRecipients(RecipientType.TO), mContacts); - CharSequence ccText = Address.toFriendly(message.getRecipients(RecipientType.CC), mContacts); + CharSequence toText = Address.toFriendly(message.getRecipients(RecipientType.TO), contacts); + CharSequence ccText = Address.toFriendly(message.getRecipients(RecipientType.CC), contacts); int color = mAccount.getChipColor(); boolean hasAttachments = ((LocalMessage) message).getAttachmentCount() > 0; diff --git a/src/com/fsck/k9/helper/Contacts.java b/src/com/fsck/k9/helper/Contacts.java index 6f85bf7f5..e95caf130 100644 --- a/src/com/fsck/k9/helper/Contacts.java +++ b/src/com/fsck/k9/helper/Contacts.java @@ -147,7 +147,14 @@ public abstract class Contacts */ public abstract Cursor searchContacts(CharSequence filter); - public abstract Cursor searchByAddress(String address); + /** + * Get the name of the contact an email address belongs to. + * + * @param address The email address to search for. + * @return The name of the contact the email address belongs to. Or + * null if there's no matching contact. + */ + public abstract String getNameForAddress(String address); /** * Extract the name from a {@link Cursor} instance returned by diff --git a/src/com/fsck/k9/helper/ContactsSdk3_4.java b/src/com/fsck/k9/helper/ContactsSdk3_4.java index f512e067d..0d15d8baa 100644 --- a/src/com/fsck/k9/helper/ContactsSdk3_4.java +++ b/src/com/fsck/k9/helper/ContactsSdk3_4.java @@ -174,21 +174,16 @@ public class ContactsSdk3_4 extends com.fsck.k9.helper.Contacts } @Override - public Cursor searchByAddress(String address) + public String getNameForAddress(String address) { - final String where; - final String[] args; if (address == null) { - where = null; - args = null; - } - else - { - where = Contacts.ContactMethods.DATA + " = ?"; - args = new String[] {address}; + return null; } + final String where = Contacts.ContactMethods.DATA + " = ?"; + final String[] args = new String[] {address}; + final Cursor c = mContentResolver.query( Contacts.ContactMethods.CONTENT_EMAIL_URI, PROJECTION, @@ -196,20 +191,18 @@ public class ContactsSdk3_4 extends com.fsck.k9.helper.Contacts args, SORT_ORDER); + String name = null; if (c != null) { - /* - * To prevent expensive execution in the UI thread: - * Cursors get lazily executed, so if you don't call anything on - * the cursor before returning it from the background thread you'll - * have a complied program for the cursor, but it won't have been - * executed to generate the data yet. Often the execution is more - * expensive than the compilation... - */ - c.getCount(); + if (c.getCount() > 0) + { + c.moveToFirst(); + name = getName(c); + } + c.close(); } - return c; + return name; } @Override diff --git a/src/com/fsck/k9/helper/ContactsSdk5.java b/src/com/fsck/k9/helper/ContactsSdk5.java index 2388dd734..1ebf26e11 100644 --- a/src/com/fsck/k9/helper/ContactsSdk5.java +++ b/src/com/fsck/k9/helper/ContactsSdk5.java @@ -150,10 +150,14 @@ public class ContactsSdk5 extends com.fsck.k9.helper.Contacts } @Override - public Cursor searchByAddress(String address) + public String getNameForAddress(String address) { - final String filter = (address == null) ? "" : address; - final Uri uri = Uri.withAppendedPath(Email.CONTENT_FILTER_URI, Uri.encode(filter)); + if (address == null) + { + return null; + } + + final Uri uri = Uri.withAppendedPath(Email.CONTENT_LOOKUP_URI, Uri.encode(address)); final Cursor c = mContentResolver.query( uri, PROJECTION, @@ -161,20 +165,18 @@ public class ContactsSdk5 extends com.fsck.k9.helper.Contacts null, SORT_ORDER); + String name = null; if (c != null) { - /* - * To prevent expensive execution in the UI thread: - * Cursors get lazily executed, so if you don't call anything on - * the cursor before returning it from the background thread you'll - * have a complied program for the cursor, but it won't have been - * executed to generate the data yet. Often the execution is more - * expensive than the compilation... - */ - c.getCount(); + if (c.getCount() > 0) + { + c.moveToFirst(); + name = getName(c); + } + c.close(); } - return c; + return name; } @Override diff --git a/src/com/fsck/k9/helper/MessageHelper.java b/src/com/fsck/k9/helper/MessageHelper.java index 5487cb6a4..b8c38b308 100644 --- a/src/com/fsck/k9/helper/MessageHelper.java +++ b/src/com/fsck/k9/helper/MessageHelper.java @@ -53,7 +53,7 @@ public class MessageHelper public void populate(final MessageInfoHolder target, final Message m, final FolderInfoHolder folder, final Account account) { - final Contacts contactHelper = Contacts.getInstance(mContext); + final Contacts contactHelper = K9.showContactName() ? Contacts.getInstance(mContext) : null; try { LocalMessage message = (LocalMessage) m; diff --git a/src/com/fsck/k9/mail/Address.java b/src/com/fsck/k9/mail/Address.java index 44dcf7f9e..ed9914d5d 100644 --- a/src/com/fsck/k9/mail/Address.java +++ b/src/com/fsck/k9/mail/Address.java @@ -1,9 +1,6 @@ package com.fsck.k9.mail; -import android.database.Cursor; -import android.graphics.Color; -import android.text.Html; import android.text.Spannable; import android.text.SpannableString; import android.text.SpannableStringBuilder; @@ -23,27 +20,27 @@ import org.apache.james.mime4j.field.address.NamedMailbox; import org.apache.james.mime4j.field.address.parser.ParseException; import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; import java.util.List; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; public class Address { + /** + * If the number of addresses exceeds this value the addresses aren't + * resolved to the names of Android contacts. + * + *

+ * TODO: This number was chosen arbitrarily and should be determined by + * performance tests. + *

+ * + * @see Address#toFriendly(Address[], Contacts) + */ + private static final int TOO_MANY_ADDRESSES = 50; /** * Immutable empty {@link Address} array */ private static final Address[] EMPTY_ADDRESS_ARRAY = new Address[0]; - private static Map sContactsName = new ConcurrentHashMap(); - - public static void clearContactsNameCache() - { - sContactsName.clear(); - } - - private static final String NO_ENTRY = ""; String mAddress; @@ -256,81 +253,46 @@ public class Address return toFriendly((Contacts)null); } - public CharSequence toFriendly(Contacts contacts) + /** + * Returns the name of the contact this email address belongs to if + * the {@link Contacts contacts} parameter is not {@code null} and a + * contact is found. Otherwise the personal portion of the {@link Address} + * is returned. If that isn't available either, the email address is + * returned. + * + * @param contacts + * A {@link Contacts} instance or {@code null}. + * @return + * A "friendly" name for this {@link Address}. + */ + public CharSequence toFriendly(final Contacts contacts) { if (contacts != null) { - String name = sContactsName.get(mAddress); + final String name = contacts.getNameForAddress(mAddress); - if (name != null && name != NO_ENTRY) + // TODO: The results should probably be cached for performance reasons. + + if (name != null) { if (K9.changeRegisteredNameColor()) { - SpannableString sname = new SpannableString(name); - sname.setSpan(new ForegroundColorSpan(K9.getRegisteredNameColor()), + final SpannableString coloredName = new SpannableString(name); + coloredName.setSpan(new ForegroundColorSpan(K9.getRegisteredNameColor()), 0, - sname.length(), + coloredName.length(), Spannable.SPAN_EXCLUSIVE_EXCLUSIVE - ); - return sname; + ); + return coloredName; } else { return name; } } - if (name == null) - { - Cursor cursor = contacts.searchByAddress(mAddress); - if (cursor != null) - { - try - { - if (cursor.getCount() > 0) - { - cursor.moveToFirst(); - name = contacts.getName(cursor); // name might return null - if (name != null) - { - sContactsName.put(mAddress, name); - - if (K9.changeRegisteredNameColor()) - { - SpannableString sname = new SpannableString(name); - sname.setSpan(new ForegroundColorSpan(K9.getRegisteredNameColor()), - 0, - sname.length(), - Spannable.SPAN_EXCLUSIVE_EXCLUSIVE - ); - return sname; - } - else - { - return name; - } - } - } - else - { - sContactsName.put(mAddress, NO_ENTRY); - } - } - finally - { - // cursor.close(); // TODO: should close cursor. - } - } - } } - if (mPersonal != null && mPersonal.length() > 0) - { - return mPersonal; - } - else - { - return mAddress; - } + return ((mPersonal != null) && (mPersonal.length() > 0)) ? mPersonal : mAddress; } public static CharSequence toFriendly(Address[] addresses) @@ -344,6 +306,13 @@ public class Address { return null; } + + if (addresses.length >= TOO_MANY_ADDRESSES) + { + // Don't look up contacts if the number of addresses is very high. + contacts = null; + } + SpannableStringBuilder sb = new SpannableStringBuilder(); for (int i = 0; i < addresses.length; i++) {