Merge "Straighten out database cursors behavior."

This commit is contained in:
Jean Chalard 2014-02-18 11:09:38 +00:00 committed by Android (Google) Code Review
commit 80d413caff
8 changed files with 178 additions and 136 deletions

View file

@ -350,7 +350,8 @@ public final class DictionaryProvider extends ContentProvider {
clientId); clientId);
if (null == results) { if (null == results) {
return Collections.<WordListInfo>emptyList(); return Collections.<WordListInfo>emptyList();
} else { }
try {
final HashMap<String, WordListInfo> dicts = new HashMap<String, WordListInfo>(); final HashMap<String, WordListInfo> dicts = new HashMap<String, WordListInfo>();
final int idIndex = results.getColumnIndex(MetadataDbHelper.WORDLISTID_COLUMN); final int idIndex = results.getColumnIndex(MetadataDbHelper.WORDLISTID_COLUMN);
final int localeIndex = results.getColumnIndex(MetadataDbHelper.LOCALE_COLUMN); final int localeIndex = results.getColumnIndex(MetadataDbHelper.LOCALE_COLUMN);
@ -416,8 +417,9 @@ public final class DictionaryProvider extends ContentProvider {
} }
} while (results.moveToNext()); } while (results.moveToNext());
} }
results.close();
return Collections.unmodifiableCollection(dicts.values()); return Collections.unmodifiableCollection(dicts.values());
} finally {
results.close();
} }
} }

View file

@ -283,63 +283,70 @@ public final class DictionarySettingsFragment extends PreferenceFragment
final ArrayList<Preference> result = new ArrayList<Preference>(); final ArrayList<Preference> result = new ArrayList<Preference>();
result.add(createErrorMessage(activity, R.string.cannot_connect_to_dict_service)); result.add(createErrorMessage(activity, R.string.cannot_connect_to_dict_service));
return result; return result;
} else if (!cursor.moveToFirst()) { }
final ArrayList<Preference> result = new ArrayList<Preference>(); try {
result.add(createErrorMessage(activity, R.string.no_dictionaries_available)); if (!cursor.moveToFirst()) {
cursor.close(); final ArrayList<Preference> result = new ArrayList<Preference>();
return result; result.add(createErrorMessage(activity, R.string.no_dictionaries_available));
} else { return result;
final String systemLocaleString = Locale.getDefault().toString(); } else {
final TreeMap<String, WordListPreference> prefMap = final String systemLocaleString = Locale.getDefault().toString();
new TreeMap<String, WordListPreference>(); final TreeMap<String, WordListPreference> prefMap =
final int idIndex = cursor.getColumnIndex(MetadataDbHelper.WORDLISTID_COLUMN); new TreeMap<String, WordListPreference>();
final int versionIndex = cursor.getColumnIndex(MetadataDbHelper.VERSION_COLUMN); final int idIndex = cursor.getColumnIndex(MetadataDbHelper.WORDLISTID_COLUMN);
final int localeIndex = cursor.getColumnIndex(MetadataDbHelper.LOCALE_COLUMN); final int versionIndex = cursor.getColumnIndex(MetadataDbHelper.VERSION_COLUMN);
final int descriptionIndex = cursor.getColumnIndex(MetadataDbHelper.DESCRIPTION_COLUMN); final int localeIndex = cursor.getColumnIndex(MetadataDbHelper.LOCALE_COLUMN);
final int statusIndex = cursor.getColumnIndex(MetadataDbHelper.STATUS_COLUMN); final int descriptionIndex =
final int filesizeIndex = cursor.getColumnIndex(MetadataDbHelper.FILESIZE_COLUMN); cursor.getColumnIndex(MetadataDbHelper.DESCRIPTION_COLUMN);
do { final int statusIndex = cursor.getColumnIndex(MetadataDbHelper.STATUS_COLUMN);
final String wordlistId = cursor.getString(idIndex); final int filesizeIndex = cursor.getColumnIndex(MetadataDbHelper.FILESIZE_COLUMN);
final int version = cursor.getInt(versionIndex); do {
final String localeString = cursor.getString(localeIndex); final String wordlistId = cursor.getString(idIndex);
final Locale locale = new Locale(localeString); final int version = cursor.getInt(versionIndex);
final String description = cursor.getString(descriptionIndex); final String localeString = cursor.getString(localeIndex);
final int status = cursor.getInt(statusIndex); final Locale locale = new Locale(localeString);
final int matchLevel = LocaleUtils.getMatchLevel(systemLocaleString, localeString); final String description = cursor.getString(descriptionIndex);
final String matchLevelString = LocaleUtils.getMatchLevelSortedString(matchLevel); final int status = cursor.getInt(statusIndex);
final int filesize = cursor.getInt(filesizeIndex); final int matchLevel =
// The key is sorted in lexicographic order, according to the match level, then LocaleUtils.getMatchLevel(systemLocaleString, localeString);
// the description. final String matchLevelString =
final String key = matchLevelString + "." + description + "." + wordlistId; LocaleUtils.getMatchLevelSortedString(matchLevel);
final WordListPreference existingPref = prefMap.get(key); final int filesize = cursor.getInt(filesizeIndex);
if (null == existingPref || existingPref.hasPriorityOver(status)) { // The key is sorted in lexicographic order, according to the match level, then
final WordListPreference oldPreference = mCurrentPreferenceMap.get(key); // the description.
final WordListPreference pref; final String key = matchLevelString + "." + description + "." + wordlistId;
if (null != oldPreference final WordListPreference existingPref = prefMap.get(key);
&& oldPreference.mVersion == version if (null == existingPref || existingPref.hasPriorityOver(status)) {
&& oldPreference.hasStatus(status) final WordListPreference oldPreference = mCurrentPreferenceMap.get(key);
&& oldPreference.mLocale.equals(locale)) { final WordListPreference pref;
// If the old preference has all the new attributes, reuse it. Ideally, we if (null != oldPreference
// should reuse the old pref even if its status is different and call && oldPreference.mVersion == version
// setStatus here, but setStatus calls Preference#setSummary() which needs && oldPreference.hasStatus(status)
// to be done on the UI thread and we're not on the UI thread here. We && oldPreference.mLocale.equals(locale)) {
// could do all this work on the UI thread, but in this case it's probably // If the old preference has all the new attributes, reuse it. Ideally,
// lighter to stay on a background thread and throw this old preference out. // we should reuse the old pref even if its status is different and call
pref = oldPreference; // setStatus here, but setStatus calls Preference#setSummary() which
} else { // needs to be done on the UI thread and we're not on the UI thread
// Otherwise, discard it and create a new one instead. // here. We could do all this work on the UI thread, but in this case
// TODO: when the status is different from the old one, we need to // it's probably lighter to stay on a background thread and throw this
// animate the old one out before animating the new one in. // old preference out.
pref = new WordListPreference(activity, mDictionaryListInterfaceState, pref = oldPreference;
mClientId, wordlistId, version, locale, description, status, } else {
filesize); // Otherwise, discard it and create a new one instead.
// TODO: when the status is different from the old one, we need to
// animate the old one out before animating the new one in.
pref = new WordListPreference(activity, mDictionaryListInterfaceState,
mClientId, wordlistId, version, locale, description, status,
filesize);
}
prefMap.put(key, pref);
} }
prefMap.put(key, pref); } while (cursor.moveToNext());
} mCurrentPreferenceMap = prefMap;
} while (cursor.moveToNext()); return prefMap.values();
}
} finally {
cursor.close(); cursor.close();
mCurrentPreferenceMap = prefMap;
return prefMap.values();
} }
} }

View file

@ -533,12 +533,17 @@ public class MetadataDbHelper extends SQLiteOpenHelper {
PENDINGID_COLUMN + "= ?", PENDINGID_COLUMN + "= ?",
new String[] { Long.toString(id) }, new String[] { Long.toString(id) },
null, null, null); null, null, null);
// There should never be more than one result. If because of some bug there are, returning if (null == cursor) {
// only one result is the right thing to do, because we couldn't handle several anyway return null;
// and we should still handle one. }
final ContentValues result = getFirstLineAsContentValues(cursor); try {
cursor.close(); // There should never be more than one result. If because of some bug there are,
return result; // returning only one result is the right thing to do, because we couldn't handle
// several anyway and we should still handle one.
return getFirstLineAsContentValues(cursor);
} finally {
cursor.close();
}
} }
/** /**
@ -559,11 +564,16 @@ public class MetadataDbHelper extends SQLiteOpenHelper {
new String[] { id, Integer.toString(STATUS_INSTALLED), new String[] { id, Integer.toString(STATUS_INSTALLED),
Integer.toString(STATUS_DELETING) }, Integer.toString(STATUS_DELETING) },
null, null, null); null, null, null);
// There should only be one result, but if there are several, we can't tell which if (null == cursor) {
// is the best, so we just return the first one. return null;
final ContentValues result = getFirstLineAsContentValues(cursor); }
cursor.close(); try {
return result; // There should only be one result, but if there are several, we can't tell which
// is the best, so we just return the first one.
return getFirstLineAsContentValues(cursor);
} finally {
cursor.close();
}
} }
/** /**
@ -622,10 +632,15 @@ public class MetadataDbHelper extends SQLiteOpenHelper {
METADATA_TABLE_COLUMNS, METADATA_TABLE_COLUMNS,
WORDLISTID_COLUMN + "= ? AND " + VERSION_COLUMN + "= ?", WORDLISTID_COLUMN + "= ? AND " + VERSION_COLUMN + "= ?",
new String[] { id, Integer.toString(version) }, null, null, null); new String[] { id, Integer.toString(version) }, null, null, null);
// This is a lookup by primary key, so there can't be more than one result. if (null == cursor) {
final ContentValues result = getFirstLineAsContentValues(cursor); return null;
cursor.close(); }
return result; try {
// This is a lookup by primary key, so there can't be more than one result.
return getFirstLineAsContentValues(cursor);
} finally {
cursor.close();
}
} }
/** /**
@ -641,10 +656,15 @@ public class MetadataDbHelper extends SQLiteOpenHelper {
METADATA_TABLE_COLUMNS, METADATA_TABLE_COLUMNS,
WORDLISTID_COLUMN + "= ?", WORDLISTID_COLUMN + "= ?",
new String[] { id }, null, null, VERSION_COLUMN + " DESC", "1"); new String[] { id }, null, null, VERSION_COLUMN + " DESC", "1");
// This is a lookup by primary key, so there can't be more than one result. if (null == cursor) {
final ContentValues result = getFirstLineAsContentValues(cursor); return null;
cursor.close(); }
return result; try {
// This is a lookup by primary key, so there can't be more than one result.
return getFirstLineAsContentValues(cursor);
} finally {
cursor.close();
}
} }
/** /**

View file

@ -44,8 +44,7 @@ public class MetadataHandler {
*/ */
private static List<WordListMetadata> makeMetadataObject(final Cursor results) { private static List<WordListMetadata> makeMetadataObject(final Cursor results) {
final ArrayList<WordListMetadata> buildingMetadata = new ArrayList<WordListMetadata>(); final ArrayList<WordListMetadata> buildingMetadata = new ArrayList<WordListMetadata>();
if (null != results && results.moveToFirst()) {
if (results.moveToFirst()) {
final int localeColumn = results.getColumnIndex(MetadataDbHelper.LOCALE_COLUMN); final int localeColumn = results.getColumnIndex(MetadataDbHelper.LOCALE_COLUMN);
final int typeColumn = results.getColumnIndex(MetadataDbHelper.TYPE_COLUMN); final int typeColumn = results.getColumnIndex(MetadataDbHelper.TYPE_COLUMN);
final int descriptionColumn = final int descriptionColumn =
@ -61,7 +60,6 @@ public class MetadataHandler {
final int versionIndex = results.getColumnIndex(MetadataDbHelper.VERSION_COLUMN); final int versionIndex = results.getColumnIndex(MetadataDbHelper.VERSION_COLUMN);
final int formatVersionIndex = final int formatVersionIndex =
results.getColumnIndex(MetadataDbHelper.FORMATVERSION_COLUMN); results.getColumnIndex(MetadataDbHelper.FORMATVERSION_COLUMN);
do { do {
buildingMetadata.add(new WordListMetadata(results.getString(idIndex), buildingMetadata.add(new WordListMetadata(results.getString(idIndex),
results.getInt(typeColumn), results.getInt(typeColumn),
@ -75,8 +73,6 @@ public class MetadataHandler {
results.getInt(formatVersionIndex), results.getInt(formatVersionIndex),
0, results.getString(localeColumn))); 0, results.getString(localeColumn)));
} while (results.moveToNext()); } while (results.moveToNext());
results.close();
} }
return Collections.unmodifiableList(buildingMetadata); return Collections.unmodifiableList(buildingMetadata);
} }
@ -92,9 +88,14 @@ public class MetadataHandler {
// If clientId is null, we get a cursor on the default database (see // If clientId is null, we get a cursor on the default database (see
// MetadataDbHelper#getInstance() for more on this) // MetadataDbHelper#getInstance() for more on this)
final Cursor results = MetadataDbHelper.queryCurrentMetadata(context, clientId); final Cursor results = MetadataDbHelper.queryCurrentMetadata(context, clientId);
final List<WordListMetadata> resultList = makeMetadataObject(results); // If null, we should return makeMetadataObject(null), so we go through.
results.close(); try {
return resultList; return makeMetadataObject(results);
} finally {
if (null != results) {
results.close();
}
}
} }
/** /**

View file

@ -142,7 +142,7 @@ public final class BinaryDictionaryFileDumper {
final ContentProviderClient client = context.getContentResolver(). final ContentProviderClient client = context.getContentResolver().
acquireContentProviderClient(getProviderUriBuilder("").build()); acquireContentProviderClient(getProviderUriBuilder("").build());
if (null == client) return Collections.<WordListInfo>emptyList(); if (null == client) return Collections.<WordListInfo>emptyList();
Cursor cursor = null;
try { try {
final Uri.Builder builder = getContentUriBuilderForType(clientId, client, final Uri.Builder builder = getContentUriBuilderForType(clientId, client,
QUERY_PATH_DICT_INFO, locale.toString()); QUERY_PATH_DICT_INFO, locale.toString());
@ -154,24 +154,22 @@ public final class BinaryDictionaryFileDumper {
final boolean isProtocolV2 = (QUERY_PARAMETER_PROTOCOL_VALUE.equals( final boolean isProtocolV2 = (QUERY_PARAMETER_PROTOCOL_VALUE.equals(
queryUri.getQueryParameter(QUERY_PARAMETER_PROTOCOL))); queryUri.getQueryParameter(QUERY_PARAMETER_PROTOCOL)));
Cursor c = client.query(queryUri, DICTIONARY_PROJECTION, null, null, null); cursor = client.query(queryUri, DICTIONARY_PROJECTION, null, null, null);
if (isProtocolV2 && null == c) { if (isProtocolV2 && null == cursor) {
reinitializeClientRecordInDictionaryContentProvider(context, client, clientId); reinitializeClientRecordInDictionaryContentProvider(context, client, clientId);
c = client.query(queryUri, DICTIONARY_PROJECTION, null, null, null); cursor = client.query(queryUri, DICTIONARY_PROJECTION, null, null, null);
} }
if (null == c) return Collections.<WordListInfo>emptyList(); if (null == cursor) return Collections.<WordListInfo>emptyList();
if (c.getCount() <= 0 || !c.moveToFirst()) { if (cursor.getCount() <= 0 || !cursor.moveToFirst()) {
c.close();
return Collections.<WordListInfo>emptyList(); return Collections.<WordListInfo>emptyList();
} }
final ArrayList<WordListInfo> list = CollectionUtils.newArrayList(); final ArrayList<WordListInfo> list = CollectionUtils.newArrayList();
do { do {
final String wordListId = c.getString(0); final String wordListId = cursor.getString(0);
final String wordListLocale = c.getString(1); final String wordListLocale = cursor.getString(1);
if (TextUtils.isEmpty(wordListId)) continue; if (TextUtils.isEmpty(wordListId)) continue;
list.add(new WordListInfo(wordListId, wordListLocale)); list.add(new WordListInfo(wordListId, wordListLocale));
} while (c.moveToNext()); } while (cursor.moveToNext());
c.close();
return list; return list;
} catch (RemoteException e) { } catch (RemoteException e) {
// The documentation is unclear as to in which cases this may happen, but it probably // The documentation is unclear as to in which cases this may happen, but it probably
@ -186,6 +184,9 @@ public final class BinaryDictionaryFileDumper {
Log.e(TAG, "Unexpected exception communicating with the dictionary pack", e); Log.e(TAG, "Unexpected exception communicating with the dictionary pack", e);
return Collections.<WordListInfo>emptyList(); return Collections.<WordListInfo>emptyList();
} finally { } finally {
if (null != cursor) {
cursor.close();
}
client.release(); client.release();
} }
} }

View file

@ -139,23 +139,24 @@ public class ContactsBinaryDictionary extends ExpandableBinaryDictionary {
} }
private void loadDictionaryAsyncForUri(final Uri uri) { private void loadDictionaryAsyncForUri(final Uri uri) {
Cursor cursor = null;
try { try {
Cursor cursor = mContext.getContentResolver() cursor = mContext.getContentResolver().query(uri, PROJECTION, null, null, null);
.query(uri, PROJECTION, null, null, null); if (null == cursor) {
if (cursor != null) { return;
try { }
if (cursor.moveToFirst()) { if (cursor.moveToFirst()) {
sContactCountAtLastRebuild = getContactCount(); sContactCountAtLastRebuild = getContactCount();
addWords(cursor); addWords(cursor);
}
} finally {
cursor.close();
}
} }
} catch (final SQLiteException e) { } catch (final SQLiteException e) {
Log.e(TAG, "SQLiteException in the remote Contacts process.", e); Log.e(TAG, "SQLiteException in the remote Contacts process.", e);
} catch (final IllegalStateException e) { } catch (final IllegalStateException e) {
Log.e(TAG, "Contacts DB is having problems", e); Log.e(TAG, "Contacts DB is having problems", e);
} finally {
if (null != cursor) {
cursor.close();
}
} }
} }
@ -186,18 +187,20 @@ public class ContactsBinaryDictionary extends ExpandableBinaryDictionary {
private int getContactCount() { private int getContactCount() {
// TODO: consider switching to a rawQuery("select count(*)...") on the database if // TODO: consider switching to a rawQuery("select count(*)...") on the database if
// performance is a bottleneck. // performance is a bottleneck.
Cursor cursor = null;
try { try {
final Cursor cursor = mContext.getContentResolver().query( cursor = mContext.getContentResolver().query(Contacts.CONTENT_URI, PROJECTION_ID_ONLY,
Contacts.CONTENT_URI, PROJECTION_ID_ONLY, null, null, null); null, null, null);
if (cursor != null) { if (null == cursor) {
try { return 0;
return cursor.getCount();
} finally {
cursor.close();
}
} }
return cursor.getCount();
} catch (final SQLiteException e) { } catch (final SQLiteException e) {
Log.e(TAG, "SQLiteException in the remote Contacts process.", e); Log.e(TAG, "SQLiteException in the remote Contacts process.", e);
} finally {
if (null != cursor) {
cursor.close();
}
} }
return 0; return 0;
} }
@ -281,26 +284,27 @@ public class ContactsBinaryDictionary extends ExpandableBinaryDictionary {
// Check all contacts since it's not possible to find out which names have changed. // Check all contacts since it's not possible to find out which names have changed.
// This is needed because it's possible to receive extraneous onChange events even when no // This is needed because it's possible to receive extraneous onChange events even when no
// name has changed. // name has changed.
Cursor cursor = mContext.getContentResolver().query( final Cursor cursor = mContext.getContentResolver().query(Contacts.CONTENT_URI, PROJECTION,
Contacts.CONTENT_URI, PROJECTION, null, null, null); null, null, null);
if (cursor != null) { if (null == cursor) {
try { return false;
if (cursor.moveToFirst()) { }
while (!cursor.isAfterLast()) { try {
String name = cursor.getString(INDEX_NAME); if (cursor.moveToFirst()) {
if (isValidName(name) && !isNameInDictionary(name)) { while (!cursor.isAfterLast()) {
if (DEBUG) { String name = cursor.getString(INDEX_NAME);
Log.d(TAG, "Contact name missing: " + name + " (runtime = " if (isValidName(name) && !isNameInDictionary(name)) {
+ (SystemClock.uptimeMillis() - startTime) + " ms)"); if (DEBUG) {
} Log.d(TAG, "Contact name missing: " + name + " (runtime = "
return true; + (SystemClock.uptimeMillis() - startTime) + " ms)");
} }
cursor.moveToNext(); return true;
} }
cursor.moveToNext();
} }
} finally {
cursor.close();
} }
} finally {
cursor.close();
} }
if (DEBUG) { if (DEBUG) {
Log.d(TAG, "No contacts changed. (runtime = " + (SystemClock.uptimeMillis() - startTime) Log.d(TAG, "No contacts changed. (runtime = " + (SystemClock.uptimeMillis() - startTime)

View file

@ -61,12 +61,17 @@ public class UserDictionaryList extends PreferenceFragment {
if (null == cursor) { if (null == cursor) {
// The user dictionary service is not present or disabled. Return null. // The user dictionary service is not present or disabled. Return null.
return null; return null;
} else if (cursor.moveToFirst()) { }
final int columnIndex = cursor.getColumnIndex(UserDictionary.Words.LOCALE); try {
do { if (cursor.moveToFirst()) {
final String locale = cursor.getString(columnIndex); final int columnIndex = cursor.getColumnIndex(UserDictionary.Words.LOCALE);
localeSet.add(null != locale ? locale : ""); do {
} while (cursor.moveToNext()); final String locale = cursor.getString(columnIndex);
localeSet.add(null != locale ? locale : "");
} while (cursor.moveToNext());
}
} finally {
cursor.close();
} }
if (!UserDictionarySettings.IS_SHORTCUT_API_SUPPORTED) { if (!UserDictionarySettings.IS_SHORTCUT_API_SUPPORTED) {
// For ICS, we need to show "For all languages" in case that the keyboard locale // For ICS, we need to show "For all languages" in case that the keyboard locale

View file

@ -140,6 +140,8 @@ public class UserDictionarySettings extends ListFragment {
} }
mLocale = locale; mLocale = locale;
// WARNING: The following cursor is never closed! TODO: don't put that in a member, and
// make sure all cursors are correctly closed.
mCursor = createCursor(locale); mCursor = createCursor(locale);
TextView emptyView = (TextView) getView().findViewById(android.R.id.empty); TextView emptyView = (TextView) getView().findViewById(android.R.id.empty);
emptyView.setText(R.string.user_dict_settings_empty_text); emptyView.setText(R.string.user_dict_settings_empty_text);