Remove tab from parent before we remove it from the tree.

This resolves an issue in update_tab_titles() when deleting a tab with
collapsed children. Since we were doing all of the super() action before
removing the node from the tree the super() method was calling
on_current_changed() before we had done our bit of updating the tree.
update_tab_titles() was getting called from that which was calling into
get_tab_fields() in TreeTabWidget which was complaining that the tree and
widget didn't match up.

We don't have any automated tests covering this behaviour currently but moving
the super() call to the end works so far. Possibly it was that way due to some
earlier refactoring.

I've changed a local variable name since it was shadowing a method arg that is
now used further down.

TODO:
* are there other pieces of code where we should look whether we are doing
  tree stuff or widget stuff first and enshrine that as a pattern?
* the note about `_add_undo_entry()` points to unfortunate code that we should
  look into. It doesn't seem correct to be manipulating the tree in that
  method and the explanatory comment is too vague to explain anything

1 files changed, 13 insertions, 9 deletions

diff --git a/qutebrowser/mainwindow/treetabbedbrowser.py b/qutebrowser/mainwindow/treetabbedbrowser.py
index 0294eb0eb..a9f134bd2 100644
--- a/qutebrowser/mainwindow/treetabbedbrowser.py
+++ b/qutebrowser/mainwindow/treetabbedbrowser.py
@@ -86,9 +86,6 @@ class TreeTabbedBrowser(TabbedBrowser):
         # to save descendents
         descendents = tuple(node.traverse(render_collapsed=True))
 
-        super()._remove_tab(tab, add_undo=False, new_undo=False,
-                            crashed=crashed)
-
         if not tab.url().isEmpty() and tab.url().isValid() and add_undo:
             idx = self.widget.indexOf(tab)
             self._add_undo_entry(tab, idx, new_undo)
@@ -96,13 +93,16 @@ class TreeTabbedBrowser(TabbedBrowser):
         parent = node.parent
 
         if node.collapsed:
-            # node will already be removed from tree
-            # but we need to manually close the tab processes
+            # Collapsed nodes have already been removed from the TabWidget so
+            # we can't ask super() to disposed of them and need to do it
+            # ourselves.
+            # The node is removed from the tree in _add_undo_entry() currently
+            # (see fixme comment).
             for descendent in descendents:
-                tab = descendent.value
-                tab.private_api.shutdown()
-                tab.deleteLater()
-                tab.layout().unwrap()
+                descendent_tab = descendent.value
+                descendent_tab.private_api.shutdown()
+                descendent_tab.deleteLater()
+                descendent_tab.layout().unwrap()
         elif parent:
             siblings = list(parent.children)
             children = node.children
@@ -121,6 +121,10 @@ class TreeTabbedBrowser(TabbedBrowser):
                 node.children = ()
 
             node.parent = None
+
+        super()._remove_tab(tab, add_undo=False, new_undo=False,
+                            crashed=crashed)
+
         self.widget.tree_tab_update()
 
     def _add_undo_entry(self, tab, idx, new_undo):