am 80d413ca: Merge "Straighten out database cursors behavior."

* commit '80d413caff342149aea958199cd343ebc75c8e91':
  Straighten out database cursors behavior.

java/src/com/android/inputmethod/dictionarypack/DictionaryProvider.java

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

java/src/com/android/inputmethod/dictionarypack/DictionarySettingsFragment.java

@@ -283,63 +283,70 @@ public final class DictionarySettingsFragment extends PreferenceFragment
             final ArrayList<Preference> result = new ArrayList<Preference>();
             result.add(createErrorMessage(activity, R.string.cannot_connect_to_dict_service));
             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();
-            mCurrentPreferenceMap = prefMap;
-            return prefMap.values();
         }
     }
 

java/src/com/android/inputmethod/dictionarypack/MetadataDbHelper.java

@@ -533,12 +533,17 @@ public class MetadataDbHelper extends SQLiteOpenHelper {
                 PENDINGID_COLUMN + "= ?",
                 new String[] { Long.toString(id) },
                 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),
                         Integer.toString(STATUS_DELETING) },
                 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,
                 WORDLISTID_COLUMN + "= ? AND " + VERSION_COLUMN + "= ?",
                 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,
                 WORDLISTID_COLUMN + "= ?",
                 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();
+        }
     }
 
     /**

java/src/com/android/inputmethod/dictionarypack/MetadataHandler.java

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

java/src/com/android/inputmethod/latin/BinaryDictionaryFileDumper.java

@@ -142,7 +142,7 @@ public final class BinaryDictionaryFileDumper {
         final ContentProviderClient client = context.getContentResolver().
                 acquireContentProviderClient(getProviderUriBuilder("").build());
         if (null == client) return Collections.<WordListInfo>emptyList();
-
+        Cursor cursor = null;
         try {
             final Uri.Builder builder = getContentUriBuilderForType(clientId, client,
                     QUERY_PATH_DICT_INFO, locale.toString());
@@ -154,24 +154,22 @@ public final class BinaryDictionaryFileDumper {
             final boolean isProtocolV2 = (QUERY_PARAMETER_PROTOCOL_VALUE.equals(
                     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);
-                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();
             }
             final ArrayList<WordListInfo> list = CollectionUtils.newArrayList();
             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;
                 list.add(new WordListInfo(wordListId, wordListLocale));
-            } while (c.moveToNext());
+            } while (cursor.moveToNext());
-            c.close();
             return list;
         } catch (RemoteException e) {
             // 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);
             return Collections.<WordListInfo>emptyList();
         } finally {
+            if (null != cursor) {
+                cursor.close();
+            }
             client.release();
         }
     }

java/src/com/android/inputmethod/latin/ContactsBinaryDictionary.java

@@ -139,23 +139,24 @@ public class ContactsBinaryDictionary extends ExpandableBinaryDictionary {
     }
 
     private void loadDictionaryAsyncForUri(final Uri uri) {
+        Cursor cursor = null;
         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) {
             Log.e(TAG, "SQLiteException in the remote Contacts process.", e);
         } catch (final IllegalStateException 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() {
         // TODO: consider switching to a rawQuery("select count(*)...") on the database if
         // performance is a bottleneck.
+        Cursor cursor = null;
         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) {
             Log.e(TAG, "SQLiteException in the remote Contacts process.", e);
+        } finally {
+            if (null != cursor) {
+                cursor.close();
+            }
         }
         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.
         // This is needed because it's possible to receive extraneous onChange events even when no
         // 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) {
             Log.d(TAG, "No contacts changed. (runtime = " + (SystemClock.uptimeMillis() - startTime)

java/src/com/android/inputmethod/latin/userdictionary/UserDictionaryList.java

@@ -61,12 +61,17 @@ public class UserDictionaryList extends PreferenceFragment {
         if (null == cursor) {
             // The user dictionary service is not present or disabled. 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) {
             // For ICS, we need to show "For all languages" in case that the keyboard locale

java/src/com/android/inputmethod/latin/userdictionary/UserDictionarySettings.java

@@ -140,6 +140,8 @@ public class UserDictionarySettings extends ListFragment {
         }
 
         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);
         TextView emptyView = (TextView) getView().findViewById(android.R.id.empty);
         emptyView.setText(R.string.user_dict_settings_empty_text);