From ef2bc0b3e9f1fde01fe625906d0fa9b803a30471 Mon Sep 17 00:00:00 2001 From: Makoto Onuki Date: Wed, 2 Mar 2011 17:25:27 -0800 Subject: [PATCH] DO NOT MERGE: Fix ANR: Run getPreviewIcon on bg thread The new class EmailAsyncTask might look overkill, but this is what I've been wanting for long time. In many activities we store all AsyncTasks we start to member fields so that we can cancel them in onDestroy(). (e.g. MessageViewFragmentBase.mLoadMessageTask and mReloadMessageTask) With EmailAsyncTask these fields will no longer be necessary. We'll be able to just fire up as many AsyncTasks as we want, and clean them up in onDestroy() with just cancellAllInterrupt(). Bug 3480136 Backport of Id8aa1ba1500eee58cfab8b562b95e9ed852b3e29 Change-Id: I2d2966ff878862a5246c031d1d4e221da5a7e81a --- .../emailcommon/utility/EmailAsyncTask.java | 154 ++++++++++++++++++ .../activity/MessageViewFragmentBase.java | 76 ++++++--- .../utility/EmailAsyncTaskTests.java | 111 +++++++++++++ 3 files changed, 319 insertions(+), 22 deletions(-) create mode 100644 emailcommon/src/com/android/emailcommon/utility/EmailAsyncTask.java create mode 100644 tests/src/com/android/emailcommon/utility/EmailAsyncTaskTests.java diff --git a/emailcommon/src/com/android/emailcommon/utility/EmailAsyncTask.java b/emailcommon/src/com/android/emailcommon/utility/EmailAsyncTask.java new file mode 100644 index 000000000..c157e2ff3 --- /dev/null +++ b/emailcommon/src/com/android/emailcommon/utility/EmailAsyncTask.java @@ -0,0 +1,154 @@ +/* + * Copyright (C) 2011 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.emailcommon.utility; + +import android.os.AsyncTask; + +import java.util.LinkedList; +import java.util.concurrent.ExecutionException; + +/** + * {@link AsyncTask} substitution for the email app. + * + * Modeled after {@link AsyncTask}; the basic usage is the same, with extra features: + * - Bulk cancellation of multiple tasks. This is mainly used by UI to cancell pending tasks + * in onDestroy() or similar places. + * - More features to come... + * + * Note this class isn't 100% compatible to the regular {@link AsyncTask}, e.g. it lacks + * {@link AsyncTask#onProgressUpdate}. Add these when necessary. + */ +public abstract class EmailAsyncTask { + /** + * Tracks {@link EmailAsyncTask}. + * + * Call {@link #cancellAllInterrupt()} to cancel all tasks registered. + */ + public static class Tracker { + private final LinkedList> mTasks = + new LinkedList>(); + + private void add(EmailAsyncTask task) { + synchronized (mTasks) { + mTasks.add(task); + } + } + + private void remove(EmailAsyncTask task) { + synchronized (mTasks) { + mTasks.remove(task); + } + } + + /** + * Cancel all registered tasks. + */ + public void cancellAllInterrupt() { + synchronized (mTasks) { + for (EmailAsyncTask task : mTasks) { + task.cancel(true); + } + mTasks.clear(); + } + } + + /* package */ int getTaskCountForTest() { + return mTasks.size(); + } + } + + private final Tracker mTracker; + + private static class InnerTask + extends AsyncTask { + private final EmailAsyncTask mOwner; + + public InnerTask(EmailAsyncTask owner) { + mOwner = owner; + } + + @Override + protected Result2 doInBackground(Params2... params) { + return mOwner.doInBackground(params); + } + + @Override + public void onCancelled(Result2 result) { + mOwner.unregisterSelf(); + mOwner.onCancelled(result); + } + + @Override + public void onPostExecute(Result2 result) { + mOwner.unregisterSelf(); + mOwner.onPostExecute(result); + } + } + + private final InnerTask mInnerTask; + + public EmailAsyncTask(Tracker tracker) { + mTracker = tracker; + if (mTracker != null) { + mTracker.add(this); + } + mInnerTask = new InnerTask(this); + } + + /* package */ void unregisterSelf() { + if (mTracker != null) { + mTracker.remove(this); + } + } + + protected abstract Result doInBackground(Params... params); + + public final boolean cancel(boolean mayInterruptIfRunning) { + return mInnerTask.cancel(mayInterruptIfRunning); + } + + protected void onCancelled(Result result) { + } + + protected void onPostExecute(Result result) { + } + + public final EmailAsyncTask execute(Params... params) { + mInnerTask.execute(params); + return this; + } + + public final Result get() throws InterruptedException, ExecutionException { + return mInnerTask.get(); + } + + public final boolean isCancelled() { + return mInnerTask.isCancelled(); + } + + /* package */ Result callDoInBackgroundForTest(Params... params) { + return mInnerTask.doInBackground(params); + } + + /* package */ void callOnCancelledForTest(Result result) { + mInnerTask.onCancelled(result); + } + + /* package */ void callOnPostExecuteForTest(Result result) { + mInnerTask.onPostExecute(result); + } +} diff --git a/src/com/android/email/activity/MessageViewFragmentBase.java b/src/com/android/email/activity/MessageViewFragmentBase.java index 903acd02c..54ea10be3 100644 --- a/src/com/android/email/activity/MessageViewFragmentBase.java +++ b/src/com/android/email/activity/MessageViewFragmentBase.java @@ -34,6 +34,7 @@ import com.android.emailcommon.provider.EmailContent.Body; import com.android.emailcommon.provider.EmailContent.Mailbox; import com.android.emailcommon.provider.EmailContent.Message; import com.android.emailcommon.utility.AttachmentUtilities; +import com.android.emailcommon.utility.EmailAsyncTask; import com.android.emailcommon.utility.Utility; import org.apache.commons.io.IOUtils; @@ -205,6 +206,9 @@ public abstract class MessageViewFragmentBase extends Fragment implements View.O private boolean mRestoredPictureLoaded; + private final EmailAsyncTask.Tracker mUpdatePreviewIconTaskTracker + = new EmailAsyncTask.Tracker(); + /** * Zoom scales for webview. Values correspond to {@link Preferences#TEXT_ZOOM_TINY}.. * {@link Preferences#TEXT_ZOOM_HUGE}. @@ -423,6 +427,7 @@ public abstract class MessageViewFragmentBase extends Fragment implements View.O private void cancelAllTasks() { mMessageObserver.unregister(); + mUpdatePreviewIconTaskTracker.cancellAllInterrupt(); Utility.cancelTaskInterrupt(mLoadMessageTask); mLoadMessageTask = null; Utility.cancelTaskInterrupt(mReloadMessageTask); @@ -1154,12 +1159,12 @@ public abstract class MessageViewFragmentBase extends Fragment implements View.O } } - private Bitmap getPreviewIcon(AttachmentInfo attachment) { + private static Bitmap getPreviewIcon(Context context, AttachmentInfo attachment) { try { return BitmapFactory.decodeStream( - mContext.getContentResolver().openInputStream( + context.getContentResolver().openInputStream( AttachmentUtilities.getAttachmentThumbnailUri( - mAccountId, attachment.mId, + attachment.mAccountKey, attachment.mId, PREVIEW_ICON_WIDTH, PREVIEW_ICON_HEIGHT))); } catch (Exception e) { @@ -1168,20 +1173,6 @@ public abstract class MessageViewFragmentBase extends Fragment implements View.O } } - private void updateAttachmentThumbnail(long attachmentId) { - for (int i = 0, count = mAttachments.getChildCount(); i < count; i++) { - MessageViewAttachmentInfo attachment = - (MessageViewAttachmentInfo) mAttachments.getChildAt(i).getTag(); - if (attachment.mId == attachmentId) { - Bitmap previewIcon = getPreviewIcon(attachment); - if (previewIcon != null) { - attachment.iconView.setImageBitmap(previewIcon); - } - return; - } - } - } - /** * Subclass of AttachmentInfo which includes our views and buttons related to attachment * handling, as well as our determination of suitability for viewing (based on availability of @@ -1318,10 +1309,7 @@ public abstract class MessageViewFragmentBase extends Fragment implements View.O loadButton.setVisibility(View.GONE); cancelButton.setVisibility(View.GONE); - Bitmap previewIcon = getPreviewIcon(attachmentInfo); - if (previewIcon != null) { - attachmentIcon.setImageBitmap(previewIcon); - } + updatePreviewIcon(attachmentInfo); } else { // The attachment is not loaded, so present UI to start downloading it @@ -1398,6 +1386,17 @@ public abstract class MessageViewFragmentBase extends Fragment implements View.O mAttachments.setVisibility(View.VISIBLE); } + private MessageViewAttachmentInfo findAttachmentInfoFromView(long attachmentId) { + for (int i = 0, count = mAttachments.getChildCount(); i < count; i++) { + MessageViewAttachmentInfo attachmentInfo = + (MessageViewAttachmentInfo) mAttachments.getChildAt(i).getTag(); + if (attachmentInfo.mId == attachmentId) { + return attachmentInfo; + } + } + return null; + } + /** * Reload the UI from a provider cursor. {@link LoadMessageTask#onPostExecute} calls it. * @@ -1656,7 +1655,11 @@ public abstract class MessageViewFragmentBase extends Fragment implements View.O showAttachmentProgress(attachmentId, progress); switch (progress) { case 100: - updateAttachmentThumbnail(attachmentId); + final MessageViewAttachmentInfo attachmentInfo = + findAttachmentInfoFromView(attachmentId); + if (attachmentInfo != null) { + updatePreviewIcon(attachmentInfo); + } doFinishLoadAttachment(attachmentId); break; default: @@ -1757,6 +1760,35 @@ public abstract class MessageViewFragmentBase extends Fragment implements View.O } } + private void updatePreviewIcon(MessageViewAttachmentInfo attachmentInfo) { + new UpdatePreviewIconTask(attachmentInfo).execute(); + } + + private class UpdatePreviewIconTask extends EmailAsyncTask { + @SuppressWarnings("hiding") + private final Context mContext; + private final MessageViewAttachmentInfo mAttachmentInfo; + + public UpdatePreviewIconTask(MessageViewAttachmentInfo attachmentInfo) { + super(mUpdatePreviewIconTaskTracker); + mContext = getActivity(); + mAttachmentInfo = attachmentInfo; + } + + @Override + protected Bitmap doInBackground(Void... params) { + return getPreviewIcon(mContext, mAttachmentInfo); + } + + @Override + protected void onPostExecute(Bitmap result) { + if (result == null) { + return; + } + mAttachmentInfo.iconView.setImageBitmap(result); + } + } + public boolean isMessageLoadedForTest() { return mIsMessageLoadedForTest; } diff --git a/tests/src/com/android/emailcommon/utility/EmailAsyncTaskTests.java b/tests/src/com/android/emailcommon/utility/EmailAsyncTaskTests.java new file mode 100644 index 000000000..b0315cfa4 --- /dev/null +++ b/tests/src/com/android/emailcommon/utility/EmailAsyncTaskTests.java @@ -0,0 +1,111 @@ +/* + * Copyright (C) 2011 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.emailcommon.utility; + +import android.test.AndroidTestCase; +import android.test.MoreAsserts; + +public class EmailAsyncTaskTests extends AndroidTestCase { + public void testAll() throws Exception { + // Because AsyncTask relies on the UI thread and how we use threads in test, we can't + // execute() these tasks. + // Instead, we directly call onPostExecute/onCancel. + + final EmailAsyncTask.Tracker tracker = new EmailAsyncTask.Tracker(); + + // Initially empty + assertEquals(0, tracker.getTaskCountForTest()); + + // Start 4 tasks + final MyTask task1 = new MyTask(tracker); + assertEquals(1, tracker.getTaskCountForTest()); + + final MyTask task2 = new MyTask(tracker); + assertEquals(2, tracker.getTaskCountForTest()); + + final MyTask task3 = new MyTask(tracker); + assertEquals(3, tracker.getTaskCountForTest()); + + final MyTask task4 = new MyTask(tracker); + assertEquals(4, tracker.getTaskCountForTest()); + + // Check the piping for doInBackground + task1.mDoInBackgroundResult = "R"; + assertEquals("R", task1.callDoInBackgroundForTest("1", "2")); + MoreAsserts.assertEquals(new String[] {"1", "2"}, task1.mDoInBackgroundArg); + + // Finish task1 + task1.callOnPostExecuteForTest("a"); + + // onPostExecute should unregister the instance + assertEquals(3, tracker.getTaskCountForTest()); + // and call onPostExecuteInternal + assertEquals("a", task1.mOnPostExecuteArg); + assertNull(task1.mOnCancelledArg); + + // Cancel task 3 + task3.callOnCancelledForTest("b"); + // onCancelled should unregister the instance too + assertEquals(2, tracker.getTaskCountForTest()); + // and call onCancelledInternal + assertNull(task3.mOnPostExecuteArg); + assertEquals("b", task3.mOnCancelledArg); + + // Task 2 and 4 are still registered. + + // Cancel all left + tracker.cancellAllInterrupt(); + + // Check if they're canceled + assertTrue(task2.isCancelled()); + assertTrue(task4.isCancelled()); + assertEquals(0, tracker.getTaskCountForTest()); + } + + // Make sure null tracker will be accepted + public void testNullTracker() { + final MyTask task1 = new MyTask(null); + task1.unregisterSelf(); + } + + private static class MyTask extends EmailAsyncTask { + public String[] mDoInBackgroundArg; + public String mDoInBackgroundResult; + public String mOnCancelledArg; + public String mOnPostExecuteArg; + + public MyTask(Tracker tracker) { + super(tracker); + } + + @Override + protected String doInBackground(String... params) { + mDoInBackgroundArg = params; + return mDoInBackgroundResult; + } + + @Override + protected void onCancelled(String result) { + mOnCancelledArg = result; + } + + @Override + protected void onPostExecute(String result) { + mOnPostExecuteArg = result; + } + } +}