Bug 1346004 - Part 2 - Tabs helper functions for searching next/previous tab should take the Tab type into account. r?sebastian draft
authorJan Henning <jh+bugzilla@buttercookie.de>
Sat, 11 Mar 2017 20:29:17 +0100
changeset 502490 355a2c876b8affd1b09ac97a40905a325b04096b
parent 502489 c2231bc42ec48cf4645d1cf2a8fb50ef4f1b810d
child 502491 2ab97b16955c8f43b5d41582c25b22a81fc1112f
push id50298
push usermozilla@buttercookie.de
push dateTue, 21 Mar 2017 21:08:11 +0000
reviewerssebastian
bugs1346004
milestone55.0a1
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
@@ -2390,22 +2390,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);