Separate between Close Tab Requested and Tab Closed flows

src/cascadia/TerminalApp/AppActionHandlers.cpp

@@ -498,7 +498,7 @@ namespace winrt::TerminalApp::implementation
                 return;
             }
 
-            // Since _RemoveTab is asynchronous, create a snapshot of the  tabs we want to remove
+            // Since _RemoveTabs is asynchronous, create a snapshot of the  tabs we want to remove
             std::vector<winrt::TerminalApp::TabBase> tabsToRemove;
             if (index > 0)
             {
@@ -537,7 +537,7 @@ namespace winrt::TerminalApp::implementation
                 return;
             }
 
-            // Since _RemoveTab is asynchronous, create a snapshot of the  tabs we want to remove
+            // Since _RemoveTabs is asynchronous, create a snapshot of the  tabs we want to remove
             std::vector<winrt::TerminalApp::TabBase> tabsToRemove;
             std::copy(begin(_tabs) + index + 1, end(_tabs), std::back_inserter(tabsToRemove));
             _RemoveTabs(tabsToRemove);

src/cascadia/TerminalApp/Pane.cpp

@@ -416,11 +416,8 @@ void Pane::_ControlLostFocusHandler(winrt::Windows::Foundation::IInspectable con
 // - <none>
 void Pane::Close()
 {
-    if (!_isClosing.exchange(true))
-    {
-        // Fire our Closed event to tell our parent that we should be removed.
-        _ClosedHandlers(nullptr, nullptr);
-    }
+    // Fire our Closed event to tell our parent that we should be removed.
+    _ClosedHandlers(nullptr, nullptr);
 }
 
 // Method Description:

src/cascadia/TerminalApp/Pane.h

@@ -120,8 +120,6 @@ class Pane : public std::enable_shared_from_this<Pane>
 
     Borders _borders{ Borders::None };
 
-    std::atomic<bool> _isClosing{ false };
-
     bool _zoomed{ false };
 
     bool _IsLeaf() const noexcept;

src/cascadia/TerminalApp/TabBase.cpp

@@ -54,7 +54,7 @@ namespace winrt::TerminalApp::implementation
         closeTabMenuItem.Click([weakThis](auto&&, auto&&) {
             if (auto tab{ weakThis.get() })
             {
-                tab->_ClosedHandlers(nullptr, nullptr);
+                tab->_CloseRequestedHandlers(nullptr, nullptr);
             }
         });
         closeTabMenuItem.Text(RS_(L"TabClose"));

src/cascadia/TerminalApp/TabBase.h

@@ -26,6 +26,7 @@ namespace winrt::TerminalApp::implementation
         void SetKeyMap(const Microsoft::Terminal::Settings::Model::KeyMapping& keymap);
 
         WINRT_CALLBACK(Closed, winrt::Windows::Foundation::EventHandler<winrt::Windows::Foundation::IInspectable>);
+        WINRT_CALLBACK(CloseRequested, winrt::Windows::Foundation::EventHandler<winrt::Windows::Foundation::IInspectable>);
         WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler);
 
         // The TabViewIndex is the index this Tab object resides in TerminalPage's _tabs vector.

src/cascadia/TerminalApp/TabManagement.cpp

@@ -318,26 +318,10 @@ namespace winrt::TerminalApp::implementation
     }
 
     // Method Description:
-    // - Look for the index of the input tabView in the tabs vector,
-    //   and call _RemoveTab
-    // Arguments:
-    // - tabViewItem: the TabViewItem in the TabView that is being removed.
-    void TerminalPage::_RemoveTabViewItem(const MUX::Controls::TabViewItem& tabViewItem)
-    {
-        uint32_t tabIndexFromControl = 0;
-        if (_tabView.TabItems().IndexOf(tabViewItem, tabIndexFromControl))
-        {
-            // If IndexOf returns true, we've actually got an index
-            auto tab{ _tabs.GetAt(tabIndexFromControl) };
-            _RemoveTab(tab);
-        }
-    }
-
-    // Method Description:
-    // - Removes the tab (both TerminalControl and XAML)
+    // - Removes the tab (both TerminalControl and XAML) after prompting for approval
     // Arguments:
     // - tab: the tab to remove
-    winrt::Windows::Foundation::IAsyncAction TerminalPage::_RemoveTab(winrt::TerminalApp::TabBase tab)
+    winrt::Windows::Foundation::IAsyncAction TerminalPage::_HandleCloseTabRequested(winrt::TerminalApp::TabBase tab)
     {
         if (tab.ReadOnly())
         {
@@ -349,12 +333,20 @@ namespace winrt::TerminalApp::implementation
                 co_return;
             }
         }
+        _RemoveTab(tab);
+    }
 
+    // Method Description:
+    // - Removes the tab (both TerminalControl and XAML)
+    // Arguments:
+    // - tab: the tab to remove
+    void TerminalPage::_RemoveTab(const winrt::TerminalApp::TabBase& tab)
+    {
         uint32_t tabIndex{};
         if (!_tabs.IndexOf(tab, tabIndex))
         {
             // The tab is already removed
-            co_return;
+            return;
         }
 
         // We use _removing flag to suppress _OnTabSelectionChanged events
@@ -438,8 +430,6 @@ namespace winrt::TerminalApp::implementation
             _rearrangeFrom = std::nullopt;
             _rearrangeTo = std::nullopt;
         }
-
-        co_return;
     }
 
     // Method Description:
@@ -534,7 +524,7 @@ namespace winrt::TerminalApp::implementation
     }
 
     // Method Description:
-    // - returns a com_ptr to the currently focused tab. This might return null,
+    // - returns the currently focused tab. This might return null,
     //   so make sure to check the result!
     winrt::TerminalApp::TabBase TerminalPage::_GetFocusedTab() const noexcept
     {
@@ -557,6 +547,20 @@ namespace winrt::TerminalApp::implementation
         return nullptr;
     }
 
+    // Method Description:
+    // - returns a tab corresponding to a view item. This might return null,
+    //   so make sure to check the result!
+    winrt::TerminalApp::TabBase TerminalPage::_GetTabByTabViewItem(const Microsoft::UI::Xaml::Controls::TabViewItem& tabViewItem) const noexcept
+    {
+        uint32_t tabIndexFromControl{};
+        if (_tabView.TabItems().IndexOf(tabViewItem, tabIndexFromControl))
+        {
+            // If IndexOf returns true, we've actually got an index
+            return _tabs.GetAt(tabIndexFromControl);
+        }
+        return nullptr;
+    }
+
     // Method Description:
     // - An async method for changing the focused tab on the UI thread. This
     //   method will _only_ set the selected item of the TabView, which will
@@ -589,7 +593,7 @@ namespace winrt::TerminalApp::implementation
         if (auto index{ _GetFocusedTabIndex() })
         {
             auto tab{ _tabs.GetAt(*index) };
-            _RemoveTab(tab);
+            _HandleCloseTabRequested(tab);
         }
     }
 
@@ -635,7 +639,7 @@ namespace winrt::TerminalApp::implementation
             const auto tab{ _tabs.GetAt(*index) };
             if (tab.try_as<TerminalApp::SettingsTab>())
             {
-                _RemoveTab(tab);
+                _HandleCloseTabRequested(tab);
             }
         }
     }
@@ -648,7 +652,7 @@ namespace winrt::TerminalApp::implementation
     {
         for (auto& tab : tabs)
         {
-            co_await _RemoveTab(tab);
+            co_await _HandleCloseTabRequested(tab);
         }
     }
     // Method Description:
@@ -689,7 +693,11 @@ namespace winrt::TerminalApp::implementation
     {
         if (eventArgs.GetCurrentPoint(*this).Properties().IsMiddleButtonPressed())
         {
-            _RemoveTabViewItem(sender.as<MUX::Controls::TabViewItem>());
+            const auto tabViewItem = sender.try_as<MUX::Controls::TabViewItem>();
+            if (auto tab{ _GetTabByTabViewItem(tabViewItem) })
+            {
+                _HandleCloseTabRequested(tab);
+            }
             eventArgs.Handled(true);
         }
         else if (eventArgs.GetCurrentPoint(*this).Properties().IsRightButtonPressed())
@@ -882,7 +890,7 @@ namespace winrt::TerminalApp::implementation
 
     void TerminalPage::_RemoveAllTabs()
     {
-        // Since _RemoveTab is asynchronous, create a snapshot of the  tabs we want to remove
+        // Since _RemoveTabs is asynchronous, create a snapshot of the  tabs we want to remove
         std::vector<winrt::TerminalApp::TabBase> tabsToRemove;
         std::copy(begin(_tabs), end(_tabs), std::back_inserter(tabsToRemove));
         _RemoveTabs(tabsToRemove);

src/cascadia/TerminalApp/TerminalPage.cpp

@@ -716,7 +716,10 @@ namespace winrt::TerminalApp::implementation
     {
         co_await winrt::resume_foreground(page->_tabView.Dispatcher());
 
-        page->_RemoveTabViewItem(tabViewItem);
+        if (auto tab{ _GetTabByTabViewItem(tabViewItem) })
+        {
+            _RemoveTab(tab);
+        }
     }
 
     // Method Description:
@@ -1799,7 +1802,10 @@ namespace winrt::TerminalApp::implementation
     void TerminalPage::_OnTabCloseRequested(const IInspectable& /*sender*/, const MUX::Controls::TabViewTabCloseRequestedEventArgs& eventArgs)
     {
         const auto tabViewItem = eventArgs.Tab();
-        _RemoveTabViewItem(tabViewItem);
+        if (auto tab{ _GetTabByTabViewItem(tabViewItem) })
+        {
+            _HandleCloseTabRequested(tab);
+        }
     }
 
     TermControl TerminalPage::_InitControl(const TerminalSettings& settings, const ITerminalConnection& connection)
@@ -2353,6 +2359,17 @@ namespace winrt::TerminalApp::implementation
 
             tabViewItem.PointerPressed({ this, &TerminalPage::_OnTabClick });
 
+            // When the tab requests close, try to close it (prompt for approval, if required)
+            newTabImpl->CloseRequested([weakTab, weakThis{ get_weak() }](auto&& /*s*/, auto&& /*e*/) {
+                auto page{ weakThis.get() };
+                auto tab{ weakTab.get() };
+
+                if (page && tab)
+                {
+                    page->_HandleCloseTabRequested(*tab);
+                }
+            });
+
             // When the tab is closed, remove it from our list of tabs.
             newTabImpl->Closed([tabViewItem, weakThis{ get_weak() }](auto&& /*s*/, auto&& /*e*/) {
                 if (auto page{ weakThis.get() })

src/cascadia/TerminalApp/TerminalPage.h

@@ -178,8 +178,9 @@ namespace winrt::TerminalApp::implementation
 
         void _DuplicateFocusedTab();
         void _DuplicateTab(const TerminalTab& tab);
-        void _RemoveTabViewItem(const Microsoft::UI::Xaml::Controls::TabViewItem& tabViewItem);
-        winrt::Windows::Foundation::IAsyncAction _RemoveTab(winrt::TerminalApp::TabBase tab);
+
+        winrt::Windows::Foundation::IAsyncAction _HandleCloseTabRequested(winrt::TerminalApp::TabBase tab);
+        void _RemoveTab(const winrt::TerminalApp::TabBase& tab);
         winrt::fire_and_forget _RemoveTabs(const std::vector<winrt::TerminalApp::TabBase> tabs);
 
         void _RegisterTerminalEvents(Microsoft::Terminal::Control::TermControl term, TerminalTab& hostingTab);
@@ -198,6 +199,7 @@ namespace winrt::TerminalApp::implementation
         std::optional<uint32_t> _GetFocusedTabIndex() const noexcept;
         TerminalApp::TabBase _GetFocusedTab() const noexcept;
         winrt::com_ptr<TerminalTab> _GetFocusedTabImpl() const noexcept;
+        TerminalApp::TabBase _GetTabByTabViewItem(const Microsoft::UI::Xaml::Controls::TabViewItem& tabViewItem) const noexcept;
 
         winrt::fire_and_forget _SetFocusedTabIndex(const uint32_t tabIndex);
         void _CloseFocusedTab();

src/cascadia/TerminalApp/TerminalTab.cpp

@@ -788,7 +788,7 @@ namespace winrt::TerminalApp::implementation
         closeTabMenuItem.Click([weakThis](auto&&, auto&&) {
             if (auto tab{ weakThis.get() })
             {
-                tab->_ClosedHandlers(nullptr, nullptr);
+                tab->_CloseRequestedHandlers(nullptr, nullptr);
             }
         });
         closeTabMenuItem.Text(RS_(L"TabClose"));