Fix the "onPostExecute executed even when cancelled" issue

Renamed onPostExecute to onSuccess and made sure it won't called
if a task is cancelled in time.

Also removed isCancelled().  To implement it right we should make sure
that onPostExecute() isn't finished when setting mCancelled, but it's a bit
of a pain to implement right, and we don't really have to use it.

Change-Id: I3a0baf504506ffc4952a5553f7098a8415842fa3
This commit is contained in:
Makoto Onuki 2011-06-28 19:31:23 -07:00
parent 4a893e60b4
commit 50d934360d
15 changed files with 42 additions and 35 deletions

View File

@ -29,9 +29,11 @@ import java.util.concurrent.Executor;
* 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 cancel pending tasks
* in onDestroy() or similar places.
* - More features to come...
* - Instead of {@link AsyncTask#onPostExecute}, it has {@link #onSuccess(Object)}, as the
* regular {@link AsyncTask#onPostExecute} is a bit hard to predict when it'll be called and
* whel it won't.
*
* Note this class isn't 100% compatible to the regular {@link AsyncTask}, e.g. it lacks
* Note this class is missing some of the {@link AsyncTask} features, e.g. it lacks
* {@link AsyncTask#onProgressUpdate}. Add these when necessary.
*/
public abstract class EmailAsyncTask<Params, Progress, Result> {
@ -125,11 +127,16 @@ public abstract class EmailAsyncTask<Params, Progress, Result> {
@Override
public void onPostExecute(Result2 result) {
mOwner.unregisterSelf();
mOwner.onPostExecute(result);
if (mOwner.mCancelled) {
mOwner.onCancelled(result);
} else {
mOwner.onSuccess(result);
}
}
}
private final InnerTask<Params, Progress, Result> mInnerTask;
private volatile boolean mCancelled;
public EmailAsyncTask(Tracker tracker) {
mTracker = tracker;
@ -150,16 +157,23 @@ public abstract class EmailAsyncTask<Params, Progress, Result> {
/** @see AsyncTask#cancel(boolean) */
public final boolean cancel(boolean mayInterruptIfRunning) {
return mInnerTask.cancel(mayInterruptIfRunning);
public final void cancel(boolean mayInterruptIfRunning) {
mCancelled = true;
mInnerTask.cancel(mayInterruptIfRunning);
}
/** @see AsyncTask#onCancelled */
protected void onCancelled(Result result) {
}
/** @see AsyncTask#onPostExecute */
protected void onPostExecute(Result result) {
/**
* Similar to {@link AsyncTask#onPostExecute}, but this will never be executed if
* {@link #cancel(boolean)} has been called before its execution, even if
* {@link #doInBackground(Object...)} has completed when cancelled.
*
* @see AsyncTask#onPostExecute
*/
protected void onSuccess(Result result) {
}
/**
@ -246,11 +260,6 @@ public abstract class EmailAsyncTask<Params, Progress, Result> {
return mInnerTask.get();
}
/** @see AsyncTask#isCancelled */
public final boolean isCancelled() {
return mInnerTask.isCancelled();
}
/* package */ final Result callDoInBackgroundForTest(Params... params) {
return mInnerTask.doInBackground(params);
}

View File

@ -509,7 +509,7 @@ public class EmailActivity extends Activity implements View.OnClickListener, Fra
}
@Override
protected void onPostExecute(String accountName) {
protected void onSuccess(String accountName) {
String message =
MessagingExceptionStrings.getErrorString(EmailActivity.this, result);
if (!TextUtils.isEmpty(accountName)) {

View File

@ -200,7 +200,7 @@ public class MailboxFinder {
}
@Override
protected void onPostExecute(Long mailboxId) {
protected void onSuccess(Long mailboxId) {
switch (mResult) {
case RESULT_ACCOUNT_SECURITY_HOLD:
Log.w(Logging.LOG_TAG, "MailboxFinder: Account security hold.");

View File

@ -673,7 +673,7 @@ public class MailboxListFragment extends ListFragment implements OnItemClickList
}
@Override
protected void onPostExecute(Long[] result) {
protected void onSuccess(Long[] result) {
mCallback.onResult(result[0], result[1], result[2]);
}
}

View File

@ -984,7 +984,7 @@ public class MessageCompose extends Activity implements OnClickListener, OnFocus
}
@Override
protected void onPostExecute(Object[] results) {
protected void onSuccess(Object[] results) {
if ((results == null) || (results.length != 3)) {
mCallback.onLoadFailed();
return;
@ -1023,7 +1023,7 @@ public class MessageCompose extends Activity implements OnClickListener, OnFocus
}
@Override
protected void onPostExecute(Attachment[] attachments) {
protected void onSuccess(Attachment[] attachments) {
if (attachments == null) {
attachments = new Attachment[0];
}
@ -1296,7 +1296,7 @@ public class MessageCompose extends Activity implements OnClickListener, OnFocus
}
@Override
protected void onPostExecute(Long draftId) {
protected void onSuccess(Long draftId) {
// Note that send or save tasks are always completed, even if the activity
// finishes earlier.
sActiveSaveTasks.remove(mTaskId);

View File

@ -113,7 +113,7 @@ public class MessageFileView extends Activity implements MessageViewFragmentBase
}
@Override
protected void onPostExecute(String filename) {
protected void onSuccess(String filename) {
if (filename == null) {
return;
}

View File

@ -49,7 +49,7 @@ public class MessageList extends Activity {
}
@Override
protected void onPostExecute(Long accountId) {
protected void onSuccess(Long accountId) {
if ((accountId == null) || (accountId == Account.NO_ACCOUNT)) {
// Account deleted?
Utility.showToast(me, R.string.toast_account_not_found);

View File

@ -761,7 +761,7 @@ public class MessageListFragment extends ListFragment
}
@Override
protected void onPostExecute(Integer type) {
protected void onSuccess(Integer type) {
if (type == null) {
return;
}

View File

@ -284,7 +284,7 @@ public class MessageOrderManager {
}
@Override
protected void onPostExecute(Cursor cursor) {
protected void onSuccess(Cursor cursor) {
onCursorOpenDone(cursor);
}
}

View File

@ -858,7 +858,7 @@ public abstract class MessageViewFragmentBase extends Fragment implements View.O
return null;
}
@Override
protected void onPostExecute(Void result) {
protected void onSuccess(Void result) {
// If the timeout completes and the attachment has not loaded, show cancel
if (!attachment.loaded) {
attachment.cancelButton.setVisibility(View.VISIBLE);
@ -1058,7 +1058,7 @@ public abstract class MessageViewFragmentBase extends Fragment implements View.O
}
@Override
protected void onPostExecute(Message message) {
protected void onSuccess(Message message) {
if (message == null) {
resetView();
mCallback.onMessageNotExists();
@ -1092,7 +1092,7 @@ public abstract class MessageViewFragmentBase extends Fragment implements View.O
}
@Override
protected void onPostExecute(Message message) {
protected void onSuccess(Message message) {
if (message == null || message.mMailboxKey != mMessage.mMailboxKey) {
// Message deleted or moved.
mCallback.onMessageNotExists();
@ -1153,7 +1153,7 @@ public abstract class MessageViewFragmentBase extends Fragment implements View.O
}
@Override
protected void onPostExecute(String[] results) {
protected void onSuccess(String[] results) {
if (results == null) {
if (mErrorLoadingMessageBody) {
Utility.showToast(getActivity(), R.string.error_loading_message_body);
@ -1185,7 +1185,7 @@ public abstract class MessageViewFragmentBase extends Fragment implements View.O
}
@Override
protected void onPostExecute(Attachment[] attachments) {
protected void onSuccess(Attachment[] attachments) {
try {
if (attachments == null) {
return;
@ -1903,7 +1903,7 @@ public abstract class MessageViewFragmentBase extends Fragment implements View.O
}
@Override
protected void onPostExecute(Bitmap result) {
protected void onSuccess(Bitmap result) {
if (result == null) {
return;
}

View File

@ -741,7 +741,7 @@ class UIControllerTwoPane extends UIControllerBase implements
* Do the actual refresh.
*/
@Override
protected void onPostExecute(Boolean isCurrentMailboxRefreshable) {
protected void onSuccess(Boolean isCurrentMailboxRefreshable) {
if (isCurrentMailboxRefreshable == null) {
return;
}

View File

@ -344,7 +344,7 @@ public class Welcome extends Activity {
}
@Override
protected void onPostExecute(Void noResult) {
protected void onSuccess(Void noResult) {
final Activity activity = Welcome.this;
if (mStartAccountSetup) {

View File

@ -180,7 +180,7 @@ public class AccountSettingsEditQuickResponsesFragment extends Fragment
}
@Override
protected void onPostExecute(QuickResponse[] quickResponseItems) {
protected void onSuccess(QuickResponse[] quickResponseItems) {
ArrayAdapter<QuickResponse> adapter;
if (mIsEditable) {
adapter = new ArrayAdapterWithButtons(

View File

@ -291,7 +291,7 @@ public class FindParentMailboxTaskTest extends AndroidTestCase {
highlightedMailboxId, result);
// Can't execute an async task on the test thread, so emulate execution...
task.onPostExecute(task.doInBackground((Void[]) null));
task.onSuccess(task.doInBackground((Void[]) null));
assertEquals("parent", expectedNextParent, result.mNextParentMailboxId);
assertEquals("highlighted", expectedNextHighlighted, result.mNextHighlightedMailboxId);

View File

@ -73,8 +73,6 @@ public class EmailAsyncTaskTests extends AndroidTestCase {
tracker.cancellAllInterrupt();
// Check if they're canceled
assertTrue(task2.isCancelled());
assertTrue(task4.isCancelled());
assertEquals(0, tracker.getTaskCountForTest());
}
@ -137,7 +135,7 @@ public class EmailAsyncTaskTests extends AndroidTestCase {
}
@Override
protected void onPostExecute(String result) {
protected void onSuccess(String result) {
mOnPostExecuteArg = result;
}
}