Bug 963288 - Remove unnecessary synchronization in Tabs. r=rnewman
authorBrian Nicholson <bnicholson@mozilla.com>
Tue, 28 Jan 2014 11:56:26 -0800
changeset 165633 24979a717e7d9eed81675a4e57a3e5ae62458b33
parent 165632 ea49291871ef8e22ef8ba632787779c5df834a92
child 165634 733028be599d1fbe4402f4e79f925f9c923bc353
push id4625
push userbnicholson@mozilla.com
push dateTue, 28 Jan 2014 23:17:54 +0000
treeherderfx-team@24979a717e7d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrnewman
bugs963288
milestone29.0a1
Bug 963288 - Remove unnecessary synchronization in Tabs. r=rnewman
mobile/android/base/BrowserApp.java
mobile/android/base/Tabs.java
mobile/android/base/favicons/Favicons.java
mobile/android/base/home/TwoLinePageRow.java
--- a/mobile/android/base/BrowserApp.java
+++ b/mobile/android/base/BrowserApp.java
@@ -1357,25 +1357,25 @@ abstract public class BrowserApp extends
      * @return true if we successfully switched to a tab, false otherwise.
      */
     private boolean maybeSwitchToTab(String url, EnumSet<OnUrlOpenListener.Flags> flags) {
         if (!flags.contains(OnUrlOpenListener.Flags.ALLOW_SWITCH_TO_TAB)) {
             return false;
         }
 
         final Tabs tabs = Tabs.getInstance();
-        final int tabId = tabs.getTabIdForUrl(url, tabs.getSelectedTab().isPrivate());
-        if (tabId < 0) {
+        final Tab tab = tabs.getFirstTabForUrl(url, tabs.getSelectedTab().isPrivate());
+        if (tab == null) {
             return false;
         }
 
         // Set the target tab to null so it does not get selected (on editing
         // mode exit) in lieu of the tab we are about to select.
         mTargetTabForEditingMode = null;
-        Tabs.getInstance().selectTab(tabId);
+        tabs.selectTab(tab.getId());
 
         mBrowserToolbar.cancelEdit();
 
         return true;
     }
 
     private void openUrlAndStopEditing(String url) {
         openUrlAndStopEditing(url, null, false);
--- a/mobile/android/base/Tabs.java
+++ b/mobile/android/base/Tabs.java
@@ -158,17 +158,17 @@ public class Tabs implements GeckoEventL
         for (Tab tab : mOrder) {
             if (tab.isPrivate() == getPrivate) {
                 count++;
             }
         }
         return count;
     }
 
-    public synchronized int isOpen(String url) {
+    public int isOpen(String url) {
         for (Tab tab : mOrder) {
             if (tab.getURL().equals(url)) {
                 return tab.getId();
             }
         }
         return -1;
     }
 
@@ -640,42 +640,54 @@ public class Tabs implements GeckoEventL
 
     private void registerEventListener(String event) {
         GeckoAppShell.getEventDispatcher().registerEventListener(event, this);
     }
 
     /**
      * Looks for an open tab with the given URL.
      * @param url       the URL of the tab we're looking for
+     *
+     * @return first Tab with the given URL, or null if there is no such tab.
+     */
+    public Tab getFirstTabForUrl(String url) {
+        return getFirstTabForUrlHelper(url, null);
+    }
+
+    /**
+     * Looks for an open tab with the given URL and private state.
+     * @param url       the URL of the tab we're looking for
      * @param isPrivate if true, only look for tabs that are private. if false,
      *                  only look for tabs that are non-private.
      *
-     * @return id of an open tab with the given URL; -1 if the tab doesn't exist.
+     * @return first Tab with the given URL, or null if there is no such tab.
      */
-    public int getTabIdForUrl(String url, boolean isPrivate) {
+    public Tab getFirstTabForUrl(String url, boolean isPrivate) {
+        return getFirstTabForUrlHelper(url, isPrivate);
+    }
+
+    private Tab getFirstTabForUrlHelper(String url, Boolean isPrivate) {
+        if (url == null) {
+            return null;
+        }
+
         for (Tab tab : mOrder) {
+            if (isPrivate != null && isPrivate != tab.isPrivate()) {
+                continue;
+            }
             String tabUrl = tab.getURL();
             if (AboutPages.isAboutReader(tabUrl)) {
                 tabUrl = ReaderModeUtils.getUrlFromAboutReader(tabUrl);
             }
-            if (TextUtils.equals(tabUrl, url) && isPrivate == tab.isPrivate()) {
-                return tab.getId();
+            if (url.equals(tabUrl)) {
+                return tab;
             }
         }
 
-        return -1;
-    }
-
-    public int getTabIdForUrl(String url) {
-        return getTabIdForUrl(url, Tabs.getInstance().getSelectedTab().isPrivate());
-    }
-
-    public synchronized Tab getTabForUrl(String url) {
-        int tabId = getTabIdForUrl(url);
-        return getTab(tabId);
+        return null;
     }
 
     /**
      * Loads a tab with the given URL in the currently selected tab.
      *
      * @param url URL of page to load, or search term used if searchEngine is given
      */
     @RobocopTarget
--- a/mobile/android/base/favicons/Favicons.java
+++ b/mobile/android/base/favicons/Favicons.java
@@ -234,17 +234,17 @@ public class Favicons {
      * @param pageURL The URL of a webpage with a Favicon.
      * @return The URL of the Favicon used by that webpage, according to either the History database
      *         or a somewhat educated guess.
      */
     public static String getFaviconURLForPageURL(String pageURL) {
         // Attempt to determine the Favicon URL from the Tabs datastructure. Can dodge having to use
         // the database sometimes by doing this.
         String targetURL;
-        Tab theTab = Tabs.getInstance().getTabForUrl(pageURL);
+        Tab theTab = Tabs.getInstance().getFirstTabForUrl(pageURL);
         if (theTab != null) {
             targetURL = theTab.getFaviconURL();
             if (targetURL != null) {
                 return targetURL;
             }
         }
 
         targetURL = BrowserDB.getFaviconUrlForHistoryUrl(sContext.getContentResolver(), pageURL);
--- a/mobile/android/base/home/TwoLinePageRow.java
+++ b/mobile/android/base/home/TwoLinePageRow.java
@@ -115,18 +115,18 @@ public class TwoLinePageRow extends TwoL
 
     /**
      * Replaces the page URL with "Switch to tab" if there is already a tab open with that URL.
      * Only looks for tabs that are either private or non-private, depending on the current 
      * selected tab.
      */
     private void updateDisplayedUrl() {
         boolean isPrivate = Tabs.getInstance().getSelectedTab().isPrivate();
-        int tabId = Tabs.getInstance().getTabIdForUrl(mPageUrl, isPrivate);
-        if (!mShowIcons || tabId < 0) {
+        Tab tab = Tabs.getInstance().getFirstTabForUrl(mPageUrl, isPrivate);
+        if (!mShowIcons || tab == null) {
             setSecondaryText(mPageUrl);
             setSecondaryIcon(NO_ICON);
         } else {
             setSecondaryText(R.string.switch_to_tab);
             setSecondaryIcon(R.drawable.ic_url_bar_tab);
         }
     }