Bug 888326 - Part 4: LoadFaviconTask's use mNextFaviconLoadId is now thread safe. (Use AtomicInteger instead of long.) r=mleibovic
authorChris Kitching <ckitching@mozilla.com>
Thu, 12 Sep 2013 10:50:03 -0400
changeset 159705 84992b91dc391223a6ea354035df9c10e0fd3bbc
parent 159704 856365a2e58b3a36ef4d909923d4949449aab364
child 159706 1fe12c9597f45c54ae2d1bb0e7faa84e0e374a3c
push id2961
push userlsblakk@mozilla.com
push dateMon, 28 Oct 2013 21:59:28 +0000
treeherdermozilla-beta@73ef4f13486f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmleibovic
bugs888326
milestone26.0a1
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
Bug 888326 - Part 4: LoadFaviconTask's use mNextFaviconLoadId is now thread safe. (Use AtomicInteger instead of long.) r=mleibovic
mobile/android/base/BrowserApp.java
mobile/android/base/Tab.java
mobile/android/base/favicons/Favicons.java
mobile/android/base/favicons/LoadFaviconTask.java
--- a/mobile/android/base/BrowserApp.java
+++ b/mobile/android/base/BrowserApp.java
@@ -1330,17 +1330,17 @@ abstract public class BrowserApp extends
         Tabs.getInstance().loadUrl(ABOUT_HOME, Tabs.LOADURL_READING_LIST);
     }
 
     /* Favicon methods */
     private void loadFavicon(final Tab tab) {
         maybeCancelFaviconLoad(tab);
 
         int flags = LoadFaviconTask.FLAG_SCALE | ( (tab.isPrivate() || tab.getErrorType() != Tab.ErrorType.NONE) ? 0 : LoadFaviconTask.FLAG_PERSIST);
-        long id = Favicons.loadFavicon(tab.getURL(), tab.getFaviconURL(), flags,
+        int id = Favicons.loadFavicon(tab.getURL(), tab.getFaviconURL(), flags,
                         new OnFaviconLoadedListener() {
 
             @Override
             public void onFaviconLoaded(String pageUrl, Bitmap favicon) {
                 // Leave favicon UI untouched if we failed to load the image
                 // for some reason.
                 if (favicon == null)
                     return;
@@ -1356,17 +1356,17 @@ abstract public class BrowserApp extends
                 Tabs.getInstance().notifyListeners(tab, Tabs.TabEvents.FAVICON);
             }
         });
 
         tab.setFaviconLoadId(id);
     }
 
     private void maybeCancelFaviconLoad(Tab tab) {
-        long faviconLoadId = tab.getFaviconLoadId();
+        int faviconLoadId = tab.getFaviconLoadId();
 
         if (faviconLoadId == Favicons.NOT_LOADING)
             return;
 
         // Cancel pending favicon load task
         Favicons.cancelFaviconLoad(faviconLoadId);
 
         // Reset favicon load state
--- a/mobile/android/base/Tab.java
+++ b/mobile/android/base/Tab.java
@@ -49,17 +49,17 @@ public class Tab {
     private BitmapDrawable mThumbnail;
     private int mHistoryIndex;
     private int mHistorySize;
     private int mParentId;
     private HomePager.Page mAboutHomePage;
     private boolean mExternal;
     private boolean mBookmark;
     private boolean mReadingListItem;
-    private long mFaviconLoadId;
+    private int mFaviconLoadId;
     private String mContentType;
     private boolean mHasTouchListeners;
     private ZoomConstraints mZoomConstraints;
     private boolean mIsRTL;
     private ArrayList<View> mPluginViews;
     private HashMap<Object, Layer> mPluginLayers;
     private int mBackgroundColor;
     private int mState;
@@ -346,21 +346,21 @@ public class Tab {
     public void setHasTouchListeners(boolean aValue) {
         mHasTouchListeners = aValue;
     }
 
     public boolean getHasTouchListeners() {
         return mHasTouchListeners;
     }
 
-    public void setFaviconLoadId(long faviconLoadId) {
+    public void setFaviconLoadId(int faviconLoadId) {
         mFaviconLoadId = faviconLoadId;
     }
 
-    public long getFaviconLoadId() {
+    public int getFaviconLoadId() {
         return mFaviconLoadId;
     }
 
     public void updateFavicon(Bitmap favicon) {
         mFavicon = favicon;
     }
 
     public synchronized void updateFaviconURL(String faviconUrl, int size) {
--- a/mobile/android/base/favicons/Favicons.java
+++ b/mobile/android/base/favicons/Favicons.java
@@ -19,27 +19,27 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Set;
 
 public class Favicons {
     private static final String LOGTAG = "GeckoFavicons";
 
-    public static final long NOT_LOADING = 0;
-    public static final long FAILED_EXPIRY_NEVER = -1;
+    public static final int NOT_LOADING = 0;
+    public static final int FAILED_EXPIRY_NEVER = -1;
     public static final int FLAG_PERSIST = 1;
     public static final int FLAG_SCALE = 2;
 
     private static int sFaviconSmallSize = -1;
     private static int sFaviconLargeSize = -1;
 
     protected static Context sContext;
 
-    private static final Map<Long,LoadFaviconTask> sLoadTasks = Collections.synchronizedMap(new HashMap<Long, LoadFaviconTask>());
+    private static final Map<Integer, LoadFaviconTask> sLoadTasks = Collections.synchronizedMap(new HashMap<Integer, LoadFaviconTask>());
     private static final LruCache<String, Bitmap> sFaviconCache = new LruCache<String, Bitmap>(1024 * 1024) {
         @Override
         protected int sizeOf(String url, Bitmap image) {
             return image.getRowBytes() * image.getHeight();
         }
     };
 
     // A cache of the Favicon which have recently failed to download - prevents us from repeatedly
@@ -63,17 +63,17 @@ public class Favicons {
             }
         });
     }
 
     public static String getFaviconUrlForPageUrl(String pageUrl) {
         return BrowserDB.getFaviconUrlForHistoryUrl(sContext.getContentResolver(), pageUrl);
     }
 
-    public static long loadFavicon(String pageUrl, String faviconUrl, int flags,
+    public static int loadFavicon(String pageUrl, String faviconUrl, int flags,
             OnFaviconLoadedListener listener) {
 
         // Handle the case where page url is empty
         if (pageUrl == null || pageUrl.length() == 0) {
             dispatchResult(null, null, listener);
             return -1;
         }
 
@@ -87,17 +87,17 @@ public class Favicons {
         Bitmap image = getFaviconFromMemCache(pageUrl);
         if (image != null) {
             dispatchResult(pageUrl, image, listener);
             return -1;
         }
 
         LoadFaviconTask task = new LoadFaviconTask(ThreadUtils.getBackgroundHandler(), pageUrl, faviconUrl, flags, listener);
 
-        long taskId = task.getmId();
+        int taskId = task.getId();
         sLoadTasks.put(taskId, task);
 
         task.execute();
 
         return taskId;
     }
 
     public static Bitmap getFaviconFromMemCache(String pageUrl) {
@@ -129,17 +129,17 @@ public class Favicons {
     public static void putFaviconInFailedCache(String pageUrl, long fetchTime) {
         sFailedCache.put(pageUrl, fetchTime);
     }
 
     public static void clearFailedCache() {
         sFailedCache.evictAll();
     }
 
-    public static boolean cancelFaviconLoad(long taskId) {
+    public static boolean cancelFaviconLoad(int taskId) {
         Log.d(LOGTAG, "Requesting cancelation of favicon load (" + taskId + ")");
 
         boolean cancelled = false;
         synchronized (sLoadTasks) {
             if (!sLoadTasks.containsKey(taskId))
                 return false;
 
             Log.d(LOGTAG, "Cancelling favicon load (" + taskId + ")");
@@ -150,20 +150,20 @@ public class Favicons {
         return cancelled;
     }
 
     public static void close() {
         Log.d(LOGTAG, "Closing Favicons database");
 
         // Cancel any pending tasks
         synchronized (sLoadTasks) {
-            Set<Long> taskIds = sLoadTasks.keySet();
-            Iterator<Long> iter = taskIds.iterator();
+            Set<Integer> taskIds = sLoadTasks.keySet();
+            Iterator<Integer> iter = taskIds.iterator();
             while (iter.hasNext()) {
-                long taskId = iter.next();
+                int taskId = iter.next();
                 cancelFaviconLoad(taskId);
             }
         }
 
         LoadFaviconTask.closeHTTPClient();
     }
 
     public static boolean isLargeFavicon(Bitmap image) {
--- a/mobile/android/base/favicons/LoadFaviconTask.java
+++ b/mobile/android/base/favicons/LoadFaviconTask.java
@@ -21,46 +21,45 @@ import org.mozilla.gecko.util.GeckoJarRe
 import org.mozilla.gecko.util.UiAsyncTask;
 import static org.mozilla.gecko.favicons.Favicons.sContext;
 
 import java.io.InputStream;
 import java.net.MalformedURLException;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URL;
+import java.util.concurrent.atomic.AtomicInteger;
 
 /**
  * Class representing the asynchronous task to load a Favicon which is not currently in the in-memory
  * cache.
  * The implementation initially tries to get the Favicon from the database. Upon failure, the icon
  * is loaded from the internet.
  */
 public class LoadFaviconTask extends UiAsyncTask<Void, Void, Bitmap> {
     private static final String LOGTAG = "LoadFaviconTask";
 
     public static final int FLAG_PERSIST = 1;
     public static final int FLAG_SCALE = 2;
 
-    private long mNextFaviconLoadId;
-    private long mId;
+    private static AtomicInteger mNextFaviconLoadId = new AtomicInteger(0);
+    private int mId;
     private String mPageUrl;
     private String mFaviconUrl;
     private OnFaviconLoadedListener mListener;
     private int mFlags;
 
     static AndroidHttpClient sHttpClient = AndroidHttpClient.newInstance(GeckoAppShell.getGeckoInterface().getDefaultUAString());
 
     public LoadFaviconTask(Handler backgroundThreadHandler,
                            String aPageUrl, String aFaviconUrl, int aFlags,
                            OnFaviconLoadedListener aListener) {
         super(backgroundThreadHandler);
 
-        synchronized(this) {
-            mId = ++mNextFaviconLoadId;
-        }
+        mId = mNextFaviconLoadId.incrementAndGet();
 
         mPageUrl = aPageUrl;
         mFaviconUrl = aFaviconUrl;
         mListener = aListener;
         mFlags = aFlags;
     }
 
     // Runs in background thread
@@ -196,17 +195,17 @@ public class LoadFaviconTask extends UiA
     @Override
     protected void onCancelled() {
         Favicons.removeLoadTask(mId);
 
         // Note that we don't call the listener callback if the
         // favicon load is cancelled.
     }
 
-    long getId() {
+    int getId() {
         return mId;
     }
 
     static void closeHTTPClient() {
         if (sHttpClient != null) {
             sHttpClient.close();
         }
     }