From 2ac164f609faea134b8fa1d6c0ebc90ccda91166 Mon Sep 17 00:00:00 2001
From: Makoto Onuki <omakoto@google.com>
Date: Thu, 30 Jun 2011 16:59:42 -0700
Subject: [PATCH] Fix bug 4982804 / nested fragment transaction

Post MessageOrderManager callbacks instead of calling them directly
to break the loop.

Change-Id: I033121f7bdbadf6edd7a0fab87b960b737da820e
---
 .../email/activity/MessageOrderManager.java   | 50 ++++++++++++++++++-
 .../activity/MessageOrderManagerTest.java     | 17 ++++++-
 2 files changed, 65 insertions(+), 2 deletions(-)

diff --git a/src/com/android/email/activity/MessageOrderManager.java b/src/com/android/email/activity/MessageOrderManager.java
index 9e359b15d..6e5cacbc6 100644
--- a/src/com/android/email/activity/MessageOrderManager.java
+++ b/src/com/android/email/activity/MessageOrderManager.java
@@ -19,8 +19,10 @@ package com.android.email.activity;
 import com.android.emailcommon.provider.EmailContent;
 import com.android.emailcommon.provider.EmailContent.Message;
 import com.android.emailcommon.provider.Mailbox;
+import com.android.emailcommon.utility.DelayedOperations;
 import com.android.emailcommon.utility.EmailAsyncTask;
 import com.android.emailcommon.utility.Utility;
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 
 import android.content.ContentResolver;
@@ -56,6 +58,7 @@ public class MessageOrderManager {
     private final long mMailboxId;
     private final ContentObserver mObserver;
     private final Callback mCallback;
+    private final DelayedOperations mDelayedOperations;
 
     private LoadMessageListTask mLoadMessageListTask;
     private Cursor mCursor;
@@ -81,12 +84,56 @@ public class MessageOrderManager {
         public void onMessageNotFound();
     }
 
+    /**
+     * Wrapper for {@link Callback}, which uses {@link DelayedOperations#post(Runnable)} to
+     * kick callbacks rather than calling them directly.  This is used to avoid the "nested fragment
+     * transaction" exception.  e.g. {@link #moveTo} is often called during a fragment transaction,
+     * and if the message no longer exists we call {@link #onMessageNotFound}, which most probably
+     * triggers another fragment transaction.
+     */
+    private class PostingCallback implements Callback {
+        private final Callback mOriginal;
+
+        private PostingCallback(Callback original) {
+            mOriginal = original;
+        }
+
+        private final Runnable mOnMessagesChangedRunnable = new Runnable() {
+            @Override public void run() {
+                mOriginal.onMessagesChanged();
+            }
+        };
+
+        @Override
+        public void onMessagesChanged() {
+            mDelayedOperations.post(mOnMessagesChangedRunnable);
+        }
+
+        private final Runnable mOnMessageNotFoundRunnable = new Runnable() {
+            @Override public void run() {
+                mOriginal.onMessageNotFound();
+            }
+        };
+
+        @Override
+        public void onMessageNotFound() {
+            mDelayedOperations.post(mOnMessageNotFoundRunnable);
+        }
+    }
+
     public MessageOrderManager(Context context, long mailboxId, Callback callback) {
+        this(context, mailboxId, callback, new DelayedOperations(Utility.getMainThreadHandler()));
+    }
+
+    @VisibleForTesting
+    MessageOrderManager(Context context, long mailboxId, Callback callback,
+            DelayedOperations delayedOperations) {
         Preconditions.checkArgument(mailboxId != Mailbox.NO_MAILBOX);
         mContext = context.getApplicationContext();
         mContentResolver = mContext.getContentResolver();
+        mDelayedOperations = delayedOperations;
         mMailboxId = mailboxId;
-        mCallback = callback;
+        mCallback = new PostingCallback(callback);
         mObserver = new ContentObserver(getHandlerForContentObserver()) {
                 @Override public void onChange(boolean selfChange) {
                     if (mClosed) {
@@ -173,6 +220,7 @@ public class MessageOrderManager {
      */
     public void close() {
         mClosed = true;
+        mDelayedOperations.removeCallbacks();
         cancelTask();
         closeCursor();
     }
diff --git a/tests/src/com/android/email/activity/MessageOrderManagerTest.java b/tests/src/com/android/email/activity/MessageOrderManagerTest.java
index 4f85ac042..3bbc1f73a 100644
--- a/tests/src/com/android/email/activity/MessageOrderManagerTest.java
+++ b/tests/src/com/android/email/activity/MessageOrderManagerTest.java
@@ -18,6 +18,7 @@ package com.android.email.activity;
 
 import com.android.email.provider.EmailProvider;
 import com.android.emailcommon.provider.EmailContent;
+import com.android.emailcommon.utility.DelayedOperations;
 
 import android.content.Context;
 import android.database.AbstractCursor;
@@ -284,6 +285,20 @@ public class MessageOrderManagerTest extends ProviderTestCase2<EmailProvider> {
         }
     }
 
+    /**
+     *  "Non" delayed operation -- runs the runnable immediately
+     */
+    private static final class NonDelayedOperations extends DelayedOperations {
+        public NonDelayedOperations() {
+            super(new Handler());
+        }
+
+        @Override
+        public void post(Runnable r) {
+            r.run();
+        }
+    }
+
     /**
      * MessageOrderManager for test.  Overrides {@link #startQuery}
      */
@@ -292,7 +307,7 @@ public class MessageOrderManagerTest extends ProviderTestCase2<EmailProvider> {
         public boolean mStartQueryCalled;
 
         public MessageOrderManagerForTest(Context context, long mailboxId, Callback callback) {
-            super(context, mailboxId, callback);
+            super(context, mailboxId, callback, new NonDelayedOperations());
         }
 
         @Override void startQuery() {