Bug 1346004 - Part 2 - Tabs helper functions for searching next/previous tab should take the Tab type into account. r=sebastian
authorJan Henning <jh+bugzilla@buttercookie.de>
Sat, 11 Mar 2017 20:29:17 +0100
changeset 399194 83e6ed90512040870c4227664097c28d2f2d72a1
parent 399193 d5baa00b8365d335682257c238b39f2864701011
child 399195 1fcdba303d8885447762933dca99e6d1745f6205
push id1490
push usermtabara@mozilla.com
push dateMon, 31 Jul 2017 14:08:16 +0000
treeherdermozilla-release@70e32e6bf15e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssebastian
bugs1346004
milestone55.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 1346004 - Part 2 - Tabs helper functions for searching next/previous tab should take the Tab type into account. r=sebastian MozReview-Commit-ID: DRBjBrndDWW
mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
mobile/android/base/java/org/mozilla/gecko/Tabs.java
mobile/android/base/java/org/mozilla/gecko/home/TwoLinePageRow.java
--- a/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
+++ b/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
@@ -2387,22 +2387,23 @@ public class BrowserApp extends GeckoApp
      * @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 Tab selectedTab = tabs.getSelectedTab();
         final Tab tab;
 
         if (AboutPages.isAboutReader(url)) {
-            tab = tabs.getFirstReaderTabForUrl(url, tabs.getSelectedTab().isPrivate());
+            tab = tabs.getFirstReaderTabForUrl(url, selectedTab.isPrivate(), selectedTab.getType());
         } else {
-            tab = tabs.getFirstTabForUrl(url, tabs.getSelectedTab().isPrivate());
+            tab = tabs.getFirstTabForUrl(url, selectedTab.isPrivate(), selectedTab.getType());
         }
 
         if (tab == null) {
             return false;
         }
 
         return maybeSwitchToTab(tab.getId());
     }
--- a/mobile/android/base/java/org/mozilla/gecko/Tabs.java
+++ b/mobile/android/base/java/org/mozilla/gecko/Tabs.java
@@ -178,32 +178,33 @@ public class Tabs implements BundleEvent
         if (mBookmarksContentObserver != null) {
             // It's safe to use the db here since we aren't doing any I/O.
             final GeckoProfile profile = GeckoProfile.get(context);
             BrowserDB.from(profile).registerBookmarkObserver(getContentResolver(), mBookmarksContentObserver);
         }
     }
 
     /**
-     * Gets the tab count corresponding to the private state of the selected
-     * tab.
+     * Gets the tab count corresponding to the category and private state of the
+     * selected tab.
      *
      * If the selected tab is a non-private tab, this will return the number of
      * non-private tabs; likewise, if this is a private tab, this will return
      * the number of private tabs.
      *
      * @return the number of tabs in the current private state
      */
     public synchronized int getDisplayCount() {
         // Once mSelectedTab is non-null, it cannot be null for the remainder
         // of the object's lifetime.
         boolean getPrivate = mSelectedTab != null && mSelectedTab.isPrivate();
+        TabType type = mSelectedTab != null ? mSelectedTab.getType() : TabType.BROWSING;
         int count = 0;
         for (Tab tab : mOrder) {
-            if (tab.isPrivate() == getPrivate) {
+            if (tab.isPrivate() == getPrivate && tab.getType() == type) {
                 count++;
             }
         }
         return count;
     }
 
     public int isOpen(String url) {
         for (Tab tab : mOrder) {
@@ -247,29 +248,30 @@ public class Tabs implements BundleEvent
             } else {
                 mOrder.add(tab);
             }
         }
 
         // Suppress the ADDED event to prevent animation of tabs created via session restore.
         if (mInitialTabsAdded) {
             notifyListeners(tab, TabEvents.ADDED,
-                    Integer.toString(getPrivacySpecificTabIndex(tabIndex, isPrivate)));
+                    Integer.toString(getPrivacySpecificTabIndex(tabIndex, isPrivate, type)));
         }
 
         return tab;
     }
 
-    // Return the index, among those tabs whose privacy setting matches isPrivate, of the tab at
-    // position index in mOrder.  Returns -1, for "new last tab", when index is -1.
-    private int getPrivacySpecificTabIndex(int index, boolean isPrivate) {
+    // Return the index, among those tabs of the chosen type whose privacy setting matches
+    // isPrivate, of the tab at position index in mOrder.  Returns -1, for "new last tab",
+    // when index is -1.
+    private int getPrivacySpecificTabIndex(int index, boolean isPrivate, TabType type) {
         int privacySpecificIndex = -1;
         for (int i = 0; i <= index; i++) {
             final Tab tab = mOrder.get(i);
-            if (tab.isPrivate() == isPrivate) {
+            if (tab.isPrivate() == isPrivate && tab.getType() == type) {
                 privacySpecificIndex++;
             }
         }
         return privacySpecificIndex;
     }
 
     public synchronized void removeTab(int id) {
         if (mTabs.containsKey(id)) {
@@ -319,33 +321,33 @@ public class Tabs implements BundleEvent
         selectTab(mOrder.get(mOrder.size() - 1).getId());
         return true;
     }
 
     private int getIndexOf(Tab tab) {
         return mOrder.lastIndexOf(tab);
     }
 
-    private Tab getNextTabFrom(Tab tab, boolean getPrivate) {
+    private Tab getNextTabFrom(Tab tab, boolean getPrivate, TabType type) {
         int numTabs = mOrder.size();
         int index = getIndexOf(tab);
         for (int i = index + 1; i < numTabs; i++) {
             Tab next = mOrder.get(i);
-            if (next.isPrivate() == getPrivate) {
+            if (next.isPrivate() == getPrivate && next.getType() == type) {
                 return next;
             }
         }
         return null;
     }
 
-    private Tab getPreviousTabFrom(Tab tab, boolean getPrivate) {
+    private Tab getPreviousTabFrom(Tab tab, boolean getPrivate, TabType type) {
         int index = getIndexOf(tab);
         for (int i = index - 1; i >= 0; i--) {
             Tab prev = mOrder.get(i);
-            if (prev.isPrivate() == getPrivate) {
+            if (prev.isPrivate() == getPrivate && prev.getType() == type) {
                 return prev;
             }
         }
         return null;
     }
 
     /**
      * Gets the selected tab.
@@ -436,26 +438,27 @@ public class Tabs implements BundleEvent
 
     /** Return the tab that will be selected by default after this one is closed */
     public Tab getNextTab(Tab tab) {
         Tab selectedTab = getSelectedTab();
         if (selectedTab != tab)
             return selectedTab;
 
         boolean getPrivate = tab.isPrivate();
-        Tab nextTab = getNextTabFrom(tab, getPrivate);
+        TabType type = tab.getType();
+        Tab nextTab = getNextTabFrom(tab, getPrivate, type);
         if (nextTab == null)
-            nextTab = getPreviousTabFrom(tab, getPrivate);
+            nextTab = getPreviousTabFrom(tab, getPrivate, type);
         if (nextTab == null && getPrivate) {
             // If there are no private tabs remaining, get the last normal tab
             Tab lastTab = mOrder.get(mOrder.size() - 1);
             if (!lastTab.isPrivate()) {
                 nextTab = lastTab;
             } else {
-                nextTab = getPreviousTabFrom(lastTab, false);
+                nextTab = getPreviousTabFrom(lastTab, false, type);
             }
         }
 
         Tab parent = getTab(tab.getParentId());
         if (parent != null) {
             // If the next tab is a sibling, switch to it. Otherwise go back to the parent.
             if (nextTab != null && nextTab.getParentId() == tab.getParentId())
                 return nextTab;
@@ -796,38 +799,39 @@ public class Tabs implements BundleEvent
 
     /**
      * 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);
+        return getFirstTabForUrlHelper(url, null, TabType.BROWSING);
     }
 
     /**
      * 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.
+     * @param type      the type 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, boolean isPrivate) {
-        return getFirstTabForUrlHelper(url, isPrivate);
+    public Tab getFirstTabForUrl(String url, boolean isPrivate, TabType type) {
+        return getFirstTabForUrlHelper(url, isPrivate, type);
     }
 
-    private Tab getFirstTabForUrlHelper(String url, Boolean isPrivate) {
+    private Tab getFirstTabForUrlHelper(String url, Boolean isPrivate, TabType type) {
         if (url == null) {
             return null;
         }
 
         for (Tab tab : mOrder) {
-            if (isPrivate != null && isPrivate != tab.isPrivate()) {
+            if (isPrivate != null && isPrivate != tab.isPrivate() || type != tab.getType()) {
                 continue;
             }
             if (url.equals(tab.getURL())) {
                 return tab;
             }
         }
 
         return null;
@@ -838,29 +842,31 @@ public class Tabs implements BundleEvent
      * state.
      *
      * @param url
      *            The URL of the tab we're looking for. The url parameter can be
      *            the actual article URL or the reader mode article URL.
      * @param isPrivate
      *            If true, only look for tabs that are private. If false, only
      *            look for tabs that are not private.
+     * @param type
+     *            The type of the tab we're looking for.
      *
      * @return The first Tab with the given URL, or null if there is no such
      *         tab.
      */
-    public Tab getFirstReaderTabForUrl(String url, boolean isPrivate) {
+    public Tab getFirstReaderTabForUrl(String url, boolean isPrivate, TabType type) {
         if (url == null) {
             return null;
         }
 
         url = ReaderModeUtils.stripAboutReaderUrl(url);
 
         for (Tab tab : mOrder) {
-            if (isPrivate != tab.isPrivate()) {
+            if (isPrivate != tab.isPrivate() || type != tab.getType()) {
                 continue;
             }
             String tabUrl = tab.getURL();
             if (AboutPages.isAboutReader(tabUrl)) {
                 tabUrl = ReaderModeUtils.stripAboutReaderUrl(tabUrl);
                 if (url.equals(tabUrl)) {
                     return tab;
                 }
--- a/mobile/android/base/java/org/mozilla/gecko/home/TwoLinePageRow.java
+++ b/mobile/android/base/java/org/mozilla/gecko/home/TwoLinePageRow.java
@@ -200,22 +200,23 @@ public class TwoLinePageRow extends Line
     /**
      * 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.
      */
     protected void updateDisplayedUrl() {
         final Tab selectedTab = Tabs.getInstance().getSelectedTab();
         final boolean isPrivate = (selectedTab != null) && (selectedTab.isPrivate());
+        final Tab.TabType type = selectedTab != null ? selectedTab.getType() : Tab.TabType.BROWSING;
 
         // We always want to display the underlying page url, however for readermode pages
         // we navigate to the about:reader equivalent, hence we need to use that url when finding
         // existing tabs
         final String navigationUrl = mHasReaderCacheItem ? ReaderModeUtils.getAboutReaderForUrl(mPageUrl) : mPageUrl;
-        Tab tab = Tabs.getInstance().getFirstTabForUrl(navigationUrl, isPrivate);
+        Tab tab = Tabs.getInstance().getFirstTabForUrl(navigationUrl, isPrivate, type);
 
 
         if (!mShowIcons || tab == null) {
             setUrl(mPageUrl);
             setSwitchToTabIcon(NO_ICON);
         } else {
             setUrl(R.string.switch_to_tab);
             setSwitchToTabIcon(R.drawable.ic_url_bar_tab);