Repaint every row sharing a toggled profile's name, not just one

vibrance.GUI/common/ProfileToggleFixture.cs

@@ -1112,10 +1112,13 @@ private static void CheckClearSuppressionLeavesADifferentSuppressionAlone(Checkl
 
         // ------------------------------------------------------------------
         // Application list marker - VibranceGUI.DescribeListItem (the pure decision
-        // ApplyApplicationListItemAppearance turns into ListViewItem.Text/ForeColor/ToolTipText)
-        // and VibranceGUI.ShouldRefreshListItemForToggleResult (the gate
-        // RefreshToggledListItemAppearance opens on). Both are internal statics called directly -
-        // no reflection, no Form: ApplyApplicationListItemAppearance and
+        // ApplyApplicationListItemAppearance turns into ListViewItem.Text/ForeColor/ToolTipText),
+        // VibranceGUI.ShouldRefreshListItemForToggleResult (the gate
+        // RefreshToggledListItemAppearance opens on), and VibranceGUI.FindApplicationSettingsByName
+        // (the decision behind that same method's repaint - which of possibly SEVERAL settings
+        // sharing one Name need their row redrawn, see that method's own comment for why two
+        // settings can share a Name at all). All three are internal statics called directly - no
+        // reflection, no Form: ApplyApplicationListItemAppearance and
         // RefreshToggledListItemAppearance themselves need a real ListView on a real Form
         // (VibranceGUI's own constructor calls getProxy(...)), which is exactly why the decision
         // each one makes is pulled out into something a fixture CAN reach - the real gate, not a
@@ -1124,7 +1127,7 @@ private static void CheckClearSuppressionLeavesADifferentSuppressionAlone(Checkl
 
         private static void RunListItemMarkerChecks(Checklist checklist)
         {
-            checklist.Lines.Add("Application list marker (VibranceGUI.DescribeListItem / ShouldRefreshListItemForToggleResult, real methods, no reflection):");
+            checklist.Lines.Add("Application list marker (VibranceGUI.DescribeListItem / ShouldRefreshListItemForToggleResult / FindApplicationSettingsByName, real methods, no reflection):");
 
             CheckDescribeListItemNeitherFlag(checklist);
             CheckDescribeListItemUnconfirmedOnly(checklist);
@@ -1136,6 +1139,13 @@ private static void RunListItemMarkerChecks(Checklist checklist)
             CheckDescribeListItemNullNameDoesNotThrow(checklist);
             CheckMarkerSuffixesAreDistinct(checklist);
             CheckShouldRefreshOnlyOnConfirmedToggles(checklist);
+            CheckFindApplicationSettingsByNameReturnsOnlyTheMatchingOne(checklist);
+            CheckFindApplicationSettingsByNameReturnsEveryProfileSharingTheName(checklist);
+            CheckFindApplicationSettingsByNameExcludesADifferentName(checklist);
+            CheckFindApplicationSettingsByNameIsCaseInsensitive(checklist);
+            CheckFindApplicationSettingsByNameOnNullSettingsReturnsEmpty(checklist);
+            CheckFindApplicationSettingsByNameOnNullOrEmptyNameReturnsEmpty(checklist);
+            CheckFindApplicationSettingsByNameSkipsNullEntries(checklist);
 
             checklist.Lines.Add(string.Empty);
         }
@@ -1313,6 +1323,168 @@ private static void CheckShouldRefreshOnlyOnConfirmedToggles(Checklist checklist
                     onOk, offOk, noneOk, engineNotReadyOk, writeFailedOk));
         }
 
+        // O1-O7. VibranceGUI.FindApplicationSettingsByName - the decision behind
+        // RefreshToggledListItemAppearance's repaint. Two ApplicationSetting entries CAN share one
+        // Name: Name is Path.GetFileNameWithoutExtension of whatever executable the user picked
+        // (VibranceSettings.resolveApplicationName), and nothing stops two installs - a demo and
+        // the full game, two store copies, two unrelated games - from producing the same bare file
+        // name while their FileName (the full path) stays distinct. ProfileToggleHelper's
+        // suppression set is keyed by that Name, so one hotkey press suppresses every entry that
+        // shares it; this function is what turns "the one setting ApplicationSettingMatcher.
+        // FindMatch resolved" into "every row whose suppression state just changed together with
+        // it" - the fix for the defect where only the resolved row repainted and any other
+        // same-Name row went stale.
+
+        // O1. The common case: one setting has the toggled Name, a second has a different one.
+        // Mutation this guards: returning every setting regardless of Name (e.g. the guard clause
+        // deleted), or comparing the wrong field (FileName instead of Name).
+        private static void CheckFindApplicationSettingsByNameReturnsOnlyTheMatchingOne(Checklist checklist)
+        {
+            ApplicationSetting target = new ApplicationSetting { Name = "game", FileName = @"D:\A\game.exe" };
+            ApplicationSetting other = new ApplicationSetting { Name = "otherGame", FileName = @"D:\B\otherGame.exe" };
+            List<ApplicationSetting> settings = new List<ApplicationSetting> { target, other };
+
+            List<ApplicationSetting> result = VibranceGUI.FindApplicationSettingsByName(settings, "game");
+
+            checklist.Check(result.Count == 1 && result[0] == target,
+                string.Format("O1: only the setting whose Name matches is returned, got count={0} containsTarget={1}",
+                    result.Count, result.Contains(target)));
+        }
+
+        // O2. The defect itself: TWO settings share the toggled Name (distinct FileName, as two
+        // installs of a same-named executable would be) - both must come back, not just the one
+        // ApplicationSettingMatcher.FindMatch happened to resolve. Mutation this guards: an early
+        // "return" the moment one match is found instead of continuing the loop - exactly the bug
+        // RefreshToggledListItemAppearance used to have via FindMatch's own single-result contract.
+        private static void CheckFindApplicationSettingsByNameReturnsEveryProfileSharingTheName(Checklist checklist)
+        {
+            ApplicationSetting first = new ApplicationSetting { Name = "game", FileName = @"D:\A\game.exe" };
+            ApplicationSetting second = new ApplicationSetting { Name = "game", FileName = @"D:\B\game.exe" };
+            List<ApplicationSetting> settings = new List<ApplicationSetting> { first, second };
+
+            List<ApplicationSetting> result = VibranceGUI.FindApplicationSettingsByName(settings, "game");
+
+            checklist.Check(result.Count == 2 && result.Contains(first) && result.Contains(second),
+                string.Format("O2: two settings sharing one Name (distinct FileName) both come back, got count={0} containsFirst={1} containsSecond={2}",
+                    result.Count, result.Contains(first), result.Contains(second)));
+        }
+
+        // O3. Companion to O2 - a third, unrelated setting must NOT be swept in alongside the two
+        // that share the toggled Name. Mutation this guards: the loop's condition dropped
+        // entirely (returning the whole list unfiltered) - undetectable by O2 alone, since O2's
+        // two settings legitimately share a Name and a "return everything" bug would still pass
+        // O1 if settings only ever held two entries, but not once a third, non-matching one is
+        // added to the same list.
+        private static void CheckFindApplicationSettingsByNameExcludesADifferentName(Checklist checklist)
+        {
+            ApplicationSetting first = new ApplicationSetting { Name = "game", FileName = @"D:\A\game.exe" };
+            ApplicationSetting second = new ApplicationSetting { Name = "game", FileName = @"D:\B\game.exe" };
+            ApplicationSetting unrelated = new ApplicationSetting { Name = "otherGame", FileName = @"D:\C\otherGame.exe" };
+            List<ApplicationSetting> settings = new List<ApplicationSetting> { first, second, unrelated };
+
+            List<ApplicationSetting> result = VibranceGUI.FindApplicationSettingsByName(settings, "game");
+
+            checklist.Check(result.Count == 2 && !result.Contains(unrelated),
+                string.Format("O3: a differently-named setting in the same list is excluded, got count={0} containsUnrelated={1}",
+                    result.Count, result.Contains(unrelated)));
+        }
+
+        // O4. The comparison must be case-insensitive, exactly like ProfileToggleHelper's own
+        // suppression set (NameMatches, ApplicationSettingMatcher.cs:89-94) - and, per
+        // ProfileToggleHelper.NameComparer's own comment, derived from that SAME comparer rather
+        // than a second, independent one. Mutation this guards: comparing with
+        // StringComparison.Ordinal (or plain "==") instead of ProfileToggleHelper.NameComparer.
+        private static void CheckFindApplicationSettingsByNameIsCaseInsensitive(Checklist checklist)
+        {
+            ApplicationSetting setting = new ApplicationSetting { Name = "Game", FileName = @"D:\A\Game.exe" };
+            List<ApplicationSetting> settings = new List<ApplicationSetting> { setting };
+
+            List<ApplicationSetting> result = VibranceGUI.FindApplicationSettingsByName(settings, "GAME");
+
+            checklist.Check(result.Count == 1 && result[0] == setting,
+                string.Format("O4: Name comparison is case-insensitive (stored \"Game\", toggled \"GAME\"), got count={0}", result.Count));
+        }
+
+        // O5. A null settings list (the empty-startup-list window RefreshToggledListItemAppearance's
+        // own comment describes) never throws and yields an empty, non-null list rather than null -
+        // the caller's for loop would NullReferenceException on a null return.
+        private static void CheckFindApplicationSettingsByNameOnNullSettingsReturnsEmpty(Checklist checklist)
+        {
+            bool threw = false;
+            List<ApplicationSetting> result = null;
+            try
+            {
+                result = VibranceGUI.FindApplicationSettingsByName(null, "game");
+            }
+            catch (Exception)
+            {
+                threw = true;
+            }
+
+            checklist.Check(!threw && result != null && result.Count == 0,
+                string.Format("O5: a null settings list never throws and returns an empty (not null) list, got threw={0} result={1}",
+                    threw, result == null ? "null" : "count=" + result.Count));
+        }
+
+        // O6. A null or empty toggled name (IsSuppressed itself already refuses both - see
+        // ProfileToggleHelper.IsSuppressed) must not match a setting whose own Name happens to be
+        // null or empty, and must not throw doing the comparison.
+        private static void CheckFindApplicationSettingsByNameOnNullOrEmptyNameReturnsEmpty(Checklist checklist)
+        {
+            ApplicationSetting blankNamed = new ApplicationSetting { Name = null, FileName = @"D:\A\blank.exe" };
+            List<ApplicationSetting> settings = new List<ApplicationSetting> { blankNamed };
+
+            bool threwOnNull = false;
+            bool threwOnEmpty = false;
+            List<ApplicationSetting> resultForNull = null;
+            List<ApplicationSetting> resultForEmpty = null;
+            try
+            {
+                resultForNull = VibranceGUI.FindApplicationSettingsByName(settings, null);
+            }
+            catch (Exception)
+            {
+                threwOnNull = true;
+            }
+
+            try
+            {
+                resultForEmpty = VibranceGUI.FindApplicationSettingsByName(settings, string.Empty);
+            }
+            catch (Exception)
+            {
+                threwOnEmpty = true;
+            }
+
+            checklist.Check(!threwOnNull && !threwOnEmpty && resultForNull.Count == 0 && resultForEmpty.Count == 0,
+                string.Format("O6: a null or empty toggled name never throws and never matches a null-Name setting, got threwOnNull={0} threwOnEmpty={1} countForNull={2} countForEmpty={3}",
+                    threwOnNull, threwOnEmpty, resultForNull == null ? -1 : resultForNull.Count, resultForEmpty == null ? -1 : resultForEmpty.Count));
+        }
+
+        // O7. Defensive: a null entry inside the settings list (never produced by
+        // SettingsController today, but ApplicationSettingMatcher.FindMatch itself already guards
+        // the same way - see its own filter null-check) must be skipped, not dereferenced.
+        private static void CheckFindApplicationSettingsByNameSkipsNullEntries(Checklist checklist)
+        {
+            ApplicationSetting target = new ApplicationSetting { Name = "game", FileName = @"D:\A\game.exe" };
+            List<ApplicationSetting> settings = new List<ApplicationSetting> { null, target };
+
+            bool threw = false;
+            List<ApplicationSetting> result = null;
+            try
+            {
+                result = VibranceGUI.FindApplicationSettingsByName(settings, "game");
+            }
+            catch (Exception)
+            {
+                threw = true;
+            }
+
+            checklist.Check(!threw && result != null && result.Count == 1 && result[0] == target,
+                string.Format("O7: a null entry in the settings list is skipped, not dereferenced, got threw={0} count={1}",
+                    threw, result == null ? -1 : result.Count));
+        }
+
         private static void InvokeNvidiaOnWinEventHook(string processName, IntPtr handle)
         {
             MethodInfo onWinEventHook = typeof(NvidiaDynamicVibranceProxy).GetMethod(

vibrance.GUI/common/ProfileToggleHelper.cs

@@ -65,8 +65,23 @@ internal static class ProfileToggleHelper
         // keeps happening for a suppressed game (see the "restore branch stays ungated" note in
         // both proxies' OnWinEventHook) while the apply-on-alt-tab-in does not. Two different
         // lifetimes, so two different pieces of state.
+        //
+        // NameComparer is the single source for that OrdinalIgnoreCase choice - the HashSet below
+        // is built from it rather than restating StringComparer.OrdinalIgnoreCase inline, and
+        // VibranceGUI.FindApplicationSettingsByName (the list-repaint decision behind
+        // RefreshToggledListItemAppearance) compares through this same field rather than a second
+        // copy of its own. Two different ApplicationSetting entries CAN legitimately share one
+        // Name - VibranceSettings.resolveApplicationName is Path.GetFileNameWithoutExtension of
+        // whatever executable the user picked, and nothing stops two installs (a demo and the
+        // full game, two store copies, two unrelated games literally called "game.exe") from
+        // producing the same bare file name - so a toggle keyed by Name suppresses BOTH at once,
+        // and every row for that Name has to repaint together or one of them goes stale. If this
+        // comparer and that one ever disagree, the set of rows repainted stops matching the set of
+        // profiles actually suppressed - the bug this all exists to close, back in a subtler form.
+        internal static readonly StringComparer NameComparer = StringComparer.OrdinalIgnoreCase;
+
         private static readonly HashSet<string> _suppressedProfileNames =
-            new HashSet<string>(StringComparer.OrdinalIgnoreCase);
+            new HashSet<string>(NameComparer);
 
         internal static bool IsSuppressed(string name)
         {

vibrance.GUI/common/VibranceGUI.cs

@@ -851,11 +851,21 @@ private void OnToggleHotkeyPressed()
         /// the constructor's original (empty) list. That never actually mismatches the setting
         /// resolved here: an empty list can only make Decide return None, and
         /// ShouldRefreshListItemForToggleResult below already excludes that outcome, so this
-        /// method never gets past its own gate while the two references disagree. A
-        /// lookup miss on the ListViewItem side (the matched profile's item does not exist yet) is
-        /// silently skipped, not an error: the item picks up the current suppression state on its
-        /// own the moment ApplyApplicationListItemAppearance creates it, because that method reads
-        /// ProfileToggleHelper.IsSuppressed fresh every time rather than from a snapshot.
+        /// method never gets past its own gate while the two references disagree.
+        ///
+        /// The repaint itself is keyed by Name, not by the one setting FindMatch happened to
+        /// resolve: ProfileToggleHelper's suppression set is keyed by Name (see its own comment),
+        /// so a single hotkey press can suppress every ApplicationSetting that shares that Name at
+        /// once - two entries whose executables merely happen to share a bare file name, e.g.
+        /// D:\A\game.exe and D:\B\game.exe (both "game" via VibranceSettings.
+        /// resolveApplicationName), toggle together. FindApplicationSettingsByName below turns
+        /// "one resolved setting" into "every row whose suppression state just changed";
+        /// repainting only the resolved setting's own row would leave any other same-Name row
+        /// showing a stale marker. A lookup miss on the ListViewItem side (a matched profile's
+        /// item does not exist yet) is silently skipped per row, not an error: the item picks up
+        /// the current suppression state on its own the moment ApplyApplicationListItemAppearance
+        /// creates it, because that method reads ProfileToggleHelper.IsSuppressed fresh every time
+        /// rather than from a snapshot.
         /// </summary>
         private void RefreshToggledListItemAppearance(ProfileToggleResult result, string processName, string processImagePath)
         {
@@ -870,11 +880,49 @@ private void RefreshToggledListItemAppearance(ProfileToggleResult result, string
                 return;
             }
 
-            ListViewItem lvi = FindApplicationListItem(setting.FileName);
-            if (lvi != null)
+            List<ApplicationSetting> toRepaint = FindApplicationSettingsByName(_applicationSettings, setting.Name);
+            for (int i = 0; i < toRepaint.Count; i++)
             {
-                ApplyApplicationListItemAppearance(lvi, setting);
+                ApplicationSetting matched = toRepaint[i];
+                ListViewItem lvi = FindApplicationListItem(matched.FileName);
+                if (lvi != null)
+                {
+                    ApplyApplicationListItemAppearance(lvi, matched);
+                }
+            }
+        }
+
+        /// <summary>
+        /// The decision behind RefreshToggledListItemAppearance's repaint: every ApplicationSetting
+        /// in settings whose Name matches name, compared through ProfileToggleHelper.NameComparer -
+        /// the exact comparer the suppression set itself is keyed by, so this can never select a
+        /// different set of rows than the set of profiles whose suppression state the hotkey just
+        /// changed. See ProfileToggleHelper's own field comment for why two settings CAN share one
+        /// Name, and why that makes this a list rather than a single match.
+        ///
+        /// No device, no Screen, no ListView - a List&lt;ApplicationSetting&gt; in, the matching
+        /// subset out, so ProfileToggleFixture can pin this without constructing a real Form (its
+        /// constructor calls getProxy(...)), the same reason DescribeListItem above is a pure
+        /// static rather than inlined into ApplyApplicationListItemAppearance.
+        /// </summary>
+        internal static List<ApplicationSetting> FindApplicationSettingsByName(List<ApplicationSetting> settings, string name)
+        {
+            List<ApplicationSetting> matches = new List<ApplicationSetting>();
+            if (settings == null || string.IsNullOrEmpty(name))
+            {
+                return matches;
+            }
+
+            for (int i = 0; i < settings.Count; i++)
+            {
+                ApplicationSetting setting = settings[i];
+                if (setting != null && ProfileToggleHelper.NameComparer.Equals(setting.Name, name))
+                {
+                    matches.Add(setting);
+                }
             }
+
+            return matches;
         }
 
         /// <summary>