hey look, it builds now

zadjii-msft · zadjii-msft · commit ef6bb8a73ca2 · 2023-07-17T12:35:27.000-05:00
diff --git a/src/cascadia/TerminalApp/AppActionHandlers.cpp b/src/cascadia/TerminalApp/AppActionHandlers.cpp
@@ -1279,7 +1279,14 @@ namespace winrt::TerminalApp::implementation
         {
             if (const auto activePane{ activeTab->GetActivePane() })
             {
-                _restartPaneConnection(activePane);
+                activePane;
+                // TODO! If we don't expose the IPaneContent, then there's no
+                // way to get a TerminalPaneContent to pass to
+                // _restartPaneConnection / _duplicateConnectionForRestart. We
+                // probably need to change the signature to accept a
+                // TermControl&Profile
+
+                // _restartPaneConnection(activePane);
             }
         }
         args.Handled(true);
diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp
@@ -1251,19 +1251,6 @@ void Pane::Shutdown()
     // modify our tree
     std::unique_lock lock{ _createCloseLock };
 
-    // TODO! This was added in main after I started working on this
-    // // Clear out our media player callbacks, and stop any playing media. This
-    // // will prevent the callback from being triggered after we've closed, and
-    // // also make sure that our sound stops when we're closed.
-    // if (_bellPlayer)
-    // {
-    //     _bellPlayer.Pause();
-    //     _bellPlayer.Source(nullptr);
-    //     _bellPlayer.Close();
-    //     _bellPlayer = nullptr;
-    //     _bellPlayerCreated = false;
-    // }
-
     if (_IsLeaf())
     {
         _content.Close();
@@ -3173,7 +3160,7 @@ void Pane::FinalizeConfigurationGivenDefault()
 {
     if (const auto& terminalPane{ _content.try_as<TerminalPaneContent>() })
     {
-        terminalPane.FinalizeConfigurationGivenDefault();
+        terminalPane.MarkAsDefterm();
     }
     // _isDefTermSession = true;
 }
diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h
@@ -216,7 +216,7 @@ class Pane : public std::enable_shared_from_this<Pane>
     WINRT_CALLBACK(LostFocus, winrt::delegate<std::shared_ptr<Pane>>);
     WINRT_CALLBACK(PaneRaiseBell, winrt::Windows::Foundation::EventHandler<bool>);
     WINRT_CALLBACK(Detached, winrt::delegate<std::shared_ptr<Pane>>);
-    WINRT_CALLBACK(RestartTerminalRequested, winrt::delegate<std::shared_ptr<Pane>>);
+    // WINRT_CALLBACK(RestartTerminalRequested, winrt::delegate<std::shared_ptr<Pane>>);
 
 private:
     struct PanePoint;
@@ -256,10 +256,6 @@ class Pane : public std::enable_shared_from_this<Pane>
 
     bool _zoomed{ false };
 
-    // TODO! these were added in main after I started working on this
-    // winrt::Windows::Media::Playback::MediaPlayer _bellPlayer{ nullptr };
-    // bool _bellPlayerCreated{ false };
-
     bool _IsLeaf() const noexcept;
     bool _HasFocusedChild() const noexcept;
     void _SetupChildCloseHandlers();
diff --git a/src/cascadia/TerminalApp/TabManagement.cpp b/src/cascadia/TerminalApp/TabManagement.cpp
@@ -63,7 +63,7 @@ namespace winrt::TerminalApp::implementation
     // - existingConnection: An optional connection that is already established to a PTY
     //   for this tab to host instead of creating one.
     //   If not defined, the tab will create the connection.
-    HRESULT TerminalPage::_OpenNewTab(const NewTerminalArgs& newTerminalArgs, winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection existingConnection)
+    HRESULT TerminalPage::_OpenNewTab(const NewTerminalArgs& newTerminalArgs)
     try
     {
         const auto profile{ _settings.GetProfileForArgs(newTerminalArgs) };
@@ -86,7 +86,7 @@ namespace winrt::TerminalApp::implementation
         //
         // This call to _MakePane won't return nullptr, we already checked that
         // case above with the _maybeElevate call.
-        _CreateNewTabFromPane(_MakePane(newTerminalArgs, nullptr, existingConnection));
+        _CreateNewTabFromPane(_MakePane(newTerminalArgs, nullptr));
         return S_OK;
     }
     CATCH_RETURN();
diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp
@@ -1282,15 +1282,20 @@ namespace winrt::TerminalApp::implementation
         return connection;
     }
 
-    TerminalConnection::ITerminalConnection TerminalPage::_duplicateConnectionForRestart(std::shared_ptr<Pane> pane)
+    TerminalConnection::ITerminalConnection TerminalPage::_duplicateConnectionForRestart(const TerminalApp::TerminalPaneContent& paneContent)
     {
-        const auto& control{ pane->GetTerminalControl() };
+        if (paneContent == nullptr)
+        {
+            return nullptr;
+        }
+
+        const auto& control{ paneContent.GetTerminal() };
         if (control == nullptr)
         {
             return nullptr;
         }
         const auto& connection = control.Connection();
-        auto profile{ pane->GetProfile() };
+        auto profile{ paneContent.GetProfile() };
 
         TerminalSettingsCreateResult controlSettings{ nullptr };
 
@@ -2922,10 +2927,9 @@ namespace winrt::TerminalApp::implementation
             // Don't need to worry about duplicating or anything - we'll
             // serialize the actual profile's GUID along with the content guid.
             const auto& profile = _settings.GetProfileForArgs(newTerminalArgs);
-
             const auto control = _AttachControlToContent(newTerminalArgs.ContentId());
-
-            return std::make_shared<Pane>(profile, control);
+            auto terminalPane{ winrt::make<TerminalPaneContent>(profile, control) };
+            return std::make_shared<Pane>(terminalPane);
         }
 
         TerminalSettingsCreateResult controlSettings{ nullptr };
@@ -3003,16 +3007,18 @@ namespace winrt::TerminalApp::implementation
             original->SetActive();
         }
 
-        resultPane->RestartTerminalRequested({ get_weak(), &TerminalPage::_restartPaneConnection });
+        terminalPane.RestartTerminalRequested({ get_weak(), &TerminalPage::_restartPaneConnection });
 
         return resultPane;
     }
 
-    void TerminalPage::_restartPaneConnection(const std::shared_ptr<Pane>& pane)
+    void TerminalPage::_restartPaneConnection(
+        const TerminalApp::TerminalPaneContent& paneContent,
+        const winrt::Windows::Foundation::IInspectable&)
     {
-        if (const auto& connection{ _duplicateConnectionForRestart(pane) })
+        if (const auto& connection{ _duplicateConnectionForRestart(paneContent) })
         {
-            pane->GetTerminalControl().Connection(connection);
+            paneContent.GetTerminal().Connection(connection);
             connection.Start();
         }
     }
diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h
@@ -294,14 +294,14 @@ namespace winrt::TerminalApp::implementation
         winrt::Windows::UI::Xaml::Controls::MenuFlyoutItem _CreateNewTabFlyoutProfile(const Microsoft::Terminal::Settings::Model::Profile profile, int profileIndex);
 
         void _OpenNewTabDropdown();
-        HRESULT _OpenNewTab(const Microsoft::Terminal::Settings::Model::NewTerminalArgs& newTerminalArgs, winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection existingConnection = nullptr);
+        HRESULT _OpenNewTab(const Microsoft::Terminal::Settings::Model::NewTerminalArgs& newTerminalArgs);
         void _CreateNewTabFromPane(std::shared_ptr<Pane> pane, uint32_t insertPosition = -1);
 
         std::wstring _evaluatePathForCwd(std::wstring_view path);
 
         winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection _CreateConnectionFromSettings(Microsoft::Terminal::Settings::Model::Profile profile, Microsoft::Terminal::Settings::Model::TerminalSettings settings, const bool inheritCursor);
-        winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection _duplicateConnectionForRestart(std::shared_ptr<Pane> pane);
-        void _restartPaneConnection(const std::shared_ptr<Pane>& pane);
+        winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection _duplicateConnectionForRestart(const TerminalApp::TerminalPaneContent& paneContent);
+        void _restartPaneConnection(const TerminalApp::TerminalPaneContent&, const winrt::Windows::Foundation::IInspectable&);
 
         winrt::fire_and_forget _OpenNewWindow(const Microsoft::Terminal::Settings::Model::NewTerminalArgs newTerminalArgs);
 
diff --git a/src/cascadia/TerminalApp/TerminalPaneContent.cpp b/src/cascadia/TerminalApp/TerminalPaneContent.cpp
@@ -57,14 +57,14 @@ namespace winrt::TerminalApp::implementation
         // Clear out our media player callbacks, and stop any playing media. This
         // will prevent the callback from being triggered after we've closed, and
         // also make sure that our sound stops when we're closed.
-        _mediaEndedRevoker.revoke();
         if (_bellPlayer)
         {
             _bellPlayer.Pause();
             _bellPlayer.Source(nullptr);
             _bellPlayer.Close();
+            _bellPlayer = nullptr;
+            _bellPlayerCreated = false;
         }
-        _bellPlayer = nullptr;
     }
 
     // Method Description:
@@ -168,19 +168,15 @@ namespace winrt::TerminalApp::implementation
         co_await wil::resume_foreground(_control.Dispatcher());
         if (auto pane{ weakThis.get() })
         {
-            // BODGY
-            // GH#12258: We learned that if you leave the MediaPlayer open, and
-            // press the media keys (like play/pause), then the OS will _replay the
-            // bell_. So we have to re-create the MediaPlayer each time we want to
-            // play the bell, to make sure a subsequent play doesn't come through
-            // and reactivate the old one.
-
-            if (!_bellPlayer)
+            if (!_bellPlayerCreated)
             {
                 // The MediaPlayer might not exist on Windows N SKU.
                 try
                 {
+                    _bellPlayerCreated = true;
                     _bellPlayer = winrt::Windows::Media::Playback::MediaPlayer();
+                    // GH#12258: The media keys (like play/pause) should have no effect on our bell sound.
+                    _bellPlayer.CommandManager().IsEnabled(false);
                 }
                 CATCH_LOG();
             }
@@ -190,27 +186,6 @@ namespace winrt::TerminalApp::implementation
                 const auto item{ winrt::Windows::Media::Playback::MediaPlaybackItem(source) };
                 _bellPlayer.Source(item);
                 _bellPlayer.Play();
-
-                // This lambda will clean up the bell player when we're done with it.
-                auto weakThis2{ get_weak() };
-                _mediaEndedRevoker = _bellPlayer.MediaEnded(winrt::auto_revoke, [weakThis2](auto&&, auto&&) {
-                    if (auto self{ weakThis2.get() })
-                    {
-                        if (self->_bellPlayer)
-                        {
-                            // We need to make sure clear out the current track
-                            // that's being played, again, so that the system can't
-                            // come through and replay it. In testing, we needed to
-                            // do this, closing the MediaPlayer alone wasn't good
-                            // enough.
-                            self->_bellPlayer.Pause();
-                            self->_bellPlayer.Source(nullptr);
-                            self->_bellPlayer.Close();
-                        }
-                        self->_mediaEndedRevoker.revoke();
-                        self->_bellPlayer = nullptr;
-                    }
-                });
             }
         }
     }
@@ -223,7 +198,7 @@ namespace winrt::TerminalApp::implementation
     void TerminalPaneContent::_RestartTerminalRequestedHandler(const winrt::Windows::Foundation::IInspectable& /*sender*/,
                                                                const winrt::Windows::Foundation::IInspectable& /*args*/)
     {
-        _RestartTerminalRequestedHandlers(*this, nullptr);
+        RestartTerminalRequested.raise(*this, nullptr);
     }
 
     void TerminalPaneContent::UpdateSettings(const TerminalSettingsCreateResult& settings,
@@ -232,4 +207,13 @@ namespace winrt::TerminalApp::implementation
         _profile = profile;
         _control.UpdateControlSettings(settings.DefaultSettings(), settings.UnfocusedSettings());
     }
+
+    // Method Description:
+    // - Should be called when this pane is created via a default terminal handoff
+    // - Finalizes our configuration given the information that we have been
+    //   created via default handoff
+    void TerminalPaneContent::MarkAsDefterm()
+    {
+        _isDefTermSession = true;
+    }
 }
diff --git a/src/cascadia/TerminalApp/TerminalPaneContent.h b/src/cascadia/TerminalApp/TerminalPaneContent.h
@@ -4,6 +4,7 @@
 #pragma once
 #include "TerminalPaneContent.g.h"
 #include "../../cascadia/inc/cppwinrt_utils.h"
+#include <til/winrt.h>
 
 namespace winrt::TerminalApp::implementation
 {
@@ -21,6 +22,8 @@ namespace winrt::TerminalApp::implementation
         void UpdateSettings(const winrt::Microsoft::Terminal::Settings::Model::TerminalSettingsCreateResult& settings,
                             const winrt::Microsoft::Terminal::Settings::Model::Profile& profile);
 
+        void MarkAsDefterm();
+
         winrt::Microsoft::Terminal::Settings::Model::Profile GetProfile() const
         {
             return _profile;
@@ -31,14 +34,16 @@ namespace winrt::TerminalApp::implementation
         uint64_t TaskbarProgress() { return _control.TaskbarProgress(); }
         bool ReadOnly() { return _control.ReadOnly(); }
 
+        til::typed_event<TerminalApp::TerminalPaneContent, winrt::Windows::Foundation::IInspectable> RestartTerminalRequested;
+
     private:
         winrt::Microsoft::Terminal::Control::TermControl _control{ nullptr };
         winrt::Microsoft::Terminal::TerminalConnection::ConnectionState _connectionState{ winrt::Microsoft::Terminal::TerminalConnection::ConnectionState::NotConnected };
         winrt::Microsoft::Terminal::Settings::Model::Profile _profile{ nullptr };
         bool _isDefTermSession{ false };
 
         winrt::Windows::Media::Playback::MediaPlayer _bellPlayer{ nullptr };
-        winrt::Windows::Media::Playback::MediaPlayer::MediaEnded_revoker _mediaEndedRevoker;
+        bool _bellPlayerCreated{ false };
 
         struct ControlEventTokens
         {
diff --git a/src/cascadia/TerminalApp/TerminalPaneContent.idl b/src/cascadia/TerminalApp/TerminalPaneContent.idl
@@ -12,6 +12,11 @@ namespace TerminalApp
         void UpdateSettings(const Microsoft.Terminal.Settings.Model.TerminalSettingsCreateResult settings,
                             const Microsoft.Terminal.Settings.Model.Profile profile);
 
+        void MarkAsDefterm();
+
         Microsoft.Terminal.Settings.Model.Profile GetProfile();
+
+
+        event Windows.Foundation.TypedEventHandler<TerminalPaneContent, Object> RestartTerminalRequested;
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -1279,7 +1279,14 @@ namespace winrt::TerminalApp::implementation`
`1279`	`1279`	`{`
`1280`	`1280`	`if (const auto activePane{ activeTab->GetActivePane() })`
`1281`	`1281`	`{`
`1282`		`- _restartPaneConnection(activePane);`
	`1282`	`+ activePane;`
	`1283`	`+ // TODO! If we don't expose the IPaneContent, then there's no`
	`1284`	`+ // way to get a TerminalPaneContent to pass to`
	`1285`	`+ // _restartPaneConnection / _duplicateConnectionForRestart. We`
	`1286`	`+ // probably need to change the signature to accept a`
	`1287`	`+ // TermControl&Profile`
	`1288`	`+`
	`1289`	`+ // _restartPaneConnection(activePane);`
`1283`	`1290`	`}`
`1284`	`1291`	`}`
`1285`	`1292`	`args.Handled(true);`
Original file line number	Diff line number	Diff line change
`@@ -1251,19 +1251,6 @@ void Pane::Shutdown()`
`1251`	`1251`	`// modify our tree`
`1252`	`1252`	`std::unique_lock lock{ _createCloseLock };`
`1253`	`1253`
`1254`		`- // TODO! This was added in main after I started working on this`
`1255`		`- // // Clear out our media player callbacks, and stop any playing media. This`
`1256`		`- // // will prevent the callback from being triggered after we've closed, and`
`1257`		`- // // also make sure that our sound stops when we're closed.`
`1258`		`- // if (_bellPlayer)`
`1259`		`- // {`
`1260`		`- // _bellPlayer.Pause();`
`1261`		`- // _bellPlayer.Source(nullptr);`
`1262`		`- // _bellPlayer.Close();`
`1263`		`- // _bellPlayer = nullptr;`
`1264`		`- // _bellPlayerCreated = false;`
`1265`		`- // }`
`1266`		`-`
`1267`	`1254`	`if (_IsLeaf())`
`1268`	`1255`	`{`
`1269`	`1256`	`_content.Close();`
`@@ -3173,7 +3160,7 @@ void Pane::FinalizeConfigurationGivenDefault()`
`3173`	`3160`	`{`
`3174`	`3161`	`if (const auto& terminalPane{ _content.try_as<TerminalPaneContent>() })`
`3175`	`3162`	`{`
`3176`		`- terminalPane.FinalizeConfigurationGivenDefault();`
	`3163`	`+ terminalPane.MarkAsDefterm();`
`3177`	`3164`	`}`
`3178`	`3165`	`// _isDefTermSession = true;`
`3179`	`3166`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1282,15 +1282,20 @@ namespace winrt::TerminalApp::implementation`
`1282`	`1282`	`return connection;`
`1283`	`1283`	`}`
`1284`	`1284`
`1285`		`- TerminalConnection::ITerminalConnection TerminalPage::_duplicateConnectionForRestart(std::shared_ptr<Pane> pane)`
	`1285`	`+ TerminalConnection::ITerminalConnection TerminalPage::_duplicateConnectionForRestart(const TerminalApp::TerminalPaneContent& paneContent)`
`1286`	`1286`	`{`
`1287`		`- const auto& control{ pane->GetTerminalControl() };`
	`1287`	`+ if (paneContent == nullptr)`
	`1288`	`+ {`
	`1289`	`+ return nullptr;`
	`1290`	`+ }`
	`1291`	`+`
	`1292`	`+ const auto& control{ paneContent.GetTerminal() };`
`1288`	`1293`	`if (control == nullptr)`
`1289`	`1294`	`{`
`1290`	`1295`	`return nullptr;`
`1291`	`1296`	`}`
`1292`	`1297`	`const auto& connection = control.Connection();`
`1293`		`- auto profile{ pane->GetProfile() };`
	`1298`	`+ auto profile{ paneContent.GetProfile() };`
`1294`	`1299`
`1295`	`1300`	`TerminalSettingsCreateResult controlSettings{ nullptr };`
`1296`	`1301`
`@@ -2922,10 +2927,9 @@ namespace winrt::TerminalApp::implementation`
`2922`	`2927`	`// Don't need to worry about duplicating or anything - we'll`
`2923`	`2928`	`// serialize the actual profile's GUID along with the content guid.`
`2924`	`2929`	`const auto& profile = _settings.GetProfileForArgs(newTerminalArgs);`
`2925`		`-`
`2926`	`2930`	`const auto control = _AttachControlToContent(newTerminalArgs.ContentId());`
`2927`		`-`
`2928`		`- return std::make_shared<Pane>(profile, control);`
	`2931`	`+ auto terminalPane{ winrt::make<TerminalPaneContent>(profile, control) };`
	`2932`	`+ return std::make_shared<Pane>(terminalPane);`
`2929`	`2933`	`}`
`2930`	`2934`
`2931`	`2935`	`TerminalSettingsCreateResult controlSettings{ nullptr };`
`@@ -3003,16 +3007,18 @@ namespace winrt::TerminalApp::implementation`
`3003`	`3007`	`original->SetActive();`
`3004`	`3008`	`}`
`3005`	`3009`
`3006`		`- resultPane->RestartTerminalRequested({ get_weak(), &TerminalPage::_restartPaneConnection });`
	`3010`	`+ terminalPane.RestartTerminalRequested({ get_weak(), &TerminalPage::_restartPaneConnection });`
`3007`	`3011`
`3008`	`3012`	`return resultPane;`
`3009`	`3013`	`}`
`3010`	`3014`
`3011`		`- void TerminalPage::_restartPaneConnection(const std::shared_ptr<Pane>& pane)`
	`3015`	`+ void TerminalPage::_restartPaneConnection(`
	`3016`	`+ const TerminalApp::TerminalPaneContent& paneContent,`
	`3017`	`+ const winrt::Windows::Foundation::IInspectable&)`
`3012`	`3018`	`{`
`3013`		`- if (const auto& connection{ _duplicateConnectionForRestart(pane) })`
	`3019`	`+ if (const auto& connection{ _duplicateConnectionForRestart(paneContent) })`
`3014`	`3020`	`{`
`3015`		`- pane->GetTerminalControl().Connection(connection);`
	`3021`	`+ paneContent.GetTerminal().Connection(connection);`
`3016`	`3022`	`connection.Start();`
`3017`	`3023`	`}`
`3018`	`3024`	`}`
Original file line number	Diff line number	Diff line change
`@@ -12,6 +12,11 @@ namespace TerminalApp`
`12`	`12`	`void UpdateSettings(const Microsoft.Terminal.Settings.Model.TerminalSettingsCreateResult settings,`
`13`	`13`	`const Microsoft.Terminal.Settings.Model.Profile profile);`
`14`	`14`
	`15`	`+ void MarkAsDefterm();`
	`16`	`+`
`15`	`17`	`Microsoft.Terminal.Settings.Model.Profile GetProfile();`
	`18`	`+`
	`19`	`+`
	`20`	`+ event Windows.Foundation.TypedEventHandler<TerminalPaneContent, Object> RestartTerminalRequested;`
`16`	`21`	`}`
`17`	`22`	`}`