Handle children no-long existing when undoing a close tab

If we are restoring a tab whos old child has since been removed then
`root.get_descendent_by_uid(child_uid)` can return None and we should
skip that child uid.

Also added some tests around undoing tree tab closes, one happy path and
a few odd cases. We could theoretically be a little cleverer about where
we put tabs back, if if the parent is gone but the children are still
there maybe we could insert it above the children instead of dragging
them with us up to the top of the tree? Oh well, these are edge cases,
they can be improved later.

Minor changes:
* remove redundant `list()` call
* add explanatory comment on why we are worrying about non-TreeTabUndo
  type undo entries. (Actually all of the entries in the list at that
  point we'll be ignoring so we could skip earlier.)

2 files changed, 64 insertions, 2 deletions

diff --git a/qutebrowser/mainwindow/treetabbedbrowser.py b/qutebrowser/mainwindow/treetabbedbrowser.py
index b4c3add89..4e729942a 100644
--- a/qutebrowser/mainwindow/treetabbedbrowser.py
+++ b/qutebrowser/mainwindow/treetabbedbrowser.py
@@ -154,11 +154,13 @@ class TreeTabbedBrowser(TabbedBrowser):
     def undo(self, depth=1):
         """Undo removing of a tab or tabs."""
         # save entries before super().undo() pops them
-        entries = list(self.undo_stack[-depth])
+        entries = self.undo_stack[-depth]
         new_tabs = super().undo(depth)
 
         for entry, tab in zip(reversed(entries), new_tabs):
             if not isinstance(entry, _TreeUndoEntry):
+                # Likely we are undoing the close of a non-tree tab window
+                # while a tree tab window is focused.
                 continue
             root = self.widget.tree_root
             uid = entry.uid
@@ -170,7 +172,8 @@ class TreeTabbedBrowser(TabbedBrowser):
             children = []
             for child_uid in entry.children_node_uids:
                 child_node = root.get_descendent_by_uid(child_uid)
-                children.append(child_node)
+                if child_node:
+                    children.append(child_node)
             tab.node.parent = None  # Remove the node from the tree
             tab.node = notree.Node(tab, parent_node,
                                    children, uid)
diff --git a/tests/end2end/features/treetabs.feature b/tests/end2end/features/treetabs.feature
index 27bc72fa4..5fd96f82b 100644
--- a/tests/end2end/features/treetabs.feature
+++ b/tests/end2end/features/treetabs.feature
@@ -91,3 +91,62 @@ Feature: Tree tab management
             - data/numbers/2.txt (active) (collapsed)
               - data/numbers/3.txt
           - data/numbers/4.txt
+
+    Scenario: Undo a tab close restores tree structure
+        # Restored node should be put back in the right place in the tree with
+        # same parent and child.
+        When I open about:blank?grandparent
+        And I open about:blank?parent in a new related tab
+        And I open about:blank?child in a new related tab
+        And I run :tab-select ?parent
+        And I run :tab-close
+        And I run :undo
+        Then the following tabs should be open:
+          - about:blank?grandparent
+            - about:blank?parent (active)
+              - about:blank?child
+
+    Scenario: Undo a tab close when the parent has already been closed
+        # Close the child first, then the parent. When the child is restored
+        # it should be placed back at the root.
+        When I open about:blank?grandparent
+        And I open about:blank?parent in a new related tab
+        And I open about:blank?child in a new related tab
+        And I run :tab-close
+        And I run :tab-close
+        And I run :undo 2
+        Then the following tabs should be open:
+          - about:blank?child (active)
+          - about:blank?grandparent
+
+    Scenario: Undo a tab close when the parent has already been closed - with children
+        # Close the child first, then the parent. When the child is restored
+        # it should be placed back at the root, and its previous child should
+        # be re-attached to it. (Not sure if this is the best behavior.)
+        When I open about:blank?grandparent
+        And I open about:blank?parent in a new related tab
+        And I open about:blank?child in a new related tab
+        And I open about:blank?leaf in a new related tab
+        And I run :tab-select ?child
+        And I run :tab-close
+        And I run :tab-select ?parent
+        And I run :tab-close
+        And I run :undo 2
+        Then the following tabs should be open:
+          - about:blank?child (active)
+            - about:blank?leaf
+          - about:blank?grandparent
+
+    Scenario: Undo a tab close when the child has already been closed
+        # Close the parent first, then the child. Make sure we don't crash
+        # when trying to re-parent the child.
+        When I open about:blank?grandparent
+        And I open about:blank?parent in a new related tab
+        And I open about:blank?child in a new related tab
+        And I run :tab-select ?parent
+        And I run :tab-close
+        And I run :tab-close
+        And I run :undo 2
+        Then the following tabs should be open:
+          - about:blank?grandparent
+            - about:blank?parent (active)