Merge "[RF1] Remove files that don't match the expected format, step 1"

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

@@ -52,4 +52,12 @@ public final class AssetFileAddress {
         if (!f.isFile()) return null;
         return new AssetFileAddress(filename, offset, length);
     }
+
+    public boolean pointsToPhysicalFile() {
+        return 0 == mOffset;
+    }
+
+    public void deleteUnderlyingFile() {
+        new File(mFilename).delete();
+    }
 }

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

@@ -121,6 +121,7 @@ public final class BinaryDictionary extends Dictionary {
             String[] attributeKeyStringArray, String[] attributeValueStringArray);
     private static native long openNative(String sourceDir, long dictOffset, long dictSize,
             boolean isUpdatable);
+    private static native boolean hasValidContentsNative(long dict);
     private static native void flushNative(long dict, String filePath);
     private static native boolean needsToRunGCNative(long dict, boolean mindsBlockByGC);
     private static native void flushWithGCNative(long dict, String filePath);
@@ -242,6 +243,10 @@ public final class BinaryDictionary extends Dictionary {
         return mNativeDict != 0;
     }
 
+    public boolean hasValidContents() {
+        return hasValidContentsNative(mNativeDict);
+    }
+
     public int getFormatVersion() {
         return getFormatVersionNative(mNativeDict);
     }
@@ -380,7 +385,7 @@ public final class BinaryDictionary extends Dictionary {
     private void reopen() {
         close();
         final File dictFile = new File(mDictFilePath);
-        // WARNING: Because we pass 0 as the offstet and file.length() as the length, this can
+        // WARNING: Because we pass 0 as the offset and file.length() as the length, this can
         // only be called for actual files. Right now it's only called by the flush() family of
         // functions, which require an updatable dictionary, so it's okay. But beware.
         loadDictionary(dictFile.getAbsolutePath(), 0 /* startOffset */,

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

@@ -98,7 +98,7 @@ public final class BinaryDictionaryFileDumper {
      * This creates a URI builder able to build a URI pointing to the dictionary
      * pack content provider for a specific dictionary id.
      */
-    private static Uri.Builder getProviderUriBuilder(final String path) {
+    public static Uri.Builder getProviderUriBuilder(final String path) {
         return new Uri.Builder().scheme(ContentResolver.SCHEME_CONTENT)
                 .authority(DictionaryPackConstants.AUTHORITY).appendPath(path);
     }
@@ -339,15 +339,25 @@ public final class BinaryDictionaryFileDumper {
         Log.e(TAG, "Could not copy a word list. Will not be able to use it.");
         // If we can't copy it we should warn the dictionary provider so that it can mark it
         // as invalid.
+        reportBrokenFileToDictionaryProvider(providerClient, clientId, wordlistId);
+    }
+
+    public static boolean reportBrokenFileToDictionaryProvider(
+            final ContentProviderClient providerClient, final String clientId,
+            final String wordlistId) {
+        try {
+            final Uri.Builder wordListUriBuilder = getContentUriBuilderForType(clientId,
+                    providerClient, QUERY_PATH_DATAFILE, wordlistId /* extraPath */);
             wordListUriBuilder.appendQueryParameter(QUERY_PARAMETER_DELETE_RESULT,
                     QUERY_PARAMETER_FAILURE);
-        try {
             if (0 >= providerClient.delete(wordListUriBuilder.build(), null, null)) {
-                Log.e(TAG, "In addition, we were unable to delete it.");
+                Log.e(TAG, "Unable to delete a word list.");
             }
         } catch (RemoteException e) {
-            Log.e(TAG, "In addition, communication with the dictionary provider was cut", e);
+            Log.e(TAG, "Communication with the dictionary provider was cut", e);
+            return false;
         }
+        return true;
     }
 
     // Ideally the two following methods should be merged, but AssetFileDescriptor does not

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

@@ -16,6 +16,7 @@
 
 package com.android.inputmethod.latin;
 
+import android.content.ContentProviderClient;
 import android.content.Context;
 import android.content.res.AssetFileDescriptor;
 import android.content.res.Resources;
@@ -62,8 +63,12 @@ public final class DictionaryFactory {
                 final ReadOnlyBinaryDictionary readOnlyBinaryDictionary =
                         new ReadOnlyBinaryDictionary(f.mFilename, f.mOffset, f.mLength,
                                 useFullEditDistance, locale, Dictionary.TYPE_MAIN);
-                if (readOnlyBinaryDictionary.isValidDictionary()) {
+                if (readOnlyBinaryDictionary.hasValidContents()) {
                     dictList.add(readOnlyBinaryDictionary);
+                } else {
+                    readOnlyBinaryDictionary.close();
+                    // Prevent this dictionary to do any further harm.
+                    killDictionary(context, f);
                 }
             }
         }
@@ -74,6 +79,51 @@ public final class DictionaryFactory {
         return new DictionaryCollection(Dictionary.TYPE_MAIN, dictList);
     }
 
+    /**
+     * Kills a dictionary so that it is never used again, if possible.
+     * @param context The context to contact the dictionary provider, if possible.
+     * @param f A file address to the dictionary to kill.
+     */
+    private static void killDictionary(final Context context, final AssetFileAddress f) {
+        if (f.pointsToPhysicalFile()) {
+            f.deleteUnderlyingFile();
+            // Warn the dictionary provider if the dictionary came from there.
+            final ContentProviderClient providerClient;
+            try {
+                providerClient = context.getContentResolver().acquireContentProviderClient(
+                        BinaryDictionaryFileDumper.getProviderUriBuilder("").build());
+            } catch (final SecurityException e) {
+                Log.e(TAG, "No permission to communicate with the dictionary provider", e);
+                return;
+            }
+            if (null == providerClient) {
+                Log.e(TAG, "Can't establish communication with the dictionary provider");
+                return;
+            }
+            final String wordlistId =
+                    DictionaryInfoUtils.getWordListIdFromFileName(new File(f.mFilename).getName());
+            if (null != wordlistId) {
+                // TODO: this is a reasonable last resort, but it is suboptimal.
+                // The following will remove the entry for this dictionary with the dictionary
+                // provider. When the metadata is downloaded again, we will try downloading it
+                // again.
+                // However, in the practice that will mean the user will find themselves without
+                // the new dictionary. That's fine for languages where it's included in the APK,
+                // but for other languages it will leave the user without a dictionary at all until
+                // the next update, which may be a few days away.
+                // Ideally, we would trigger a new download right away, and use increasing retry
+                // delays for this particular id/version combination.
+                // Then again, this is expected to only ever happen in case of human mistake. If
+                // the wrong file is on the server, the following is still doing the right thing.
+                // If it's a file left over from the last version however, it's not great.
+                BinaryDictionaryFileDumper.reportBrokenFileToDictionaryProvider(
+                        providerClient,
+                        context.getString(R.string.dictionary_pack_client_id),
+                        wordlistId);
+            }
+        }
+    }
+
     /**
      * Initializes a main dictionary collection from a dictionary pack, with default flags.
      *

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

@@ -44,6 +44,15 @@ public final class ReadOnlyBinaryDictionary extends Dictionary {
                 locale, dictType, false /* isUpdatable */);
     }
 
+    public boolean hasValidContents() {
+        mLock.readLock().lock();
+        try {
+            return mBinaryDictionary.hasValidContents();
+        } finally {
+            mLock.readLock().unlock();
+        }
+    }
+
     public boolean isValidDictionary() {
         return mBinaryDictionary.isValidDictionary();
     }

native/jni/com_android_inputmethod_latin_BinaryDictionary.cpp

@@ -135,6 +135,14 @@ static void latinime_BinaryDictionary_close(JNIEnv *env, jclass clazz, jlong dic
     delete dictionary;
 }
 
+static bool latinime_BinaryDictionary_hasValidContents(JNIEnv *env, jclass clazz,
+        jlong dict) {
+    Dictionary *dictionary = reinterpret_cast<Dictionary *>(dict);
+    if (!dictionary) return false;
+    // TODO: check format version
+    return true;
+}
+
 static int latinime_BinaryDictionary_getFormatVersion(JNIEnv *env, jclass clazz, jlong dict) {
     Dictionary *dictionary = reinterpret_cast<Dictionary *>(dict);
     if (!dictionary) return 0;
@@ -437,6 +445,11 @@ static const JNINativeMethod sMethods[] = {
         const_cast<char *>("(J)V"),
         reinterpret_cast<void *>(latinime_BinaryDictionary_close)
     },
+    {
+        const_cast<char *>("hasValidContentsNative"),
+        const_cast<char *>("(J)Z"),
+        reinterpret_cast<void *>(latinime_BinaryDictionary_hasValidContents)
+    },
     {
         const_cast<char *>("getFormatVersionNative"),
         const_cast<char *>("(J)I"),