From ee82e34a28ec87fe17a79873fc4e32de79423761 Mon Sep 17 00:00:00 2001
From: Makoto Onuki <omakoto@google.com>
Date: Wed, 2 Mar 2011 13:38:34 -0800
Subject: [PATCH] Fix ANR/crash when you open & close a large EML

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
---
 .../activity/MessageFileViewFragment.java     | 53 ++++++++-----------
 1 file changed, 23 insertions(+), 30 deletions(-)

diff --git a/src/com/android/email/activity/MessageFileViewFragment.java b/src/com/android/email/activity/MessageFileViewFragment.java
index 24417c1ef..3da137c73 100644
--- a/src/com/android/email/activity/MessageFileViewFragment.java
+++ b/src/com/android/email/activity/MessageFileViewFragment.java
@@ -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;
     }
 
     /**