Fix for Memory issue with context menu items (#4597)

* Added the inotifyPropertyChanged to all the properties and that stops the memory for shooting up

* some more inotify properties added

(cherry picked from commit 26fa05d9b661dadc5ab0257d540ab838a07c43a6)

* Revert "some more inotify properties added"

This reverts commit 845a94c9b2.

* Removed unnecessary inotifypropertychanged interfaces and cleaned up the code

* removed the ctrl+c from folder plugin

* removed unnecessary init

* Added unit test to check if PropertyChanged is called

* renamed var

* refactored the tests

* formatting and adding comments

* changed access modifier in test

* Used observable collection instead of a list

* clearing the observable collection instead of setting it to a new one

src/modules/launcher/Wox.Plugin/BaseModel.cs

@@ -11,7 +11,7 @@ namespace Wox.Plugin
         public event PropertyChangedEventHandler PropertyChanged;
 
         [NotifyPropertyChangedInvocator]
-        protected void OnPropertyChanged([CallerMemberName] string propertyName = null)
+        protected virtual void OnPropertyChanged([CallerMemberName] string propertyName = null)
         {
             PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
         }

src/modules/launcher/Wox.Test/Wox.Test.csproj

@@ -44,6 +44,7 @@
     <ProjectReference Include="..\Wox.Core\Wox.Core.csproj" />
     <ProjectReference Include="..\Wox.Infrastructure\Wox.Infrastructure.csproj" />
     <ProjectReference Include="..\Wox.Plugin\Wox.Plugin.csproj" />
+    <ProjectReference Include="..\Wox\Wox.csproj" />
   </ItemGroup>
 
   <ItemGroup>

src/modules/launcher/Wox.Test/WoxTest.cs

@@ -0,0 +1,58 @@
+using System.Windows.Input;
+using NUnit.Framework;
+using Wox.Plugin;
+using Wox.ViewModel;
+using System.Runtime.CompilerServices;
+
+namespace Wox.Test
+{
+    [TestFixture]
+    public class Wox
+    {
+        // A Dummy class to test that OnPropertyChanged() is called while we set the variable
+        public class DummyTestClass : BaseModel
+        {
+            public bool isFunctionCalled = false;
+            private ICommand _item;
+
+            public ICommand Item
+            {
+                get
+                {
+                    return this._item;
+                }
+
+                set
+                {
+                    if (value != this._item)
+                    {
+                        this._item = value;
+                        OnPropertyChanged();
+                    }
+                }
+            }
+
+            // Overriding the OnPropertyChanged() function to test if it is being called
+            protected override void OnPropertyChanged([CallerMemberName] string propertyName = null)
+            {
+                isFunctionCalled = true;
+            }
+        }
+
+
+        [Test]
+        public void AnyVariable_MustCallOnPropertyChanged_WhenSet()
+        {
+            // Arrange
+            DummyTestClass testClass = new DummyTestClass();
+
+            // Act
+            testClass.Item = new RelayCommand(null);
+
+            // Assert
+            Assert.IsTrue(testClass.isFunctionCalled);
+
+        }
+
+    }
+}

src/modules/launcher/Wox/ViewModel/ContextMenuItemViewModel.cs

@@ -9,11 +9,29 @@ namespace Wox.ViewModel
 {
     public class ContextMenuItemViewModel : BaseModel
     {
+        private ICommand _command;
+
         public string PluginName { get; set; }
         public string Title { get; set; }
         public string Glyph { get; set; }
         public string FontFamily { get; set; }
-        public ICommand Command { get; set; }
+        public ICommand Command {
+            get
+            {
+                return this._command;
+            }
+
+            set
+            {
+                // ICommand does not implement the INotifyPropertyChanged interface and must call OnPropertyChanged() to prevent memory leaks
+                if (value != this._command)
+                {
+                    this._command = value;
+                    OnPropertyChanged();
+                }
+            }
+        }
+
         public Key AcceleratorKey { get; set; }
         public ModifierKeys AcceleratorModifiers { get; set; }
         public bool IsAcceleratorKeyEnabled { get; set; }

src/modules/launcher/Wox/ViewModel/ResultViewModel.cs

@@ -22,7 +22,7 @@ namespace Wox.ViewModel
             Hover
         };
 
-        public List<ContextMenuItemViewModel> ContextMenuItems { get; set; }
+        public ObservableCollection<ContextMenuItemViewModel> ContextMenuItems { get; set; } = new ObservableCollection<ContextMenuItemViewModel>();
 
         public ICommand ActivateContextButtonsHoverCommand { get; set; }
         public ICommand ActivateContextButtonsSelectionCommand { get; set; }
@@ -46,7 +46,10 @@ namespace Wox.ViewModel
             {
                 Result = result;
             }
+            
             ContextMenuSelectedIndex = NoSelectionIndex;
+            LoadContextMenu();
+            
             ActivateContextButtonsHoverCommand = new RelayCommand(ActivateContextButtonsHoverAction);
             ActivateContextButtonsSelectionCommand = new RelayCommand(ActivateContextButtonsSelectionAction);
             DeactivateContextButtonsHoverCommand = new RelayCommand(DeactivateContextButtonsHoverAction);
@@ -64,11 +67,6 @@ namespace Wox.ViewModel
         }
         public void ActivateContextButtons(ActivationType activationType)
         {
-            if (ContextMenuItems == null)
-            {
-                LoadContextMenu();
-            }
-
             // Result does not contain any context menu items - we don't need to show the context menu ListView at all.
             if (ContextMenuItems.Count > 0)
             {
@@ -78,14 +76,13 @@ namespace Wox.ViewModel
             {
                 AreContextButtonsActive = false;
             }
-         
 
             if (activationType == ActivationType.Selection)
             {
                 IsSelected = true;
                 EnableContextMenuAcceleratorKeys();
             }
-            else if(activationType == ActivationType.Hover)
+            else if (activationType == ActivationType.Hover)
             {
                 IsHovered = true;
             }
@@ -122,17 +119,17 @@ namespace Wox.ViewModel
             else
             {
                 AreContextButtonsActive = false;
-            }  
+            }
         }
 
 
         public void LoadContextMenu()
         {
             var results = PluginManager.GetContextMenusForPlugin(Result);
-            var newItems = new List<ContextMenuItemViewModel>();
+            ContextMenuItems.Clear();
             foreach (var r in results)
             {
-                newItems.Add(new ContextMenuItemViewModel
+                ContextMenuItems.Add(new ContextMenuItemViewModel()
                 {
                     PluginName = r.PluginName,
                     Title = r.Title,
@@ -155,13 +152,11 @@ namespace Wox.ViewModel
                     })
                 });
             }
-
-            ContextMenuItems = newItems;
         }
 
         private void EnableContextMenuAcceleratorKeys()
         {
-            foreach(var i in ContextMenuItems)
+            foreach (var i in ContextMenuItems)
             {
                 i.IsAcceleratorKeyEnabled = true;
             }
@@ -192,7 +187,7 @@ namespace Wox.ViewModel
                         imagePath = ImageLoader.ErrorIconPath;
                     }
                 }
-                
+
                 // will get here either when icoPath has value\icon delegate is null\when had exception in delegate
                 return ImageLoader.Load(imagePath);
             }
@@ -201,10 +196,10 @@ namespace Wox.ViewModel
         //Returns false if we've already reached the last item.
         public bool SelectNextContextButton()
         {
-            if(ContextMenuSelectedIndex == (ContextMenuItems.Count -1))
+            if (ContextMenuSelectedIndex == (ContextMenuItems.Count - 1))
             {
                 ContextMenuSelectedIndex = NoSelectionIndex;
-                return false; 
+                return false;
             }
 
             ContextMenuSelectedIndex++;