Fix panes being dropped when tearing off tabs (#18627)

I don't actually know why this is happening, because it doesn't
happen with startup actions specified in the settings file.
In any case, it's fixed with more delays.

Closes #18572

## Validation Steps Performed
* Create a tab with 2 panes
* Tear it off into a new window
* New window has 1 tab with 2 panes ✅

Author: lhecker

src/cascadia/LocalTests_TerminalApp/TabTests.cpp

@@ -287,7 +287,7 @@ namespace TerminalAppLocalTests
             NewTabArgs args{ newTerminalArgs };
             ActionAndArgs newTabAction{ ShortcutAction::NewTab, args };
             // push the arg onto the front
-            page->_startupActions.Append(newTabAction);
+            page->_startupActions.push_back(std::move(newTabAction));
             Log::Comment(L"Added a single newTab action");
 
             auto app = ::winrt::Windows::UI::Xaml::Application::Current();

src/cascadia/TerminalApp/AppActionHandlers.cpp

@@ -752,13 +752,11 @@ namespace winrt::TerminalApp::implementation
     {
         if (const auto& realArgs = actionArgs.ActionArgs().try_as<ExecuteCommandlineArgs>())
         {
-            auto actions = winrt::single_threaded_vector<ActionAndArgs>(
-                TerminalPage::ConvertExecuteCommandlineToActions(realArgs));
-
-            if (actions.Size() != 0)
+            auto actions = ConvertExecuteCommandlineToActions(realArgs);
+            if (!actions.empty())
             {
                 actionArgs.Handled(true);
-                ProcessStartupActions(actions, false);
+                ProcessStartupActions(std::move(actions), false);
             }
         }
     }

src/cascadia/TerminalApp/TerminalPage.cpp

@@ -62,7 +62,6 @@ namespace winrt::TerminalApp::implementation
     TerminalPage::TerminalPage(TerminalApp::WindowProperties properties, const TerminalApp::ContentManager& manager) :
         _tabs{ winrt::single_threaded_observable_vector<TerminalApp::TabBase>() },
         _mruTabs{ winrt::single_threaded_observable_vector<TerminalApp::TabBase>() },
-        _startupActions{ winrt::single_threaded_vector<ActionAndArgs>() },
         _manager{ manager },
         _hostingHwnd{},
         _WindowProperties{ std::move(properties) }
@@ -297,7 +296,7 @@ namespace winrt::TerminalApp::implementation
         // GH#12267: Don't forget about defterm handoff here. If we're being
         // created for embedding, then _yea_, we don't need to handoff to an
         // elevated window.
-        if (!_startupActions || IsRunningElevated() || _shouldStartInboundListener || _startupActions.Size() == 0)
+        if (_startupActions.empty() || IsRunningElevated() || _shouldStartInboundListener)
         {
             // there aren't startup actions, or we're elevated. In that case, go for it.
             return false;
@@ -375,7 +374,7 @@ namespace winrt::TerminalApp::implementation
     // - <none>
     void TerminalPage::HandoffToElevated(const CascadiaSettings& settings)
     {
-        if (!_startupActions)
+        if (_startupActions.empty())
         {
             return;
         }
@@ -489,7 +488,7 @@ namespace winrt::TerminalApp::implementation
         {
             _startupState = StartupState::InStartup;
 
-            ProcessStartupActions(_startupActions, true);
+            ProcessStartupActions(std::move(_startupActions), true);
 
             // If we were told that the COM server needs to be started to listen for incoming
             // default application connections, start it now.
@@ -546,80 +545,56 @@ namespace winrt::TerminalApp::implementation
     //   nt -d .` from inside another directory to work as expected.
     // Return Value:
     // - <none>
-    safe_void_coroutine TerminalPage::ProcessStartupActions(Windows::Foundation::Collections::IVector<ActionAndArgs> actions,
-                                                            const bool initial,
-                                                            const winrt::hstring cwd,
-                                                            const winrt::hstring env)
+    safe_void_coroutine TerminalPage::ProcessStartupActions(std::vector<ActionAndArgs> actions, const bool initial, const winrt::hstring cwd, const winrt::hstring env)
     {
-        auto weakThis{ get_weak() };
-
-        // Handle it on a subsequent pass of the UI thread.
-        co_await wil::resume_foreground(Dispatcher(), CoreDispatcherPriority::Normal);
+        const auto strong = get_strong();
 
         // If the caller provided a CWD, "switch" to that directory, then switch
-        // back once we're done. This looks weird though, because we have to set
-        // up the scope_exit _first_. We'll release the scope_exit if we don't
-        // actually need it.
-
+        // back once we're done.
         auto originalVirtualCwd{ _WindowProperties.VirtualWorkingDirectory() };
-        auto restoreCwd = wil::scope_exit([&originalVirtualCwd, this]() {
-            // ignore errors, we'll just power on through. We'd rather do
-            // something rather than fail silently if the directory doesn't
-            // actually exist.
-            _WindowProperties.VirtualWorkingDirectory(originalVirtualCwd);
-        });
-
-        // Literally the same thing with env vars too
         auto originalVirtualEnv{ _WindowProperties.VirtualEnvVars() };
-        auto restoreEnv = wil::scope_exit([&originalVirtualEnv, this]() {
-            _WindowProperties.VirtualEnvVars(originalVirtualEnv);
+        auto restoreCwd = wil::scope_exit([&]() {
+            if (!cwd.empty())
+            {
+                // ignore errors, we'll just power on through. We'd rather do
+                // something rather than fail silently if the directory doesn't
+                // actually exist.
+                _WindowProperties.VirtualWorkingDirectory(originalVirtualCwd);
+                _WindowProperties.VirtualEnvVars(originalVirtualEnv);
+            }
         });
+        _WindowProperties.VirtualWorkingDirectory(cwd);
+        _WindowProperties.VirtualEnvVars(env);
 
-        if (cwd.empty())
+        for (size_t i = 0; i < actions.size(); ++i)
         {
-            // We didn't actually need to change the virtual CWD, so we don't
-            // need to restore it
-            restoreCwd.release();
-        }
-        else
-        {
-            _WindowProperties.VirtualWorkingDirectory(cwd);
-        }
+            if (i != 0)
+            {
+                // Each action may rely on the XAML layout of a preceding action.
+                // Most importantly, this is the case for the combination of NewTab + SplitPane,
+                // as the former appears to only have a layout size after at least 1 resume_foreground,
+                // while the latter relies on that information. This is also why it uses Low priority.
+                //
+                // Curiously, this does not seem to be required when using startupActions, but only when
+                // tearing out a tab (this currently creates a new window with injected startup actions).
+                // This indicates that this is really more of an architectural issue and not a fundamental one.
+                co_await wil::resume_foreground(Dispatcher(), CoreDispatcherPriority::Low);
+            }
 
-        if (env.empty())
-        {
-            restoreEnv.release();
-        }
-        else
-        {
-            _WindowProperties.VirtualEnvVars(env);
+            _actionDispatch->DoAction(actions[i]);
         }
 
-        if (auto page{ weakThis.get() })
+        // GH#6586: now that we're done processing all startup commands,
+        // focus the active control. This will work as expected for both
+        // commandline invocations and for `wt` action invocations.
+        if (const auto& terminalTab{ _GetFocusedTabImpl() })
         {
-            for (const auto& action : actions)
+            if (const auto& content{ terminalTab->GetActiveContent() })
             {
-                if (auto page{ weakThis.get() })
-                {
-                    _actionDispatch->DoAction(action);
-                }
-                else
-                {
-                    co_return;
-                }
-            }
-
-            // GH#6586: now that we're done processing all startup commands,
-            // focus the active control. This will work as expected for both
-            // commandline invocations and for `wt` action invocations.
-            if (const auto& terminalTab{ _GetFocusedTabImpl() })
-            {
-                if (const auto& content{ terminalTab->GetActiveContent() })
-                {
-                    content.Focus(FocusState::Programmatic);
-                }
+                content.Focus(FocusState::Programmatic);
             }
         }
+
         if (initial)
         {
             _CompleteInitialization();
@@ -3670,13 +3645,9 @@ namespace winrt::TerminalApp::implementation
     // - actions: a list of Actions to process on startup.
     // Return Value:
     // - <none>
-    void TerminalPage::SetStartupActions(std::vector<ActionAndArgs>& actions)
+    void TerminalPage::SetStartupActions(std::vector<ActionAndArgs> actions)
     {
-        // The fastest way to copy all the actions out of the std::vector and
-        // put them into a winrt::IVector is by making a copy, then moving the
-        // copy into the winrt vector ctor.
-        auto listCopy = actions;
-        _startupActions = winrt::single_threaded_vector<ActionAndArgs>(std::move(listCopy));
+        _startupActions = std::move(actions);
     }
 
     // Routine Description:

src/cascadia/TerminalApp/TerminalPage.h

@@ -128,7 +128,7 @@ namespace winrt::TerminalApp::implementation
         void Maximized(bool newMaximized);
         void RequestSetMaximized(bool newMaximized);
 
-        void SetStartupActions(std::vector<Microsoft::Terminal::Settings::Model::ActionAndArgs>& actions);
+        void SetStartupActions(std::vector<Microsoft::Terminal::Settings::Model::ActionAndArgs> actions);
 
         void SetInboundListener(bool isEmbedding);
         static std::vector<Microsoft::Terminal::Settings::Model::ActionAndArgs> ConvertExecuteCommandlineToActions(const Microsoft::Terminal::Settings::Model::ExecuteCommandlineArgs& args);
@@ -146,7 +146,7 @@ namespace winrt::TerminalApp::implementation
         void ActionSaveFailed(winrt::hstring message);
         void ShowTerminalWorkingDirectory();
 
-        safe_void_coroutine ProcessStartupActions(Windows::Foundation::Collections::IVector<Microsoft::Terminal::Settings::Model::ActionAndArgs> actions,
+        safe_void_coroutine ProcessStartupActions(std::vector<Microsoft::Terminal::Settings::Model::ActionAndArgs> actions,
                                                   const bool initial,
                                                   const winrt::hstring cwd = winrt::hstring{},
                                                   const winrt::hstring env = winrt::hstring{});
@@ -255,7 +255,7 @@ namespace winrt::TerminalApp::implementation
         winrt::Windows::UI::Xaml::Controls::Grid::LayoutUpdated_revoker _layoutUpdatedRevoker;
         StartupState _startupState{ StartupState::NotInitialized };
 
-        Windows::Foundation::Collections::IVector<Microsoft::Terminal::Settings::Model::ActionAndArgs> _startupActions;
+        std::vector<Microsoft::Terminal::Settings::Model::ActionAndArgs> _startupActions;
         bool _shouldStartInboundListener{ false };
         bool _isEmbeddingInboundListener{ false };
 

src/cascadia/TerminalApp/TerminalWindow.cpp

@@ -1054,12 +1054,8 @@ namespace winrt::TerminalApp::implementation
     {
         _contentBounds = bounds;
 
-        const auto& args = _contentStringToActions(content, true);
-
-        for (const auto& action : args)
-        {
-            _initialContentArgs.push_back(action);
-        }
+        const auto args = _contentStringToActions(content, true);
+        _initialContentArgs = wil::to_vector(args);
     }
 
     // Method Description:
@@ -1085,7 +1081,7 @@ namespace winrt::TerminalApp::implementation
         if (_appArgs->ExitCode() == 0)
         {
             auto& parsedArgs = _appArgs->ParsedArgs();
-            auto actions = winrt::single_threaded_vector<ActionAndArgs>(std::move(parsedArgs.GetStartupActions()));
+            auto& actions = parsedArgs.GetStartupActions();
 
             _root->ProcessStartupActions(actions, false, _appArgs->CurrentDirectory(), _appArgs->CurrentEnvironment());
 
@@ -1200,7 +1196,7 @@ namespace winrt::TerminalApp::implementation
     {
         try
         {
-            const auto& args = ActionAndArgs::Deserialize(content);
+            const auto args = ActionAndArgs::Deserialize(content);
             if (args == nullptr ||
                 args.Size() == 0)
             {
@@ -1244,9 +1240,9 @@ namespace winrt::TerminalApp::implementation
 
             const bool replaceFirstWithNewTab = tabIndex >= _root->NumberOfTabs();
 
-            const auto& args = _contentStringToActions(content, replaceFirstWithNewTab);
+            auto args = _contentStringToActions(content, replaceFirstWithNewTab);
 
-            _root->AttachContent(args, tabIndex);
+            _root->AttachContent(std::move(args), tabIndex);
         }
     }
     void TerminalWindow::SendContentToOther(winrt::TerminalApp::RequestReceiveContentArgs args)