On restore, Delete bookmarks before closing the active tab [#377]

When restoring and removing tabs from the stash, during the "restore"
phase, we close the active tab if it's a "blank" tab (i.e. the new-tab
page, the homepage,`about:blank`, and so on).  Only after closing the
blank tab do we then delete the stashed tabs from the bookmark store.

The problem is that Tab Stash itself might be the "blank" tab (e.g. if
Tab Stash is set as the user's homepage).  If Tab Stash closes itself,
it cannot finish cleaning up the bookmarks for the restored tabs.

This patch deletes the bookmarks (if needed) before attempting to close
the active "blank" tab, so the bookmarks will always get cleaned up
regardless of whether Tab Stash is considered to be a "blank" tab.

src/model/index.ts

@@ -483,10 +483,26 @@ export class Model {
    *
    * Note that if a tab is already open and not hidden, we will do nothing,
    * since we don't want to open duplicate tabs.  Such tabs will not be
-   * included in the returned list. */
+   * included in the returned list.
+   *
+   * After restoring tabs, if the previously-active tab was a blank tab, it will
+   * be closed.  Note that this tab may be the Tab Stash tab itself (e.g. if Tab
+   * Stash is the homepage or the new-tab page).  In that situation, this
+   * function may not return (since the tab running it will be closed). */
   async restoreTabs(
     items: StashLeaf[],
-    options: {background?: boolean},
+    options: {
+      /** Should tabs be opened in the background? */
+      background?: boolean;
+
+      /** Run this function after restoring tabs, but before closing the active
+       * new tab (if any).  This hook exists in case Tab Stash is itself the
+       * active new tab--in which case, this function never returns.
+       *
+       * Note that this function always runs exactly once (even if there is no
+       * tab to close.) */
+      beforeClosing?: (tabs: Tabs.Tab[]) => Promise<void>;
+    },
   ): Promise<Tabs.Tab[]> {
     const toWindow = this.tabs.targetWindow.value;
     if (toWindow === undefined) {
@@ -519,6 +535,8 @@ export class Model {
       toWindowId: toWindow.id,
     });
 
+    if (options.beforeClosing) await options.beforeClosing(tabs);
+
     if (!options.background) {
       // Switch to the last tab that we restored (if desired).  We choose
       // the LAST tab to behave similarly to the user having just opened a

src/stash-list/bookmark.vue

@@ -243,8 +243,8 @@ export default defineComponent({
         const bg = bgKeyPressed(ev);
         await the.model.restoreTabs([this.bookmark], {
           background: bg,
+          beforeClosing: () => the.model.deleteBookmark(this.bookmark),
         });
-        await the.model.deleteBookmark(this.bookmark);
       });
     },
 

src/stash-list/folder.vue

@@ -620,13 +620,13 @@ export default defineComponent({
       this.attempt(async () => {
         const bg = bgKeyPressed(ev);
 
-        await the.model.restoreTabs(this.leafChildren, {background: bg});
-
-        if (this.leafChildren.length === this.folder.children.length) {
-          await the.model.deleteBookmarkTree(this.folder.id);
-        } else {
-          await the.model.deleteItems(this.leafChildren);
-        }
+        await the.model.restoreTabs(this.leafChildren, {
+          background: bg,
+          beforeClosing: () =>
+            this.leafChildren.length === this.folder.children.length
+              ? the.model.deleteBookmarkTree(this.folder.id)
+              : the.model.deleteItems(this.leafChildren),
+        });
       });
     },