From d0ee1e404db67a238d7017fb77757a0ffb2a4a1b Mon Sep 17 00:00:00 2001 From: Makoto Onuki Date: Wed, 26 Jan 2011 13:40:53 -0800 Subject: [PATCH] Fix bug where mailbox list won't update in combined view The problem was that CombinedMailboxesLoader used the cursor returned by super.loadInBackground() (which contains accounts), to build a matrix cursor (which contains special mailboxes and accounts and will be returned), and *it closed the first cursor* after building the matrix cursor. However, because this first cursor is the one that CursorLoader sets an observer, it shouldn't be closed until the returned matix cursor closes. In other words the two cursors should have the same lifecycle. Fixed it by using ClosingMatrixCursor that used by AccountsLoader, which is doing a similar thing, but properly. Bug 3387730 Change-Id: I554ade001dc25afa869eefb4dcf9887495e6753e --- .../activity/AccountSelectorAdapter.java | 21 +--------- .../email/activity/MailboxesAdapter.java | 39 +++++++++-------- .../email/data/ClosingMatrixCursor.java | 42 +++++++++++++++++++ .../email/activity/MailboxesAdapterTest.java | 2 +- 4 files changed, 63 insertions(+), 41 deletions(-) create mode 100644 src/com/android/email/data/ClosingMatrixCursor.java diff --git a/src/com/android/email/activity/AccountSelectorAdapter.java b/src/com/android/email/activity/AccountSelectorAdapter.java index c1d7322e2..7b67073bc 100644 --- a/src/com/android/email/activity/AccountSelectorAdapter.java +++ b/src/com/android/email/activity/AccountSelectorAdapter.java @@ -16,9 +16,9 @@ package com.android.email.activity; -import com.android.email.Email; import com.android.email.R; import com.android.email.Utility; +import com.android.email.data.ClosingMatrixCursor; import com.android.email.data.ThrottlingCursorLoader; import com.android.email.provider.EmailContent; import com.android.email.provider.EmailContent.Account; @@ -208,23 +208,4 @@ public class AccountSelectorAdapter extends CursorAdapter { return Utility.CloseTraceCursorWrapper.get(resultCursor); } } - - /** - * {@link MatrixCursor} which takes an extra {@link Cursor} to the constructor, and close - * it when self is closed. - */ - private static class ClosingMatrixCursor extends MatrixCursor { - private final Cursor mInnerCursor; - - public ClosingMatrixCursor(String[] columnNames, Cursor innerCursor) { - super(columnNames); - mInnerCursor = innerCursor; - } - - @Override - public void close() { - mInnerCursor.close(); - super.close(); - } - } } diff --git a/src/com/android/email/activity/MailboxesAdapter.java b/src/com/android/email/activity/MailboxesAdapter.java index 31d8e1980..076324fda 100644 --- a/src/com/android/email/activity/MailboxesAdapter.java +++ b/src/com/android/email/activity/MailboxesAdapter.java @@ -20,6 +20,7 @@ import com.android.email.Email; import com.android.email.R; import com.android.email.ResourceHelper; import com.android.email.Utility; +import com.android.email.data.ClosingMatrixCursor; import com.android.email.data.ThrottlingCursorLoader; import com.android.email.provider.EmailContent; import com.android.email.provider.EmailContent.Account; @@ -423,33 +424,31 @@ import android.widget.TextView; @Override public Cursor loadInBackground() { - final MatrixCursor combinedWithAccounts = getSpecialMailboxesCursor(mContext); final Cursor accounts = super.loadInBackground(); - try { - accounts.moveToPosition(-1); - while (accounts.moveToNext()) { - RowBuilder row = combinedWithAccounts.newRow(); - final long accountId = accounts.getLong(COLUMN_ACCOUND_ID); - row.add(accountId); - row.add(accountId); - row.add(accounts.getString(COLUMN_ACCOUNT_DISPLAY_NAME)); - row.add(-1); // No mailbox type. Shouldn't really be used. - final int unreadCount = Mailbox.getUnreadCountByAccountAndMailboxType( - mContext, accountId, Mailbox.TYPE_INBOX); - row.add(unreadCount); - row.add(unreadCount); - row.add(ROW_TYPE_ACCOUNT); - } - } finally { - accounts.close(); + final MatrixCursor combinedWithAccounts = getSpecialMailboxesCursor(mContext, accounts); + + accounts.moveToPosition(-1); + while (accounts.moveToNext()) { + RowBuilder row = combinedWithAccounts.newRow(); + final long accountId = accounts.getLong(COLUMN_ACCOUND_ID); + row.add(accountId); + row.add(accountId); + row.add(accounts.getString(COLUMN_ACCOUNT_DISPLAY_NAME)); + row.add(-1); // No mailbox type. Shouldn't really be used. + final int unreadCount = Mailbox.getUnreadCountByAccountAndMailboxType( + mContext, accountId, Mailbox.TYPE_INBOX); + row.add(unreadCount); + row.add(unreadCount); + row.add(ROW_TYPE_ACCOUNT); } return Utility.CloseTraceCursorWrapper.get(combinedWithAccounts); } } - /* package */ static MatrixCursor getSpecialMailboxesCursor(Context context) { - MatrixCursor cursor = new MatrixCursor(PROJECTION); + /* package */ static MatrixCursor getSpecialMailboxesCursor(Context context, + Cursor innerCursor) { + MatrixCursor cursor = new ClosingMatrixCursor(PROJECTION, innerCursor); // Combined inbox -- show unread count addSummaryMailboxRow(context, cursor, Mailbox.QUERY_ALL_INBOXES, Mailbox.TYPE_INBOX, diff --git a/src/com/android/email/data/ClosingMatrixCursor.java b/src/com/android/email/data/ClosingMatrixCursor.java new file mode 100644 index 000000000..a01017433 --- /dev/null +++ b/src/com/android/email/data/ClosingMatrixCursor.java @@ -0,0 +1,42 @@ +/* + * 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.email.data; + +import android.database.Cursor; +import android.database.MatrixCursor; + + +/** + * {@link MatrixCursor} which takes an extra {@link Cursor} to the constructor, and close + * it when self is closed. + */ +public class ClosingMatrixCursor extends MatrixCursor { + private final Cursor mInnerCursor; + + public ClosingMatrixCursor(String[] columnNames, Cursor innerCursor) { + super(columnNames); + mInnerCursor = innerCursor; + } + + @Override + public void close() { + if (mInnerCursor != null) { + mInnerCursor.close(); + } + super.close(); + } +} diff --git a/tests/src/com/android/email/activity/MailboxesAdapterTest.java b/tests/src/com/android/email/activity/MailboxesAdapterTest.java index 6f45b6229..ce168b757 100644 --- a/tests/src/com/android/email/activity/MailboxesAdapterTest.java +++ b/tests/src/com/android/email/activity/MailboxesAdapterTest.java @@ -73,7 +73,7 @@ public class MailboxesAdapterTest extends ProviderTestCase2 { createMessage(c, b2t, true, true); // Starred message in trash; All Starred excludes it. // Kick the method - Cursor cursor = MailboxesAdapter.getSpecialMailboxesCursor(c); + Cursor cursor = MailboxesAdapter.getSpecialMailboxesCursor(c, null); // Check the result assertEquals(4, cursor.getCount());