am 0b03f13c: Sanitize the usage of executors.

* commit '0b03f13cabec84d2d841fde47ce9fec0d531b6a1':
  Sanitize the usage of executors.

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

@@ -23,22 +23,21 @@ import android.os.SystemClock;
 import android.provider.ContactsContract.Contacts;
 import android.util.Log;
 
-import com.android.inputmethod.annotations.UsedForTesting;
 import com.android.inputmethod.latin.ContactsManager.ContactsChangedListener;
 import com.android.inputmethod.latin.utils.ExecutorUtils;
 
 import java.util.ArrayList;
-import java.util.concurrent.ExecutorService;
 
 /**
- * A content observer that listens to updates to content provider {@link Contacts.CONTENT_URI}.
+ * A content observer that listens to updates to content provider {@link Contacts#CONTENT_URI}.
  */
 // TODO:add test
-public class ContactsContentObserver {
+public class ContactsContentObserver implements Runnable {
     private static final String TAG = ContactsContentObserver.class.getSimpleName();
     private static final boolean DEBUG = false;
 
-    private ContentObserver mObserver;
+    private ContentObserver mContentObserver;
+    private ContactsChangedListener mContactsChangedListener;
 
     private final Context mContext;
     private final ContactsManager mManager;
@@ -52,29 +51,27 @@ public class ContactsContentObserver {
         if (DEBUG) {
             Log.d(TAG, "Registered Contacts Content Observer");
         }
-        mObserver = new ContentObserver(null /* handler */) {
+        mContactsChangedListener = listener;
+        mContentObserver = new ContentObserver(null /* handler */) {
             @Override
             public void onChange(boolean self) {
-                getBgExecutor().execute(new Runnable() {
+                // TODO(zivkovic): Limit the queue to 1 instance of ContactsContentObserver.
-                    @Override
+                ExecutorUtils.getExecutorForDynamicLanguageModelUpdate()
-                    public void run() {
+                        .execute(ContactsContentObserver.this);
-                        if (haveContentsChanged()) {
-                            if (DEBUG) {
-                                Log.d(TAG, "Contacts have changed; notifying listeners");
-                            }
-                            listener.onContactsChange();
-                        }
-                    }
-                });
             }
         };
         final ContentResolver contentResolver = mContext.getContentResolver();
-        contentResolver.registerContentObserver(Contacts.CONTENT_URI, true, mObserver);
+        contentResolver.registerContentObserver(Contacts.CONTENT_URI, true, mContentObserver);
     }
 
-    @UsedForTesting
+    @Override
-    private ExecutorService getBgExecutor() {
+    public void run() {
-        return ExecutorUtils.getExecutor("Check Contacts");
+        if (haveContentsChanged()) {
+            if (DEBUG) {
+                Log.d(TAG, "Contacts have changed; notifying listeners");
+            }
+            mContactsChangedListener.onContactsChange();
+        }
     }
 
     private boolean haveContentsChanged() {
@@ -105,6 +102,6 @@ public class ContactsContentObserver {
     }
 
     public void unregister() {
-        mContext.getContentResolver().unregisterContentObserver(mObserver);
+        mContext.getContentResolver().unregisterContentObserver(mContentObserver);
     }
 }

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

@@ -443,7 +443,7 @@ public class DictionaryFacilitatorImpl implements DictionaryFacilitator {
             final Locale[] locales, final DictionaryInitializationListener listener) {
         final CountDownLatch latchForWaitingLoadingMainDictionary = new CountDownLatch(1);
         mLatchForWaitingLoadingMainDictionaries = latchForWaitingLoadingMainDictionary;
-        ExecutorUtils.getExecutor("InitializeBinaryDictionary").execute(new Runnable() {
+        ExecutorUtils.getExecutorForStaticLanguageModelUpdate().execute(new Runnable() {
             @Override
             public void run() {
                 doReloadUninitializedMainDictionaries(

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

@@ -164,12 +164,11 @@ abstract public class ExpandableBinaryDictionary extends Dictionary {
     }
 
     private void asyncExecuteTaskWithWriteLock(final Runnable task) {
-        asyncExecuteTaskWithLock(mLock.writeLock(), mDictName /* executorName */, task);
+        asyncExecuteTaskWithLock(mLock.writeLock(), task);
     }
 
-    private static void asyncExecuteTaskWithLock(final Lock lock, final String executorName,
+    private static void asyncExecuteTaskWithLock(final Lock lock, final Runnable task) {
-            final Runnable task) {
+        ExecutorUtils.getExecutorForDynamicLanguageModelUpdate().execute(new Runnable() {
-        ExecutorUtils.getExecutor(executorName).execute(new Runnable() {
             @Override
             public void run() {
                 lock.lock();
@@ -663,7 +662,7 @@ abstract public class ExpandableBinaryDictionary extends Dictionary {
         final String dictName = mDictName;
         final File dictFile = mDictFile;
         final AsyncResultHolder<DictionaryStats> result = new AsyncResultHolder<>();
-        asyncExecuteTaskWithLock(mLock.readLock(), dictName /* executorName */, new Runnable() {
+        asyncExecuteTaskWithLock(mLock.readLock(), new Runnable() {
             @Override
             public void run() {
                 final BinaryDictionary binaryDictionary = getBinaryDictionary();
@@ -714,7 +713,7 @@ abstract public class ExpandableBinaryDictionary extends Dictionary {
         reloadDictionaryIfRequired();
         final String tag = TAG;
         final String dictName = mDictName;
-        asyncExecuteTaskWithLock(mLock.readLock(), "dumpAllWordsForDebug", new Runnable() {
+        asyncExecuteTaskWithLock(mLock.readLock(), new Runnable() {
             @Override
             public void run() {
                 Log.d(tag, "Dump dictionary: " + dictName + " for " + mLocale);
@@ -752,7 +751,7 @@ abstract public class ExpandableBinaryDictionary extends Dictionary {
     public WordProperty[] getWordPropertiesForSyncing() {
         reloadDictionaryIfRequired();
         final AsyncResultHolder<WordProperty[]> result = new AsyncResultHolder<>();
-        asyncExecuteTaskWithLock(mLock.readLock(), "sync-read", new Runnable() {
+        asyncExecuteTaskWithLock(mLock.readLock(), new Runnable() {
             @Override
             public void run() {
                 final ArrayList<WordProperty> wordPropertyList = new ArrayList<>();

java/src/com/android/inputmethod/latin/spellcheck/UserDictionaryLookup.java

@@ -26,14 +26,13 @@ import android.util.Log;
 
 import com.android.inputmethod.annotations.UsedForTesting;
 import com.android.inputmethod.latin.common.LocaleUtils;
+import com.android.inputmethod.latin.utils.ExecutorUtils;
 
 import java.io.Closeable;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Locale;
 import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.concurrent.Executors;
-import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
 
@@ -82,12 +81,6 @@ public class UserDictionaryLookup implements Closeable {
 
     private final ContentResolver mResolver;
 
-    /**
-     *  Executor on which to perform the initial load and subsequent reloads (after a delay).
-     */
-    private final ScheduledExecutorService mLoadExecutor =
-            Executors.newSingleThreadScheduledExecutor();
-
     /**
      * Runnable that calls loadUserDictionary().
      */
@@ -150,7 +143,8 @@ public class UserDictionaryLookup implements Closeable {
             }
 
             // Schedule a new reload after RELOAD_DELAY_MS.
-            mReloadFuture = mLoadExecutor.schedule(mLoader, RELOAD_DELAY_MS, TimeUnit.MILLISECONDS);
+            mReloadFuture = ExecutorUtils.getExecutorForDynamicLanguageModelUpdate().schedule(
+                    mLoader, RELOAD_DELAY_MS, TimeUnit.MILLISECONDS);
         }
     }
     private final ContentObserver mObserver = new UserDictionaryContentObserver();
@@ -192,7 +186,8 @@ public class UserDictionaryLookup implements Closeable {
         // Schedule the initial load to run immediately.  It's possible that the first call to
         // isValidWord occurs before the dictionary has actually loaded, so it should not
         // assume that the dictionary has been loaded.
-        mLoadExecutor.schedule(mLoader, 0, TimeUnit.MILLISECONDS);
+        ExecutorUtils.getExecutorForDynamicLanguageModelUpdate().schedule(
+                mLoader, 0, TimeUnit.MILLISECONDS);
 
         // Register the observer to be notified on changes to the UserDictionary and all individual
         // items.
@@ -236,9 +231,6 @@ public class UserDictionaryLookup implements Closeable {
             Log.d(TAG, "Close called (no pun intended), cleaning up executor and observer");
         }
         if (mIsClosed.compareAndSet(false, true)) {
-            // Shut down the load executor.
-            mLoadExecutor.shutdown();
-
             // Unregister the content observer.
             mResolver.unregisterContentObserver(mObserver);
         }
@@ -342,8 +334,7 @@ public class UserDictionaryLookup implements Closeable {
         if (DEBUG) {
             Log.d(TAG, "Loading UserDictionary");
         }
-        HashMap<String, ArrayList<Locale>> dictWords =
+        HashMap<String, ArrayList<Locale>> dictWords = new HashMap<>();
-                new HashMap<String, ArrayList<Locale>>();
         // Load the UserDictionary.  Request that items be returned in the default sort order
         // for UserDictionary, which is by frequency.
         Cursor cursor = mResolver.query(UserDictionary.Words.CONTENT_URI,
@@ -413,7 +404,7 @@ public class UserDictionaryLookup implements Closeable {
                         Log.d(TAG, "Word [" + dictWord +
                                 "] not seen for other locales, creating new entry");
                     }
-                    dictLocales = new ArrayList<Locale>();
+                    dictLocales = new ArrayList<>();
                     dictWords.put(dictWord, dictLocales);
                 }
                 // Append the locale to the list of locales this word is in.

java/src/com/android/inputmethod/latin/utils/ExecutorUtils.java

@@ -16,10 +16,12 @@
 
 package com.android.inputmethod.latin.utils;
 
+import android.util.Log;
+
 import com.android.inputmethod.annotations.UsedForTesting;
 
+import java.lang.Thread.UncaughtExceptionHandler;
 import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ThreadFactory;
@@ -28,33 +30,49 @@ import java.util.concurrent.ThreadFactory;
  * Utilities to manage executors.
  */
 public class ExecutorUtils {
-    static final ConcurrentHashMap<String, ScheduledExecutorService> sExecutorMap =
+
+    private static final String STATIC_LANGUAGE_MODEL_UPDATE = "StaticLanguageModelUpdate";
+    private static final String DYNAMIC_LANGUAGE_MODEL_UPDATE = "DynamicLanguageModelUpdate";
+
+    private static final ConcurrentHashMap<String, ScheduledExecutorService> sExecutorMap =
             new ConcurrentHashMap<>();
 
-    private static class ThreadFactoryWithId implements ThreadFactory {
+    @UsedForTesting
-        private final String mId;
+    private static ScheduledExecutorService sExecutorServiceForTests;
 
-        public ThreadFactoryWithId(final String id) {
+    @UsedForTesting
-            mId = id;
+    public static void setExecutorServiceForTests(
-        }
+            final ScheduledExecutorService executorServiceForTests) {
+        sExecutorServiceForTests = executorServiceForTests;
+    }
 
-        @Override
+    /**
-        public Thread newThread(final Runnable r) {
+     * @return scheduled executor service used to update static language models
-            return new Thread(r, "Executor - " + mId);
+     */
-        }
+    public static ScheduledExecutorService getExecutorForStaticLanguageModelUpdate() {
+        return getExecutor(STATIC_LANGUAGE_MODEL_UPDATE);
+    }
+
+    /**
+     * @return scheduled executor service used to update dynamic language models
+     */
+    public static ScheduledExecutorService getExecutorForDynamicLanguageModelUpdate() {
+        return getExecutor(DYNAMIC_LANGUAGE_MODEL_UPDATE);
     }
 
     /**
      * Gets the executor for the given id.
      */
-    public static ScheduledExecutorService getExecutor(final String id) {
+    private static ScheduledExecutorService getExecutor(final String id) {
+        if (sExecutorServiceForTests != null) {
+            return sExecutorServiceForTests;
+        }
         ScheduledExecutorService executor = sExecutorMap.get(id);
         if (executor == null) {
             synchronized (sExecutorMap) {
                 executor = sExecutorMap.get(id);
                 if (executor == null) {
-                    executor = Executors.newSingleThreadScheduledExecutor(
+                    executor = Executors.newSingleThreadScheduledExecutor(new ExecutorFactory(id));
-                            new ThreadFactoryWithId(id));
                     sExecutorMap.put(id, executor);
                 }
             }
@@ -69,14 +87,42 @@ public class ExecutorUtils {
     public static void shutdownAllExecutors() {
         synchronized (sExecutorMap) {
             for (final ScheduledExecutorService executor : sExecutorMap.values()) {
-                executor.execute(new Runnable() {
+                executor.execute(new ExecutorShutdown(executor));
-                    @Override
-                    public void run() {
-                        executor.shutdown();
-                        sExecutorMap.remove(executor);
-                    }
-                });
             }
+            sExecutorMap.clear();
+        }
+    }
+
+    private static class ExecutorFactory implements ThreadFactory {
+        private final String mThreadName;
+
+        public ExecutorFactory(final String threadName) {
+            mThreadName = threadName;
+        }
+
+        @Override
+        public Thread newThread(final Runnable runnable) {
+            Thread thread = new Thread(runnable, mThreadName);
+            thread.setUncaughtExceptionHandler(new UncaughtExceptionHandler() {
+                @Override
+                public void uncaughtException(Thread thread, Throwable ex) {
+                    Log.w(mThreadName + "-" + runnable.getClass().getSimpleName(), ex);
+                }
+            });
+            return thread;
+        }
+    }
+
+    private static class ExecutorShutdown implements Runnable {
+        private final ScheduledExecutorService mExecutor;
+
+        public ExecutorShutdown(final ScheduledExecutorService executor) {
+            mExecutor = executor;
+        }
+
+        @Override
+        public void run() {
+            mExecutor.shutdown();
         }
     }
 }

tests/src/com/android/inputmethod/latin/utils/ExecutorUtilsTests.java

@@ -25,18 +25,17 @@ import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 
 /**
- * Unit tests for ExecutorUtils.
+ * Unit tests for {@link ExecutorUtils}.
  */
 @MediumTest
 public class ExecutorUtilsTests extends AndroidTestCase {
     private static final String TAG = ExecutorUtilsTests.class.getSimpleName();
 
-    private static final String TEST_EXECUTOR_ID = "test";
     private static final int NUM_OF_TASKS = 10;
     private static final int DELAY_FOR_WAITING_TASKS_MILLISECONDS = 500;
 
     public void testExecute() {
-        final ExecutorService executor = ExecutorUtils.getExecutor(TEST_EXECUTOR_ID);
+        final ExecutorService executor = ExecutorUtils.getExecutorForDynamicLanguageModelUpdate();
         final AtomicInteger v = new AtomicInteger(0);
         for (int i = 0; i < NUM_OF_TASKS; ++i) {
             executor.execute(new Runnable() {