Bug 946074 - Awesomebar cursor processing is inefficient. r=mfinkle
☠☠ backed out by 871fab42fa64 ☠ ☠
authorRichard Newman <rnewman@mozilla.com>
Tue, 03 Dec 2013 22:02:28 -0800
changeset 174806 f118b840cabe5a9bd752ea45081006358674566a
parent 174805 96af68a913f468febf09cf1e031a39ee8dce439d
child 174807 8f90c80597331603929133d6d0f43fa306226fd6
push id445
push userffxbld
push dateMon, 10 Mar 2014 22:05:19 +0000
treeherdermozilla-release@dc38b741b04e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmfinkle
bugs946074
milestone28.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 946074 - Awesomebar cursor processing is inefficient. r=mfinkle
mobile/android/base/home/BrowserSearch.java
--- a/mobile/android/base/home/BrowserSearch.java
+++ b/mobile/android/base/home/BrowserSearch.java
@@ -320,66 +320,132 @@ public class BrowserSearch extends HomeF
     }
 
     @Override
     protected void load() {
         SearchLoader.init(getLoaderManager(), LOADER_ID_SEARCH, mCursorLoaderCallbacks, mSearchTerm);
     }
 
     private void handleAutocomplete(String searchTerm, Cursor c) {
-        if (TextUtils.isEmpty(mSearchTerm) || c == null || mAutocompleteHandler == null) {
+        if (c == null ||
+            mAutocompleteHandler == null ||
+            TextUtils.isEmpty(mSearchTerm)) {
             return;
         }
 
         // Avoid searching the path if we don't have to. Currently just
-        // decided by if there is a '/' character in the string.
-        final boolean searchPath = (searchTerm.indexOf("/") > 0);
+        // decided by whether there is a '/' character in the string.
+        final boolean searchPath = searchTerm.indexOf('/') > 0;
         final String autocompletion = findAutocompletion(searchTerm, c, searchPath);
 
-        if (autocompletion != null && mAutocompleteHandler != null) {
-            mAutocompleteHandler.onAutocomplete(autocompletion);
-            mAutocompleteHandler = null;
+        if (autocompletion == null || mAutocompleteHandler == null) {
+            return;
         }
+
+        mAutocompleteHandler.onAutocomplete(autocompletion);
+        mAutocompleteHandler = null;
+    }
+
+    /**
+     * Returns the substring of a provided URI, starting at the given offset,
+     * and extending up to the end of the path segment in which the provided
+     * index is found.
+     *
+     * For example, given
+     *
+     *   "www.reddit.com/r/boop/abcdef", 0, ?
+     *
+     * this method returns
+     *
+     *   ?=2:  "www.reddit.com/"
+     *   ?=17: "www.reddit.com/r/boop/"
+     *   ?=21: "www.reddit.com/r/boop/"
+     *   ?=22: "www.reddit.com/r/boop/abcdef"
+     *
+     */
+    private static String uriSubstringUpToMatchedPath(final String url, final int offset, final int begin) {
+        final int afterEnd = url.length();
+
+        // We want to include the trailing slash, but not other characters.
+        int chop = url.indexOf('/', begin);
+        if (chop != -1) {
+            ++chop;
+            if (chop < offset) {
+                // This isn't supposed to happen. Fall back to returning the whole damn thing.
+                return url;
+            }
+        } else {
+            chop = url.indexOf('?', begin);
+            if (chop == -1) {
+                chop = url.indexOf('#', begin);
+            }
+            if (chop == -1) {
+                chop = afterEnd;
+            }
+        }
+
+        return url.substring(offset, chop);
     }
 
     private String findAutocompletion(String searchTerm, Cursor c, boolean searchPath) {
         if (!c.moveToFirst()) {
             return null;
         }
 
+        final int searchLength = searchTerm.length();
         final int urlIndex = c.getColumnIndexOrThrow(URLColumns.URL);
         int searchCount = 0;
 
         do {
-            final Uri url = Uri.parse(c.getString(urlIndex));
-            final String host = StringUtils.stripCommonSubdomains(url.getHost());
+            final String url = c.getString(urlIndex);
 
-            // Host may be null for about pages
+            // Does the completion match against the whole URL? This will match
+            // about: pages, as well as user input including "http://...".
+            if (url.startsWith(searchTerm)) {
+                return uriSubstringUpToMatchedPath(url, 0, searchLength);
+            }
+
+            final Uri uri = Uri.parse(url);
+            final String host = uri.getHost();
+
+            // Host may be null for about pages.
             if (host == null) {
                 continue;
             }
 
-            final StringBuilder hostBuilder = new StringBuilder(host);
-            if (hostBuilder.indexOf(searchTerm) == 0) {
-                return hostBuilder.append("/").toString();
+            if (host.startsWith(searchTerm)) {
+                return host + "/";
+            }
+
+            final String strippedHost = StringUtils.stripCommonSubdomains(host);
+            if (strippedHost.startsWith(searchTerm)) {
+                return strippedHost + "/";
+            }
+
+            ++searchCount;
+
+            if (!searchPath) {
+                continue;
             }
 
-            if (searchPath) {
-                final List<String> path = url.getPathSegments();
-
-                for (String s : path) {
-                    hostBuilder.append("/").append(s);
-
-                    if (hostBuilder.indexOf(searchTerm) == 0) {
-                        return hostBuilder.append("/").toString();
-                    }
-                }
+            // Otherwise, if we're matching paths, let's compare against the string itself.
+            final int hostOffset = url.indexOf(strippedHost);
+            if (hostOffset == -1) {
+                // This was a URL string that parsed to a different host (normalized?).
+                // Give up.
+                continue;
             }
 
-            searchCount++;
+            // We already matched the non-stripped host, so now we're
+            // substring-searching in the part of the URL without the common
+            // subdomains.
+            if (url.startsWith(searchTerm, hostOffset)) {
+                // Great! Return including the rest of the path segment.
+                return uriSubstringUpToMatchedPath(url, hostOffset, hostOffset + searchLength);
+            }
         } while (searchCount < MAX_AUTOCOMPLETE_SEARCH && c.moveToNext());
 
         return null;
     }
 
     private void filterSuggestions() {
         if (mSuggestClient == null || !mSuggestionsEnabled) {
             return;
@@ -782,17 +848,17 @@ public class BrowserSearch extends HomeF
             return SearchLoader.createInstance(getActivity(), args);
         }
 
         @Override
         public void onLoadFinished(Loader<Cursor> loader, Cursor c) {
             mAdapter.swapCursor(c);
 
             // We should handle autocompletion based on the search term
-            // associated with the currently loader that has just provided
+            // associated with the loader that has just provided
             // the results.
             SearchCursorLoader searchLoader = (SearchCursorLoader) loader;
             handleAutocomplete(searchLoader.getSearchTerm(), c);
         }
 
         @Override
         public void onLoaderReset(Loader<Cursor> loader) {
             mAdapter.swapCursor(null);