Speculative fixes for search engine crashes.

chrome/android/java/src/org/chromium/chrome/browser/preferences/SearchEngineAdapter.java

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.chrome.browser.preferences;
 
 import android.content.Context;
 import android.content.Intent;
 import android.content.SharedPreferences;
 import android.content.res.Resources;
@@ skipping 18 matching lines @@
 import org.chromium.base.metrics.RecordUserAction;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ChromeFeatureList;
 import org.chromium.chrome.browser.locale.LocaleManager;
 import org.chromium.chrome.browser.omnibox.geo.GeolocationHeader;
 import org.chromium.chrome.browser.preferences.website.ContentSetting;
 import org.chromium.chrome.browser.preferences.website.GeolocationInfo;
 import org.chromium.chrome.browser.preferences.website.SingleWebsitePreferences;
 import org.chromium.chrome.browser.preferences.website.WebsitePreferenceBridge;
 import org.chromium.chrome.browser.search_engines.TemplateUrlService;
-import org.chromium.chrome.browser.search_engines.TemplateUrlService.LoadListener;
 import org.chromium.chrome.browser.search_engines.TemplateUrlService.TemplateUrl;
 import org.chromium.components.location.LocationUtils;
 import org.chromium.ui.text.SpanApplier;
 import org.chromium.ui.text.SpanApplier.SpanInfo;
 
 import java.util.ArrayList;
 import java.util.List;
 
 /**
 * A custom adapter for listing search engines.
 */
-public class SearchEngineAdapter extends BaseAdapter implements LoadListener, OnClickListener {
+public class SearchEngineAdapter extends BaseAdapter
+        implements TemplateUrlService.LoadListener, TemplateUrlService.TemplateUrlServiceObserver,
+                OnClickListener {
     private static final int VIEW_TYPE_ITEM = 0;
     private static final int VIEW_TYPE_DIVIDER = 1;
     private static final int VIEW_TYPE_COUNT = 2;
 
     /** The current context. */
     private Context mContext;
 
     /** The layout inflater to use for the custom views. */
     private LayoutInflater mLayoutInflater;
 
     /** The list of prepopluated and default search engines. */
     private List<TemplateUrl> mPrepopulatedSearchEngines = new ArrayList<>();
 
     /** The list of recently visited search engines. */
     private List<TemplateUrl> mRecentSearchEngines = new ArrayList<>();
 
     /**
      * The position (index into mPrepopulatedSearchEngines) of the currently selected search engine.
      * Can be -1 if current search engine is managed and set to something other than the
      * pre-populated values.
      */
     private int mSelectedSearchEnginePosition = -1;
 
     /** The position of the default search engine before user's action. */
     private int mInitialEnginePosition = -1;
 
+    private boolean mHasLoadObserver;
+
     /**
      * Construct a SearchEngineAdapter.
      * @param context The current context.
      */
     public SearchEngineAdapter(Context context) {
         mContext = context;
         mLayoutInflater = (LayoutInflater) mContext.getSystemService(
                 Context.LAYOUT_INFLATER_SERVICE);
+    }
 
-        initEntries();
+    /**
+     * Start the adapter to gather the available search engines and listen for updates.
+     */
+    public void start() {
+        refreshData();
+        TemplateUrlService.getInstance().addObserver(this);
+    }
+
+    /**
+     * Stop the adapter from listening for future search engine updates.
+     */
+    public void stop() {
+        if (mHasLoadObserver) {
+            TemplateUrlService.getInstance().unregisterLoadListener(this);
+            mHasLoadObserver = false;
+        }
+        TemplateUrlService.getInstance().removeObserver(this);
     }
 
     @VisibleForTesting
     String getValueForTesting() {
         return Integer.toString(mSelectedSearchEnginePosition);
     }
 
     @VisibleForTesting
     String setValueForTesting(String value) {
         return searchEngineSelected(Integer.parseInt(value));
     }
 
     @VisibleForTesting
     String getKeywordForTesting(int index) {
         return toKeyword(index);
     }
 
-    /**
-     * Initialize the search engine list.
-     */
-    private void initEntries() {
-        TemplateUrlService templateUrlService = TemplateUrlService.getInstance();
-        if (!templateUrlService.isLoaded()) {
-            templateUrlService.registerLoadListener(this);
-            templateUrlService.load();
-            return;  // Flow continues in onTemplateUrlServiceLoaded below.
-        }
+    private void initializeSearchEngineGroups(List<TemplateUrl> templateUrls) {
+        mPrepopulatedSearchEngines = new ArrayList<>();
+        mRecentSearchEngines = new ArrayList<>();
 
-        int defaultSearchEngineIndex = templateUrlService.getDefaultSearchEngineIndex();
-        for (TemplateUrl templateUrl : templateUrlService.getSearchEngines()) {
+        for (int i = 0; i < templateUrls.size(); i++) {
+            TemplateUrl templateUrl = templateUrls.get(i);
             if (templateUrl.getType() == TemplateUrlService.TYPE_PREPOPULATED
                     || templateUrl.getType() == TemplateUrlService.TYPE_DEFAULT) {
                 mPrepopulatedSearchEngines.add(templateUrl);
             } else {
                 mRecentSearchEngines.add(templateUrl);
             }
         }
+    }
+
+    /**
+     * Initialize the search engine list.
+     */
+    private void refreshData() {
+        TemplateUrlService templateUrlService = TemplateUrlService.getInstance();
+        if (!templateUrlService.isLoaded()) {
+            mHasLoadObserver = true;
+            templateUrlService.registerLoadListener(this);
+            templateUrlService.load();
+            return;  // Flow continues in onTemplateUrlServiceLoaded below.
+        }
+
+        List<TemplateUrl> templateUrls = templateUrlService.getSearchEngines();
+        boolean searchEnginesChanged = templateUrls.size()
+                != mPrepopulatedSearchEngines.size() + mRecentSearchEngines.size();
+        if (!searchEnginesChanged) {
+            for (int i = 0; i < templateUrls.size(); i++) {
+                TemplateUrl templateUrl = templateUrls.get(i);
+                if (!mPrepopulatedSearchEngines.contains(templateUrl)
+                        && !mRecentSearchEngines.contains(templateUrl)) {
+                    searchEnginesChanged = true;
+                    break;
+                }
+            }
+        }
+        if (searchEnginesChanged) initializeSearchEngineGroups(templateUrls);
+
+        mSelectedSearchEnginePosition = -1;
+        mInitialEnginePosition = -1;

[Theresa, 2017/01/27 23:25:44]

Since this is set below, on line 192, does setting it to -1 do anything?

[Ted C, 2017/01/28 00:01:30]

Nah, not needed...removed.  Used to reset all the variables at the top but it
has now changed.

+
+        int defaultSearchEngineIndex = templateUrlService.getDefaultSearchEngineIndex();
 
         // Convert the TemplateUrl index into an index of mSearchEngines.
         mSelectedSearchEnginePosition = -1;

[Theresa, 2017/01/27 23:25:43]

nit: this is a no-op since line 167 sets it to -1 as well.

[Ted C, 2017/01/28 00:01:30]

now that I removed above, this is now the only line that remains

         for (int i = 0; i < mPrepopulatedSearchEngines.size(); ++i) {
             if (mPrepopulatedSearchEngines.get(i).getIndex() == defaultSearchEngineIndex) {
                 mSelectedSearchEnginePosition = i;
             }
         }
 
         for (int i = 0; i < mRecentSearchEngines.size(); ++i) {
             if (mRecentSearchEngines.get(i).getIndex() == defaultSearchEngineIndex) {
                 // Add one to offset the title for the recent search engine list.
                 mSelectedSearchEnginePosition = i + computeStartIndexForRecentSearchEngines();
             }
         }
 
+        if (mSelectedSearchEnginePosition == -1) {
+            throw new IllegalStateException(
+                    "Default search engine index did not match any available search engines.");
+        }
+
         mInitialEnginePosition = mSelectedSearchEnginePosition;
 
-        TemplateUrlService.getInstance().setSearchEngine(toKeyword(mSelectedSearchEnginePosition));
+        notifyDataSetChanged();
     }
 
     private String toKeyword(int position) {
         if (position < mPrepopulatedSearchEngines.size()) {
             return mPrepopulatedSearchEngines.get(position).getKeyword();
         } else {
             position -= computeStartIndexForRecentSearchEngines();
             return mRecentSearchEngines.get(position).getKeyword();
         }
     }
@@ skipping 134 matching lines @@
         }
 
         return view;
     }
 
     // TemplateUrlService.LoadListener
 
     @Override
     public void onTemplateUrlServiceLoaded() {
         TemplateUrlService.getInstance().unregisterLoadListener(this);
-        initEntries();
-        notifyDataSetChanged();
+        mHasLoadObserver = false;
+        refreshData();
+    }
+
+    @Override
+    public void onTemplateURLServiceChanged() {
+        refreshData();
     }
 
     // OnClickListener:
 
     @Override
     public void onClick(View view) {
         if (view.getTag() == null) {
             onLocationLinkClicked();
         } else {
             searchEngineSelected((int) view.getTag());
@@ skipping 80 matching lines @@
     private int computeStartIndexForRecentSearchEngines() {
         // If there are custom search engines to show, add 1 for showing the  "Recently visited"
         // header.
         if (mRecentSearchEngines.size() > 0) {
             return mPrepopulatedSearchEngines.size() + 1;
         } else {
             return mPrepopulatedSearchEngines.size();
         }
     }
 }