Skip to content

Commit

Permalink
Prevent Tab Switcher from increasing Tab refcount (#8653)
Browse files Browse the repository at this point in the history
Fix `TabPaletteItem` to hold only a weak reference to a tab.
This way we guarantee that the refcount of the closed tab 
gets to 0 immediately
(and that command palette cannot "raise it from the dead").

While this seems a correct thing to do, 
it is still not clear why the `FilteredCommand` itself 
(the one holding the `TabPaletteItem`) doesn't get released
until the UI is refreshed.

There is an impact of not registering to PropertyChanged event:
if the tab title changes during Tab Switcher navigation
the Tab Switcher item won't be updated immediately
(the change will apply next time the Tab Switcher is open).

Due to this change we need to make sure that the tabs binding
in #8427
doesn't break the title / icon update.

## Validation Steps Performed
* Manual testing

Closes #8651
  • Loading branch information
Don-Vito authored Jan 4, 2021
1 parent 9b07cb8 commit 68e0af4
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 3 deletions.
5 changes: 4 additions & 1 deletion src/cascadia/TerminalApp/CommandPalette.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,10 @@ namespace winrt::TerminalApp::implementation
{
if (const auto tabPaletteItem{ filteredCommand.Item().try_as<winrt::TerminalApp::TabPaletteItem>() })
{
_SwitchToTabRequestedHandlers(*this, tabPaletteItem.Tab());
if (const auto tab{ tabPaletteItem.Tab() })
{
_SwitchToTabRequestedHandlers(*this, tab);
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/TabPaletteItem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ using namespace winrt::Microsoft::Terminal::Settings::Model;
namespace winrt::TerminalApp::implementation
{
TabPaletteItem::TabPaletteItem(winrt::TerminalApp::TabBase const& tab) :
_Tab(tab)
_tab(tab)
{
Name(tab.Title());
Icon(tab.Icon());
Expand Down
6 changes: 5 additions & 1 deletion src/cascadia/TerminalApp/TabPaletteItem.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,13 @@ namespace winrt::TerminalApp::implementation
TabPaletteItem() = default;
TabPaletteItem(winrt::TerminalApp::TabBase const& tab);

GETSET_PROPERTY(winrt::TerminalApp::TabBase, Tab, nullptr);
winrt::TerminalApp::TabBase Tab() const noexcept
{
return _tab.get();
}

private:
winrt::weak_ref<winrt::TerminalApp::TabBase> _tab;
Windows::UI::Xaml::Data::INotifyPropertyChanged::PropertyChanged_revoker _tabChangedRevoker;
};
}
Expand Down

0 comments on commit 68e0af4

Please sign in to comment.