Bug 1076438 - Undo ID and imageurl/bgcolor cursor changes. r=lucasr
--- a/mobile/android/base/db/BrowserContract.java
+++ b/mobile/android/base/db/BrowserContract.java
@@ -102,22 +102,16 @@ public class BrowserContract {
}
public interface DeletedColumns {
public static final String ID = "id";
public static final String GUID = "guid";
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() {}
public static final String TABLE_NAME = "favicons";
public static final Uri CONTENT_URI = Uri.withAppendedPath(AUTHORITY_URI, "favicons");
@@ -411,17 +405,17 @@ public class BrowserContract {
// Minimum fields required to create a reading list item.
public static final String[] REQUIRED_FIELDS = { Bookmarks.URL, Bookmarks.TITLE };
public static final String TABLE_NAME = "reading_list";
}
@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;
public static final int TYPE_TOP = 1;
public static final int TYPE_PINNED = 2;
public static final int TYPE_SUGGESTED = 3;
public static final String BOOKMARK_ID = "bookmark_id";
@@ -436,14 +430,14 @@ public class BrowserContract {
public static final String CONTENT_TYPE = "vnd.android.cursor.dir/searchhistory";
public static final String QUERY = "query";
public static final String TABLE_NAME = "searchhistory";
public static final Uri CONTENT_URI = Uri.withAppendedPath(SEARCH_HISTORY_AUTHORITY_URI, "searchhistory");
}
@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");
}
}
--- a/mobile/android/base/db/BrowserDB.java
+++ b/mobile/android/base/db/BrowserDB.java
@@ -16,16 +16,17 @@ import org.mozilla.gecko.favicons.decode
import org.mozilla.gecko.mozglue.RobocopTarget;
import org.mozilla.gecko.util.StringUtils;
import android.content.ContentResolver;
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;
/**
* A utility wrapper for accessing a static {@link LocalBrowserDB},
* manually initialized with a particular profile, and similarly
* managing a static instance of {@link SuggestedSites}.
*
* Be careful using this class: if you're not BrowserApp, you probably
@@ -247,9 +248,26 @@ public class BrowserDB {
@RobocopTarget
public static Cursor getBookmarkForUrl(ContentResolver cr, String url) {
return sDb.getBookmarkForUrl(cr, url);
}
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;
+ }
}
--- a/mobile/android/base/db/SuggestedSites.java
+++ b/mobile/android/base/db/SuggestedSites.java
@@ -75,82 +75,71 @@ public class SuggestedSites {
// Locale used to generate the current suggested sites.
public static final String PREF_SUGGESTED_SITES_LOCALE = "suggestedSites.locale";
// File in profile dir with the list of suggested sites.
private static final String FILENAME = "suggestedsites.json";
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);
this.bgColor = json.getString(JSON_KEY_BG_COLOR);
validate();
}
- public Site(String trackingId, String url, String title, String imageUrl, String bgColor) {
- this.trackingId = trackingId;
+ public Site(String url, String title, String imageUrl, String bgColor) {
this.url = url;
this.title = title;
this.imageUrl = imageUrl;
this.bgColor = bgColor;
validate();
}
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) ||
TextUtils.isEmpty(bgColor)) {
throw new IllegalStateException("Suggested sites must have a URL, title, " +
"image URL, and background color.");
}
}
@Override
public String toString() {
- return "{ id = " + trackingId + "\n" +
- "url = " + url + "\n" +
+ return "{ url = " + url + "\n" +
"title = " + title + "\n" +
"imageUrl = " + imageUrl + "\n" +
"bgColor = " + bgColor + " }";
}
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);
json.put(JSON_KEY_BG_COLOR, bgColor);
return json;
}
}
@@ -496,29 +485,40 @@ public class SuggestedSites {
}
if (excludeUrls != null && excludeUrls.contains(site.url)) {
continue;
}
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(),
BrowserContract.SuggestedSites.CONTENT_URI);
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>();
final SharedPreferences preferences = GeckoSharedPrefs.forProfile(context);
final String sitesString = preferences.getString(PREF_SUGGESTED_SITES_HIDDEN, null);
if (sitesString != null) {
--- a/mobile/android/base/db/TopSitesCursorWrapper.java
+++ b/mobile/android/base/db/TopSitesCursorWrapper.java
@@ -33,34 +33,27 @@ public class TopSitesCursorWrapper imple
private static final String[] columnNames = new String[] {
TopSites._ID,
TopSites.URL,
TopSites.TITLE,
TopSites.BOOKMARK_ID,
TopSites.HISTORY_ID,
TopSites.TYPE,
- TopSites.IMAGEURL,
- TopSites.BGCOLOR,
- TopSites.TRACKING_ID,
};
private static final Map<String, Integer> columnIndexes =
new HashMap<String, Integer>(columnNames.length);
static {
for (int i = 0; i < columnNames.length; i++) {
columnIndexes.put(columnNames[i], i);
}
}
- 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;
private SparseIntArray suggestedIndexes;
// Type of content in the current position
private RowType currentRowType;
@@ -242,33 +235,22 @@ public class TopSitesCursorWrapper imple
break;
case SUGGESTED:
map = suggestedIndexes;
break;
}
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;
}
@Override
public int getCount() {
return count;
--- a/mobile/android/base/home/TopSitesPanel.java
+++ b/mobile/android/base/home/TopSitesPanel.java
@@ -520,22 +520,25 @@ public class TopSitesPanel extends HomeF
// Thumbnails are delivered late, so we can't short-circuit any
// sooner than this. But we can avoid a duplicate favicon
// fetch...
if (!updated) {
debug("bindView called twice for same values; short-circuiting.");
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));
- view.displayThumbnail(imageUrl, Color.parseColor(bgColor));
+ final int bgColor = BrowserDB.getSuggestedBackgroundColorForUrl(decodedUrl);
+ view.displayThumbnail(imageUrl, bgColor);
return;
}
// If thumbnails are still being loaded, don't try to load favicons
// just yet. If we sent in a thumbnail, we're done now.
if (mThumbnailInfos == null || thumbnail != null) {
return;
}
@@ -616,21 +619,20 @@ public class TopSitesPanel extends HomeF
if (!c.moveToFirst()) {
return;
}
final ArrayList<String> urls = new ArrayList<String>();
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;
}
urls.add(url);
} while (i++ < mMaxGridEntries && c.moveToNext());
if (urls.isEmpty()) {
// Short-circuit empty results to the UI.
--- a/mobile/android/tests/browser/junit3/src/TestSuggestedSites.java
+++ b/mobile/android/tests/browser/junit3/src/TestSuggestedSites.java
@@ -147,17 +147,16 @@ public class TestSuggestedSites extends
}
private String generateSites(int n, String prefix) {
JSONArray sites = new JSONArray();
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);
site.put("bgcolor", prefix + "bgColor" + i);
sites.put(site);
}
} catch (Exception e) {
@@ -234,52 +233,31 @@ public class TestSuggestedSites extends
// Empty string = empty cursor
checkCursorCount("", 0);
// Invalid json string = empty cursor
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));
Cursor c = new SuggestedSites(context).get(DEFAULT_LIMIT);
assertEquals(3, c.getCount());
c.moveToPosition(-1);
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();
}
public void testExcludeUrls() {
resources.setSuggestedSitesResource(generateSites(6));
@@ -356,16 +334,50 @@ public class TestSuggestedSites extends
.commit();
c = new SuggestedSites(context).get(DEFAULT_LIMIT);
assertNotNull(c);
assertEquals(0, c.getCount());
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));
SuggestedSites suggestedSites = new SuggestedSites(context);
// Initial load with predefined locale
Cursor c = suggestedSites.get(DEFAULT_LIMIT, Locale.UK);
assertEquals(3, c.getCount());