Bug 1390685: Replace InvalidHighlightCandidateException with boolean return. r=sebastian

In the event that many of these exceptions are thrown (which I witnessed
locally after importing a large number of desktop bookmarks), it could have a
negative impact on performance so we're making a fix. Note: we don't know how
much it happens in practice.

MozReview-Commit-ID: 1hQo7B704LV

--HG--
extra : rebase_source : b4dd1921b9886b8c403b6676fa9e49de5402b316

mobile/android/base/java/org/mozilla/gecko/activitystream/ranking/HighlightCandidate.java

@@ -7,6 +7,7 @@ package org.mozilla.gecko.activitystream.ranking;
 
 import android.database.Cursor;
 import android.net.Uri;
+import android.support.annotation.CheckResult;
 import android.support.annotation.IntDef;
 import android.support.annotation.Nullable;
 import android.support.annotation.VisibleForTesting;
@@ -73,13 +74,19 @@ import java.lang.annotation.RetentionPolicy;
     private String host;
     private double score;
 
-    public static HighlightCandidate fromCursor(final Cursor cursor, final HighlightCandidateCursorIndices cursorIndices)
-            throws InvalidHighlightCandidateException {
+    /**
+     * @return the HighlightCandidate, or null if the candidate is invalid.
+     */
+    @Nullable
+    public static HighlightCandidate fromCursor(final Cursor cursor, final HighlightCandidateCursorIndices cursorIndices) {
         final HighlightCandidate candidate = new HighlightCandidate();
 
         extractHighlight(candidate, cursor, cursorIndices);
-        extractFeatures(candidate, cursor, cursorIndices);
+        final boolean isSuccess = extractFeatures(candidate, cursor, cursorIndices);
 
+        if (!isSuccess) {
+            return null;
+        }
         return candidate;
     }
 
@@ -93,9 +100,23 @@ import java.lang.annotation.RetentionPolicy;
 
     /**
      * Extract and assign features that will be used for ranking.
+     *
+     * @return true if the candidate is valid, false otherwise.
      */
-    private static void extractFeatures(final HighlightCandidate candidate, final Cursor cursor,
-            final HighlightCandidateCursorIndices cursorIndices) throws InvalidHighlightCandidateException {
+    @CheckResult
+    private static boolean extractFeatures(final HighlightCandidate candidate, final Cursor cursor,
+            final HighlightCandidateCursorIndices cursorIndices) {
+        final Uri uri = Uri.parse(candidate.highlight.getUrl());
+
+        // We don't support opaque URIs (such as mailto:...), or URIs which do not have a valid host.
+        // The latter might simply be URIs with invalid characters in them (such as underscore...).
+        // See Bug 1363521 and Bug 1378967.
+        if (!uri.isHierarchical() || uri.getHost() == null) {
+            // Note: we used to throw an exception but sometimes many Exceptions were thrown, potentially
+            // impacting performance so we changed it to a boolean return.
+            return false;
+        }
+
         candidate.features.put(
                 FEATURE_AGE_IN_DAYS,
                 (System.currentTimeMillis() - cursor.getDouble(cursorIndices.historyDateLastVisitedColumnIndex))
@@ -154,15 +175,6 @@ import java.lang.annotation.RetentionPolicy;
                 FEATURE_DESCRIPTION_LENGTH,
                 (double) candidate.highlight.getFastDescriptionLength());
 
-        final Uri uri = Uri.parse(candidate.highlight.getUrl());
-
-        // We don't support opaque URIs (such as mailto:...), or URIs which do not have a valid host.
-        // The latter might simply be URIs with invalid characters in them (such as underscore...).
-        // See Bug 1363521 and Bug 1378967.
-        if (!uri.isHierarchical() || uri.getHost() == null) {
-            throw new InvalidHighlightCandidateException();
-        }
-
         candidate.host = uri.getHost();
 
         candidate.features.put(
@@ -173,6 +185,8 @@ import java.lang.annotation.RetentionPolicy;
         candidate.features.put(
                 FEATURE_QUERY_LENGTH,
                 (double) uri.getQueryParameterNames().size());
+
+        return true;
     }
 
     @VisibleForTesting HighlightCandidate() {
@@ -206,8 +220,4 @@ import java.lang.annotation.RetentionPolicy;
     /* package-private */ Highlight getHighlight() {
         return highlight;
     }
-
-    /* package-private */ static class InvalidHighlightCandidateException extends Exception {
-        private static final long serialVersionUID = 949263104621445850L;
-    }
 }

mobile/android/base/java/org/mozilla/gecko/activitystream/ranking/HighlightsRanking.java

@@ -133,12 +133,12 @@ public class HighlightsRanking {
         return looselyMapCursor(cursor, new Func1<Cursor, HighlightCandidate>() {
             @Override
             public HighlightCandidate call(Cursor cursor) {
-                try {
-                    return HighlightCandidate.fromCursor(cursor, cursorIndices);
-                } catch (HighlightCandidate.InvalidHighlightCandidateException e) {
-                    Log.w(LOG_TAG, "Skipping invalid highlight item", e);
+                final HighlightCandidate candidate = HighlightCandidate.fromCursor(cursor, cursorIndices);
+                if (candidate == null) {
+                    Log.w(LOG_TAG, "Skipping invalid highlight item.");
                     return null;
                 }
+                return candidate;
             }
         });
     }