From: Seungkeun Lee Date: Mon, 26 Sep 2016 22:57:52 +0000 (+0900) Subject: Fix callback order issue, memory leak issue in GenItem,GenItemClass X-Git-Tag: submit/trunk/20170823.075128~110^2~263^2~1 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=7897611bf88d0479a22c926f404a54fd4b556937;p=platform%2Fcore%2Fcsapi%2Ftizenfx.git Fix callback order issue, memory leak issue in GenItem,GenItemClass - GenItemClass::DeleteHandler was called, after related item was cleanup So, developer can't access to related item in DeleteHandler. I change the source of DeleteHandler - Memory leak on ItemObject s_IdToItemTable and s_HandleToItemTable was not removed when Item was deleted, So ItemObject can't released. Change-Id: Ibe5ada852f1c1d777ece5667b3aafa5be4cf119d --- diff --git a/src/ElmSharp/ElmSharp/GenGrid.cs b/src/ElmSharp/ElmSharp/GenGrid.cs index 8601e1a..3d3db3b 100644 --- a/src/ElmSharp/ElmSharp/GenGrid.cs +++ b/src/ElmSharp/ElmSharp/GenGrid.cs @@ -236,15 +236,15 @@ namespace ElmSharp _unrealized = new Interop.SmartEvent(this, Handle, "unrealized", GenGridItemEventArgs.CreateFromSmartEvent); _longpressed = new Interop.SmartEvent(this, Handle, "longpressed", GenGridItemEventArgs.CreateFromSmartEvent); - _selected.On += (s, e) => { ItemSelected?.Invoke(this, e); }; - _unselected.On += (s, e) => { ItemUnselected?.Invoke(this, e); }; - _activated.On += (s, e) => { ItemActivated?.Invoke(this, e); }; - _pressed.On += (s, e) => { ItemPressed?.Invoke(this, e); }; - _released.On += (s, e) => { ItemReleased?.Invoke(this, e); }; - _doubleClicked.On += (s, e) => { ItemDoubleClicked?.Invoke(this, e); }; - _realized.On += (s, e) => { ItemRealized?.Invoke(this, e); }; - _unrealized.On += (s, e) => { ItemUnrealized?.Invoke(this, e); }; - _longpressed.On += (s, e) => { ItemLongPressed?.Invoke(this, e); }; + _selected.On += (s, e) => { if (e.Item != null) ItemSelected?.Invoke(this, e); }; + _unselected.On += (s, e) => { if (e.Item != null) ItemUnselected?.Invoke(this, e); }; + _activated.On += (s, e) => { if (e.Item != null) ItemActivated?.Invoke(this, e); }; + _pressed.On += (s, e) => { if (e.Item != null) ItemPressed?.Invoke(this, e); }; + _released.On += (s, e) => { if (e.Item != null) ItemReleased?.Invoke(this, e); }; + _doubleClicked.On += (s, e) => { if (e.Item != null) ItemDoubleClicked?.Invoke(this, e); }; + _realized.On += (s, e) => { if (e.Item != null) ItemRealized?.Invoke(this, e); }; + _unrealized.On += (s, e) => { if (e.Item != null) ItemUnrealized?.Invoke(this, e); }; + _longpressed.On += (s, e) => { if (e.Item != null) ItemLongPressed?.Invoke(this, e); }; } diff --git a/src/ElmSharp/ElmSharp/GenItem.cs b/src/ElmSharp/ElmSharp/GenItem.cs index 67fcf65..8b6528f 100644 --- a/src/ElmSharp/ElmSharp/GenItem.cs +++ b/src/ElmSharp/ElmSharp/GenItem.cs @@ -1,4 +1,4 @@ -using System; +using System; namespace ElmSharp { @@ -16,6 +16,7 @@ namespace ElmSharp public abstract void Update(); protected override void OnInvalidate() { + ItemClass?.SendItemDeleted(Data); Data = null; ItemClass = null; } diff --git a/src/ElmSharp/ElmSharp/GenItemClass.cs b/src/ElmSharp/ElmSharp/GenItemClass.cs index 6d64b66..fa31f02 100644 --- a/src/ElmSharp/ElmSharp/GenItemClass.cs +++ b/src/ElmSharp/ElmSharp/GenItemClass.cs @@ -73,6 +73,12 @@ namespace ElmSharp GC.SuppressFinalize(this); } + internal void SendItemDeleted(object data) + { + // data is user inserted value with GenItem + DeleteHandler?.Invoke(data); + } + private string GetTextCallback(IntPtr data, IntPtr obj, IntPtr part) { GenItem item = ItemObject.GetItemById((int)data) as GenItem; @@ -85,8 +91,12 @@ namespace ElmSharp } private void DelCallback(IntPtr data, IntPtr obj) { - GenItem item = ItemObject.GetItemById((int)data) as GenItem; - DeleteHandler?.Invoke(item?.Data); + // We can't use this callback + // because, when item was deleted + // First, ItemObject deleted callback was called + // and We need to clean up ItemObject related objects + // This callback was called after ItemObject deleted callback was completed. + // So, We can't get resource reletated with ItemObject } } diff --git a/src/ElmSharp/ElmSharp/GenList.cs b/src/ElmSharp/ElmSharp/GenList.cs index 3be368e..3f9cfc4 100644 --- a/src/ElmSharp/ElmSharp/GenList.cs +++ b/src/ElmSharp/ElmSharp/GenList.cs @@ -216,16 +216,16 @@ namespace ElmSharp _scrollAnimationStopped = new Interop.SmartEvent(this, Handle, "scroll,anim,stop"); _changed = new Interop.SmartEvent(this, Handle, "changed"); - _selected.On += (s, e) => { ItemSelected?.Invoke(this, e); }; - _unselected.On += (s, e) => { ItemUnselected?.Invoke(this, e); }; - _activated.On += (s, e) => { ItemActivated?.Invoke(this, e); }; - _pressed.On += (s, e) => { ItemPressed?.Invoke(this, e); }; - _released.On += (s, e) => { ItemReleased?.Invoke(this, e); }; - _doubleClicked.On += (s, e) => { ItemDoubleClicked?.Invoke(this, e); }; - _expanded.On += (s, e) => { ItemExpanded?.Invoke(this, e); }; - _realized.On += (s, e) => { ItemRealized?.Invoke(this, e); }; - _unrealized.On += (s, e) => { ItemUnrealized?.Invoke(this, e); }; - _longpressed.On += (s, e) => { ItemLongPressed?.Invoke(this, e); }; + _selected.On += (s, e) => { if (e.Item != null) ItemSelected?.Invoke(this, e); }; + _unselected.On += (s, e) => { if (e.Item != null) ItemUnselected?.Invoke(this, e); }; + _activated.On += (s, e) => { if (e.Item != null) ItemActivated?.Invoke(this, e); }; + _pressed.On += (s, e) => { if (e.Item != null) ItemPressed?.Invoke(this, e); }; + _released.On += (s, e) => { if (e.Item != null) ItemReleased?.Invoke(this, e); }; + _doubleClicked.On += (s, e) => { if (e.Item != null) ItemDoubleClicked?.Invoke(this, e); }; + _expanded.On += (s, e) => { if (e.Item != null) ItemExpanded?.Invoke(this, e); }; + _realized.On += (s, e) => { if (e.Item != null) ItemRealized?.Invoke(this, e); }; + _unrealized.On += (s, e) => { if (e.Item != null) ItemUnrealized?.Invoke(this, e); }; + _longpressed.On += (s, e) => { if (e.Item != null) ItemLongPressed?.Invoke(this, e); }; } void AddInternal(GenListItem item) diff --git a/src/ElmSharp/ElmSharp/ItemObject.cs b/src/ElmSharp/ElmSharp/ItemObject.cs index 65f28dc..e8c9be7 100644 --- a/src/ElmSharp/ElmSharp/ItemObject.cs +++ b/src/ElmSharp/ElmSharp/ItemObject.cs @@ -117,6 +117,14 @@ namespace ElmSharp { Deleted?.Invoke(this, EventArgs.Empty); OnInvalidate(); + if (s_IdToItemTable.ContainsKey(Id)) + { + s_IdToItemTable.Remove(Id); + } + if (s_HandleToItemTable.ContainsKey(_handle)) + { + s_HandleToItemTable.Remove(_handle); + } _handle = IntPtr.Zero; } diff --git a/test/ElmSharp.Test/ElmSharp.Test.csproj b/test/ElmSharp.Test/ElmSharp.Test.csproj index 57edf88..4dbd9e0 100644 --- a/test/ElmSharp.Test/ElmSharp.Test.csproj +++ b/test/ElmSharp.Test/ElmSharp.Test.csproj @@ -57,6 +57,7 @@ + @@ -120,4 +121,14 @@ <_FullFrameworkReferenceAssemblyPaths>$(MSBuildThisFileDirectory) true - + + + + + + + + + + + \ No newline at end of file diff --git a/test/ElmSharp.Test/TC/GenListTest6.cs b/test/ElmSharp.Test/TC/GenListTest6.cs new file mode 100644 index 0000000..a6006aa --- /dev/null +++ b/test/ElmSharp.Test/TC/GenListTest6.cs @@ -0,0 +1,125 @@ +using System; +using ElmSharp; + +namespace ElmSharp.Test +{ + class GenListTest6 : TestCaseBase + { + public override string TestName => "GenListTest6"; + public override string TestDescription => "To test deletion of GenListItem"; + GenListItem selected = null; + + + public override void Run(Window window) + { + Conformant conformant = new Conformant(window); + conformant.Show(); + Box box = new Box(window) + { + AlignmentX = -1, + AlignmentY = -1, + WeightX = 1, + WeightY = 1, + }; + box.Show(); + conformant.SetContent(box); + + + GenList list = new GenList(window) + { + Homogeneous = true, + AlignmentX = -1, + AlignmentY = -1, + WeightX = 1, + WeightY = 1 + }; + + GenItemClass defaultClass = new GenItemClass("default") + { + GetTextHandler = (obj, part) => + { + return string.Format("{0} - {1}", (string)obj, part); + }, + DeleteHandler = new GenItemClass.DeleteDelegate((obj) => + { + Console.WriteLine("DeleteHandler was called with... {0}", (string)obj); + }), + }; + GenListItem[] items = new GenListItem[100]; + for (int i = 0; i < 100; i++) + { + items[i] = list.Append(defaultClass, string.Format("{0} Item", i)); + } + list.Show(); + list.ItemSelected += List_ItemSelected; + list.ItemActivated += List_ItemActivated; + list.ItemUnselected += List_ItemUnselected; + list.ItemPressed += List_ItemPressed; + list.ItemRealized += List_ItemRealized; + list.ItemReleased += List_ItemReleased; + list.ItemUnrealized += List_ItemUnrealized; + list.ItemLongPressed += List_ItemLongPressed; + list.ItemDoubleClicked += List_ItemDoubleClicked; + box.PackEnd(list); + Button first = new Button(window) + { + Text = "Delete", + AlignmentX = -1, + WeightX = 1, + }; + first.Clicked += (s, e) => + { + selected?.Delete(); + }; + first.Show(); + box.PackEnd(first); + + } + + private void List_ItemSelected(object sender, GenListItemEventArgs e) + { + selected = e.Item; + Console.WriteLine("{0} Item was selected", (string)(e.Item.Data)); + } + private void List_ItemDoubleClicked(object sender, GenListItemEventArgs e) + { + Console.WriteLine("{0} Item was double clicked", (string)(e.Item.Data)); + } + + private void List_ItemLongPressed(object sender, GenListItemEventArgs e) + { + Console.WriteLine("{0} Item was Long pressed", (string)(e.Item.Data)); + } + + private void List_ItemUnrealized(object sender, GenListItemEventArgs e) + { + Console.WriteLine("!!!! Item was Unrealized!!!"); + Console.WriteLine("{0} Item was unrealzed", (string)(e.Item.Data)); + } + + private void List_ItemReleased(object sender, GenListItemEventArgs e) + { + Console.WriteLine("{0} Item was released", (string)(e.Item.Data)); + } + + private void List_ItemRealized(object sender, GenListItemEventArgs e) + { + Console.WriteLine("{0} Item was Realized", (string)(e.Item.Data)); + } + + private void List_ItemPressed(object sender, GenListItemEventArgs e) + { + Console.WriteLine("{0} Item was Pressed", (string)(e.Item.Data)); + } + + private void List_ItemUnselected(object sender, GenListItemEventArgs e) + { + Console.WriteLine("{0} Item was unselected", (string)(e.Item.Data)); + } + + private void List_ItemActivated(object sender, GenListItemEventArgs e) + { + Console.WriteLine("{0} Item was Activated", (string)(e.Item.Data)); + } + } +}