diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index dd396f251fe..b53ddf6bb0f 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -25,10 +25,6 @@ Tab::Tab(const GUID& profile, const TermControl& control) _activePane = _rootPane; - _AttachEventHandlersToPane(_rootPane); - - _AttachEventHandlersToControl(control); - _MakeTabViewItem(); } @@ -108,6 +104,19 @@ std::optional Tab::GetFocusedProfile() const noexcept return _activePane->GetFocusedProfile(); } +// Method Description: +// - Called after construction of a Tab object to bind event handlers to its +// associated Pane and TermControl object +// Arguments: +// - control: reference to the TermControl object to bind event to +// Return Value: +// - +void Tab::BindEventHandlers(const TermControl& control) noexcept +{ + _AttachEventHandlersToPane(_rootPane); + _AttachEventHandlersToControl(control); +} + // Method Description: // - Attempts to update the settings of this tab's tree of panes. // Arguments: @@ -147,8 +156,13 @@ void Tab::UpdateIcon(const winrt::hstring iconPath) _lastIconPath = iconPath; - _tabViewItem.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [this]() { - _tabViewItem.IconSource(GetColoredIcon(_lastIconPath)); + std::weak_ptr weakThis{ shared_from_this() }; + + _tabViewItem.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [weakThis]() { + if (auto tab{ weakThis.lock() }) + { + tab->_tabViewItem.IconSource(GetColoredIcon(tab->_lastIconPath)); + } }); } @@ -175,8 +189,13 @@ void Tab::SetTabText(const winrt::hstring& text) { // Copy the hstring, so we don't capture a dead reference winrt::hstring textCopy{ text }; - _tabViewItem.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [text = std::move(textCopy), this]() { - _tabViewItem.Header(winrt::box_value(text)); + std::weak_ptr weakThis{ shared_from_this() }; + + _tabViewItem.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [text = std::move(textCopy), weakThis]() { + if (auto tab{ weakThis.lock() }) + { + tab->_tabViewItem.Header(winrt::box_value(text)); + } }); } @@ -296,12 +315,16 @@ void Tab::ClosePane() // - void Tab::_AttachEventHandlersToControl(const TermControl& control) { - control.TitleChanged([this](auto newTitle) { - // The title of the control changed, but not necessarily the title - // of the tab. Get the title of the active pane of the tab, and set - // the tab's text to the active panes' text. - auto newTabTitle = GetActiveTitle(); - SetTabText(newTabTitle); + std::weak_ptr weakThis{ shared_from_this() }; + + control.TitleChanged([weakThis](auto newTitle) { + // Check if Tab's lifetime has expired + if (auto tab{ weakThis.lock() }) + { + // The title of the control changed, but not necessarily the title of the tab. + // Set the tab's text to the active panes' text. + tab->SetTabText(tab->GetActiveTitle()); + } }); } @@ -316,22 +339,24 @@ void Tab::_AttachEventHandlersToControl(const TermControl& control) // - void Tab::_AttachEventHandlersToPane(std::shared_ptr pane) { - pane->GotFocus([this](std::shared_ptr sender) { - // Do nothing if it's the same pane as before. - if (sender == _activePane) + std::weak_ptr weakThis{ shared_from_this() }; + + pane->GotFocus([weakThis](std::shared_ptr sender) { + // Do nothing if the Tab's lifetime is expired or pane isn't new. + auto tab{ weakThis.lock() }; + if (tab && sender != tab->_activePane) { - return; - } - // Clear the active state of the entire tree, and mark only the sender as active. - _rootPane->ClearActive(); - _activePane = sender; - _activePane->SetActive(); + // Clear the active state of the entire tree, and mark only the sender as active. + tab->_rootPane->ClearActive(); + tab->_activePane = sender; + tab->_activePane->SetActive(); - // Update our own title text to match the newly-active pane. - SetTabText(GetActiveTitle()); + // Update our own title text to match the newly-active pane. + tab->SetTabText(tab->GetActiveTitle()); - // Raise our own ActivePaneChanged event. - _ActivePaneChangedHandlers(); + // Raise our own ActivePaneChanged event. + tab->_ActivePaneChangedHandlers(); + } }); } diff --git a/src/cascadia/TerminalApp/Tab.h b/src/cascadia/TerminalApp/Tab.h index 429031dc193..4797b602ca7 100644 --- a/src/cascadia/TerminalApp/Tab.h +++ b/src/cascadia/TerminalApp/Tab.h @@ -5,11 +5,14 @@ #include #include "Pane.h" -class Tab +class Tab : public std::enable_shared_from_this { public: Tab(const GUID& profile, const winrt::Microsoft::Terminal::TerminalControl::TermControl& control); + // Called after construction to setup events with weak_ptr + void BindEventHandlers(const winrt::Microsoft::Terminal::TerminalControl::TermControl& control) noexcept; + winrt::Microsoft::UI::Xaml::Controls::TabViewItem GetTabViewItem(); winrt::Windows::UI::Xaml::UIElement GetRootElement(); winrt::Microsoft::Terminal::TerminalControl::TermControl GetActiveTerminalControl() const; diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 6edb8c0ba4c..5ba822f1463 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -60,23 +60,36 @@ namespace winrt::TerminalApp::implementation _tabView = _tabRow.TabView(); _rearranging = false; - _tabView.TabDragStarting([this](auto&& /*o*/, auto&& /*a*/) { - _rearranging = true; - _rearrangeFrom = std::nullopt; - _rearrangeTo = std::nullopt; - }); + // weak_ptr to this TerminalPage object lambda capturing + auto weakThis{ get_weak() }; - _tabView.TabDragCompleted([this](auto&& /*o*/, auto&& /*a*/) { - if (_rearrangeFrom.has_value() && _rearrangeTo.has_value() && _rearrangeTo != _rearrangeFrom) + _tabView.TabDragStarting([weakThis](auto&& /*o*/, auto&& /*a*/) { + if (auto page{ weakThis.get() }) { - auto tab = _tabs.at(_rearrangeFrom.value()); - _tabs.erase(_tabs.begin() + _rearrangeFrom.value()); - _tabs.insert(_tabs.begin() + _rearrangeTo.value(), tab); + page->_rearranging = true; + page->_rearrangeFrom = std::nullopt; + page->_rearrangeTo = std::nullopt; } + }); + + _tabView.TabDragCompleted([weakThis](auto&& /*o*/, auto&& /*a*/) { + if (auto page{ weakThis.get() }) + { + auto& from{ page->_rearrangeFrom }; + auto& to{ page->_rearrangeTo }; - _rearranging = false; - _rearrangeFrom = std::nullopt; - _rearrangeTo = std::nullopt; + if (from.has_value() && to.has_value() && to != from) + { + auto& tabs{ page->_tabs }; + auto tab = tabs.at(from.value()); + tabs.erase(tabs.begin() + from.value()); + tabs.insert(tabs.begin() + to.value(), tab); + } + + page->_rearranging = false; + from = std::nullopt; + to = std::nullopt; + } }); auto tabRowImpl = winrt::get_self(_tabRow); @@ -100,8 +113,11 @@ namespace winrt::TerminalApp::implementation _RegisterActionCallbacks(); //Event Bindings (Early) - _newTabButton.Click([this](auto&&, auto&&) { - this->_OpenNewTab(std::nullopt); + _newTabButton.Click([weakThis](auto&&, auto&&) { + if (auto page{ weakThis.get() }) + { + page->_OpenNewTab(std::nullopt); + } }); _tabView.SelectionChanged({ this, &TerminalPage::_OnTabSelectionChanged }); _tabView.TabCloseRequested({ this, &TerminalPage::_OnTabCloseRequested }); @@ -301,8 +317,13 @@ namespace winrt::TerminalApp::implementation profileMenuItem.FontWeight(FontWeights::Bold()); } - profileMenuItem.Click([this, profileIndex](auto&&, auto&&) { - this->_OpenNewTab({ profileIndex }); + auto weakThis{ get_weak() }; + + profileMenuItem.Click([profileIndex, weakThis](auto&&, auto&&) { + if (auto page{ weakThis.get() }) + { + page->_OpenNewTab({ profileIndex }); + } }); newTabFlyout.Items().Append(profileMenuItem); } @@ -440,17 +461,21 @@ namespace winrt::TerminalApp::implementation // Don't capture a strong ref to the tab. If the tab is removed as this // is called, we don't really care anymore about handling the event. std::weak_ptr weakTabPtr = newTab; + auto weakThis{ get_weak() }; + // When the tab's active pane changes, we'll want to lookup a new icon // for it, and possibly propogate the title up to the window. - newTab->ActivePaneChanged([this, weakTabPtr]() { - if (auto tab = weakTabPtr.lock()) + newTab->ActivePaneChanged([weakTabPtr, weakThis]() { + auto page{ weakThis.get() }; + auto tab{ weakTabPtr.lock() }; + + if (page && tab) { // Possibly update the icon of the tab. - _UpdateTabIcon(tab); - + page->_UpdateTabIcon(tab); // Possibly update the title of the tab, window to match the newly // focused pane. - _UpdateTitle(tab); + page->_UpdateTitle(tab); } }); @@ -467,10 +492,16 @@ namespace winrt::TerminalApp::implementation tabViewItem.PointerPressed({ this, &TerminalPage::_OnTabClick }); // When the tab is closed, remove it from our list of tabs. - newTab->Closed([tabViewItem, this](auto&& /*s*/, auto&& /*e*/) { - _tabView.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [tabViewItem, this]() { - _RemoveTabViewItem(tabViewItem); - }); + newTab->Closed([tabViewItem, weakThis](auto&& /*s*/, auto&& /*e*/) { + if (auto page{ weakThis.get() }) + { + page->_tabView.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [tabViewItem, weakThis]() { + if (auto page{ weakThis.get() }) + { + page->_RemoveTabViewItem(tabViewItem); + } + }); + } }); // This is one way to set the tab's selected background color. @@ -762,19 +793,25 @@ namespace winrt::TerminalApp::implementation // Add an event handler when the terminal wants to paste data from the Clipboard. term.PasteFromClipboard({ this, &TerminalPage::_PasteFromClipboardHandler }); + // Bind Tab events to the TermControl and the Tab's Pane + hostingTab->BindEventHandlers(term); + // Don't capture a strong ref to the tab. If the tab is removed as this // is called, we don't really care anymore about handling the event. std::weak_ptr weakTabPtr = hostingTab; - term.TitleChanged([this, weakTabPtr](auto newTitle) { - auto tab = weakTabPtr.lock(); - if (!tab) + auto weakThis{ get_weak() }; + + term.TitleChanged([weakTabPtr, weakThis](auto newTitle) { + auto page{ weakThis.get() }; + auto tab{ weakTabPtr.lock() }; + + if (page && tab) { - return; + // The title of the control changed, but not necessarily the title + // of the tab. Get the title of the focused pane of the tab, and set + // the tab's text to the focused panes' text. + page->_UpdateTitle(tab); } - // The title of the control changed, but not necessarily the title - // of the tab. Get the title of the focused pane of the tab, and set - // the tab's text to the focused panes' text. - _UpdateTitle(tab); }); } @@ -849,9 +886,12 @@ namespace winrt::TerminalApp::implementation // GH#1117: This is a workaround because _tabView.SelectedIndex(tabIndex) // sometimes set focus to an incorrect tab after removing some tabs auto tab = _tabs.at(tabIndex); - _tabView.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [tab, this]() { - auto tabViewItem = tab->GetTabViewItem(); - _tabView.SelectedItem(tabViewItem); + auto weakThis{ get_weak() }; + _tabView.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [tab, weakThis]() { + if (auto page{ weakThis.get() }) + { + page->_tabView.SelectedItem(tab->GetTabViewItem()); + } }); } @@ -1350,10 +1390,14 @@ namespace winrt::TerminalApp::implementation _UpdateTitle(tab); } - this->Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [this]() { + auto weakThis{ get_weak() }; + this->Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [weakThis]() { // repopulate the new tab button's flyout with entries for each // profile, which might have changed - _CreateNewTabFlyout(); + if (auto page{ weakThis.get() }) + { + page->_CreateNewTabFlyout(); + } }); }