Bug 1076438 - Undo ID and imageurl/bgcolor cursor changes. r=lucasr

mobile/android/base/db/BrowserContract.java

@@ -107,12 +107,6 @@ public class BrowserContract {
         public static final String TIME_DELETED = "timeDeleted";
     }
 
-    public interface SuggestedSitesColumns {
-        public static final String BGCOLOR = "bgcolor";
-        public static final String IMAGEURL = "imageurl";
-        public static final String TRACKING_ID = "tracking_id";
-    }
-
     @RobocopTarget
     public static final class Favicons implements CommonColumns, DateSyncColumns {
         private Favicons() {}
@@ -416,7 +410,7 @@ public class BrowserContract {
     }
 
     @RobocopTarget
-    public static final class TopSites implements CommonColumns, URLColumns, SuggestedSitesColumns {
+    public static final class TopSites implements CommonColumns, URLColumns {
         private TopSites() {}
 
         public static final int TYPE_BLANK = 0;
@@ -441,7 +435,7 @@ public class BrowserContract {
     }
 
     @RobocopTarget
-    public static final class SuggestedSites implements CommonColumns, URLColumns, SuggestedSitesColumns {
+    public static final class SuggestedSites implements CommonColumns, URLColumns {
         private SuggestedSites() {}
 
         public static final Uri CONTENT_URI = Uri.withAppendedPath(AUTHORITY_URI, "suggestedsites");

mobile/android/base/db/BrowserDB.java

@@ -21,6 +21,7 @@ import android.content.ContentValues;
 import android.content.Context;
 import android.database.ContentObserver;
 import android.database.Cursor;
+import android.graphics.Color;
 import android.graphics.drawable.BitmapDrawable;
 
 /**
@@ -252,4 +253,21 @@ public class BrowserDB {
     public static void setEnableContentProviders(boolean enableContentProviders) {
         sAreContentProvidersEnabled = enableContentProviders;
     }
+
+    public static boolean hasSuggestedImageUrl(String url) {
+        return sSuggestedSites.contains(url);
+    }
+
+    public static String getSuggestedImageUrlForUrl(String url) {
+        return sSuggestedSites.getImageUrlForUrl(url);
+    }
+
+    public static int getSuggestedBackgroundColorForUrl(String url) {
+        final String bgColor = sSuggestedSites.getBackgroundColorForUrl(url);
+        if (bgColor != null) {
+            return Color.parseColor(bgColor);
+        }
+
+        return 0;
+    }
 }

mobile/android/base/db/SuggestedSites.java

@@ -80,28 +80,22 @@ public class SuggestedSites {
 
     private static final String[] COLUMNS = new String[] {
         BrowserContract.SuggestedSites._ID,
-        BrowserContract.SuggestedSites.TRACKING_ID,
         BrowserContract.SuggestedSites.URL,
         BrowserContract.SuggestedSites.TITLE,
-        BrowserContract.SuggestedSites.IMAGEURL,
-        BrowserContract.SuggestedSites.BGCOLOR
     };
 
-    private static final String JSON_KEY_TRACKING_ID = "trackingid";
     private static final String JSON_KEY_URL = "url";
     private static final String JSON_KEY_TITLE = "title";
     private static final String JSON_KEY_IMAGE_URL = "imageurl";
     private static final String JSON_KEY_BG_COLOR = "bgcolor";
 
     private static class Site {
-        public final String trackingId;
         public final String url;
         public final String title;
         public final String imageUrl;
         public final String bgColor;
 
         public Site(JSONObject json) throws JSONException {
-            this.trackingId = json.isNull(JSON_KEY_TRACKING_ID) ? null : json.getString(JSON_KEY_TRACKING_ID);
             this.url = json.getString(JSON_KEY_URL);
             this.title = json.getString(JSON_KEY_TITLE);
             this.imageUrl = json.getString(JSON_KEY_IMAGE_URL);
@@ -110,8 +104,7 @@ public class SuggestedSites {
             validate();
         }
 
-        public Site(String trackingId, String url, String title, String imageUrl, String bgColor) {
+        public Site(String url, String title, String imageUrl, String bgColor) {
-            this.trackingId = trackingId;
             this.url = url;
             this.title = title;
             this.imageUrl = imageUrl;
@@ -121,7 +114,7 @@ public class SuggestedSites {
         }
 
         private void validate() {
-            // Site instances must have non-empty values for all properties except IDs.
+            // Site instances must have non-empty values for all properties.
             if (TextUtils.isEmpty(url) ||
                 TextUtils.isEmpty(title) ||
                 TextUtils.isEmpty(imageUrl) ||
@@ -133,8 +126,7 @@ public class SuggestedSites {
 
         @Override
         public String toString() {
-            return "{ id = " + trackingId + "\n" +
+            return "{ url = " + url + "\n" +
-                     "url = " + url + "\n" +
                      "title = " + title + "\n" +
                      "imageUrl = " + imageUrl + "\n" +
                      "bgColor = " + bgColor + " }";
@@ -143,9 +135,6 @@ public class SuggestedSites {
         public JSONObject toJSON() throws JSONException {
             final JSONObject json = new JSONObject();
 
-            // If trackingId is null, the key is not added.
-            json.put(JSON_KEY_TRACKING_ID, trackingId);
-
             json.put(JSON_KEY_URL, url);
             json.put(JSON_KEY_TITLE, title);
             json.put(JSON_KEY_IMAGE_URL, imageUrl);
@@ -501,11 +490,8 @@ public class SuggestedSites {
 
             final RowBuilder row = cursor.newRow();
             row.add(-1);
-            row.add(site.trackingId);
             row.add(site.url);
             row.add(site.title);
-            row.add(site.imageUrl);
-            row.add(site.bgColor);
         }
 
         cursor.setNotificationUri(context.getContentResolver(),
@@ -514,6 +500,20 @@ public class SuggestedSites {
         return cursor;
     }
 
+    public boolean contains(String url) {
+        return (getSiteForUrl(url) != null);
+    }
+
+    public String getImageUrlForUrl(String url) {
+        final Site site = getSiteForUrl(url);
+        return (site != null ? site.imageUrl : null);
+    }
+
+    public String getBackgroundColorForUrl(String url) {
+        final Site site = getSiteForUrl(url);
+        return (site != null ? site.bgColor : null);
+    }
+
     private Set<String> loadBlacklist() {
         Log.d(LOGTAG, "Loading blacklisted suggested sites from SharedPreferences.");
         final Set<String> blacklist = new HashSet<String>();

mobile/android/base/db/TopSitesCursorWrapper.java

@@ -38,9 +38,6 @@ public class TopSitesCursorWrapper implements Cursor {
         TopSites.BOOKMARK_ID,
         TopSites.HISTORY_ID,
         TopSites.TYPE,
-        TopSites.IMAGEURL,
-        TopSites.BGCOLOR,
-        TopSites.TRACKING_ID,
     };
 
     private static final Map<String, Integer> columnIndexes =
@@ -52,10 +49,6 @@ public class TopSitesCursorWrapper implements Cursor {
         }
     }
 
-    private static final int INDEX_TRACKING_ID = columnIndexes.get(TopSites.TRACKING_ID);
-    private static final int INDEX_IMAGEURL = columnIndexes.get(TopSites.IMAGEURL);
-    private static final int INDEX_BGCOLOR = columnIndexes.get(TopSites.BGCOLOR);
-
     // Maps column indexes from the wrapper to the cursor's.
     private SparseIntArray topIndexes;
     private SparseIntArray pinnedIndexes;
@@ -247,23 +240,12 @@ public class TopSitesCursorWrapper implements Cursor {
         }
 
         if (map != null) {
-            // Only look up suggested columns on suggested cursors.
-            if (currentRowType != RowType.SUGGESTED && isSuggestedSiteColumn(columnIndex)) {
-                return -1;
-            }
-
             return map.get(columnIndex, -1);
         }
 
         return -1;
     }
 
-    private boolean isSuggestedSiteColumn(int columnIndex) {
-        return columnIndex == INDEX_IMAGEURL ||
-               columnIndex == INDEX_BGCOLOR ||
-               columnIndex == INDEX_TRACKING_ID;
-    }
-
     @Override
     public int getPosition() {
         return currentPosition;

mobile/android/base/home/TopSitesPanel.java

@@ -525,12 +525,15 @@ public class TopSitesPanel extends HomeFragment {
                 return;
             }
 
+            // Make sure we query suggested images without the user-entered wrapper.
+            final String decodedUrl = StringUtils.decodeUserEnteredUrl(url);
+
             // Suggested images have precedence over thumbnails, no need to wait
             // for them to be loaded. See: CursorLoaderCallbacks.onLoadFinished()
-            final String imageUrl = cursor.getString(cursor.getColumnIndexOrThrow(TopSites.IMAGEURL));
+            final String imageUrl = BrowserDB.getSuggestedImageUrlForUrl(decodedUrl);
             if (!TextUtils.isEmpty(imageUrl)) {
-                final String bgColor = cursor.getString(cursor.getColumnIndexOrThrow(TopSites.BGCOLOR));
+                final int bgColor = BrowserDB.getSuggestedBackgroundColorForUrl(decodedUrl);
-                view.displayThumbnail(imageUrl, Color.parseColor(bgColor));
+                view.displayThumbnail(imageUrl, bgColor);
                 return;
             }
 
@@ -621,11 +624,10 @@ public class TopSitesPanel extends HomeFragment {
             int i = 1;
             do {
                 final String url = c.getString(col);
-                final String imageUrl = c.getString(c.getColumnIndexOrThrow(TopSites.IMAGEURL));
 
                 // Only try to fetch thumbnails for non-empty URLs that
                 // don't have an associated suggested image URL.
-                if (TextUtils.isEmpty(url) || !TextUtils.isEmpty(imageUrl)) {
+                if (TextUtils.isEmpty(url) || BrowserDB.hasSuggestedImageUrl(url)) {
                     continue;
                 }
 

mobile/android/tests/browser/junit3/src/TestSuggestedSites.java

@@ -152,7 +152,6 @@ public class TestSuggestedSites extends BrowserTestCase {
         try {
             for (int i = 0; i < n; i++) {
                 JSONObject site = new JSONObject();
-                site.put("trackingid", prefix + "trackingId" + i);
                 site.put("url", prefix + "url" + i);
                 site.put("title", prefix + "title" + i);
                 site.put("imageurl", prefix + "imageUrl" + i);
@@ -239,18 +238,6 @@ public class TestSuggestedSites extends BrowserTestCase {
         checkCursorCount("{ broken: }", 0);
     }
 
-    public void testNoTrackingId() {
-        String content = "[{ url: \"url\", title: \"title\", imageurl: \"imageurl\", bgcolor: \"bgcolor\" }]";
-        resources.setSuggestedSitesResource(content);
-
-        Cursor c = new SuggestedSites(context).get(DEFAULT_LIMIT);
-        assertEquals(1, c.getCount());
-        c.moveToNext();
-
-        String trackingId = c.getString(c.getColumnIndexOrThrow(BrowserContract.SuggestedSites.TRACKING_ID));
-        assertNull(trackingId);
-    }
-
     public void testCursorContent() {
         resources.setSuggestedSitesResource(generateSites(3));
 
@@ -261,20 +248,11 @@ public class TestSuggestedSites extends BrowserTestCase {
         while (c.moveToNext()) {
             int position = c.getPosition();
 
-            String trackingId = c.getString(c.getColumnIndexOrThrow(BrowserContract.SuggestedSites.TRACKING_ID));
-            assertEquals("trackingId" + position, trackingId);
-
             String url = c.getString(c.getColumnIndexOrThrow(BrowserContract.SuggestedSites.URL));
             assertEquals("url" + position, url);
 
             String title = c.getString(c.getColumnIndexOrThrow(BrowserContract.SuggestedSites.TITLE));
             assertEquals("title" + position, title);
-
-            String imageUrl = c.getString(c.getColumnIndexOrThrow(BrowserContract.SuggestedSites.IMAGEURL));
-            assertEquals("imageUrl" + position, imageUrl);
-
-            String bgColor = c.getString(c.getColumnIndexOrThrow(BrowserContract.SuggestedSites.BGCOLOR));
-            assertEquals("bgColor" + position, bgColor);
         }
 
         c.close();
@@ -361,6 +339,40 @@ public class TestSuggestedSites extends BrowserTestCase {
         c.close();
     }
 
+    public void testImageUrlAndBgColor() {
+        final int count = 3;
+        resources.setSuggestedSitesResource(generateSites(count));
+
+        SuggestedSites suggestedSites = new SuggestedSites(context);
+
+        // Suggested sites hasn't been loaded yet.
+        for (int i = 0; i < count; i++) {
+            String url = "url" + i;
+            assertFalse(suggestedSites.contains(url));
+            assertNull(suggestedSites.getImageUrlForUrl(url));
+            assertNull(suggestedSites.getBackgroundColorForUrl(url));
+        }
+
+        Cursor c = suggestedSites.get(DEFAULT_LIMIT);
+        c.moveToPosition(-1);
+
+        // We should have cached results after the get() call.
+        while (c.moveToNext()) {
+            String url = c.getString(c.getColumnIndexOrThrow(BrowserContract.SuggestedSites.URL));
+            assertTrue(suggestedSites.contains(url));
+            assertEquals("imageUrl" + c.getPosition(),
+                         suggestedSites.getImageUrlForUrl(url));
+            assertEquals("bgColor" + c.getPosition(),
+                         suggestedSites.getBackgroundColorForUrl(url));
+        }
+        c.close();
+
+        // No valid values for unknown URLs.
+        assertFalse(suggestedSites.contains("foo"));
+        assertNull(suggestedSites.getImageUrlForUrl("foo"));
+        assertNull(suggestedSites.getBackgroundColorForUrl("foo"));
+    }
+
     public void testLocaleChanges() {
         resources.setSuggestedSitesResource(generateSites(3));