Bug 697987 - Remove race conditions from Runnables [r=sriram]
authorKartikaya Gupta <kgupta@mozilla.com>
Tue, 01 Nov 2011 13:14:37 -0400
changeset 81705 8724de90b59fdfb95d9d0ea7c850b24aa0c66023
parent 81704 367ee5ef74783fd36db66814a7711ed274693a2b
child 81706 9e9b953e65433f35dfbb8e808fc6f2be758df1c8
push id21573
push userblassey@mozilla.com
push dateTue, 06 Dec 2011 18:57:07 +0000
treeherdermozilla-central@0e397568c71e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssriram
bugs697987
milestone10.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 697987 - Remove race conditions from Runnables [r=sriram] Eliminate various race conditions when using runnables, specifically when doing things to global UI state based on the selected tab. Since the selected tab might change between a particular non-UI action and the corresponding UI-thread update, we have to ensure that the selected tab is still correct on the UI runnable.
embedding/android/GeckoApp.java
embedding/android/GeckoPreferences.java
--- a/embedding/android/GeckoApp.java
+++ b/embedding/android/GeckoApp.java
@@ -576,34 +576,34 @@ abstract public class GeckoApp
     void handleLocationChange(final int tabId, final String uri) {
         final Tab tab = Tabs.getInstance().getTab(tabId);
         if (tab == null)
             return;
         
         String oldBaseURI = tab.getURL();
         tab.updateURL(uri);
 
-        if (!Tabs.getInstance().isSelectedTab(tab))
-            return;
-
         String baseURI = uri;
         if (baseURI.indexOf('#') != -1)
             baseURI = uri.substring(0, uri.indexOf('#'));
 
         if (oldBaseURI != null && oldBaseURI.indexOf('#') != -1)
             oldBaseURI = oldBaseURI.substring(0, oldBaseURI.indexOf('#'));
         
         if (baseURI.equals(oldBaseURI))
             return;
 
+        tab.updateFavicon(null);
+
         mMainHandler.post(new Runnable() { 
             public void run() {
-                mBrowserToolbar.setTitle(uri);
-                mBrowserToolbar.setFavicon(null);
-                tab.updateFavicon(null);
+                if (Tabs.getInstance().isSelectedTab(tab)) {
+                    mBrowserToolbar.setTitle(uri);
+                    mBrowserToolbar.setFavicon(null);
+                }
             }
         });
     }
 
     void addTab() {
         Intent intent = new Intent(mAppContext, AwesomeBar.class);
         intent.addFlags(Intent.FLAG_ACTIVITY_NO_ANIMATION | Intent.FLAG_ACTIVITY_NO_HISTORY);
         intent.putExtra(AwesomeBar.TYPE_KEY, AwesomeBar.Type.ADD.name());
@@ -830,135 +830,111 @@ abstract public class GeckoApp
             public void run() {
                 onTabsChanged();
                 mBrowserToolbar.updateTabs(Tabs.getInstance().getCount());
                 mDoorHanger.removeForTab(tabId);
             }
         });
     }
     
-    void handleSelectTab(final int tabId) {
+    void handleSelectTab(int tabId) {
         final Tab tab = Tabs.getInstance().selectTab(tabId);
         if (tab == null)
             return;
 
         mMainHandler.post(new Runnable() { 
             public void run() {
-                mBrowserToolbar.setTitle(tab.getDisplayTitle());
-                mBrowserToolbar.setFavicon(tab.getFavicon());
-                mBrowserToolbar.setProgressVisibility(tab.isLoading());
-                mDoorHanger.updateForTab(tabId);
+                if (Tabs.getInstance().isSelectedTab(tab)) {
+                    mBrowserToolbar.setTitle(tab.getDisplayTitle());
+                    mBrowserToolbar.setFavicon(tab.getFavicon());
+                    mBrowserToolbar.setProgressVisibility(tab.isLoading());
+                    mDoorHanger.updateForTab(tab.getId());
+                }
             }
         });
     }
 
-    void handleDocumentStart(final int tabId) {
-        Tab tab = Tabs.getInstance().getTab(tabId);
-
+    void handleDocumentStart(int tabId) {
+        final Tab tab = Tabs.getInstance().getTab(tabId);
         if (tab == null)
             return;
 
         tab.setLoading(true);
         
         mMainHandler.post(new Runnable() {
             public void run() {
+                if (Tabs.getInstance().isSelectedTab(tab))
+                    mBrowserToolbar.setProgressVisibility(true);
                 onTabsChanged();
             }
         });
-
-        if (!Tabs.getInstance().isSelectedTab(tab))
-            return;
-
-        mMainHandler.post(new Runnable() { 
-            public void run() {
-                mBrowserToolbar.setProgressVisibility(true);
-            }
-        });
     }
 
-    void handleDocumentStop(final int tabId) {
-        Tab tab = Tabs.getInstance().getTab(tabId);
-
+    void handleDocumentStop(int tabId) {
+        final Tab tab = Tabs.getInstance().getTab(tabId);
         if (tab == null)
             return;
 
-         tab.setLoading(false);
-         
+        tab.setLoading(false);
+
         mMainHandler.post(new Runnable() {
             public void run() {
+                if (Tabs.getInstance().isSelectedTab(tab))
+                    mBrowserToolbar.setProgressVisibility(false);
+                surfaceView.hideStartupBitmap();
                 onTabsChanged();
             }
         });
-        
-        if (!Tabs.getInstance().isSelectedTab(tab))
-            return;
-
-        mMainHandler.post(new Runnable() { 
-            public void run() {
-                mBrowserToolbar.setProgressVisibility(false);
-                surfaceView.hideStartupBitmap();
-            }
-        });
     }
 
     void handleShowToast(final String message, final String duration) {
         mMainHandler.post(new Runnable() {
             public void run() {
                 Toast toast;
                 if (duration.equals("long"))
                     toast = Toast.makeText(mAppContext, message, Toast.LENGTH_LONG);
                 else
                     toast = Toast.makeText(mAppContext, message, Toast.LENGTH_SHORT);
                 toast.show();
             }
         });
     }
 
-    void handleContentLoaded(final int tabId, final String uri, final String title) {
+    void handleContentLoaded(int tabId, String uri, String title) {
+        final Tab tab = Tabs.getInstance().getTab(tabId);
+        if (tab == null)
+            return;
+
+        tab.updateTitle(title);
+        if (tab.getFavicon() == null)
+            downloadDefaultFavicon(tabId);
+
+        mMainHandler.post(new Runnable() {
+            public void run() {
+                if (Tabs.getInstance().isSelectedTab(tab))
+                    mBrowserToolbar.setTitle(tab.getDisplayTitle());
+                onTabsChanged();
+            }
+        });
+    }
+
+    void handleTitleChanged(int tabId, String title) {
         final Tab tab = Tabs.getInstance().getTab(tabId);
         if (tab == null)
             return;
 
         tab.updateTitle(title);
 
         mMainHandler.post(new Runnable() {
             public void run() {
+                if (Tabs.getInstance().isSelectedTab(tab))
+                    mBrowserToolbar.setTitle(tab.getDisplayTitle());
                 onTabsChanged();
             }
         });
-
-        if (tab.getFavicon() == null)
-            downloadDefaultFavicon(tabId);
-
-        if (!Tabs.getInstance().isSelectedTab(tab))
-            return;
-
-        mMainHandler.post(new Runnable() {
-            public void run() {
-                mBrowserToolbar.setTitle(tab.getDisplayTitle());
-            }
-        });
-    }
-
-    void handleTitleChanged(final int tabId, final String title) {
-        final Tab tab = Tabs.getInstance().getTab(tabId);
-        if (tab == null)
-            return;
-
-        tab.updateTitle(title);
-        
-        if (!Tabs.getInstance().isSelectedTab(tab))
-            return;
-
-        mMainHandler.post(new Runnable() { 
-            public void run() {
-                onTabsChanged();
-                mBrowserToolbar.setTitle(tab.getDisplayTitle());
-            }
-        });
     }
 
     void handleLinkAdded(final int tabId, String rel, final String href) {
         if (rel.indexOf("icon") != -1) {
             new DownloadFaviconTask(tabId).execute(href);
         }
     }
 
--- a/embedding/android/GeckoPreferences.java
+++ b/embedding/android/GeckoPreferences.java
@@ -105,25 +105,23 @@ public class GeckoPreferences
     }
 
     // Use values received from Gecko to update preferences UI
     public static void refresh(JSONArray jsonPrefs) {
         try {
             if (mPreferenceScreen == null)
                 return;
 
+            // set the current page URL for the "Home page" preference
             final String[] homepageValues = sContext.getResources().getStringArray(R.array.pref_homepage_values);
-
-            // set the current page URL for the "Home page" preference
-            Tab tab = Tabs.getInstance().getSelectedTab();
-            String currentUrl = tab.getURL();
             final Preference homepagePref = mPreferenceScreen.findPreference("browser.startup.homepage");
-            homepageValues[2] = currentUrl;
             GeckoAppShell.getMainHandler().post(new Runnable() {
                 public void run() {
+                    Tab tab = Tabs.getInstance().getSelectedTab();
+                    homepageValues[2] = tab.getURL();
                     ((ListPreference)homepagePref).setEntryValues(homepageValues);
                 }
             });
 
             final int length = jsonPrefs.length();
             for (int i = 0; i < length; i++) {
                 JSONObject jPref = jsonPrefs.getJSONObject(i);
                 final String prefName = jPref.getString("name");