From 11f7cae094720c3ab47e6c18772b1fc44e9e5372 Mon Sep 17 00:00:00 2001
From: Keisuke Kuroyanagi <ksk@google.com>
Date: Thu, 3 Oct 2013 20:55:34 +0900
Subject: [PATCH] Fix UserHistoryDictionaryTests.

Bug: 6669677
Bug: 10667710

Change-Id: I6cdc6a6c9cacc7f276fda3a26ec31e3eb928471c
---
 .../latin/ExpandableBinaryDictionary.java     | 30 ++++--
 ...ecayingExpandableBinaryDictionaryBase.java |  1 +
 ...oid_inputmethod_latin_BinaryDictionary.cpp |  1 -
 .../UserHistoryDictionaryTests.java           | 92 +++++++++----------
 4 files changed, 70 insertions(+), 54 deletions(-)

diff --git a/java/src/com/android/inputmethod/latin/ExpandableBinaryDictionary.java b/java/src/com/android/inputmethod/latin/ExpandableBinaryDictionary.java
index 0985aae586..c79a4ff90f 100644
--- a/java/src/com/android/inputmethod/latin/ExpandableBinaryDictionary.java
+++ b/java/src/com/android/inputmethod/latin/ExpandableBinaryDictionary.java
@@ -249,6 +249,9 @@ abstract public class ExpandableBinaryDictionary extends Dictionary {
                     final File file = new File(mContext.getFilesDir(), mFilename);
                     BinaryDictionary.createEmptyDictFile(file.getAbsolutePath(),
                             DICTIONARY_FORMAT_VERSION, getHeaderAttributeMap());
+                    mBinaryDictionary = new BinaryDictionary(
+                            file.getAbsolutePath(), 0 /* offset */, file.length(),
+                            true /* useFullEditDistance */, null, mDictType, mIsUpdatable);
                 } else {
                     mDictionaryWriter.clear();
                 }
@@ -273,11 +276,26 @@ abstract public class ExpandableBinaryDictionary extends Dictionary {
                 lastModifiedTime);
     }
 
+    /**
+     * Check whether GC is needed and run GC if required.
+     */
     protected void runGCIfRequired(final boolean mindsBlockByGC) {
         if (!ENABLE_BINARY_DICTIONARY_DYNAMIC_UPDATE) return;
+        getExecutor(mFilename).execute(new Runnable() {
+            @Override
+            public void run() {
+                runGCIfRequiredInternalLocked(mindsBlockByGC);
+            }
+        });
+    }
+
+    private void runGCIfRequiredInternalLocked(final boolean mindsBlockByGC) {
+        if (!ENABLE_BINARY_DICTIONARY_DYNAMIC_UPDATE) return;
+        // Calls to needsToRunGC() need to be serialized.
         if (mBinaryDictionary.needsToRunGC(mindsBlockByGC)) {
             if (setIsRegeneratingIfNotRegenerating()) {
-                getExecutor(mFilename).execute(new Runnable() {
+                // Run GC after currently existing time sensitive operations.
+                getExecutor(mFilename).executePrioritized(new Runnable() {
                     @Override
                     public void run() {
                         try {
@@ -300,11 +318,11 @@ abstract public class ExpandableBinaryDictionary extends Dictionary {
             Log.w(TAG, "addWordDynamically is called for non-updatable dictionary: " + mFilename);
             return;
         }
-        runGCIfRequired(true /* mindsBlockByGC */);
         getExecutor(mFilename).execute(new Runnable() {
             @Override
             public void run() {
                 if (ENABLE_BINARY_DICTIONARY_DYNAMIC_UPDATE) {
+                    runGCIfRequiredInternalLocked(true /* mindsBlockByGC */);
                     mBinaryDictionary.addUnigramWord(word, frequency);
                 } else {
                     // TODO: Remove.
@@ -324,11 +342,11 @@ abstract public class ExpandableBinaryDictionary extends Dictionary {
                     + mFilename);
             return;
         }
-        runGCIfRequired(true /* mindsBlockByGC */);
         getExecutor(mFilename).execute(new Runnable() {
             @Override
             public void run() {
                 if (ENABLE_BINARY_DICTIONARY_DYNAMIC_UPDATE) {
+                    runGCIfRequiredInternalLocked(true /* mindsBlockByGC */);
                     mBinaryDictionary.addBigramWords(word0, word1, frequency);
                 } else {
                     // TODO: Remove.
@@ -348,11 +366,11 @@ abstract public class ExpandableBinaryDictionary extends Dictionary {
                     + mFilename);
             return;
         }
-        runGCIfRequired(true /* mindsBlockByGC */);
         getExecutor(mFilename).execute(new Runnable() {
             @Override
             public void run() {
                 if (ENABLE_BINARY_DICTIONARY_DYNAMIC_UPDATE) {
+                    runGCIfRequiredInternalLocked(true /* mindsBlockByGC */);
                     mBinaryDictionary.removeBigramWords(word0, word1);
                 } else {
                     // TODO: Remove.
@@ -479,8 +497,8 @@ abstract public class ExpandableBinaryDictionary extends Dictionary {
         final long length = file.length();
 
         // Build the new binary dictionary
-        final BinaryDictionary newBinaryDictionary = new BinaryDictionary(filename, 0, length,
-                true /* useFullEditDistance */, null, mDictType, mIsUpdatable);
+        final BinaryDictionary newBinaryDictionary = new BinaryDictionary(filename, 0 /* offset */,
+                length, true /* useFullEditDistance */, null, mDictType, mIsUpdatable);
 
         // Ensure all threads accessing the current dictionary have finished before
         // swapping in the new one.
diff --git a/java/src/com/android/inputmethod/latin/personalization/DecayingExpandableBinaryDictionaryBase.java b/java/src/com/android/inputmethod/latin/personalization/DecayingExpandableBinaryDictionaryBase.java
index 2662164106..c8b62b6c8f 100644
--- a/java/src/com/android/inputmethod/latin/personalization/DecayingExpandableBinaryDictionaryBase.java
+++ b/java/src/com/android/inputmethod/latin/personalization/DecayingExpandableBinaryDictionaryBase.java
@@ -230,6 +230,7 @@ public abstract class DecayingExpandableBinaryDictionaryBase extends ExpandableB
         mSessions.remove(session);
     }
 
+    @UsedForTesting
     public void clearAndFlushDictionary() {
         // Clear the node structure on memory
         clear();
diff --git a/native/jni/com_android_inputmethod_latin_BinaryDictionary.cpp b/native/jni/com_android_inputmethod_latin_BinaryDictionary.cpp
index 38159b0f3f..8f21c50ecd 100644
--- a/native/jni/com_android_inputmethod_latin_BinaryDictionary.cpp
+++ b/native/jni/com_android_inputmethod_latin_BinaryDictionary.cpp
@@ -67,7 +67,6 @@ static jboolean latinime_BinaryDictionary_createEmptyDictFile(JNIEnv *env, jclas
         valueChars[valueUtf8Length] = '\0';
         HeaderReadWriteUtils::AttributeMap::mapped_type value;
         HeaderReadWriteUtils::insertCharactersIntoVector(valueChars, &value);
-
         attributeMap[key] = value;
     }
 
diff --git a/tests/src/com/android/inputmethod/latin/personalization/UserHistoryDictionaryTests.java b/tests/src/com/android/inputmethod/latin/personalization/UserHistoryDictionaryTests.java
index ddc9546c53..7c1decb716 100644
--- a/tests/src/com/android/inputmethod/latin/personalization/UserHistoryDictionaryTests.java
+++ b/tests/src/com/android/inputmethod/latin/personalization/UserHistoryDictionaryTests.java
@@ -109,35 +109,54 @@ public class UserHistoryDictionaryTests extends AndroidTestCase {
         dict.close();
     }
 
+    /**
+     * Clear all entries in the user history dictionary.
+     * @param testFilenameSuffix file name suffix used for testing.
+     */
+    private void clearHistory(final String testFilenameSuffix) {
+        final UserHistoryDictionary dict =
+                PersonalizationHelper.getUserHistoryDictionary(getContext(),
+                        testFilenameSuffix /* locale */, mPrefs);
+        dict.clearAndFlushDictionary();
+        dict.close();
+    }
+
+    /**
+     * Shut down executer and wait until all operations of user history are done.
+     * @param testFilenameSuffix file name suffix used for testing.
+     */
+    private void waitForWriting(final String testFilenameSuffix) {
+        try {
+            final UserHistoryDictionary dict =
+                    PersonalizationHelper.getUserHistoryDictionary(getContext(),
+                            testFilenameSuffix, mPrefs);
+            dict.shutdownExecutorForTests();
+            while (!dict.isTerminatedForTests()) {
+                Thread.sleep(WAIT_TERMINATING_IN_MILLISECONDS);
+            }
+        } catch (InterruptedException e) {
+            Log.d(TAG, "InterruptedException: ", e);
+        }
+    }
+
     public void testRandomWords() {
-        File dictFile = null;
         Log.d(TAG, "This test can be used for profiling.");
         Log.d(TAG, "Usage: please set UserHistoryDictionary.PROFILE_SAVE_RESTORE to true.");
         final String testFilenameSuffix = "testRandomWords" + System.currentTimeMillis();
+        final String fileName = UserHistoryDictionary.NAME + "." + testFilenameSuffix
+                + ExpandableBinaryDictionary.DICT_FILE_EXTENSION;
+
         final int numberOfWords = 1000;
         final Random random = new Random(123456);
 
         try {
+            clearHistory(testFilenameSuffix);
             addAndWriteRandomWords(testFilenameSuffix, numberOfWords, random,
                     true /* checksContents */);
         } finally {
-            try {
-                final UserHistoryDictionary dict =
-                        PersonalizationHelper.getUserHistoryDictionary(getContext(),
-                                testFilenameSuffix, mPrefs);
-                Log.d(TAG, "waiting for writing ...");
-                dict.shutdownExecutorForTests();
-                while (!dict.isTerminatedForTests()) {
-                    Thread.sleep(WAIT_TERMINATING_IN_MILLISECONDS);
-                }
-            } catch (InterruptedException e) {
-                Log.d(TAG, "InterruptedException: " + e);
-            }
-
-            final String fileName = UserHistoryDictionary.NAME + "." + testFilenameSuffix
-                    + ExpandableBinaryDictionary.DICT_FILE_EXTENSION;
-            dictFile = new File(getContext().getFilesDir(), fileName);
-
+            Log.d(TAG, "waiting for writing ...");
+            waitForWriting(testFilenameSuffix);
+            final File dictFile = new File(getContext().getFilesDir(), fileName);
             if (dictFile != null) {
                 assertTrue(dictFile.exists());
                 assertTrue(dictFile.length() >= MIN_USER_HISTORY_DICTIONARY_FILE_SIZE);
@@ -162,6 +181,7 @@ public class UserHistoryDictionaryTests extends AndroidTestCase {
                 final String fileName = UserHistoryDictionary.NAME + "." +
                         testFilenameSuffixes[i] + ExpandableBinaryDictionary.DICT_FILE_EXTENSION;
                 dictFiles[i] = new File(getContext().getFilesDir(), fileName);
+                clearHistory(testFilenameSuffixes[i]);
             }
 
             final long start = System.currentTimeMillis();
@@ -178,19 +198,9 @@ public class UserHistoryDictionaryTests extends AndroidTestCase {
             Log.d(TAG, "testStressTestForSwitchingLanguageAndAddingWords took "
                     + (end - start) + " ms");
         } finally {
-            try {
-                Log.d(TAG, "waiting for writing ...");
-                for (int i = 0; i < numberOfLanguages; i++) {
-                    final UserHistoryDictionary dict =
-                            PersonalizationHelper.getUserHistoryDictionary(getContext(),
-                                    testFilenameSuffixes[i], mPrefs);
-                    dict.shutdownExecutorForTests();
-                    while (!dict.isTerminatedForTests()) {
-                        Thread.sleep(WAIT_TERMINATING_IN_MILLISECONDS);
-                    }
-                }
-            } catch (InterruptedException e) {
-                Log.d(TAG, "InterruptedException: " + e);
+            Log.d(TAG, "waiting for writing ...");
+            for (int i = 0; i < numberOfLanguages; i++) {
+                waitForWriting(testFilenameSuffixes[i]);
             }
             for (final File file : dictFiles) {
                 if (file != null) {
@@ -203,33 +213,21 @@ public class UserHistoryDictionaryTests extends AndroidTestCase {
     }
 
     public void testAddManyWords() {
-        File dictFile = null;
         final String testFilenameSuffix = "testRandomWords" + System.currentTimeMillis();
         final int numberOfWords =
                 ExpandableBinaryDictionary.ENABLE_BINARY_DICTIONARY_DYNAMIC_UPDATE ?
                         10000 : 1000;
         final Random random = new Random(123456);
-
-        UserHistoryDictionary dict =
-                PersonalizationHelper.getUserHistoryDictionary(getContext(),
-                        testFilenameSuffix, mPrefs);
+        clearHistory(testFilenameSuffix);
         try {
             addAndWriteRandomWords(testFilenameSuffix, numberOfWords, random,
                     true /* checksContents */);
-            dict.close();
         } finally {
-            try {
-                Log.d(TAG, "waiting for writing ...");
-                dict.shutdownExecutorForTests();
-                while (!dict.isTerminatedForTests()) {
-                    Thread.sleep(WAIT_TERMINATING_IN_MILLISECONDS);
-                }
-            } catch (InterruptedException e) {
-                Log.d(TAG, "InterruptedException: ", e);
-            }
+            Log.d(TAG, "waiting for writing ...");
+            waitForWriting(testFilenameSuffix);
             final String fileName = UserHistoryDictionary.NAME + "." + testFilenameSuffix
                     + ExpandableBinaryDictionary.DICT_FILE_EXTENSION;
-            dictFile = new File(getContext().getFilesDir(), fileName);
+            final File dictFile = new File(getContext().getFilesDir(), fileName);
             if (dictFile != null) {
                 assertTrue(dictFile.exists());
                 assertTrue(dictFile.length() >= MIN_USER_HISTORY_DICTIONARY_FILE_SIZE);
-- 
GitLab