Bug 931918 - Part 2: check affinity between favicon fetches and their view. r=bnicholson

mobile/android/base/home/TopSitesGridItemView.java

@@ -132,6 +132,39 @@ public class TopSitesGridItemView extends RelativeLayout {
         mTitleView.setCompoundDrawablesWithIntrinsicBounds(pinned ? R.drawable.pin : 0, 0, 0, 0);
     }
 
+    /**
+     * Updates the title, URL, and pinned state of this view.
+     *
+     * Also resets our loadId to NOT_LOADING.
+     *
+     * Returns true if any fields changed.
+     */
+    public boolean updateState(final String title, final String url, final boolean pinned) {
+        boolean changed = false;
+        if (mUrl == null || !mUrl.equals(url)) {
+            mUrl = url;
+            changed = true;
+        }
+
+        if (mTitle == null || !mTitle.equals(title)) {
+            mTitle = title;
+            changed = true;
+        }
+
+        if (changed) {
+            updateTitleView();
+            setLoadId(Favicons.NOT_LOADING);
+        }
+
+        if (mIsPinned != pinned) {
+            mIsPinned = pinned;
+            mTitleView.setCompoundDrawablesWithIntrinsicBounds(pinned ? R.drawable.pin : 0, 0, 0, 0);
+            changed = true;
+        }
+
+        return changed;
+    }
+
     /**
      * Display the thumbnail from a resource.
      *
@@ -162,6 +195,17 @@ public class TopSitesGridItemView extends RelativeLayout {
         mThumbnailView.setBackgroundDrawable(null);
     }
 
+    public void displayFavicon(Bitmap favicon, String faviconURL, int expectedLoadId) {
+        if (mLoadId != Favicons.NOT_LOADING &&
+            mLoadId != expectedLoadId) {
+            // View recycled.
+            return;
+        }
+
+        // Yes, there's a chance of a race here.
+        displayFavicon(favicon, faviconURL);
+    }
+
     /**
      * Display the thumbnail from a favicon.
      *
@@ -169,6 +213,7 @@ public class TopSitesGridItemView extends RelativeLayout {
      */
     public void displayFavicon(Bitmap favicon, String faviconURL) {
         if (mThumbnail != null) {
+            // Already showing a thumbnail; do nothing.
             return;
         }
 

mobile/android/base/home/TopSitesPage.java

@@ -558,9 +558,10 @@ public class TopSitesPage extends HomeFragment {
             }
 
             final TopSitesGridItemView view = (TopSitesGridItemView) bindView;
-            view.setTitle(title);
-            view.setUrl(url);
-            view.setPinned(pinned);
+
+            // Debounce bindView calls to avoid redundant redraws and favicon
+            // fetches.
+            final boolean updated = view.updateState(title, url, pinned);
 
             // If there is no url, then show "add bookmark".
             if (TextUtils.isEmpty(url)) {
@@ -570,15 +571,24 @@ public class TopSitesPage extends HomeFragment {
                 Bitmap thumbnail = (mThumbnails != null ? mThumbnails.get(url) : null);
                 if (thumbnail != null) {
                     view.displayThumbnail(thumbnail);
-                } else {
-                    // If we have no thumbnail, attempt to show a Favicon instead.
-                    view.setLoadId(Favicons.getSizedFaviconForPageFromLocal(url, new OnFaviconLoadedListener() {
-                        @Override
-                        public void onFaviconLoaded(String url, String faviconURL, Bitmap favicon) {
-                            view.displayFavicon(favicon, faviconURL);
-                        }
-                    }));
+                    return;
                 }
+
+                // Thumbnails are delivered late, so we can't short-circuit any
+                // sooner than this. But we can avoid a duplicate favicon
+                // fetch...
+                if (!updated) {
+                    Log.d(LOGTAG, "bindView called twice for same values; short-circuiting.");
+                    return;
+                }
+
+                // If we have no thumbnail, attempt to show a Favicon instead.
+                LoadIDAwareFaviconLoadedListener listener = new LoadIDAwareFaviconLoadedListener(view);
+                final int loadId = Favicons.getSizedFaviconForPageFromLocal(url, listener);
+
+                // Give each side enough information to shake hands later.
+                listener.setLoadId(loadId);
+                view.setLoadId(loadId);
             }
         }
 
@@ -588,6 +598,23 @@ public class TopSitesPage extends HomeFragment {
         }
     }
 
+    private class LoadIDAwareFaviconLoadedListener implements OnFaviconLoadedListener {
+        private volatile int loadId = Favicons.NOT_LOADING;
+        private final TopSitesGridItemView view;
+        public LoadIDAwareFaviconLoadedListener(TopSitesGridItemView view) {
+            this.view = view;
+        }
+
+        public void setLoadId(int id) {
+            this.loadId = id;
+        }
+
+        @Override
+        public void onFaviconLoaded(String url, String faviconURL, Bitmap favicon) {
+            this.view.displayFavicon(favicon, faviconURL, this.loadId);
+        }
+    }
+
     private class CursorLoaderCallbacks implements LoaderCallbacks<Cursor> {
         @Override
         public Loader<Cursor> onCreateLoader(int id, Bundle args) {