From b8a83fba1ba30f074733d45ce9242be4895ea9b2 Mon Sep 17 00:00:00 2001 From: Davide Giacometti Date: Fri, 15 Sep 2023 08:34:17 +0200 Subject: [PATCH] [BugReport]Fix hang when bug report is launched (#28506) * fix hang when bug report is launched from flyout * Normalize output dir for release --- .pipelines/ESRPSigning_core.json | 2 +- installer/PowerToysSetup/Tools.wxs | 2 +- src/runner/bug_report.cpp | 33 ++++++++++++++ src/runner/bug_report.h | 3 ++ src/runner/runner.vcxproj | 2 + src/runner/runner.vcxproj.filters | 6 +++ src/runner/settings_window.cpp | 41 +++++++----------- src/runner/tray_icon.cpp | 43 +++++-------------- .../SettingsXAML/Flyout/LaunchPage.xaml.cs | 4 ++ .../BugReportTool/BugReportTool.vcxproj | 12 ++---- 10 files changed, 78 insertions(+), 70 deletions(-) create mode 100644 src/runner/bug_report.cpp create mode 100644 src/runner/bug_report.h diff --git a/.pipelines/ESRPSigning_core.json b/.pipelines/ESRPSigning_core.json index d5c6ff120b..c3c91deb6a 100644 --- a/.pipelines/ESRPSigning_core.json +++ b/.pipelines/ESRPSigning_core.json @@ -14,7 +14,7 @@ "PowerToys.exe", "PowerToys.FilePreviewCommon.dll", "PowerToys.Interop.dll", - "BugReportTool\\PowerToys.BugReportTool.exe", + "Tools\\PowerToys.BugReportTool.exe", "WebcamReportTool\\PowerToys.WebcamReportTool.exe", "StylesReportTool\\PowerToys.StylesReportTool.exe", "Telemetry.dll", diff --git a/installer/PowerToysSetup/Tools.wxs b/installer/PowerToysSetup/Tools.wxs index a9fe0bf306..57b6269f09 100644 --- a/installer/PowerToysSetup/Tools.wxs +++ b/installer/PowerToysSetup/Tools.wxs @@ -10,7 +10,7 @@ - + diff --git a/src/runner/bug_report.cpp b/src/runner/bug_report.cpp new file mode 100644 index 0000000000..9abfe6fa18 --- /dev/null +++ b/src/runner/bug_report.cpp @@ -0,0 +1,33 @@ +#include "pch.h" +#include "bug_report.h" +#include "Generated files/resource.h" +#include +#include + +std::atomic_bool isBugReportThreadRunning = false; + +void launch_bug_report() noexcept +{ + std::wstring bug_report_path = get_module_folderpath(); + bug_report_path += L"\\Tools\\PowerToys.BugReportTool.exe"; + + bool expected_isBugReportThreadRunning = false; + if (isBugReportThreadRunning.compare_exchange_strong(expected_isBugReportThreadRunning, true)) + { + std::thread([bug_report_path]() { + SHELLEXECUTEINFOW sei{ sizeof(sei) }; + sei.fMask = { SEE_MASK_FLAG_NO_UI | SEE_MASK_NOASYNC | SEE_MASK_NOCLOSEPROCESS | SEE_MASK_NO_CONSOLE }; + sei.lpFile = bug_report_path.c_str(); + sei.nShow = SW_HIDE; + if (ShellExecuteExW(&sei)) + { + WaitForSingleObject(sei.hProcess, INFINITE); + CloseHandle(sei.hProcess); + static const std::wstring bugreport_success = GET_RESOURCE_STRING(IDS_BUGREPORT_SUCCESS); + MessageBoxW(nullptr, bugreport_success.c_str(), L"PowerToys", MB_OK); + } + + isBugReportThreadRunning.store(false); + }).detach(); + } +} diff --git a/src/runner/bug_report.h b/src/runner/bug_report.h new file mode 100644 index 0000000000..2d7084ea21 --- /dev/null +++ b/src/runner/bug_report.h @@ -0,0 +1,3 @@ +#pragma once + +void launch_bug_report() noexcept; \ No newline at end of file diff --git a/src/runner/runner.vcxproj b/src/runner/runner.vcxproj index c05245142b..3671cc3d56 100644 --- a/src/runner/runner.vcxproj +++ b/src/runner/runner.vcxproj @@ -48,6 +48,7 @@ + @@ -67,6 +68,7 @@ + diff --git a/src/runner/runner.vcxproj.filters b/src/runner/runner.vcxproj.filters index a289bb95dd..e8fad83d26 100644 --- a/src/runner/runner.vcxproj.filters +++ b/src/runner/runner.vcxproj.filters @@ -42,6 +42,9 @@ Utils + + Utils + @@ -87,6 +90,9 @@ Utils + + Utils + diff --git a/src/runner/settings_window.cpp b/src/runner/settings_window.cpp index f2bf54b585..bcba0915b1 100644 --- a/src/runner/settings_window.cpp +++ b/src/runner/settings_window.cpp @@ -26,6 +26,7 @@ #include #include #include "settings_window.h" +#include "bug_report.h" #define BUFSIZE 1024 @@ -108,7 +109,7 @@ std::optional dispatch_json_action_to_module(const json::JsonObjec else if (action == L"check_for_updates") { bool expected_isUpdateCheckThreadRunning = false; - if (isUpdateCheckThreadRunning.compare_exchange_strong(expected_isUpdateCheckThreadRunning,true)) + if (isUpdateCheckThreadRunning.compare_exchange_strong(expected_isUpdateCheckThreadRunning, true)) { std::thread([]() { CheckForUpdatesCallback(); @@ -222,19 +223,7 @@ void dispatch_received_json(const std::wstring& json_to_parse) } else if (name == L"bugreport") { - std::wstring bug_report_path = get_module_folderpath(); - bug_report_path += L"\\Tools\\PowerToys.BugReportTool.exe"; - SHELLEXECUTEINFOW sei{ sizeof(sei) }; - sei.fMask = { SEE_MASK_FLAG_NO_UI | SEE_MASK_NOASYNC | SEE_MASK_NOCLOSEPROCESS | SEE_MASK_NO_CONSOLE }; - sei.lpFile = bug_report_path.c_str(); - sei.nShow = SW_HIDE; - if (ShellExecuteExW(&sei)) - { - WaitForSingleObject(sei.hProcess, INFINITE); - CloseHandle(sei.hProcess); - static const std::wstring bugreport_success = GET_RESOURCE_STRING(IDS_BUGREPORT_SUCCESS); - MessageBoxW(nullptr, bugreport_success.c_str(), L"PowerToys", MB_OK); - } + launch_bug_report(); } else if (name == L"killrunner") { @@ -410,18 +399,18 @@ void run_settings_window(bool show_oobe_window, bool show_scoobe_window, std::op PTSettingsHelper::save_general_settings(save_settings.to_json()); std::wstring executable_args = fmt::format(L"\"{}\" {} {} {} {} {} {} {} {} {} {} {}", - executable_path, - powertoys_pipe_name, - settings_pipe_name, - std::to_wstring(powertoys_pid), - settings_theme, - settings_elevatedStatus, - settings_isUserAnAdmin, - settings_showOobe, - settings_showScoobe, - settings_showFlyout, - settings_containsSettingsWindow, - settings_containsFlyoutPosition); + executable_path, + powertoys_pipe_name, + settings_pipe_name, + std::to_wstring(powertoys_pid), + settings_theme, + settings_elevatedStatus, + settings_isUserAnAdmin, + settings_showOobe, + settings_showScoobe, + settings_showFlyout, + settings_containsSettingsWindow, + settings_containsFlyoutPosition); if (settings_window.has_value()) { diff --git a/src/runner/tray_icon.cpp b/src/runner/tray_icon.cpp index 7dab06ac7d..6987a6d1cc 100644 --- a/src/runner/tray_icon.cpp +++ b/src/runner/tray_icon.cpp @@ -6,17 +6,18 @@ #include "centralized_kb_hook.h" #include -#include #include #include #include #include +#include "bug_report.h" namespace { HWND tray_icon_hwnd = NULL; - enum { + enum + { wm_icon_notify = WM_APP, wm_run_on_main_ui_thread, }; @@ -44,8 +45,6 @@ struct run_on_main_ui_thread_msg PVOID data; }; -std::atomic_bool isBugReportThreadRunning = false; - bool dispatch_run_on_main_ui_thread(main_loop_callback_function _callback, PVOID data) { if (tray_icon_hwnd == NULL) @@ -74,11 +73,11 @@ void handle_tray_command(HWND window, const WPARAM command_id, LPARAM lparam) switch (command_id) { case ID_SETTINGS_MENU_COMMAND: - { - std::wstring settings_window{ winrt::to_hstring(ESettingsWindowNames_to_string(static_cast(lparam))) }; - open_settings_window(settings_window, false); - } - break; + { + std::wstring settings_window{ winrt::to_hstring(ESettingsWindowNames_to_string(static_cast(lparam))) }; + open_settings_window(settings_window, false); + } + break; case ID_EXIT_MENU_COMMAND: if (h_menu) { @@ -97,29 +96,7 @@ void handle_tray_command(HWND window, const WPARAM command_id, LPARAM lparam) break; case ID_REPORT_BUG_COMMAND: { - std::wstring bug_report_path = get_module_folderpath(); - bug_report_path += L"\\Tools\\PowerToys.BugReportTool.exe"; - - bool expected_isBugReportThreadRunning = false; - if (isBugReportThreadRunning.compare_exchange_strong(expected_isBugReportThreadRunning, true)) - { - std::thread([bug_report_path]() { - SHELLEXECUTEINFOW sei{ sizeof(sei) }; - sei.fMask = { SEE_MASK_FLAG_NO_UI | SEE_MASK_NOASYNC | SEE_MASK_NOCLOSEPROCESS | SEE_MASK_NO_CONSOLE }; - sei.lpFile = bug_report_path.c_str(); - sei.nShow = SW_HIDE; - if (ShellExecuteExW(&sei)) - { - WaitForSingleObject(sei.hProcess, INFINITE); - CloseHandle(sei.hProcess); - static const std::wstring bugreport_success = GET_RESOURCE_STRING(IDS_BUGREPORT_SUCCESS); - MessageBoxW(nullptr, bugreport_success.c_str(), L"PowerToys", MB_OK); - } - - isBugReportThreadRunning.store(false); - }).detach(); - } - + launch_bug_report(); break; } @@ -230,7 +207,7 @@ LRESULT __stdcall tray_icon_window_proc(HWND window, UINT message, WPARAM wparam // start timer for detecting single or double click double_click_timer_running = true; double_clicked = false; - + UINT doubleClickTime = GetDoubleClickTime(); std::thread([doubleClickTime]() { std::this_thread::sleep_for(std::chrono::milliseconds(doubleClickTime)); diff --git a/src/settings-ui/Settings.UI/SettingsXAML/Flyout/LaunchPage.xaml.cs b/src/settings-ui/Settings.UI/SettingsXAML/Flyout/LaunchPage.xaml.cs index 1aff564223..ebb1928937 100644 --- a/src/settings-ui/Settings.UI/SettingsXAML/Flyout/LaunchPage.xaml.cs +++ b/src/settings-ui/Settings.UI/SettingsXAML/Flyout/LaunchPage.xaml.cs @@ -13,6 +13,7 @@ using Microsoft.PowerToys.Telemetry; using Microsoft.UI.Xaml; using Microsoft.UI.Xaml.Controls; using Microsoft.UI.Xaml.Media.Animation; +using WinUIEx; namespace Microsoft.PowerToys.Settings.UI.Flyout { @@ -141,6 +142,9 @@ namespace Microsoft.PowerToys.Settings.UI.Flyout private void ReportBugBtn_Click(object sender, RoutedEventArgs e) { ViewModel.StartBugReport(); + + // Closing manually the flyout since no window will steal the focus + App.GetFlyoutWindow()?.Hide(); } } } diff --git a/tools/BugReportTool/BugReportTool/BugReportTool.vcxproj b/tools/BugReportTool/BugReportTool/BugReportTool.vcxproj index e7b06e500d..0cb309d170 100644 --- a/tools/BugReportTool/BugReportTool/BugReportTool.vcxproj +++ b/tools/BugReportTool/BugReportTool/BugReportTool.vcxproj @@ -1,4 +1,4 @@ - + @@ -15,15 +15,9 @@ Application - $(SolutionDir)..\..\$(Platform)\$(Configuration)\$(ProjectName)\ - - PowerToys.$(ProjectName) + $(SolutionDir)..\..\$(Platform)\$(Configuration)\Tools\ - - PowerToys.$(ProjectName) - - @@ -45,7 +39,7 @@ - 4706;26451;4267;4244;%(DisableSpecificWarnings) + 4706;26451;4267;4244;%(DisableSpecificWarnings)