[NUI] Remove internal API to call Native signal emit directly + Empty checker method#7745
[NUI] Remove internal API to call Native signal emit directly + Empty checker method#7745hinohie wants to merge 1 commit into
Conversation
Build Error: |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors event signal handling across Tizen.NUI by removing unused Empty() and Emit() methods from internal signal classes and their Interop bindings, and updates public event unsubscription logic to use using statements for proper disposal. The code review identified critical logical errors in FocusManager.cs where impossible null checks prevent signal disconnection. Additionally, the reviewer pointed out several instances across various detectors and components where missing disposed checks, missing null assignments after disposal, or unconditional wrapper allocations could lead to native crashes, memory leaks, or inefficiencies during event unsubscription.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
hinohie
force-pushed
the
remove_signal_emit
branch
from
56d62a2 to
a293eb3
Compare
Internal API ChangedAdded: 0, Removed: 0, Changed: 2Changed/// <since_tizen>none</since_tizen
+ [Obsolete]
[EditorBrowsable(EditorBrowsableState.Never)]
System.Void Tizen.NUI.GLWindow::VisibiltyChangedSignalEmit(System.Boolean)
/// <since_tizen>none</since_tizen
+ [Obsolete]
[EditorBrowsable(EditorBrowsableState.Never)]
System.Void Tizen.NUI.Window::VisibiltyChangedSignalEmit(System.Boolean)
|
… checker method Most of event lifecycle of native side designed that signal invoked at native side, and C# just consume the events. Since most of events also need to change some native side status too, directly emit signals at C# side must not works well. To guard some mis-implements for future developer, delete those features, and just keep 3 functions - GetConnectionCount, Connect, Disconnect. TODO : Tizen.NUI.Components Navigator use AnimationSignal.Emit(), which is not recommanded. Since Navigator code implemented that finished callback comes immediatly, which is not match with DALi animation common behavior, we must change Navigator code first. After then, we can remove AnimationSignal.Emit(). TODO : AccessibilityDoAction() Looks not a common logic, and we may have similar API to emit native callback without touch emit API directly. We need to bind it first, and then remove AccessibilityDoAction() side Emit() API calls Signed-off-by: Eunki Hong <eunkiki.hong@samsung.com>
hinohie
force-pushed
the
remove_signal_emit
branch
from
a293eb3 to
f2ef845
Compare
Internal API ChangedAdded: 0, Removed: 0, Changed: 2Changed/// <since_tizen>none</since_tizen
+ [Obsolete]
[EditorBrowsable(EditorBrowsableState.Never)]
System.Void Tizen.NUI.GLWindow::VisibiltyChangedSignalEmit(System.Boolean)
/// <since_tizen>none</since_tizen
+ [Obsolete]
[EditorBrowsable(EditorBrowsableState.Never)]
System.Void Tizen.NUI.Window::VisibiltyChangedSignalEmit(System.Boolean)
|
|
@jaehyun0cho Could you please check that we can invoke callback events synchronously instead of call AnimationFinishedEmit() directly, at @dongsug-song Could you check whether some other codes(like Flux, OneUI) use internal methods what I removed at this PR? |
|
🤖 [AI Review] Reviewed — no findings. Scope checked:
No 🔴 critical issues, no 🟡 suggestions to flag. Automated review — final merge decision rests with human reviewers. |
3 participants
Most of event lifecycle of native side designed that signal invoked at native side, and C# just consume the events. Since most of events also need to change some native side status too, directly emit signals at C# side must not works well.
To guard some mis-implements for future developer, delete those features, and just keep 3 functions - GetConnectionCount, Connect, Disconnect.
TODO : Tizen.NUI.Components Navigator use AnimationSignal.Emit(), which is not recommanded. Since Navigator code implemented that finished callback comes immediatly, which is not match with DALi animation common behavior, we must change Navigator code first. After then, we can remove AnimationSignal.Emit().
TODO : AccessibilityDoAction() Looks not a common logic, and we may have similar API to emit native callback without touch emit API directly. We need to bind it first, and then remove AccessibilityDoAction() side Emit() API calls
Description of Change
API Changes