Add new TabbedBrowser.tabs() method.

We have one (1) use case where a caller wants to get all tabs from a
tabbed browser, without caring whether they are mounted in the tab bar
or not. I was wondering what an API would look like that let callers ask
that without having to worry about whether the tabbed browser had tree
tabs enabled or not, while still maintaining the more common use case of
only getting widgets that are visible, or focusable, to the user.

Right now the best option I can come up with is this
`tabs(include_hidden=True)` method. This provides a clear API that makes
it obvious how to achieve both use cases (apart from the overloaded
meaning of "hidden" regarding widget visibility). I think referring
to "tabs" is better than "widgets" too, it requires you to hold less
information in your head about what these widgets in particular are.

Am I going to put any effort into attempting to deprecate `.widgets()`?
Nah.

Despite what I think is a fine API I do have some concerns:

*extensibility* One of the goals of this attempt at tree tabs and
productionising it is to see what extensions to core logic would look
like without having to change core classes. I'm not sure if this is a
good example or not because the core class just has the `.widgets()` which
is very much tied to the QTabBar. It doesn't have the concept of tabs
that exist and are children of a TabbedBrowser but aren't in the
QTabBar. So it's a fundamentally new piece of information to express and
we can't just unilaterally return tabs that aren't in the tab bar
because I can see it'll break some things.

So perhaps it would be more fitting with the "extensibility case study"
refactor goal to require the caller to have more knowledge. But I still
don't want callers iterating trees if they don't have to be, so maybe
something like:

    class TabbedBrowser:

        def widgets():
            ...

    class TreeTabbedBrowser:

        def widgets(include_hidden=False):
            ...

    tabbed_browser = ...
    if isinstance(tabbed_browser, TreeTabbedBrowser):
      tabs = tabbed_browser.widgets(include_hidden=True)
    else:
      tabs = tabbed_browser.widgets()

Since we are going to be merging this into the core codebase I don't
want to be inflicting that on users of tabs in the extensions API (when
we add them), but perhaps it's a pattern we can recommend for actual
extensions, when they show up.

*ownership and hidden tabs* For tabs associated with a window but not
visible in the tab bar, who should they be owned by? Currently
TabbedBrowser doesn't attempt to keep track of tabs at all, that's all
handled by the TabWidget. But now we have tabs that the TabWidget
doesn't know about. They are only referenced by the tree structure under
TabbedBrowser. Should the browser be managing tabs and just telling the
tab bar what to show? (Which is essentially what is happening now but
only for the tree case.) Or should we be pushing the tab hiding
functionality into the TabWidget?

Having tabs not in the tab bar can cause issues, mainly because it
causes violates some assumptions in the code that all tabs have an index
in the tab bar. But putting the tabs in the tab bar that you are not
showing can cause usability issues with, for example, navigating to a
tab via count.

When we implement our own tab bar we are probably going to re-do the
displaying of the tree structure to be a bit more "native". When we do
that should we move the tab hiding into the tab bar too? I guess the API
would end up looking like it does with the tree case today, hidden tabs
wouldn't get indices and you would want ways to both iterate through
visible tabs and all tabs (and maybe hidden tabs too, but we don't have
that currently without traversing the tree yourself). I imagine in that
case the tree structure would be part of the tab bar and in the "flat"
case it would just always put stuff at the top level, and complain if
you tried to demote.

That rambling was mostly driven by so many callers going
`tabbed_browser.widget.currentTab()` etc. Which I don't think is a great
API. Beyond an attribute called "widget" being not very meaningful have
tab accessing and manipulating commands spread between and parent and a
child seems like it could be confusing. So I settled on the future logic
being in the tab widget (or in a tree in the tab widget), does that
change anything or dictate what they should be owned by? No. Oh well. I
kinda want to say tabs should be owned by the tabbed browser but I don't
know what practical implications that would have. The interface to
callers would remain predominantly the tabbed browser and the logic
would probably continue to be mixed between that and the tab widget.

3 files changed, 35 insertions, 11 deletions

diff --git a/qutebrowser/mainwindow/tabbedbrowser.py b/qutebrowser/mainwindow/tabbedbrowser.py
index 741123ed9..cbc1bac78 100644
--- a/qutebrowser/mainwindow/tabbedbrowser.py
+++ b/qutebrowser/mainwindow/tabbedbrowser.py
@@ -281,9 +281,11 @@ class TabbedBrowser(QWidget):
             raise TabDeletedError("index is -1!")
         return idx
 
-    def widgets(self):
+    def widgets(self) -> List[browsertab.AbstractTab]:
         """Get a list of open tab widgets.
 
+        Consider using `tabs()` instead of this method.
+
         We don't implement this as generator so we can delete tabs while
         iterating over the list.
         """
@@ -296,6 +298,18 @@ class TabbedBrowser(QWidget):
                 widgets.append(widget)
         return widgets
 
+    def tabs(
+        self,
+        include_hidden: bool = False,  # pylint: disable=unused-argument
+    ) -> List[browsertab.AbstractTab]:
+        """Get a list of tabs in this browser.
+
+        Args:
+            include_hidden: Include child tabs which are not currently in the
+                            tab bar.
+        """
+        return self.widgets()
+
     def _update_window_title(self, field=None):
         """Change the window title to match the current tab.
 
diff --git a/qutebrowser/mainwindow/treetabbedbrowser.py b/qutebrowser/mainwindow/treetabbedbrowser.py
index 75a3c55f8..2ac18c337 100644
--- a/qutebrowser/mainwindow/treetabbedbrowser.py
+++ b/qutebrowser/mainwindow/treetabbedbrowser.py
@@ -195,6 +195,25 @@ class TreeTabbedBrowser(TabbedBrowser):
 
         self.widget.tree_tab_update()
 
+    def tabs(
+        self,
+        include_hidden: bool = False,
+    ) -> List[browsertab.AbstractTab]:
+        """Get a list of tabs in this browser.
+
+        Args:
+            include_hidden: Include child tabs which are not currently in the
+                            tab bar.
+        """
+        return [
+            node.value
+            for node
+            in self.widget.tree_root.traverse(
+                render_collapsed=include_hidden,
+            )
+            if node.value
+        ]
+
     @pyqtSlot('QUrl')
     @pyqtSlot('QUrl', bool)
     @pyqtSlot('QUrl', bool, bool)
diff --git a/qutebrowser/misc/sessions.py b/qutebrowser/misc/sessions.py
index 9f7254958..c9eb70b6b 100644
--- a/qutebrowser/misc/sessions.py
+++ b/qutebrowser/misc/sessions.py
@@ -321,16 +321,7 @@ class SessionManager(QObject):
                 win_data['private'] = True
 
             win_data['tabs'] = []
-            if tabbed_browser.is_treetabbedbrowser:
-                tabs = [
-                    node.value
-                    for node
-                    in tabbed_browser.widget.tree_root.traverse()
-                    if node.value
-                ]
-            else:
-                tabs = tabbed_browser.widgets()
-            for tab in tabs:
+            for tab in tabbed_browser.tabs(include_hidden=True):
                 active = tab == tabbed_browser.current_tab()
                 tab_data = self._save_tab(tab,
                                           active,