ADHOC/replicant-packages_apps_Email
3
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Fix ANR/crash when you open & close a large EML

Browse Source 
The problem was:
- MessageFileViewFragment.clearContent (A) is called in onDestroy
- MessageFileViewFragment.openMessageSync (B) is called in a bg thread
  to load an eml file
- And both try to hold the same lock. (mLock)
- If EML is large enough, B takes _seconds_.  If you press back during this,
  onDestroy gets blocked trying to lock mLock.
- This could also cause a crash, because the task that runs openMessageSync
  won't get cancelled in this case, because that's done in clearContent.
  Because of this, the task's onPostExecute tries to touch a UI element
  when the fragment is actually being destroyed.

The lock was introduced to protect mFileEmailUri, only to keep the same
semantics for openMessage() as MessageViewFragment. i.e. openMessage can be
called multiple times for the same instance of the fragment.

However, in practice, this won't happen.  Unlike the regular message view,
we never reuse MessageFileViewFragment.  MessageFileViewFragment instances
are created per message.  This lock was just reminiscence from the early
developmen stage.

So, fix is simple -- just remove the lock.

Bug 3500487

Change-Id: If2b22a683666de535454bb1293563796fa7acfd7
This commit is contained in:
Makoto Onuki 2011-03-02 13:38:34 -08:00
parent 19b2a7ebc9
commit ee82e34a28
1 changed files with 23 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
src/com/android/email/activity/MessageFileViewFragment.java
View File

@ -40,9 +40,6 @@ public class MessageFileViewFragment extends MessageViewFragmentBase {
*/
private Uri mFileEmailUri;
/** Lock object to protect {@link #mFileEmailUri} */
private final Object mLock = new Object();
/**
* # of instances of this class. When it gets 0, and the last one is not destroying for
* a config change, we delete all the EML files.
@ -71,28 +68,26 @@ public class MessageFileViewFragment extends MessageViewFragmentBase {
if (Email.DEBUG_LIFECYCLE && Email.DEBUG) {
Log.d(Logging.LOG_TAG, "MessageFileViewFragment openMessage");
}
if (mFileEmailUri != null) {
// Unlike MessageViewFragment, this fragment doesn't support loading another message
// once it opens a message, even after clearContent().
throw new IllegalStateException();
}
if (fileEmailUri == null) {
throw new InvalidParameterException();
}
synchronized (mLock) {
mFileEmailUri = fileEmailUri;
}
mFileEmailUri = fileEmailUri;
loadMessageIfResumed();
}
@Override
public void clearContent() {
synchronized (mLock) {
super.clearContent();
mFileEmailUri = null;
}
super.clearContent();
}
@Override
protected boolean isMessageSpecified() {
synchronized (mLock) {
return mFileEmailUri != null;
}
return mFileEmailUri != null;
}
/**
@ -100,24 +95,22 @@ public class MessageFileViewFragment extends MessageViewFragmentBase {
*/
@Override
protected Message openMessageSync(Activity activity) {
synchronized (mLock) {
if (Email.DEBUG_LIFECYCLE && Email.DEBUG) {
Log.d(Logging.LOG_TAG, "MessageFileViewFragment openMessageSync");
}
Uri messageUri = mFileEmailUri;
if (messageUri == null) {
return null; // Called after clearContent().
}
// Put up a toast; this can take a little while...
Utility.showToast(activity, R.string.message_view_parse_message_toast);
Message msg = getController().loadMessageFromUri(messageUri);
if (msg == null) {
// Indicate that the attachment couldn't be loaded
Utility.showToast(activity, R.string.message_view_display_attachment_toast);
return null;
}
return msg;
if (Email.DEBUG_LIFECYCLE && Email.DEBUG) {
Log.d(Logging.LOG_TAG, "MessageFileViewFragment openMessageSync");
}
Uri messageUri = mFileEmailUri;
if (messageUri == null) {
return null; // Called after clearContent().
}
// Put up a toast; this can take a little while...
Utility.showToast(activity, R.string.message_view_parse_message_toast);
Message msg = getController().loadMessageFromUri(messageUri);
if (msg == null) {
// Indicate that the attachment couldn't be loaded
Utility.showToast(activity, R.string.message_view_display_attachment_toast);
return null;
}
return msg;
}
/**