mirror of
https://github.com/moparisthebest/Yaaic
synced 2024-11-22 08:52:18 -05:00
Hold MessageListAdapters and MessageListViews in DeckAdapter to avoid leaks
There are at least two significant memory leaks in Yaaic, which cause the client to force close after a few hours with an OutOfMemoryException: (1) The IRCService holds Conversation objects, which contain a MessageListAdapter, which have references to the ConversationActivity context. This causes Activity contexts to outlast the Activity, causing a significant memory leak over time. Fix this by holding the MessageListAdapter in the ConversationActivity's DeckAdapter instead of in the Conversation objects. The DeckAdapter's lifecycle matches that of the Activity, so this prevents the leak. (2) Every call to DeckAdapter.getView()/renderConversation() creates a new MessageListView and adds it to the deck. But adding the view to the deck causes the deck to take a reference to the view, leaking the MessageListView until the Activity is finished. (This has the effect of exacerbating the first leak, since the Activity context holds a reference to the deck.) Fix this leak by caching MessageListViews in the DeckAdapter, and returning an existing MessageListView for a Conversation in getView() if one already exists.
This commit is contained in:
parent
c4504be725
commit
ffe73b7c9f
@ -52,6 +52,7 @@ import org.yaaic.model.User;
|
||||
import org.yaaic.receiver.ConversationReceiver;
|
||||
import org.yaaic.receiver.ServerReceiver;
|
||||
import org.yaaic.view.ConversationSwitcher;
|
||||
import org.yaaic.view.MessageListView;
|
||||
|
||||
import android.app.Activity;
|
||||
import android.app.AlertDialog;
|
||||
@ -235,7 +236,7 @@ public class ConversationActivity extends Activity implements ServiceConnection,
|
||||
|
||||
// Fill view with messages that have been buffered while paused
|
||||
for (Conversation conversation : mConversations) {
|
||||
mAdapter = conversation.getMessageListAdapter();
|
||||
mAdapter = deckAdapter.getItemAdapter(conversation.getName());
|
||||
|
||||
if (mAdapter != null) {
|
||||
mAdapter.addBulkMessages(conversation.getBuffer());
|
||||
@ -401,7 +402,7 @@ public class ConversationActivity extends Activity implements ServiceConnection,
|
||||
return;
|
||||
}
|
||||
|
||||
MessageListAdapter adapter = conversation.getMessageListAdapter();
|
||||
MessageListAdapter adapter = deckAdapter.getItemAdapter(target);
|
||||
|
||||
while(conversation.hasBufferedMessages()) {
|
||||
Message message = conversation.pollBufferedMessage();
|
||||
@ -526,8 +527,10 @@ public class ConversationActivity extends Activity implements ServiceConnection,
|
||||
{
|
||||
if (keyCode == KeyEvent.KEYCODE_BACK && event.getRepeatCount() == 0) {
|
||||
if (deckAdapter.isSwitched()) {
|
||||
switcher.showNext();
|
||||
switcher.removeView(deckAdapter.getSwitchedView());
|
||||
MessageListView canvas = (MessageListView) deckAdapter.getView(deckAdapter.getPositionByName(deckAdapter.getSwitchedName()), null, switcher);
|
||||
canvas.setLayoutParams(new Gallery.LayoutParams(switcher.getWidth()*85/100, switcher.getHeight()));
|
||||
canvas.setTranscriptMode(MessageListView.TRANSCRIPT_MODE_ALWAYS_SCROLL);
|
||||
canvas.setDelegateTouchEvents(true);
|
||||
deckAdapter.setSwitched(null, null);
|
||||
return true;
|
||||
}
|
||||
|
@ -344,4 +344,4 @@ public class ServersActivity extends ListActivity implements ServiceConnection,
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
}
|
||||
|
@ -41,16 +41,28 @@ import android.widget.TextView;
|
||||
*/
|
||||
public class DeckAdapter extends BaseAdapter
|
||||
{
|
||||
private LinkedList<Conversation> conversations;
|
||||
private LinkedList<ConversationInfo> conversations;
|
||||
private MessageListView currentView;
|
||||
private String currentChannel;
|
||||
|
||||
public class ConversationInfo {
|
||||
public Conversation conv;
|
||||
public MessageListAdapter adapter;
|
||||
public MessageListView view;
|
||||
|
||||
public ConversationInfo(Conversation conv) {
|
||||
this.conv = conv;
|
||||
this.adapter = null;
|
||||
this.view = null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a new DeckAdapter instance
|
||||
*/
|
||||
public DeckAdapter()
|
||||
{
|
||||
conversations = new LinkedList<Conversation>();
|
||||
conversations = new LinkedList<ConversationInfo>();
|
||||
}
|
||||
|
||||
/**
|
||||
@ -58,7 +70,7 @@ public class DeckAdapter extends BaseAdapter
|
||||
*/
|
||||
public void clearConversations()
|
||||
{
|
||||
conversations = new LinkedList<Conversation>();
|
||||
conversations = new LinkedList<ConversationInfo>();
|
||||
}
|
||||
|
||||
/**
|
||||
@ -70,16 +82,51 @@ public class DeckAdapter extends BaseAdapter
|
||||
return conversations.size();
|
||||
}
|
||||
|
||||
/**
|
||||
* Get ConversationInfo on item at position
|
||||
*/
|
||||
private ConversationInfo getItemInfo(int position) {
|
||||
if (position >= 0 && position < conversations.size()) {
|
||||
return conversations.get(position);
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get item at position
|
||||
*/
|
||||
@Override
|
||||
public Conversation getItem(int position)
|
||||
{
|
||||
if (position >= 0 && position < conversations.size()) {
|
||||
return conversations.get(position);
|
||||
ConversationInfo convInfo = getItemInfo(position);
|
||||
if (convInfo != null) {
|
||||
return convInfo.conv;
|
||||
} else {
|
||||
return null;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get MessageListAdapter belonging to a conversation
|
||||
*
|
||||
* @param position Position of the conversation in the deck
|
||||
*/
|
||||
public MessageListAdapter getItemAdapter(int position) {
|
||||
ConversationInfo convInfo = getItemInfo(position);
|
||||
if (convInfo != null) {
|
||||
return convInfo.adapter;
|
||||
} else {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Get MessageListAdapter belonging to a conversation
|
||||
*
|
||||
* @param name Name of the conversation
|
||||
*/
|
||||
public MessageListAdapter getItemAdapter(String name) {
|
||||
return getItemAdapter(getPositionByName(name));
|
||||
}
|
||||
|
||||
/**
|
||||
@ -99,7 +146,7 @@ public class DeckAdapter extends BaseAdapter
|
||||
*/
|
||||
public void addItem(Conversation conversation)
|
||||
{
|
||||
conversations.add(conversation);
|
||||
conversations.add(new ConversationInfo(conversation));
|
||||
|
||||
notifyDataSetChanged();
|
||||
}
|
||||
@ -114,10 +161,10 @@ public class DeckAdapter extends BaseAdapter
|
||||
{
|
||||
// Optimization - cache field lookups
|
||||
int mSize = conversations.size();
|
||||
LinkedList<Conversation> mItems = this.conversations;
|
||||
LinkedList<ConversationInfo> mItems = this.conversations;
|
||||
|
||||
for (int i = 0; i < mSize; i++) {
|
||||
if (mItems.get(i).getName().equalsIgnoreCase(name)) {
|
||||
if (mItems.get(i).conv.getName().equalsIgnoreCase(name)) {
|
||||
return i;
|
||||
}
|
||||
}
|
||||
@ -187,17 +234,21 @@ public class DeckAdapter extends BaseAdapter
|
||||
@Override
|
||||
public View getView(int position, View convertView, ViewGroup parent)
|
||||
{
|
||||
Conversation conversation = getItem(position);
|
||||
ConversationInfo convInfo = getItemInfo(position);
|
||||
|
||||
// Market stack traces prove that sometimes we get a null converstion
|
||||
// because the collection changed while a view is requested for an
|
||||
// item that does not exist anymore... so we just need to reply with
|
||||
// some kind of view here.
|
||||
if (conversation == null) {
|
||||
if (convInfo == null || convInfo.conv == null) {
|
||||
return new TextView(parent.getContext());
|
||||
}
|
||||
|
||||
return renderConversation(conversation, parent);
|
||||
if (convInfo.view != null) {
|
||||
return convInfo.view;
|
||||
} else {
|
||||
return renderConversation(convInfo, parent);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
@ -207,16 +258,17 @@ public class DeckAdapter extends BaseAdapter
|
||||
* @param parent The parent view (context)
|
||||
* @return The rendered MessageListView
|
||||
*/
|
||||
public MessageListView renderConversation(Conversation conversation, ViewGroup parent)
|
||||
private MessageListView renderConversation(ConversationInfo convInfo, ViewGroup parent)
|
||||
{
|
||||
MessageListView list = new MessageListView(parent.getContext(), parent);
|
||||
convInfo.view = list;
|
||||
list.setOnItemClickListener(MessageClickListener.getInstance());
|
||||
|
||||
MessageListAdapter adapter = conversation.getMessageListAdapter();
|
||||
MessageListAdapter adapter = convInfo.adapter;
|
||||
|
||||
if (adapter == null) {
|
||||
adapter = new MessageListAdapter(conversation, parent.getContext());
|
||||
conversation.setMessageListAdapter(adapter);
|
||||
adapter = new MessageListAdapter(convInfo.conv, parent.getContext());
|
||||
convInfo.adapter = adapter;
|
||||
}
|
||||
|
||||
list.setAdapter(adapter);
|
||||
|
@ -28,7 +28,7 @@ import android.view.View;
|
||||
import android.widget.AdapterView;
|
||||
import android.widget.AdapterView.OnItemClickListener;
|
||||
import android.widget.ListView;
|
||||
import android.widget.TableLayout.LayoutParams;
|
||||
import android.widget.Gallery.LayoutParams;
|
||||
import android.widget.ViewSwitcher;
|
||||
|
||||
/**
|
||||
@ -61,13 +61,11 @@ public class ConversationClickListener implements OnItemClickListener
|
||||
{
|
||||
Conversation conversation = adapter.getItem(position);
|
||||
|
||||
MessageListView canvas = adapter.renderConversation(conversation, switcher);
|
||||
MessageListView canvas = (MessageListView) adapter.getView(position, null, switcher);
|
||||
canvas.setTranscriptMode(ListView.TRANSCRIPT_MODE_NORMAL);
|
||||
canvas.setLayoutParams(new LayoutParams(LayoutParams.FILL_PARENT, LayoutParams.FILL_PARENT));
|
||||
canvas.setDelegateTouchEvents(false); // Do not delegate
|
||||
|
||||
adapter.setSwitched(conversation.getName(), canvas);
|
||||
switcher.addView(canvas);
|
||||
switcher.showNext();
|
||||
}
|
||||
}
|
||||
|
@ -22,8 +22,6 @@ package org.yaaic.model;
|
||||
|
||||
import java.util.LinkedList;
|
||||
|
||||
import org.yaaic.adapter.MessageListAdapter;
|
||||
|
||||
/**
|
||||
* Base class for conversations
|
||||
*
|
||||
@ -48,7 +46,6 @@ public abstract class Conversation
|
||||
private final LinkedList<Message> buffer;
|
||||
private final LinkedList<Message> history;
|
||||
private final String name;
|
||||
private MessageListAdapter adapter;
|
||||
private int status = 1;
|
||||
|
||||
/**
|
||||
@ -148,22 +145,6 @@ public abstract class Conversation
|
||||
buffer.clear();
|
||||
}
|
||||
|
||||
/**
|
||||
* Store the adapter of this conversation
|
||||
*/
|
||||
public void setMessageListAdapter(MessageListAdapter adapter)
|
||||
{
|
||||
this.adapter = adapter;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the MessageList Adapter of this conversation if known
|
||||
*/
|
||||
public MessageListAdapter getMessageListAdapter()
|
||||
{
|
||||
return adapter;
|
||||
}
|
||||
|
||||
/**
|
||||
* Set status of conversation
|
||||
*
|
||||
|
Loading…
Reference in New Issue
Block a user