Backed out changeset 371d34adf87a (bug 1369604) for Android bustage at testActivityStreamContextMenu.java:297: method fromCursor in class Highlight cannot be applied to given types. r=backout
authorSebastian Hengst <archaeopteryx@coole-files.de>
Sat, 29 Jul 2017 18:36:57 +0200
changeset 420540 45f201fe8a9a04031281696a9a25b8d96d636355
parent 420539 7d2a2ae193821b6f6d21169e2599f274ffef0a3b
child 420541 770c9d02352d1af7af74f6cc18f209727db7cc7a
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1369604
milestone56.0a1
backs out371d34adf87a6e9f3ff6f878784a4eafb96b9d7e
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
Backed out changeset 371d34adf87a (bug 1369604) for Android bustage at testActivityStreamContextMenu.java:297: method fromCursor in class Highlight cannot be applied to given types. r=backout
mobile/android/base/java/org/mozilla/gecko/activitystream/Utils.java
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/model/Highlight.java
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/model/Metadata.java
mobile/android/base/java/org/mozilla/gecko/activitystream/ranking/HighlightCandidate.java
mobile/android/base/java/org/mozilla/gecko/activitystream/ranking/HighlightCandidateCursorIndices.java
mobile/android/base/java/org/mozilla/gecko/activitystream/ranking/HighlightsRanking.java
mobile/android/base/moz.build
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/Utils.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/Utils.java
@@ -1,31 +1,32 @@
 /* -*- Mode: Java; c-basic-offset: 4; tab-width: 4; 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;
 
 import android.database.Cursor;
-import org.mozilla.gecko.activitystream.ranking.HighlightCandidateCursorIndices;
+
+import org.mozilla.gecko.db.BrowserContract;
 
 /**
  * Various util methods and constants that are shared by different parts of Activity Stream.
  */
 public class Utils {
     public enum HighlightSource {
         VISITED,
         BOOKMARKED
     }
 
-    public static HighlightSource highlightSource(final Cursor cursor, final HighlightCandidateCursorIndices cursorIndices) {
-        if (-1 != cursor.getLong(cursorIndices.bookmarkIDColumnIndex)) {
+    public static HighlightSource highlightSource(final Cursor cursor) {
+        if (-1 != cursor.getLong(cursor.getColumnIndexOrThrow(BrowserContract.Highlights.BOOKMARK_ID))) {
             return HighlightSource.BOOKMARKED;
         }
 
-        if (-1 != cursor.getLong(cursorIndices.historyIDColumnIndex)) {
+        if (-1 != cursor.getLong(cursor.getColumnIndexOrThrow(BrowserContract.Highlights.HISTORY_ID))) {
             return HighlightSource.VISITED;
         }
 
         throw new IllegalArgumentException("Unknown highlight source.");
     }
 }
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/model/Highlight.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/model/Highlight.java
@@ -3,45 +3,46 @@
  * 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.model;
 
 import android.database.Cursor;
 import android.support.annotation.Nullable;
 import android.text.format.DateUtils;
+
 import org.mozilla.gecko.activitystream.Utils;
-import org.mozilla.gecko.activitystream.ranking.HighlightCandidateCursorIndices;
+import org.mozilla.gecko.db.BrowserContract;
 
 public class Highlight implements Item {
     private final String title;
     private final String url;
     private final Utils.HighlightSource source;
     private final long time;
 
     private long historyId;
 
     private Metadata metadata;
 
     private @Nullable Boolean isPinned;
     private @Nullable Boolean isBookmarked;
 
-    public static Highlight fromCursor(final Cursor cursor, final HighlightCandidateCursorIndices cursorIndices) {
-        return new Highlight(cursor, cursorIndices);
+    public static Highlight fromCursor(Cursor cursor) {
+        return new Highlight(cursor);
     }
 
-    private Highlight(final Cursor cursor, final HighlightCandidateCursorIndices cursorIndices) {
-        title = cursor.getString(cursorIndices.titleColumnIndex);
-        url = cursor.getString(cursorIndices.urlColumnIndex);
-        source = Utils.highlightSource(cursor, cursorIndices);
-        time = cursor.getLong(cursorIndices.highlightsDateColumnIndex);
+    private Highlight(Cursor cursor) {
+        title = cursor.getString(cursor.getColumnIndexOrThrow(BrowserContract.History.TITLE));
+        url = cursor.getString(cursor.getColumnIndexOrThrow(BrowserContract.Combined.URL));
+        source = Utils.highlightSource(cursor);
+        time = cursor.getLong(cursor.getColumnIndexOrThrow(BrowserContract.Highlights.DATE));
 
-        historyId = cursor.getLong(cursorIndices.historyIDColumnIndex);
+        historyId = cursor.getLong(cursor.getColumnIndexOrThrow(BrowserContract.Highlights.HISTORY_ID));
 
-        metadata = Metadata.fromCursor(cursor, cursorIndices);
+        metadata = Metadata.fromCursor(cursor);
 
         updateState();
     }
 
     private void updateState() {
         // We can only be certain of bookmark state if an item is a bookmark item.
         // Otherwise, due to the underlying highlights query, we have to look up states when
         // menus are displayed.
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/model/Metadata.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/model/Metadata.java
@@ -3,25 +3,27 @@
  * 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.model;
 
 import android.database.Cursor;
 import android.text.TextUtils;
 import android.util.Log;
+
 import org.json.JSONException;
 import org.json.JSONObject;
-import org.mozilla.gecko.activitystream.ranking.HighlightCandidateCursorIndices;
+import org.mozilla.gecko.db.BrowserContract;
 
 public class Metadata {
     private static final String LOGTAG = "GeckoMetadata";
 
-    public static Metadata fromCursor(final Cursor cursor, final HighlightCandidateCursorIndices cursorIndices) {
-        return new Metadata(cursor.getString(cursorIndices.metadataColumnIndex));
+    public static Metadata fromCursor(Cursor cursor) {
+        return new Metadata(
+                cursor.getString(cursor.getColumnIndexOrThrow(BrowserContract.Highlights.METADATA)));
     }
 
     private String provider;
     private String imageUrl;
     private int descriptionLength;
 
     private Metadata(String json) {
         if (TextUtils.isEmpty(json)) {
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/ranking/HighlightCandidate.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/ranking/HighlightCandidate.java
@@ -7,16 +7,17 @@ package org.mozilla.gecko.activitystream
 
 import android.database.Cursor;
 import android.net.Uri;
 import android.support.annotation.Nullable;
 import android.support.annotation.StringDef;
 import android.support.annotation.VisibleForTesting;
 
 import org.mozilla.gecko.activitystream.ranking.RankingUtils.Func1;
+import org.mozilla.gecko.db.BrowserContract;
 import org.mozilla.gecko.activitystream.homepanel.model.Highlight;
 
 import java.util.HashMap;
 import java.util.Map;
 
 /**
  * A highlight candidate (Highlight object + features). Ranking will determine whether this is an
  * actual highlight.
@@ -38,47 +39,44 @@ import java.util.Map;
     public @interface Feature {}
 
     @VisibleForTesting final Map<String, Double> features;
     private Highlight highlight;
     private @Nullable String imageUrl;
     private String host;
     private double score;
 
-    public static HighlightCandidate fromCursor(final Cursor cursor, final HighlightCandidateCursorIndices cursorIndices)
-            throws InvalidHighlightCandidateException {
+    public static HighlightCandidate fromCursor(Cursor cursor) throws InvalidHighlightCandidateException {
         final HighlightCandidate candidate = new HighlightCandidate();
 
-        extractHighlight(candidate, cursor, cursorIndices);
-        extractFeatures(candidate, cursor, cursorIndices);
+        extractHighlight(candidate, cursor);
+        extractFeatures(candidate, cursor);
 
         return candidate;
     }
 
     /**
      * Extract highlight object from cursor.
      */
-    private static void extractHighlight(final HighlightCandidate candidate, final Cursor cursor,
-            final HighlightCandidateCursorIndices cursorIndices) {
-        candidate.highlight = Highlight.fromCursor(cursor, cursorIndices);
+    private static void extractHighlight(HighlightCandidate candidate, Cursor cursor) {
+        candidate.highlight = Highlight.fromCursor(cursor);
     }
 
     /**
      * Extract and assign features that will be used for ranking.
      */
-    private static void extractFeatures(final HighlightCandidate candidate, final Cursor cursor,
-            final HighlightCandidateCursorIndices cursorIndices) throws InvalidHighlightCandidateException {
+    private static void extractFeatures(HighlightCandidate candidate, Cursor cursor) throws InvalidHighlightCandidateException {
         candidate.features.put(
                 FEATURE_AGE_IN_DAYS,
-                (System.currentTimeMillis() - cursor.getDouble(cursorIndices.historyDateLastVisitedColumnIndex))
+                (System.currentTimeMillis() - cursor.getDouble(cursor.getColumnIndexOrThrow(BrowserContract.History.DATE_LAST_VISITED)))
                         / (1000 * 3600 * 24));
 
         candidate.features.put(
                 FEATURE_VISITS_COUNT,
-                cursor.getDouble(cursorIndices.visitsColumnIndex));
+                cursor.getDouble(cursor.getColumnIndexOrThrow(BrowserContract.History.VISITS)));
 
         // Until we can determine those numbers we assume this domain has only been visited once
         // and the cursor returned all database entries.
         // TODO: Calculate values based using domain hash field (bug 1335817)
         final int occurrences = 1; // Number of times host shows up in history (Bug 1319485)
         final int domainCountSize = cursor.getCount(); // Number of domains visited (Bug 1319485)
 
         candidate.features.put(
@@ -105,24 +103,25 @@ import java.util.Map;
 
         // Historical note: before Bug 1335198, this value was not really the time at which the
         // bookmark was created by the user. Especially synchronized bookmarks could have a recent
         // value but have been bookmarked a long time ago.
         // Current behaviour: synchronized clients will, over time, converge DATE_CREATED field
         // to the real creation date, or the earliest one mentioned in the clients constellation.
         // We are sourcing highlights from the recent visited history - so in order to
         // show up this bookmark need to have been visited recently too.
-        if (cursor.isNull(cursorIndices.bookmarkDateCreatedColumnIndex)) {
+        final int bookmarkDateColumnIndex = cursor.getColumnIndexOrThrow(BrowserContract.Bookmarks.DATE_CREATED);
+        if (cursor.isNull(bookmarkDateColumnIndex)) {
             candidate.features.put(
                     FEATURE_BOOKMARK_AGE_IN_MILLISECONDS,
                     0d);
         } else {
             candidate.features.put(
                     FEATURE_BOOKMARK_AGE_IN_MILLISECONDS,
-                    Math.max(1, System.currentTimeMillis() - cursor.getDouble(cursorIndices.bookmarkDateCreatedColumnIndex)));
+                    Math.max(1, System.currentTimeMillis() - cursor.getDouble(bookmarkDateColumnIndex)));
         }
 
         candidate.features.put(
                 FEATURE_DESCRIPTION_LENGTH,
                 (double) candidate.highlight.getMetadata().getDescriptionLength());
 
         final Uri uri = Uri.parse(candidate.highlight.getUrl());
 
deleted file mode 100644
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/ranking/HighlightCandidateCursorIndices.java
+++ /dev/null
@@ -1,45 +0,0 @@
-/* -*- 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 org.mozilla.gecko.db.BrowserContract;
-
-/**
- * A cache of the column indices of the given Cursor.
- *
- * We use this for performance reasons: {@link Cursor#getColumnIndexOrThrow(String)} will do a HashMap look-up and
- * String comparison each time it's called, which gets expensive while iterating through HighlightCandidate results
- * (currently a maximum of 500 items), so we cache the values.
- */
-public class HighlightCandidateCursorIndices {
-
-    public final int titleColumnIndex;
-    public final int urlColumnIndex;
-    public final int visitsColumnIndex;
-    public final int metadataColumnIndex;
-
-    public final int highlightsDateColumnIndex;
-    public final int bookmarkDateCreatedColumnIndex;
-    public final int historyDateLastVisitedColumnIndex;
-
-    public final int historyIDColumnIndex;
-    public final int bookmarkIDColumnIndex;
-
-    HighlightCandidateCursorIndices(final Cursor cursor) {
-        titleColumnIndex = cursor.getColumnIndexOrThrow(BrowserContract.History.TITLE);
-        urlColumnIndex = cursor.getColumnIndexOrThrow(BrowserContract.Combined.URL);
-        visitsColumnIndex = cursor.getColumnIndexOrThrow(BrowserContract.History.VISITS);
-        metadataColumnIndex = cursor.getColumnIndexOrThrow(BrowserContract.Highlights.METADATA);
-
-        highlightsDateColumnIndex = cursor.getColumnIndexOrThrow(BrowserContract.Highlights.DATE);
-        bookmarkDateCreatedColumnIndex = cursor.getColumnIndexOrThrow(BrowserContract.Bookmarks.DATE_CREATED);
-        historyDateLastVisitedColumnIndex = cursor.getColumnIndexOrThrow(BrowserContract.History.DATE_LAST_VISITED);
-
-        historyIDColumnIndex = cursor.getColumnIndexOrThrow(BrowserContract.Highlights.HISTORY_ID);
-        bookmarkIDColumnIndex = cursor.getColumnIndexOrThrow(BrowserContract.Highlights.BOOKMARK_ID);
-    }
-}
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/ranking/HighlightsRanking.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/ranking/HighlightsRanking.java
@@ -2,18 +2,18 @@
  * 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.support.annotation.VisibleForTesting;
+import android.util.Log;
 
-import android.util.Log;
 import org.mozilla.gecko.activitystream.homepanel.model.Highlight;
 
 import java.util.Arrays;
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -91,24 +91,22 @@ public class HighlightsRanking {
 
         return createHighlightsList(highlights, limit);
     }
 
     /**
      * Extract features for every candidate. The heavy lifting is done in
      * HighlightCandidate.fromCursor().
      */
-    @VisibleForTesting static List<HighlightCandidate> extractFeatures(final Cursor cursor) {
-        // Cache column indices for performance: see class Javadoc for more info.
-        final HighlightCandidateCursorIndices cursorIndices = new HighlightCandidateCursorIndices(cursor);
+    @VisibleForTesting static List<HighlightCandidate> extractFeatures(Cursor cursor) {
         return looselyMapCursor(cursor, new Func1<Cursor, HighlightCandidate>() {
             @Override
             public HighlightCandidate call(Cursor cursor) {
                 try {
-                    return HighlightCandidate.fromCursor(cursor, cursorIndices);
+                    return HighlightCandidate.fromCursor(cursor);
                 } catch (HighlightCandidate.InvalidHighlightCandidateException e) {
                     Log.w(LOG_TAG, "Skipping invalid highlight item", e);
                     return null;
                 }
             }
         });
     }
 
--- a/mobile/android/base/moz.build
+++ b/mobile/android/base/moz.build
@@ -520,17 +520,16 @@ gbjar.sources += ['java/org/mozilla/geck
     'activitystream/homepanel/stream/WelcomePanel.java',
     'activitystream/homepanel/StreamItemAnimator.java',
     'activitystream/homepanel/StreamRecyclerAdapter.java',
     'activitystream/homepanel/topsites/TopSitesCard.java',
     'activitystream/homepanel/topsites/TopSitesPage.java',
     'activitystream/homepanel/topsites/TopSitesPageAdapter.java',
     'activitystream/homepanel/topsites/TopSitesPagerAdapter.java',
     'activitystream/ranking/HighlightCandidate.java',
-    'activitystream/ranking/HighlightCandidateCursorIndices.java',
     'activitystream/ranking/HighlightsRanking.java',
     'activitystream/ranking/RankingUtils.java',
     'activitystream/Utils.java',
     'adjust/AdjustBrowserAppDelegate.java',
     'animation/AnimationUtils.java',
     'animation/HeightChangeAnimation.java',
     'animation/PropertyAnimator.java',
     'animation/Rotate3DAnimation.java',