Skip to content

Commit e314485

Browse files
shuaiyuanxxCopilot
andauthored
Improve module enable/disable IPC and sorting reliability (#44734)
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request - Refactored the runner logic for handling individual module enable/disable updates. Instead of receiving the entire settings.json via IPC, it now processes only single-module state updates, which avoids race conditions and fixes a bug where modules could end up being skipped. - Fixed an issue where the sort order option could be deselected — it is now enforced as a mutually exclusive choice. - Fixed a potential race condition when updating the AppList control’s sorting. <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [x] Closes: #44697 <!-- - [ ] Closes: #yyy (add separate lines for additional resolved issues) --> - [x] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [x] **Tests:** Added/updated and all pass - [x] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx <!-- Provide a more detailed description of the PR, other things fixed, or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed --------- Co-authored-by: Copilot <[email protected]>
1 parent f48c4a9 commit e314485

File tree

10 files changed

+295
-103
lines changed

10 files changed

+295
-103
lines changed

src/runner/general_settings.cpp

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,102 @@ GeneralSettings get_general_settings()
193193
return settings;
194194
}
195195

196+
void apply_module_status_update(const json::JsonObject& module_config, bool save)
197+
{
198+
Logger::info(L"apply_module_status_update: {}", std::wstring{ module_config.ToString() });
199+
200+
// Expected format: {"ModuleName": true/false} - only one module per update
201+
auto iter = module_config.First();
202+
if (!iter.HasCurrent())
203+
{
204+
Logger::warn(L"apply_module_status_update: Empty module config");
205+
return;
206+
}
207+
208+
const auto& element = iter.Current();
209+
const auto value = element.Value();
210+
if (value.ValueType() != json::JsonValueType::Boolean)
211+
{
212+
Logger::warn(L"apply_module_status_update: Invalid value type for module status");
213+
return;
214+
}
215+
216+
const std::wstring name{ element.Key().c_str() };
217+
if (modules().find(name) == modules().end())
218+
{
219+
Logger::warn(L"apply_module_status_update: Module {} not found", name);
220+
return;
221+
}
222+
223+
PowertoyModule& powertoy = modules().at(name);
224+
const bool module_inst_enabled = powertoy->is_enabled();
225+
bool target_enabled = value.GetBoolean();
226+
227+
auto gpo_rule = powertoy->gpo_policy_enabled_configuration();
228+
if (gpo_rule == powertoys_gpo::gpo_rule_configured_enabled || gpo_rule == powertoys_gpo::gpo_rule_configured_disabled)
229+
{
230+
// Apply the GPO Rule.
231+
target_enabled = gpo_rule == powertoys_gpo::gpo_rule_configured_enabled;
232+
}
233+
234+
if (module_inst_enabled == target_enabled)
235+
{
236+
Logger::info(L"apply_module_status_update: Module {} already in target state {}", name, target_enabled);
237+
return;
238+
}
239+
240+
if (target_enabled)
241+
{
242+
Logger::info(L"apply_module_status_update: Enabling powertoy {}", name);
243+
powertoy->enable();
244+
auto& hkmng = HotkeyConflictDetector::HotkeyConflictManager::GetInstance();
245+
hkmng.EnableHotkeyByModule(name);
246+
247+
// Trigger AI capability detection when ImageResizer is enabled
248+
if (name == L"Image Resizer")
249+
{
250+
Logger::info(L"ImageResizer enabled, triggering AI capability detection");
251+
DetectAiCapabilitiesAsync(true); // Skip settings check since we know it's being enabled
252+
}
253+
}
254+
else
255+
{
256+
Logger::info(L"apply_module_status_update: Disabling powertoy {}", name);
257+
powertoy->disable();
258+
auto& hkmng = HotkeyConflictDetector::HotkeyConflictManager::GetInstance();
259+
hkmng.DisableHotkeyByModule(name);
260+
}
261+
// Sync the hotkey state with the module state, so it can be removed for disabled modules.
262+
powertoy.UpdateHotkeyEx();
263+
264+
if (save)
265+
{
266+
// Load existing settings and only update the specific module's enabled state
267+
json::JsonObject current_settings = PTSettingsHelper::load_general_settings();
268+
269+
json::JsonObject enabled;
270+
if (current_settings.HasKey(L"enabled"))
271+
{
272+
enabled = current_settings.GetNamedObject(L"enabled");
273+
}
274+
275+
// Check if the saved state is different from the requested state
276+
bool current_saved = enabled.HasKey(name) ? enabled.GetNamedBoolean(name, true) : true;
277+
278+
if (current_saved != target_enabled)
279+
{
280+
// Update only this module's enabled state
281+
enabled.SetNamedValue(name, json::value(target_enabled));
282+
current_settings.SetNamedValue(L"enabled", enabled);
283+
284+
PTSettingsHelper::save_general_settings(current_settings);
285+
286+
GeneralSettings settings_for_trace = get_general_settings();
287+
Trace::SettingsChanged(settings_for_trace);
288+
}
289+
}
290+
}
291+
196292
void apply_general_settings(const json::JsonObject& general_configs, bool save)
197293
{
198294
std::wstring old_settings_json_string;

src/runner/general_settings.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,4 +38,5 @@ struct GeneralSettings
3838
json::JsonObject load_general_settings();
3939
GeneralSettings get_general_settings();
4040
void apply_general_settings(const json::JsonObject& general_configs, bool save = true);
41+
void apply_module_status_update(const json::JsonObject& module_config, bool save = true);
4142
void start_enabled_powertoys();

src/runner/settings_window.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,12 @@ void dispatch_received_json(const std::wstring& json_to_parse)
215215
// current_settings_ipc->send(settings_string);
216216
// }
217217
}
218+
else if (name == L"module_status")
219+
{
220+
// Handle single module enable/disable update
221+
// Expected format: {"module_status": {"ModuleName": true/false}}
222+
apply_module_status_update(value.GetObjectW());
223+
}
218224
else if (name == L"powertoys")
219225
{
220226
dispatch_json_config_to_modules(value.GetObjectW());

src/settings-ui/QuickAccess.UI/QuickAccessXAML/Flyout/AppsListPage.xaml.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ private void SortAlphabetical_Click(object sender, RoutedEventArgs e)
5151
if (ViewModel != null)
5252
{
5353
ViewModel.DashboardSortOrder = DashboardSortOrder.Alphabetical;
54+
((ToggleMenuFlyoutItem)sender).IsChecked = true;
5455
}
5556
}
5657

@@ -59,6 +60,7 @@ private void SortByStatus_Click(object sender, RoutedEventArgs e)
5960
if (ViewModel != null)
6061
{
6162
ViewModel.DashboardSortOrder = DashboardSortOrder.ByStatus;
63+
((ToggleMenuFlyoutItem)sender).IsChecked = true;
6264
}
6365
}
6466
}

src/settings-ui/QuickAccess.UI/Services/IQuickAccessCoordinator.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
// See the LICENSE file in the project root for more information.
44

55
using System.Threading.Tasks;
6+
67
using ManagedCommon;
8+
using Microsoft.PowerToys.Settings.UI.Library;
79

810
namespace Microsoft.PowerToys.QuickAccess.Services;
911

@@ -19,10 +21,10 @@ public interface IQuickAccessCoordinator
1921

2022
Task<bool> ShowDocumentationAsync();
2123

22-
void NotifyUserSettingsInteraction();
23-
2424
bool UpdateModuleEnabled(ModuleType moduleType, bool isEnabled);
2525

26+
void SendSortOrderUpdate(GeneralSettings generalSettings);
27+
2628
void ReportBug();
2729

2830
void OnModuleLaunched(ModuleType moduleType);

src/settings-ui/QuickAccess.UI/Services/QuickAccessCoordinator.cs

Lines changed: 4 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -55,37 +55,8 @@ public Task<bool> ShowDocumentationAsync()
5555
return Task.FromResult(false);
5656
}
5757

58-
public void NotifyUserSettingsInteraction()
59-
{
60-
Logger.LogDebug("QuickAccessCoordinator.NotifyUserSettingsInteraction invoked.");
61-
}
62-
6358
public bool UpdateModuleEnabled(ModuleType moduleType, bool isEnabled)
64-
{
65-
GeneralSettings? updatedSettings = null;
66-
lock (_generalSettingsLock)
67-
{
68-
var repository = SettingsRepository<GeneralSettings>.GetInstance(_settingsUtils);
69-
var generalSettings = repository.SettingsConfig;
70-
var current = ModuleHelper.GetIsModuleEnabled(generalSettings, moduleType);
71-
if (current == isEnabled)
72-
{
73-
return false;
74-
}
75-
76-
ModuleHelper.SetIsModuleEnabled(generalSettings, moduleType, isEnabled);
77-
_settingsUtils.SaveSettings(generalSettings.ToJsonString());
78-
Logger.LogInfo($"QuickAccess updated module '{moduleType}' enabled state to {isEnabled}.");
79-
updatedSettings = generalSettings;
80-
}
81-
82-
if (updatedSettings != null)
83-
{
84-
SendGeneralSettingsUpdate(updatedSettings);
85-
}
86-
87-
return true;
88-
}
59+
=> TrySendIpcMessage($"{{\"module_status\": {{\"{ModuleHelper.GetModuleKey(moduleType)}\": {isEnabled.ToString().ToLowerInvariant()}}}}}", "module status update");
8960

9061
public void ReportBug()
9162
{
@@ -131,20 +102,10 @@ private void OnIpcMessageReceived(string message)
131102
Logger.LogDebug($"QuickAccessCoordinator received IPC payload: {message}");
132103
}
133104

134-
private void SendGeneralSettingsUpdate(GeneralSettings updatedSettings)
105+
public void SendSortOrderUpdate(GeneralSettings generalSettings)
135106
{
136-
string payload;
137-
try
138-
{
139-
payload = new OutGoingGeneralSettings(updatedSettings).ToString();
140-
}
141-
catch (Exception ex)
142-
{
143-
Logger.LogError("QuickAccessCoordinator: failed to serialize general settings payload.", ex);
144-
return;
145-
}
146-
147-
TrySendIpcMessage(payload, "general settings update");
107+
var outgoing = new OutGoingGeneralSettings(generalSettings);
108+
TrySendIpcMessage(outgoing.ToString(), "sort order update");
148109
}
149110

150111
private bool TrySendIpcMessage(string payload, string operationDescription)

src/settings-ui/QuickAccess.UI/ViewModels/AllAppsViewModel.cs

Lines changed: 57 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ namespace Microsoft.PowerToys.QuickAccess.ViewModels;
2222

2323
public sealed class AllAppsViewModel : Observable
2424
{
25+
private readonly object _sortLock = new object();
2526
private readonly IQuickAccessCoordinator _coordinator;
2627
private readonly ISettingsRepository<GeneralSettings> _settingsRepository;
2728
private readonly SettingsUtils _settingsUtils;
@@ -30,6 +31,9 @@ public sealed class AllAppsViewModel : Observable
3031
private readonly List<FlyoutMenuItem> _allFlyoutMenuItems = new();
3132
private GeneralSettings _generalSettings;
3233

34+
// Flag to prevent toggle operations during sorting to avoid race conditions.
35+
private bool _isSorting;
36+
3337
public ObservableCollection<FlyoutMenuItem> FlyoutMenuItems { get; }
3438

3539
public DashboardSortOrder DashboardSortOrder
@@ -40,9 +44,9 @@ public DashboardSortOrder DashboardSortOrder
4044
if (_generalSettings.DashboardSortOrder != value)
4145
{
4246
_generalSettings.DashboardSortOrder = value;
43-
_settingsUtils.SaveSettings(_generalSettings.ToJsonString(), _generalSettings.GetModuleName());
47+
_coordinator.SendSortOrderUpdate(_generalSettings);
4448
OnPropertyChanged();
45-
RefreshFlyoutMenuItems();
49+
SortFlyoutMenuItems();
4650
}
4751
}
4852
}
@@ -54,7 +58,6 @@ public AllAppsViewModel(IQuickAccessCoordinator coordinator)
5458
_settingsUtils = SettingsUtils.Default;
5559
_settingsRepository = SettingsRepository<GeneralSettings>.GetInstance(_settingsUtils);
5660
_generalSettings = _settingsRepository.SettingsConfig;
57-
_generalSettings.AddEnabledModuleChangeNotification(ModuleEnabledChangedOnSettingsPage);
5861
_settingsRepository.SettingsChanged += OnSettingsChanged;
5962

6063
_resourceLoader = Helpers.ResourceLoaderInstance.ResourceLoader;
@@ -87,7 +90,6 @@ private void OnSettingsChanged(GeneralSettings newSettings)
8790
_dispatcherQueue.TryEnqueue(() =>
8891
{
8992
_generalSettings = newSettings;
90-
_generalSettings.AddEnabledModuleChangeNotification(ModuleEnabledChangedOnSettingsPage);
9193
OnPropertyChanged(nameof(DashboardSortOrder));
9294
RefreshFlyoutMenuItems();
9395
});
@@ -120,48 +122,73 @@ private void RefreshFlyoutMenuItems()
120122
}
121123
}
122124

123-
var sortedItems = DashboardSortOrder switch
124-
{
125-
DashboardSortOrder.ByStatus => _allFlyoutMenuItems.OrderByDescending(x => x.IsEnabled).ThenBy(x => x.Label).ToList(),
126-
_ => _allFlyoutMenuItems.OrderBy(x => x.Label).ToList(),
127-
};
125+
SortFlyoutMenuItems();
126+
}
128127

129-
if (FlyoutMenuItems.Count == 0)
128+
private void SortFlyoutMenuItems()
129+
{
130+
if (_isSorting)
130131
{
131-
foreach (var item in sortedItems)
132-
{
133-
FlyoutMenuItems.Add(item);
134-
}
135-
136132
return;
137133
}
138134

139-
for (int i = 0; i < sortedItems.Count; i++)
135+
lock (_sortLock)
140136
{
141-
var item = sortedItems[i];
142-
var oldIndex = FlyoutMenuItems.IndexOf(item);
143-
144-
if (oldIndex != -1 && oldIndex != i)
137+
_isSorting = true;
138+
try
145139
{
146-
FlyoutMenuItems.Move(oldIndex, i);
140+
var sortedItems = DashboardSortOrder switch
141+
{
142+
DashboardSortOrder.ByStatus => _allFlyoutMenuItems.OrderByDescending(x => x.IsEnabled).ThenBy(x => x.Label).ToList(),
143+
_ => _allFlyoutMenuItems.OrderBy(x => x.Label).ToList(),
144+
};
145+
146+
if (FlyoutMenuItems.Count == 0)
147+
{
148+
foreach (var item in sortedItems)
149+
{
150+
FlyoutMenuItems.Add(item);
151+
}
152+
153+
return;
154+
}
155+
156+
for (int i = 0; i < sortedItems.Count; i++)
157+
{
158+
var item = sortedItems[i];
159+
var oldIndex = FlyoutMenuItems.IndexOf(item);
160+
161+
if (oldIndex != -1 && oldIndex != i)
162+
{
163+
FlyoutMenuItems.Move(oldIndex, i);
164+
}
165+
}
166+
}
167+
finally
168+
{
169+
// Use dispatcher to reset flag after UI updates complete
170+
_dispatcherQueue.TryEnqueue(DispatcherQueuePriority.Low, () =>
171+
{
172+
_isSorting = false;
173+
});
147174
}
148175
}
149176
}
150177

151178
private void EnabledChangedOnUI(ModuleListItem item)
152179
{
153180
var flyoutItem = (FlyoutMenuItem)item;
154-
if (_coordinator.UpdateModuleEnabled(flyoutItem.Tag, flyoutItem.IsEnabled))
155-
{
156-
_coordinator.NotifyUserSettingsInteraction();
181+
var isEnabled = flyoutItem.IsEnabled;
157182

158-
// Trigger re-sort immediately when status changes on UI
159-
RefreshFlyoutMenuItems();
183+
// Ignore toggle operations during sorting to prevent race conditions.
184+
// Revert the toggle state since UI already changed due to TwoWay binding.
185+
if (_isSorting)
186+
{
187+
flyoutItem.UpdateStatus(!isEnabled);
188+
return;
160189
}
161-
}
162190

163-
private void ModuleEnabledChangedOnSettingsPage()
164-
{
165-
RefreshFlyoutMenuItems();
191+
_coordinator.UpdateModuleEnabled(flyoutItem.Tag, flyoutItem.IsEnabled);
192+
SortFlyoutMenuItems();
166193
}
167194
}

0 commit comments

Comments
 (0)