Bug 1396054 - Filter highlight candidates based on settings. r?mcomella
MozReview-Commit-ID: 8e36KZsuh27
--- a/mobile/android/app/src/main/res/layout/preference_topsites_panel_dialog.xml
+++ b/mobile/android/app/src/main/res/layout/preference_topsites_panel_dialog.xml
@@ -23,29 +23,29 @@
<org.mozilla.gecko.widget.SwitchPreferenceView
android:id="@+id/preference_pocket"
android:layout_width="match_parent"
android:layout_height="wrap_content"
style="@style/Gecko.SwitchPreferenceView"
android:paddingBottom="@dimen/dialog_switchpreferenceview_padding"
android:text="@string/activity_stream_topstories"
gecko:androidPreferenceKey="pref_activitystream_pocket_enabled"
- gecko:defaultValue="true"/>
+ gecko:defaultValue="@bool/pref_activitystream_pocket_enabled_default"/>
<org.mozilla.gecko.widget.SwitchPreferenceView
android:layout_width="match_parent"
android:layout_height="wrap_content"
style="@style/Gecko.SwitchPreferenceView"
android:paddingBottom="@dimen/dialog_switchpreferenceview_padding"
android:text="@string/pref_dialog_activitystream_recentBookmarks"
gecko:androidPreferenceKey="pref_activitystream_recentbookmarks_enabled"
- gecko:defaultValue="true"/>
+ gecko:defaultValue="@bool/pref_activitystream_recentbookmarks_enabled_default"/>
<org.mozilla.gecko.widget.SwitchPreferenceView
android:layout_width="match_parent"
android:layout_height="wrap_content"
style="@style/Gecko.SwitchPreferenceView"
android:paddingBottom="@dimen/dialog_switchpreferenceview_padding"
android:text="@string/pref_dialog_activitystream_visited"
gecko:androidPreferenceKey="pref_activitystream_visited_enabled"
- gecko:defaultValue="true"/>
+ gecko:defaultValue="@bool/pref_activitystream_visited_enabled_default"/>
</LinearLayout>
\ No newline at end of file
--- a/mobile/android/app/src/main/res/values/bool.xml
+++ b/mobile/android/app/src/main/res/values/bool.xml
@@ -10,9 +10,12 @@
with the configuration retrieved by HardwareUtils (e.g. some custom ROMs allow the user to
choose a phone or tablet version of the UI even though the hardware stays the same). This
can cause crashes when we branch on the value returned by HardwareUtils.
In order to work around this, we define the resource size in resources with the expectation that
we branch on that value, rather than HardwareUtils, so our code is consistent with the used resources.
See bug 1277379 for a initiative to move all of the HardwareUtils code over. -->
<bool name="is_large_resource">false</bool>
+ <bool name="pref_activitystream_pocket_enabled_default">true</bool>
+ <bool name="pref_activitystream_visited_enabled_default">true</bool>
+ <bool name="pref_activitystream_recentbookmarks_enabled_default">true</bool>
</resources>
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/HighlightsLoader.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/HighlightsLoader.java
@@ -1,22 +1,26 @@
/* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*-
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
package org.mozilla.gecko.activitystream.homepanel;
import android.content.Context;
+import android.content.SharedPreferences;
+import android.content.res.Resources;
import android.database.ContentObserver;
import android.database.Cursor;
import android.os.SystemClock;
import android.support.annotation.WorkerThread;
import android.support.v4.content.AsyncTaskLoader;
+import org.mozilla.gecko.GeckoSharedPrefs;
+import org.mozilla.gecko.R;
import org.mozilla.gecko.Telemetry;
import org.mozilla.gecko.activitystream.ranking.HighlightsRanking;
import org.mozilla.gecko.db.BrowserContract;
import org.mozilla.gecko.db.BrowserDB;
import org.mozilla.gecko.activitystream.homepanel.model.Highlight;
import org.mozilla.gecko.util.ThreadUtils;
import java.util.Collections;
@@ -58,17 +62,22 @@ import java.util.List;
if (candidatesCursor == null) {
return Collections.emptyList();
}
try {
// From now on get notified about content updates and reload data - until loader is reset.
enableContentUpdates();
- final List<Highlight> highlights = HighlightsRanking.rank(candidatesCursor, highlightsLimit);
+ final SharedPreferences prefs = GeckoSharedPrefs.forProfile(getContext());
+ final Resources res = getContext().getResources();
+ final boolean includeHistory = prefs.getBoolean(ActivityStreamPanel.PREF_VISITED_ENABLED, res.getBoolean(R.bool.pref_activitystream_visited_enabled_default));
+ final boolean includeBookmarks = prefs.getBoolean(ActivityStreamPanel.PREF_BOOKMARKS_ENABLED, res.getBoolean(R.bool.pref_activitystream_recentbookmarks_enabled_default));
+
+ final List<Highlight> highlights = HighlightsRanking.rank(candidatesCursor, highlightsLimit, includeHistory, includeBookmarks);
forceLoadHighlightMetadata(highlights); // force load now that we have a short list of the data.
addToPerformanceHistogram(startTime);
return highlights;
} finally {
candidatesCursor.close();
}
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/ranking/HighlightCandidate.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/ranking/HighlightCandidate.java
@@ -1,16 +1,17 @@
/* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*-
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
package org.mozilla.gecko.activitystream.ranking;
import android.database.Cursor;
+import android.database.sqlite.SQLiteDatabase;
import android.net.Uri;
import android.support.annotation.CheckResult;
import android.support.annotation.IntDef;
import android.support.annotation.Nullable;
import android.support.annotation.VisibleForTesting;
import org.mozilla.gecko.activitystream.homepanel.model.Highlight;
import java.lang.annotation.Retention;
@@ -63,21 +64,28 @@ import java.lang.annotation.RetentionPol
return values[featureName];
}
/* package-private */ void put(final @FeatureName int featureName, final double value) {
values[featureName] = value;
}
}
+ /**
+ * The BOOKMARK_ID colmun value is set to -1 for non-bookmarks in
+ * {@link org.mozilla.gecko.db.BrowserProvider#getHighlightCandidates(SQLiteDatabase, String)}
+ */
+ private static final int COLUMN_VALUE_NON_BOOKMARK = -1;
+
@VisibleForTesting final Features features = new Features();
private Highlight highlight;
private @Nullable String imageUrl;
private String host;
private double score;
+ private boolean isBookmark;
/**
* @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();
@@ -112,16 +120,18 @@ import java.lang.annotation.RetentionPol
// 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.isBookmark = cursor.getDouble(cursorIndices.bookmarkIDColumnIndex) != COLUMN_VALUE_NON_BOOKMARK;
+
candidate.features.put(
FEATURE_AGE_IN_DAYS,
(System.currentTimeMillis() - cursor.getDouble(cursorIndices.historyDateLastVisitedColumnIndex))
/ (1000 * 3600 * 24));
candidate.features.put(
FEATURE_VISITS_COUNT,
cursor.getDouble(cursorIndices.visitsColumnIndex));
@@ -203,16 +213,20 @@ import java.lang.annotation.RetentionPol
/* package-private */ String getUrl() {
return highlight.getUrl();
}
/* package-private */ String getHost() {
return host;
}
+ /* package-private */ boolean isBookmark() {
+ return isBookmark;
+ }
+
/**
* Gets an estimate of the actual image url that should only be used to compare against other return
* values of this method. See {@link Highlight#getFastImageURLForComparison()} for more details.
*/
@Nullable
/* package-private */ String getFastImageUrlForComparison() {
return imageUrl;
}
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/ranking/HighlightsRanking.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/ranking/HighlightsRanking.java
@@ -6,27 +6,30 @@
package org.mozilla.gecko.activitystream.ranking;
import android.database.Cursor;
import android.net.Uri;
import android.support.annotation.VisibleForTesting;
import android.text.TextUtils;
import android.util.Log;
import android.util.SparseArray;
+
+import org.mozilla.gecko.SharedPreferencesHelper;
import org.mozilla.gecko.activitystream.homepanel.model.Highlight;
import org.mozilla.gecko.util.MapUtils;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
+import static android.R.attr.candidatesTextStyleSpans;
import static android.R.attr.filter;
import static java.util.Collections.sort;
import static org.mozilla.gecko.activitystream.ranking.HighlightCandidate.FEATURE_AGE_IN_DAYS;
import static org.mozilla.gecko.activitystream.ranking.HighlightCandidate.FEATURE_BOOKMARK_AGE_IN_MILLISECONDS;
import static org.mozilla.gecko.activitystream.ranking.HighlightCandidate.FEATURE_DESCRIPTION_LENGTH;
import static org.mozilla.gecko.activitystream.ranking.HighlightCandidate.FEATURE_DOMAIN_FREQUENCY;
import static org.mozilla.gecko.activitystream.ranking.HighlightCandidate.FEATURE_IMAGE_COUNT;
import static org.mozilla.gecko.activitystream.ranking.HighlightCandidate.FEATURE_IMAGE_SIZE;
@@ -98,19 +101,21 @@ public class HighlightsRanking {
private static final double BOOKMARK_AGE_DIVIDEND = 3 * 24 * 60 * 60 * 1000;
/**
* Create a list of highlights based on the candidates provided by the input cursor.
*
* THIS METHOD IS CRITICAL FOR HIGHLIGHTS PERFORMANCE AND HAS BEEN OPTIMIZED (bug 1369604):
* please be careful what you add to it!
*/
- public static List<Highlight> rank(Cursor cursor, int limit) {
+ public static List<Highlight> rank(Cursor cursor, int limit, boolean includeHistory, boolean includeBookmarks) {
List<HighlightCandidate> highlights = extractFeatures(cursor);
+ filterOutItemsPreffedOff(highlights, includeHistory, includeBookmarks);
+
normalize(highlights);
scoreEntries(highlights);
filterOutItemsWithNoScore(highlights);
sortDescendingByScore(highlights);
@@ -202,16 +207,33 @@ public class HighlightsRanking {
return 1;
} else {
return 0;
}
}
});
}
+ @VisibleForTesting static void filterOutItemsPreffedOff(List<HighlightCandidate> candidates, final boolean includeHistory, final boolean includeBookmarks) {
+ // Pinned items are not bookmarks, and will be grouped with history.
+ filter(candidates, new Func1<HighlightCandidate, Boolean>() {
+ @Override
+ public Boolean call(HighlightCandidate candidate) {
+ if (includeBookmarks && includeHistory) {
+ return true;
+ } else if (!includeBookmarks && !includeHistory) {
+ return false;
+ } else {
+ // Either B or H are enabled, but not both, so we can filter on bookmark state.
+ return includeBookmarks == candidate.isBookmark();
+ }
+ }
+ });
+ }
+
/**
* Remove all items without or with a negative score.
*/
@VisibleForTesting static void filterOutItemsWithNoScore(List<HighlightCandidate> candidates) {
filter(candidates, new Func1<HighlightCandidate, Boolean>() {
@Override
public Boolean call(HighlightCandidate candidate) {
return candidate.getScore() > 0;
--- a/mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java
@@ -1267,16 +1267,20 @@ public class BrowserProvider extends Sha
DBUtils.qualifyColumn(PageMetadata.TABLE_NAME, PageMetadata.JSON) + " AS " + Highlights.METADATA + ", " +
DBUtils.qualifyColumn(Bookmarks.TABLE_NAME, Bookmarks.DATE_CREATED) + ", " +
DBUtils.qualifyColumn(History.TABLE_NAME, History.TITLE) + " AS " + Highlights.TITLE + ", " +
DBUtils.qualifyColumn(Bookmarks.TABLE_NAME, Bookmarks.POSITION) + " AS " + Highlights.POSITION + ", " +
DBUtils.qualifyColumn(Bookmarks.TABLE_NAME, Bookmarks.PARENT) + " AS " + Highlights.PARENT + ", " +
DBUtils.qualifyColumn(History.TABLE_NAME, History._ID) + " AS " + Highlights.HISTORY_ID + ", " +
+ /**
+ * NB: Highlights filtering depends on BOOKMARK_ID, so changes to this logic should also update
+ * {@link org.mozilla.gecko.activitystream.ranking.HighlightsRanking#filterOutItemsPreffedOff(List, boolean, boolean)}
+ */
"CASE WHEN " + DBUtils.qualifyColumn(Bookmarks.TABLE_NAME, Bookmarks._ID) + " IS NOT NULL "
+ "AND " + DBUtils.qualifyColumn(Bookmarks.TABLE_NAME, Bookmarks.PARENT) + " IS NOT " + Bookmarks.FIXED_PINNED_LIST_ID + " "
+ "THEN " + DBUtils.qualifyColumn(Bookmarks.TABLE_NAME, Bookmarks._ID) + " "
+ "ELSE -1 "
+ "END AS " + Highlights.BOOKMARK_ID + ", " +
"CASE WHEN " + DBUtils.qualifyColumn(Bookmarks.TABLE_NAME, Bookmarks.DATE_CREATED) + " IS NOT NULL "
+ "THEN " + DBUtils.qualifyColumn(Bookmarks.TABLE_NAME, Bookmarks.DATE_CREATED) + " "