From 7a256707a79c0c8b397e235ec7c6ce77432f6717 Mon Sep 17 00:00:00 2001 From: Anthony Restaino Date: Sun, 17 Apr 2016 00:11:34 -0400 Subject: [PATCH] Cleaned up search suggestions code, fixed potential memory leaks --- .../lightning/activity/BrowserActivity.java | 18 +- .../browser/lightning/app/AppComponent.java | 4 +- .../search/RetrieveSuggestionsTask.java | 201 ++++++++++++++++++ .../SuggestionsAdapter.java} | 196 ++--------------- .../lightning/search/SuggestionsResult.java | 21 ++ 5 files changed, 256 insertions(+), 184 deletions(-) create mode 100644 app/src/main/java/acr/browser/lightning/search/RetrieveSuggestionsTask.java rename app/src/main/java/acr/browser/lightning/{adapter/SearchAdapter.java => search/SuggestionsAdapter.java} (59%) create mode 100644 app/src/main/java/acr/browser/lightning/search/SuggestionsResult.java diff --git a/app/src/main/java/acr/browser/lightning/activity/BrowserActivity.java b/app/src/main/java/acr/browser/lightning/activity/BrowserActivity.java index ecdb1dd..c2345a6 100644 --- a/app/src/main/java/acr/browser/lightning/activity/BrowserActivity.java +++ b/app/src/main/java/acr/browser/lightning/activity/BrowserActivity.java @@ -112,7 +112,7 @@ import acr.browser.lightning.database.HistoryItem; import acr.browser.lightning.dialog.LightningDialogBuilder; import acr.browser.lightning.fragment.BookmarksFragment; import acr.browser.lightning.fragment.TabsFragment; -import acr.browser.lightning.adapter.SearchAdapter; +import acr.browser.lightning.search.SuggestionsAdapter; import acr.browser.lightning.react.Observable; import acr.browser.lightning.react.Schedulers; import acr.browser.lightning.receiver.NetworkReceiver; @@ -164,7 +164,7 @@ public abstract class BrowserActivity extends ThemableBrowserActivity implements private View mCustomView; // Adapter - private SearchAdapter mSearchAdapter; + private SuggestionsAdapter mSuggestionsAdapter; // Callback private CustomViewCallback mCustomViewCallback; @@ -787,7 +787,7 @@ public abstract class BrowserActivity extends ThemableBrowserActivity implements ? new HistoryItem(url, title) : null; if (item != null && mBookmarkManager.addBookmark(item)) { - mSearchAdapter.refreshBookmarks(); + mSuggestionsAdapter.refreshBookmarks(); mEventBus.post(new BrowserEvents.BookmarkAdded(title, url)); } } @@ -797,7 +797,7 @@ public abstract class BrowserActivity extends ThemableBrowserActivity implements ? new HistoryItem(url, title) : null; if (item != null && mBookmarkManager.deleteBookmark(item)) { - mSearchAdapter.refreshBookmarks(); + mSuggestionsAdapter.refreshBookmarks(); mEventBus.post(new BrowserEvents.CurrentPageUrl(url)); } } @@ -1229,9 +1229,9 @@ public abstract class BrowserActivity extends ThemableBrowserActivity implements super.onResume(); final LightningView currentTab = mTabsManager.getCurrentTab(); Log.d(TAG, "onResume"); - if (mSearchAdapter != null) { - mSearchAdapter.refreshPreferences(); - mSearchAdapter.refreshBookmarks(); + if (mSuggestionsAdapter != null) { + mSuggestionsAdapter.refreshPreferences(); + mSuggestionsAdapter.refreshBookmarks(); } if (currentTab != null) { currentTab.resumeTimers(); @@ -1418,7 +1418,7 @@ public abstract class BrowserActivity extends ThemableBrowserActivity implements */ private void initializeSearchSuggestions(final AutoCompleteTextView getUrl) { - mSearchAdapter = new SearchAdapter(this, mDarkTheme, isIncognito()); + mSuggestionsAdapter = new SuggestionsAdapter(this, mDarkTheme, isIncognito()); getUrl.setThreshold(1); getUrl.setDropDownWidth(-1); @@ -1454,7 +1454,7 @@ public abstract class BrowserActivity extends ThemableBrowserActivity implements }); getUrl.setSelectAllOnFocus(true); - getUrl.setAdapter(mSearchAdapter); + getUrl.setAdapter(mSuggestionsAdapter); } /** diff --git a/app/src/main/java/acr/browser/lightning/app/AppComponent.java b/app/src/main/java/acr/browser/lightning/app/AppComponent.java index 6007148..0f8187e 100644 --- a/app/src/main/java/acr/browser/lightning/app/AppComponent.java +++ b/app/src/main/java/acr/browser/lightning/app/AppComponent.java @@ -16,7 +16,7 @@ import acr.browser.lightning.fragment.BookmarksFragment; import acr.browser.lightning.fragment.LightningPreferenceFragment; import acr.browser.lightning.fragment.PrivacySettingsFragment; import acr.browser.lightning.fragment.TabsFragment; -import acr.browser.lightning.adapter.SearchAdapter; +import acr.browser.lightning.search.SuggestionsAdapter; import acr.browser.lightning.utils.AdBlock; import acr.browser.lightning.utils.ProxyUtils; import acr.browser.lightning.view.LightningView; @@ -33,7 +33,7 @@ public interface AppComponent { void inject(BookmarkSettingsFragment fragment); - void inject(SearchAdapter adapter); + void inject(SuggestionsAdapter adapter); void inject(LightningDialogBuilder builder); diff --git a/app/src/main/java/acr/browser/lightning/search/RetrieveSuggestionsTask.java b/app/src/main/java/acr/browser/lightning/search/RetrieveSuggestionsTask.java new file mode 100644 index 0000000..3bdcbef --- /dev/null +++ b/app/src/main/java/acr/browser/lightning/search/RetrieveSuggestionsTask.java @@ -0,0 +1,201 @@ +package acr.browser.lightning.search; + +import android.app.Application; +import android.content.Context; +import android.net.ConnectivityManager; +import android.net.NetworkInfo; +import android.os.AsyncTask; +import android.support.annotation.NonNull; +import android.support.annotation.Nullable; +import android.text.TextUtils; +import android.util.Log; + +import org.xmlpull.v1.XmlPullParser; +import org.xmlpull.v1.XmlPullParserException; +import org.xmlpull.v1.XmlPullParserFactory; + +import java.io.BufferedInputStream; +import java.io.File; +import java.io.FileInputStream; +import java.io.FileOutputStream; +import java.io.InputStream; +import java.io.UnsupportedEncodingException; +import java.lang.ref.WeakReference; +import java.net.HttpURLConnection; +import java.net.URL; +import java.net.URLEncoder; +import java.util.ArrayList; +import java.util.List; +import java.util.Locale; +import java.util.regex.Pattern; + +import acr.browser.lightning.R; +import acr.browser.lightning.database.HistoryItem; +import acr.browser.lightning.utils.Utils; + +public class RetrieveSuggestionsTask extends AsyncTask> { + + private static final String TAG = RetrieveSuggestionsTask.class.getSimpleName(); + + private static final Pattern SPACE_PATTERN = Pattern.compile(" ", Pattern.LITERAL); + private static final String CACHE_FILE_TYPE = ".sgg"; + private static final String ENCODING = "ISO-8859-1"; + private static final long INTERVAL_DAY = 86400000; + private static final String DEFAULT_LANGUAGE = "en"; + private static XmlPullParser sXpp; + private static String sLanguage; + private WeakReference mResultCallback; + private Application mApplication; + private String mSearchSubtitle; + private String mQuery; + + public RetrieveSuggestionsTask(@NonNull String query, + @NonNull SuggestionsResult callback, + @NonNull Application application) { + mQuery = query; + mResultCallback = new WeakReference<>(callback); + mApplication = application; + mSearchSubtitle = mApplication.getString(R.string.suggestion); + } + + @NonNull + private static synchronized String getLanguage() { + if (sLanguage == null) { + sLanguage = Locale.getDefault().getLanguage(); + } + if (TextUtils.isEmpty(sLanguage)) { + sLanguage = DEFAULT_LANGUAGE; + } + return sLanguage; + } + + @NonNull + private static synchronized XmlPullParser getParser() throws XmlPullParserException { + if (sXpp == null) { + XmlPullParserFactory factory = XmlPullParserFactory.newInstance(); + factory.setNamespaceAware(true); + sXpp = factory.newPullParser(); + } + return sXpp; + } + + @NonNull + @Override + protected List doInBackground(Void... voids) { + List filter = new ArrayList<>(5); + try { + mQuery = SPACE_PATTERN.matcher(mQuery).replaceAll("+"); + URLEncoder.encode(mQuery, ENCODING); + } catch (UnsupportedEncodingException e) { + e.printStackTrace(); + } + File cache = downloadSuggestionsForQuery(mQuery, getLanguage(), mApplication); + if (!cache.exists()) { + return filter; + } + InputStream fileInput = null; + try { + fileInput = new BufferedInputStream(new FileInputStream(cache)); + XmlPullParser parser = getParser(); + parser.setInput(fileInput, ENCODING); + int eventType = parser.getEventType(); + int counter = 0; + while (eventType != XmlPullParser.END_DOCUMENT) { + if (eventType == XmlPullParser.START_TAG && "suggestion".equals(parser.getName())) { + String suggestion = parser.getAttributeValue(null, "data"); + filter.add(new HistoryItem(mSearchSubtitle + " \"" + suggestion + '"', + suggestion, R.drawable.ic_search)); + counter++; + if (counter >= 5) { + break; + } + } + eventType = parser.next(); + } + } catch (Exception e) { + return filter; + } finally { + Utils.close(fileInput); + } + return filter; + } + + @Override + protected void onPostExecute(@NonNull List result) { + SuggestionsResult callback = mResultCallback.get(); + if (callback != null) { + callback.resultReceived(result); + } + } + + /** + * This method downloads the search suggestions for the specific query. + * NOTE: This is a blocking operation, do not run on the UI thread. + * + * @param query the query to get suggestions for + * @return the cache file containing the suggestions + */ + @NonNull + private static File downloadSuggestionsForQuery(@NonNull String query, String language, @NonNull Application app) { + File cacheFile = new File(app.getCacheDir(), query.hashCode() + CACHE_FILE_TYPE); + if (System.currentTimeMillis() - INTERVAL_DAY < cacheFile.lastModified()) { + return cacheFile; + } + if (!isNetworkConnected(app)) { + return cacheFile; + } + InputStream in = null; + FileOutputStream fos = null; + try { + // Old API that doesn't support HTTPS + // http://google.com/complete/search?q= + query + &output=toolbar&hl= + language + URL url = new URL("https://suggestqueries.google.com/complete/search?output=toolbar&hl=" + + language + "&q=" + query); + HttpURLConnection connection = (HttpURLConnection) url.openConnection(); + connection.setDoInput(true); + connection.connect(); + if (connection.getResponseCode() >= HttpURLConnection.HTTP_MULT_CHOICE || + connection.getResponseCode() < HttpURLConnection.HTTP_OK) { + Log.e(TAG, "Search API Responded with code: " + connection.getResponseCode()); + connection.disconnect(); + return cacheFile; + } + in = connection.getInputStream(); + + if (in != null) { + //noinspection IOResourceOpenedButNotSafelyClosed + fos = new FileOutputStream(cacheFile); + int buffer; + while ((buffer = in.read()) != -1) { + fos.write(buffer); + } + fos.flush(); + } + connection.disconnect(); + cacheFile.setLastModified(System.currentTimeMillis()); + } catch (Exception e) { + Log.w(TAG, "Problem getting search suggestions", e); + } finally { + Utils.close(in); + Utils.close(fos); + } + return cacheFile; + } + + private static boolean isNetworkConnected(@NonNull Context context) { + NetworkInfo networkInfo = getActiveNetworkInfo(context); + return networkInfo != null && networkInfo.isConnected(); + } + + @Nullable + private static NetworkInfo getActiveNetworkInfo(@NonNull Context context) { + ConnectivityManager connectivity = (ConnectivityManager) context + .getApplicationContext() + .getSystemService(Context.CONNECTIVITY_SERVICE); + if (connectivity == null) { + return null; + } + return connectivity.getActiveNetworkInfo(); + } + +} \ No newline at end of file diff --git a/app/src/main/java/acr/browser/lightning/adapter/SearchAdapter.java b/app/src/main/java/acr/browser/lightning/search/SuggestionsAdapter.java similarity index 59% rename from app/src/main/java/acr/browser/lightning/adapter/SearchAdapter.java rename to app/src/main/java/acr/browser/lightning/search/SuggestionsAdapter.java index c17c64a..accb0c0 100644 --- a/app/src/main/java/acr/browser/lightning/adapter/SearchAdapter.java +++ b/app/src/main/java/acr/browser/lightning/search/SuggestionsAdapter.java @@ -1,15 +1,12 @@ -package acr.browser.lightning.adapter; +package acr.browser.lightning.search; import android.app.Application; import android.content.Context; import android.graphics.Color; import android.graphics.drawable.Drawable; -import android.net.ConnectivityManager; -import android.net.NetworkInfo; import android.os.AsyncTask; import android.support.annotation.NonNull; import android.support.annotation.Nullable; -import android.util.Log; import android.view.LayoutInflater; import android.view.View; import android.view.ViewGroup; @@ -19,26 +16,14 @@ import android.widget.Filterable; import android.widget.ImageView; import android.widget.TextView; -import org.xmlpull.v1.XmlPullParser; -import org.xmlpull.v1.XmlPullParserFactory; - -import java.io.BufferedInputStream; import java.io.File; -import java.io.FileInputStream; -import java.io.FileOutputStream; import java.io.FilenameFilter; -import java.io.InputStream; -import java.io.UnsupportedEncodingException; -import java.net.HttpURLConnection; -import java.net.URL; -import java.net.URLEncoder; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; import java.util.Iterator; import java.util.List; import java.util.Locale; -import java.util.regex.Pattern; import javax.inject.Inject; @@ -49,35 +34,29 @@ import acr.browser.lightning.database.HistoryDatabase; import acr.browser.lightning.database.HistoryItem; import acr.browser.lightning.preference.PreferenceManager; import acr.browser.lightning.utils.ThemeUtils; -import acr.browser.lightning.utils.Utils; -public class SearchAdapter extends BaseAdapter implements Filterable { +public class SuggestionsAdapter extends BaseAdapter implements Filterable, SuggestionsResult { + + private static final String TAG = SuggestionsAdapter.class.getSimpleName(); - private static final String TAG = SearchAdapter.class.getSimpleName(); - private static final Pattern SPACE_PATTERN = Pattern.compile(" ", Pattern.LITERAL); private final List mHistory = new ArrayList<>(5); private final List mBookmarks = new ArrayList<>(5); private final List mSuggestions = new ArrayList<>(5); private final List mFilteredList = new ArrayList<>(5); private final List mAllBookmarks = new ArrayList<>(5); - private final Object mLock = new Object(); @NonNull private final Context mContext; private boolean mUseGoogle = true; private boolean mIsExecuting = false; private final boolean mDarkTheme; private final boolean mIncognito; private static final String CACHE_FILE_TYPE = ".sgg"; - private static final String ENCODING = "ISO-8859-1"; - private static final String DEFAULT_LANGUAGE = "en"; private static final long INTERVAL_DAY = 86400000; private static final int MAX_SUGGESTIONS = 5; private static final SuggestionsComparator mComparator = new SuggestionsComparator(); - @NonNull private final String mSearchSubtitle; private SearchFilter mFilter; @NonNull private final Drawable mSearchDrawable; @NonNull private final Drawable mHistoryDrawable; @NonNull private final Drawable mBookmarkDrawable; - private String mLanguage; @Inject HistoryDatabase mDatabaseHandler; @@ -88,22 +67,17 @@ public class SearchAdapter extends BaseAdapter implements Filterable { @Inject PreferenceManager mPreferenceManager; - public SearchAdapter(@NonNull Context context, boolean dark, boolean incognito) { + public SuggestionsAdapter(@NonNull Context context, boolean dark, boolean incognito) { BrowserApp.getAppComponent().inject(this); mAllBookmarks.addAll(mBookmarkManager.getAllBookmarks(true)); mUseGoogle = mPreferenceManager.getGoogleSearchSuggestionsEnabled(); mContext = context; - mSearchSubtitle = mContext.getString(R.string.suggestion); mDarkTheme = dark || incognito; mIncognito = incognito; BrowserApp.getTaskThread().execute(new ClearCacheRunnable(BrowserApp.get(context))); mSearchDrawable = ThemeUtils.getThemedDrawable(context, R.drawable.ic_search, mDarkTheme); mBookmarkDrawable = ThemeUtils.getThemedDrawable(context, R.drawable.ic_bookmark, mDarkTheme); mHistoryDrawable = ThemeUtils.getThemedDrawable(context, R.drawable.ic_history, mDarkTheme); - mLanguage = Locale.getDefault().getLanguage(); - if (mLanguage.isEmpty()) { - mLanguage = DEFAULT_LANGUAGE; - } } private static class NameFilter implements FilenameFilter { @@ -125,7 +99,7 @@ public class SearchAdapter extends BaseAdapter implements Filterable { } public void refreshBookmarks() { - synchronized (mLock) { + synchronized (SuggestionsAdapter.this) { mAllBookmarks.clear(); mAllBookmarks.addAll(mBookmarkManager.getAllBookmarks(true)); } @@ -210,9 +184,10 @@ public class SearchAdapter extends BaseAdapter implements Filterable { private static class ClearCacheRunnable implements Runnable { + @NonNull private final Application app; - public ClearCacheRunnable(Application app) { + public ClearCacheRunnable(@NonNull Application app) { this.app = app; } @@ -242,13 +217,14 @@ public class SearchAdapter extends BaseAdapter implements Filterable { } String query = constraint.toString().toLowerCase(Locale.getDefault()); if (mUseGoogle && !mIncognito && !mIsExecuting) { - new RetrieveSearchSuggestions().executeOnExecutor(AsyncTask.SERIAL_EXECUTOR, query); + mIsExecuting = true; + new RetrieveSuggestionsTask(query, SuggestionsAdapter.this, BrowserApp.get(mContext)).executeOnExecutor(AsyncTask.SERIAL_EXECUTOR); } int counter = 0; synchronized (mBookmarks) { mBookmarks.clear(); - synchronized (mLock) { + synchronized (SuggestionsAdapter.this) { for (int n = 0; n < mAllBookmarks.size(); n++) { if (counter >= 5) { break; @@ -298,146 +274,20 @@ public class SearchAdapter extends BaseAdapter implements Filterable { TextView mUrl; } - private class RetrieveSearchSuggestions extends AsyncTask> { - - private XmlPullParserFactory mFactory; - private XmlPullParser mXpp; - - @NonNull - @Override - protected List doInBackground(String... arg0) { - mIsExecuting = true; - - List filter = new ArrayList<>(5); - String query = arg0[0]; - try { - query = SPACE_PATTERN.matcher(query).replaceAll("+"); - URLEncoder.encode(query, ENCODING); - } catch (UnsupportedEncodingException e) { - e.printStackTrace(); - } - File cache = downloadSuggestionsForQuery(query, mLanguage, BrowserApp.get(mContext)); - if (!cache.exists()) { - return filter; - } - InputStream fileInput = null; - try { - fileInput = new BufferedInputStream(new FileInputStream(cache)); - if (mFactory == null) { - mFactory = XmlPullParserFactory.newInstance(); - mFactory.setNamespaceAware(true); - } - if (mXpp == null) { - mXpp = mFactory.newPullParser(); - } - mXpp.setInput(fileInput, ENCODING); - int eventType = mXpp.getEventType(); - int counter = 0; - while (eventType != XmlPullParser.END_DOCUMENT) { - if (eventType == XmlPullParser.START_TAG && "suggestion".equals(mXpp.getName())) { - String suggestion = mXpp.getAttributeValue(null, "data"); - filter.add(new HistoryItem(mSearchSubtitle + " \"" + suggestion + '"', - suggestion, R.drawable.ic_search)); - counter++; - if (counter >= 5) { - break; - } - } - eventType = mXpp.next(); - } - } catch (Exception e) { - return filter; - } finally { - Utils.close(fileInput); - } - return filter; - } - - @Override - protected void onPostExecute(@NonNull List result) { - mIsExecuting = false; - synchronized (mSuggestions) { - mSuggestions.clear(); - mSuggestions.addAll(result); - } - synchronized (mFilteredList) { - mFilteredList.clear(); - List filtered = getFilteredList(); - Collections.sort(filtered, mComparator); - mFilteredList.addAll(filtered); - notifyDataSetChanged(); - } - } - - } - - /** - * This method downloads the search suggestions for the specific query. - * NOTE: This is a blocking operation, do not run on the UI thread. - * - * @param query the query to get suggestions for - * @return the cache file containing the suggestions - */ - @NonNull - private static File downloadSuggestionsForQuery(@NonNull String query, String language, @NonNull Application app) { - File cacheFile = new File(app.getCacheDir(), query.hashCode() + CACHE_FILE_TYPE); - if (System.currentTimeMillis() - INTERVAL_DAY < cacheFile.lastModified()) { - return cacheFile; - } - if (!isNetworkConnected(app)) { - return cacheFile; - } - InputStream in = null; - FileOutputStream fos = null; - try { - // Old API that doesn't support HTTPS - // http://google.com/complete/search?q= + query + &output=toolbar&hl= + language - URL url = new URL("https://suggestqueries.google.com/complete/search?output=toolbar&hl=" - + language + "&q=" + query); - HttpURLConnection connection = (HttpURLConnection) url.openConnection(); - connection.setDoInput(true); - connection.connect(); - if (connection.getResponseCode() >= HttpURLConnection.HTTP_MULT_CHOICE || - connection.getResponseCode() < HttpURLConnection.HTTP_OK) { - Log.e(TAG, "Search API Responded with code: " + connection.getResponseCode()); - connection.disconnect(); - return cacheFile; - } - in = connection.getInputStream(); - - if (in != null) { - //noinspection IOResourceOpenedButNotSafelyClosed - fos = new FileOutputStream(cacheFile); - int buffer; - while ((buffer = in.read()) != -1) { - fos.write(buffer); - } - fos.flush(); - } - connection.disconnect(); - cacheFile.setLastModified(System.currentTimeMillis()); - } catch (Exception e) { - Log.w(TAG, "Problem getting search suggestions", e); - } finally { - Utils.close(in); - Utils.close(fos); + @Override + public void resultReceived(@NonNull List searchResults) { + mIsExecuting = false; + synchronized (mSuggestions) { + mSuggestions.clear(); + mSuggestions.addAll(searchResults); } - return cacheFile; - } - - private static boolean isNetworkConnected(@NonNull Context context) { - NetworkInfo networkInfo = getActiveNetworkInfo(context); - return networkInfo != null && networkInfo.isConnected(); - } - - private static NetworkInfo getActiveNetworkInfo(@NonNull Context context) { - ConnectivityManager connectivity = (ConnectivityManager) context - .getApplicationContext() - .getSystemService(Context.CONNECTIVITY_SERVICE); - if (connectivity == null) { - return null; + synchronized (mFilteredList) { + mFilteredList.clear(); + List filtered = getFilteredList(); + Collections.sort(filtered, mComparator); + mFilteredList.addAll(filtered); + notifyDataSetChanged(); } - return connectivity.getActiveNetworkInfo(); } @NonNull diff --git a/app/src/main/java/acr/browser/lightning/search/SuggestionsResult.java b/app/src/main/java/acr/browser/lightning/search/SuggestionsResult.java new file mode 100644 index 0000000..875f119 --- /dev/null +++ b/app/src/main/java/acr/browser/lightning/search/SuggestionsResult.java @@ -0,0 +1,21 @@ +package acr.browser.lightning.search; + +import android.support.annotation.NonNull; + +import java.util.List; + +import acr.browser.lightning.database.HistoryItem; + +public interface SuggestionsResult { + + /** + * Called when the search suggestions have + * been retrieved from the server. + * + * @param searchResults the results, a valid + * list of results. May + * be empty. + */ + void resultReceived(@NonNull List searchResults); + +}