Straighten out database cursors behavior.

Some were never closed, other closed twice. This change
makes all Cursor instances behave, having the #close()
call in a finally{} clause, and puts the burden of closing
the cursor squarely on the creator rather than in the
called methods.
There is however one exception that is beyond the scope
of this change: UserDictionarySettings have a Cursor
member, it's never closed, and fixing the problem is not
obvious. This change adds a TODO for now.

It's not very clear if this change actually helps with
bug#12670151, but it may be related and it's a good
think to do anyway.

Bug: 12670151
Change-Id: I87cc44387e7dee3da1488671b93a28d9d73f7dc0

4 files changed, 64 insertions, 52 deletions

diff --git a/java/src/com/android/inputmethod/latin/BinaryDictionaryFileDumper.java b/java/src/com/android/inputmethod/latin/BinaryDictionaryFileDumper.java
index b4382bc2c..e428b1d54 100644
--- a/java/src/com/android/inputmethod/latin/BinaryDictionaryFileDumper.java
+++ b/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);
-            if (isProtocolV2 && null == c) {
+            cursor = client.query(queryUri, DICTIONARY_PROJECTION, null, null, null);
+            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 (c.getCount() <= 0 || !c.moveToFirst()) {
-                c.close();
+            if (null == cursor) return Collections.<WordListInfo>emptyList();
+            if (cursor.getCount() <= 0 || !cursor.moveToFirst()) {
                 return Collections.<WordListInfo>emptyList();
             }
             final ArrayList<WordListInfo> list = CollectionUtils.newArrayList();
             do {
-                final String wordListId = c.getString(0);
-                final String wordListLocale = c.getString(1);
+                final String wordListId = cursor.getString(0);
+                final String wordListLocale = cursor.getString(1);
                 if (TextUtils.isEmpty(wordListId)) continue;
                 list.add(new WordListInfo(wordListId, wordListLocale));
-            } while (c.moveToNext());
-            c.close();
+            } while (cursor.moveToNext());
             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();
         }
     }
diff --git a/java/src/com/android/inputmethod/latin/ContactsBinaryDictionary.java b/java/src/com/android/inputmethod/latin/ContactsBinaryDictionary.java
index d626ff926..11a9d1fe4 100644
--- a/java/src/com/android/inputmethod/latin/ContactsBinaryDictionary.java
+++ b/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()
-                    .query(uri, PROJECTION, null, null, null);
-            if (cursor != null) {
-                try {
-                    if (cursor.moveToFirst()) {
-                        sContactCountAtLastRebuild = getContactCount();
-                        addWords(cursor);
-                    }
-                } finally {
-                    cursor.close();
-                }
+            cursor = mContext.getContentResolver().query(uri, PROJECTION, null, null, null);
+            if (null == cursor) {
+                return;
+            }
+            if (cursor.moveToFirst()) {
+                sContactCountAtLastRebuild = getContactCount();
+                addWords(cursor);
             }
         } 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(
-                    Contacts.CONTENT_URI, PROJECTION_ID_ONLY, null, null, null);
-            if (cursor != null) {
-                try {
-                    return cursor.getCount();
-                } finally {
-                    cursor.close();
-                }
+            cursor = mContext.getContentResolver().query(Contacts.CONTENT_URI, PROJECTION_ID_ONLY,
+                    null, null, null);
+            if (null == cursor) {
+                return 0;
             }
+            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(
-                Contacts.CONTENT_URI, PROJECTION, null, null, null);
-        if (cursor != null) {
-            try {
-                if (cursor.moveToFirst()) {
-                    while (!cursor.isAfterLast()) {
-                        String name = cursor.getString(INDEX_NAME);
-                        if (isValidName(name) && !isNameInDictionary(name)) {
-                            if (DEBUG) {
-                                Log.d(TAG, "Contact name missing: " + name + " (runtime = "
-                                        + (SystemClock.uptimeMillis() - startTime) + " ms)");
-                            }
-                            return true;
+        final Cursor cursor = mContext.getContentResolver().query(Contacts.CONTENT_URI, PROJECTION,
+                null, null, null);
+        if (null == cursor) {
+            return false;
+        }
+        try {
+            if (cursor.moveToFirst()) {
+                while (!cursor.isAfterLast()) {
+                    String name = cursor.getString(INDEX_NAME);
+                    if (isValidName(name) && !isNameInDictionary(name)) {
+                        if (DEBUG) {
+                            Log.d(TAG, "Contact name missing: " + name + " (runtime = "
+                                    + (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)
diff --git a/java/src/com/android/inputmethod/latin/userdictionary/UserDictionaryList.java b/java/src/com/android/inputmethod/latin/userdictionary/UserDictionaryList.java
index 32c4950da..2f41ce9ce 100644
--- a/java/src/com/android/inputmethod/latin/userdictionary/UserDictionaryList.java
+++ b/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);
-            do {
-                final String locale = cursor.getString(columnIndex);
-                localeSet.add(null != locale ? locale : "");
-            } while (cursor.moveToNext());
+        }
+        try {
+            if (cursor.moveToFirst()) {
+                final int columnIndex = cursor.getColumnIndex(UserDictionary.Words.LOCALE);
+                do {
+                    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
diff --git a/java/src/com/android/inputmethod/latin/userdictionary/UserDictionarySettings.java b/java/src/com/android/inputmethod/latin/userdictionary/UserDictionarySettings.java
index 7571e87c5..220efb5d3 100644
--- a/java/src/com/android/inputmethod/latin/userdictionary/UserDictionarySettings.java
+++ b/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);