🔍 Add basic search widget test

chrome/android/java/src/org/chromium/chrome/browser/searchwidget/SearchWidgetProvider.java

Index: chrome/android/java/src/org/chromium/chrome/browser/searchwidget/SearchWidgetProvider.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/searchwidget/SearchWidgetProvider.java b/chrome/android/java/src/org/chromium/chrome/browser/searchwidget/SearchWidgetProvider.java
index 169ad676f392da5ac1b57a108a0750638ea368a3..401ef25e16cabff1e98fc1ec5ab9916aa5afc445 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/searchwidget/SearchWidgetProvider.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/searchwidget/SearchWidgetProvider.java
@@ -21,6 +21,7 @@ import android.widget.RemoteViews;
 import org.chromium.base.ContextUtils;
 import org.chromium.base.Log;
 import org.chromium.base.ThreadUtils;
+import org.chromium.base.VisibleForTesting;
 import org.chromium.base.library_loader.LibraryLoader;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.IntentHandler;
@@ -34,8 +35,43 @@ import org.chromium.chrome.browser.util.IntentUtils;
  *
  * Because this is a BroadcastReceiver, it dies immediately after it runs.  A new one is created
  * for each new broadcast.
+ *
+ * This class avoids loading the native library because it can be triggered at regular intervals by
+ * Android when it tells widgets that they need updates.
  */
 public class SearchWidgetProvider extends AppWidgetProvider {
+    /** Wraps up all things that a {@link SearchWidgetProvider} can request things from. */
+    static class Delegate {

[Ted C, 2017/04/18 04:33:02]

I prefer the names to be fully qualified even internal ones as I find them
imported in other places to be hard to grok with it like this (i.e.
SearchWidgetDelegate)

[gone, 2017/04/18 17:14:32]

Done.

+        protected final Context mContext;

[Ted C, 2017/04/18 04:33:02]

why protected but provide an accessor?

[gone, 2017/04/18 17:14:32]

Don't even remember at this point.  Fixed.

+        private final AppWidgetManager mManager;
+
+        public Delegate(Context context) {
+            mContext = context == null ? ContextUtils.getApplicationContext() : context;
+            mManager = AppWidgetManager.getInstance(mContext);
+        }
+
+        /** Returns the Context to pull resources from. */
+        protected Context getContext() {
+            return mContext;
+        }
+
+        /** See {@link ContextUtils#getAppSharedPreferences}. */
+        protected SharedPreferences getSharedPreferences() {
+            return ContextUtils.getAppSharedPreferences();
+        }
+
+        /** Returns IDs for all search widgets that exist. */
+        protected int[] getAllSearchWidgetIds() {
+            return mManager.getAppWidgetIds(
+                    new ComponentName(getContext(), SearchWidgetProvider.class.getName()));
+        }
+
+        /** See {@link AppWidgetManager#updateAppWidget}. */
+        protected void updateAppWidget(int id, RemoteViews views) {
+            mManager.updateAppWidget(id, views);
+        }
+    }
+
     /** Monitors the TemplateUrlService for changes, updating the widget when necessary. */
     private static final class SearchWidgetTemplateUrlServiceObserver
             implements LoadListener, TemplateUrlServiceObserver {
@@ -49,13 +85,24 @@ public class SearchWidgetProvider extends AppWidgetProvider {
         public void onTemplateURLServiceChanged() {
             updateCachedEngineName();
         }
+
+        private void updateCachedEngineName() {
+            assert LibraryLoader.isInitialized();
+
+            // Getting an instance of the TemplateUrlService requires that the native library be
+            // loaded, but the TemplateUrlService also itself needs to be initialized.
+            TemplateUrlService service = TemplateUrlService.getInstance();
+            assert service.isLoaded();
+            SearchWidgetProvider.updateCachedEngineName(
+                    service.getDefaultSearchEngineTemplateUrl().getShortName());
+        }
     }
 
-    private static final String ACTION_START_TEXT_QUERY =
+    static final String ACTION_START_TEXT_QUERY =
             "org.chromium.chrome.browser.searchwidget.START_TEXT_QUERY";
-    private static final String ACTION_START_VOICE_QUERY =
+    static final String ACTION_START_VOICE_QUERY =
             "org.chromium.chrome.browser.searchwidget.START_VOICE_QUERY";
-    private static final String ACTION_UPDATE_ALL_WIDGETS =
+    static final String ACTION_UPDATE_ALL_WIDGETS =
             "org.chromium.chrome.browser.searchwidget.UPDATE_ALL_WIDGETS";
 
     static final String EXTRA_START_VOICE_SEARCH =
@@ -67,9 +114,11 @@ public class SearchWidgetProvider extends AppWidgetProvider {
             "org.chromium.chrome.browser.searchwidget.SEARCH_ENGINE_SHORTNAME";
 
     private static final String TAG = "searchwidget";
+    private static final Object DELEGATE_LOCK = new Object();
     private static final Object OBSERVER_LOCK = new Object();
 
     private static SearchWidgetTemplateUrlServiceObserver sObserver;
+    private static Delegate sDelegate;
 
     /**
      * Creates the singleton instance of the observer that will monitor for search engine changes.
@@ -93,24 +142,19 @@ public class SearchWidgetProvider extends AppWidgetProvider {
 
     /** Nukes all cached information and forces all widgets to start with a blank slate. */
     public static void reset() {
-        SharedPreferences.Editor editor = ContextUtils.getAppSharedPreferences().edit();
+        SharedPreferences.Editor editor = getDelegate().getSharedPreferences().edit();
         editor.remove(PREF_IS_VOICE_SEARCH_AVAILABLE);
         editor.remove(PREF_SEARCH_ENGINE_SHORTNAME);
         editor.apply();
-        updateAllWidgets();
+
+        performUpdate(null);
     }
 
     @Override
     public void onReceive(Context context, Intent intent) {
         try {
             if (IntentHandler.isIntentChromeOrFirstParty(intent)) {
-                if (ACTION_START_TEXT_QUERY.equals(intent.getAction())) {
-                    startSearchActivity(context, false);
-                } else if (ACTION_START_VOICE_QUERY.equals(intent.getAction())) {
-                    startSearchActivity(context, true);
-                } else if (ACTION_UPDATE_ALL_WIDGETS.equals(intent.getAction())) {
-                    performUpdate(context);
-                }
+                handleAction(intent.getAction());
             } else {
                 super.onReceive(context, intent);
             }
@@ -121,8 +165,34 @@ public class SearchWidgetProvider extends AppWidgetProvider {
         }
     }
 
-    private static void startSearchActivity(Context context, boolean startVoiceSearch) {
+    @Override
+    public void onUpdate(Context context, AppWidgetManager manager, int[] ids) {
+        try {
+            performUpdate(ids);
+        } catch (Exception e) {
+            // This is necessary to prevent Android refusing to launch the widget again.
+            // See http://crbug.com/712061
+            Log.e(TAG, "Caught exception when attempting to launch widget.", e);
+        }
+    }
+
+    /** Delegates out calls to the correct functions. */

[Ted C, 2017/04/18 04:33:02]

Nit of nits, the use of "Delegates" here makes me think it calls into the
delegate.  I would just say, "Handles the intent actions to the widget" or
something of that sort.\\

[gone, 2017/04/18 17:14:32]

Done.

+    @VisibleForTesting
+    static void handleAction(String action) {
+        if (ACTION_START_TEXT_QUERY.equals(action)) {
+            startSearchActivity(false);
+        } else if (ACTION_START_VOICE_QUERY.equals(action)) {
+            startSearchActivity(true);
+        } else if (ACTION_UPDATE_ALL_WIDGETS.equals(action)) {
+            performUpdate(null);
+        } else {
+            assert false;
+        }
+    }
+
+    private static void startSearchActivity(boolean startVoiceSearch) {
         Log.d(TAG, "Launching SearchActivity: VOICE=" + startVoiceSearch);
+        Context context = getDelegate().getContext();
 
         // Launch the SearchActivity.
         Intent searchIntent = new Intent();
@@ -137,33 +207,20 @@ public class SearchWidgetProvider extends AppWidgetProvider {
         IntentUtils.safeStartActivity(context, searchIntent, optionsBundle);
     }
 
-    @Override
-    public void onUpdate(Context context, AppWidgetManager manager, int[] ids) {
-        try {
-            performUpdate(context, ids);
-        } catch (Exception e) {
-            // This is necessary to prevent Android refusing to launch the widget again.
-            // See http://crbug.com/712061
-            Log.e(TAG, "Caught exception when attempting to launch widget.", e);
-        }
-    }
+    private static void performUpdate(int[] ids) {
+        Delegate delegate = getDelegate();
 
-    private static void performUpdate(Context context) {
-        AppWidgetManager manager = AppWidgetManager.getInstance(context);
-        performUpdate(context, getAllSearchWidgetIds(manager));
-    }
-
-    private static void performUpdate(Context context, int[] ids) {
+        if (ids == null) ids = delegate.getAllSearchWidgetIds();
         if (ids.length == 0) return;
 
-        AppWidgetManager manager = AppWidgetManager.getInstance(context);
-        SharedPreferences prefs = ContextUtils.getAppSharedPreferences();
-        boolean isVoiceSearchAvailable = prefs.getBoolean(PREF_IS_VOICE_SEARCH_AVAILABLE, true);
-        String engineName = prefs.getString(PREF_SEARCH_ENGINE_SHORTNAME, null);
+        SharedPreferences prefs = delegate.getSharedPreferences();
+        boolean isVoiceSearchAvailable = getCachedVoiceSearchAvailability(prefs);
+        String engineName = getCachedEngineName(prefs);
 
         for (int id : ids) {
-            RemoteViews views = createWidgetViews(context, id, engineName, isVoiceSearchAvailable);
-            manager.updateAppWidget(id, views);
+            RemoteViews views = createWidgetViews(
+                    delegate.getContext(), id, engineName, isVoiceSearchAvailable);
+            delegate.updateAppWidget(id, views);
         }
     }
 
@@ -201,10 +258,10 @@ public class SearchWidgetProvider extends AppWidgetProvider {
 
     /** Caches whether or not a voice search is possible. */
     static void updateCachedVoiceSearchAvailability(boolean isVoiceSearchAvailable) {
-        SharedPreferences prefs = ContextUtils.getAppSharedPreferences();
-        if (prefs.getBoolean(PREF_IS_VOICE_SEARCH_AVAILABLE, true) != isVoiceSearchAvailable) {
+        SharedPreferences prefs = getDelegate().getSharedPreferences();
+        if (getCachedVoiceSearchAvailability(prefs) != isVoiceSearchAvailable) {
             prefs.edit().putBoolean(PREF_IS_VOICE_SEARCH_AVAILABLE, isVoiceSearchAvailable).apply();
-            updateAllWidgets();
+            performUpdate(null);
         }
     }
 
@@ -213,28 +270,14 @@ public class SearchWidgetProvider extends AppWidgetProvider {
      * Caching it in SharedPreferences prevents us from having to load the native library and the
      * TemplateUrlService whenever the widget is updated.
      */
-    private static void updateCachedEngineName() {
-        assert LibraryLoader.isInitialized();
-
-        // Getting an instance of the TemplateUrlService requires that the native library be
-        // loaded, but the TemplateUrlService itself needs to be initialized.
-        TemplateUrlService service = TemplateUrlService.getInstance();
-        assert service.isLoaded();
-        String engineName = service.getDefaultSearchEngineTemplateUrl().getShortName();
-
-        SharedPreferences prefs = ContextUtils.getAppSharedPreferences();
-        if (!TextUtils.equals(prefs.getString(PREF_SEARCH_ENGINE_SHORTNAME, null), engineName)) {
+    static void updateCachedEngineName(String engineName) {
+        SharedPreferences prefs = getDelegate().getSharedPreferences();
+        if (!TextUtils.equals(getCachedEngineName(prefs), engineName)) {
             prefs.edit().putString(PREF_SEARCH_ENGINE_SHORTNAME, engineName).apply();
-            updateAllWidgets();
+            performUpdate(null);
         }
     }
 
-    /** Get the IDs of all existing search widgets. */
-    private static int[] getAllSearchWidgetIds(AppWidgetManager manager) {
-        return manager.getAppWidgetIds(new ComponentName(
-                ContextUtils.getApplicationContext(), SearchWidgetProvider.class.getName()));
-    }
-
     /** Creates a trusted Intent that lets the user begin performing queries. */
     private static Intent createStartQueryIntent(Context context, String action, int widgetId) {
         Intent intent = new Intent(action, Uri.parse(String.valueOf(widgetId)));
@@ -243,8 +286,25 @@ public class SearchWidgetProvider extends AppWidgetProvider {
         return intent;
     }
 
-    /** Immediately updates all widgets. */
-    private static void updateAllWidgets() {
-        performUpdate(ContextUtils.getApplicationContext());
+    /** Sets an {@link Delegate} to interact with. */
+    @VisibleForTesting
+    static void setDelegateForTest(Delegate delegate) {
+        assert sDelegate == null;
+        sDelegate = delegate;
+    }
+
+    private static boolean getCachedVoiceSearchAvailability(SharedPreferences prefs) {
+        return prefs.getBoolean(PREF_IS_VOICE_SEARCH_AVAILABLE, true);
+    }
+
+    private static String getCachedEngineName(SharedPreferences prefs) {
+        return prefs.getString(PREF_SEARCH_ENGINE_SHORTNAME, null);
+    }
+
+    private static Delegate getDelegate() {
+        synchronized (DELEGATE_LOCK) {
+            if (sDelegate == null) sDelegate = new Delegate(null);
+        }
+        return sDelegate;
     }
 }