-
Notifications
You must be signed in to change notification settings - Fork 275
[Tizen.NUI.WindowSystem] change to use tizen_core_wl instead of efl_util#7693
Open
Inhong wants to merge 1 commit into
Open
[Tizen.NUI.WindowSystem] change to use tizen_core_wl instead of efl_util #7693Inhong wants to merge 1 commit into
Inhong wants to merge 1 commit into
Conversation
@Inhong
Inhong
requested review from
bshsqa,
dongsug-song and
hinohie
as code owners
June 12, 2026 06:49
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request migrates the input generation and gesture systems from EFL utility libraries to the Tizen Core Wayland display server (libtizen-core-wl.so.0), introducing a new TizenCoreWlDisplay class to manage the Wayland display connection. The code review feedback identifies several critical issues: throwing exceptions inside Dispose methods in InputGenerator and InputGesture (which can cause application crashes during cleanup), a potential resource leak in the TizenCoreWlDisplay constructor if initialization fails, and a lack of validation checks for disposed states or invalid native handles (IntPtr.Zero) before passing them to native Interop functions.
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.
Comment on lines
250
to
255
if (_handler != IntPtr.Zero)
{
Interop.InputGenerator.ErrorCode res = Interop.InputGenerator.Deinit(_handler);
ErrorCodeThrow(res);
int res = Interop.InputGenerator.Destroy(_handler);
ErrorCodeThrow((Interop.InputGenerator.ErrorCode)res);
_handler = IntPtr.Zero;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Throwing exceptions inside Dispose or finalizers is a major anti-pattern in C# and can crash the application or prevent other objects from being disposed. We should log the error instead of calling ErrorCodeThrow.
if (_handler != IntPtr.Zero) { int res = Interop.InputGenerator.Destroy(_handler); if (res != (int)Interop.InputGenerator.ErrorCode.None) { Tizen.Log.Error("InputGenerator", $"Failed to destroy input generator, error={res}"); } _handler = IntPtr.Zero; }
Comment on lines
253
to
258
if (_handler != global::System.IntPtr.Zero)
{
Interop.InputGesture.ErrorCode res = Interop.InputGesture.Deinitialize(_handler);
ErrorCodeThrow(res);
int res = Interop.InputGesture.Destroy(_handler);
ErrorCodeThrow((Interop.InputGesture.ErrorCode)res);
_handler = IntPtr.Zero;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Throwing exceptions inside Dispose or finalizers is a major anti-pattern in C# and can crash the application or prevent other objects from being disposed. We should log the error instead of calling ErrorCodeThrow.
if (_handler != global::System.IntPtr.Zero) { int res = Interop.InputGesture.Destroy(_handler); if (res != (int)Interop.InputGesture.ErrorCode.None) { Log.Error(Interop.InputGesture.LogTag, $"Failed to destroy input gesture, error={res}"); } _handler = IntPtr.Zero; }
Comment on lines
+60
to
+77
public TizenCoreWlDisplay()
{
Tizen.Log.Debug("TIZEN_CORE_WL", "TizenCoreWlDisplay: calling tizen_core_wl_init()");
int initRet = Interop.TizenCoreWl.Init();
if (initRet != (int)Interop.TizenCoreWl.ErrorCode.None)
{
Tizen.Log.Error("TIZEN_CORE_WL", $"TizenCoreWlDisplay: tizen_core_wl_init() failed with error={initRet}");
ErrorCodeThrow((Interop.TizenCoreWl.ErrorCode)initRet);
}
int ret = Interop.TizenCoreWl.DisplayCreate(out _display);
if (ret != (int)Interop.TizenCoreWl.ErrorCode.None)
{
Tizen.Log.Error("TIZEN_CORE_WL", $"TizenCoreWlDisplay: DisplayCreate() failed with error={ret}");
ErrorCodeThrow((Interop.TizenCoreWl.ErrorCode)ret);
}
Tizen.Log.Debug("TIZEN_CORE_WL", $"TizenCoreWlDisplay: DisplayCreate() succeeded, handle=0x{_display:X}");
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If Interop.TizenCoreWl.Init() succeeds but Interop.TizenCoreWl.DisplayCreate fails, the constructor throws an exception, but tizen_core_wl_init is not shut down, leading to a resource leak. We should wrap the creation in a try-catch block and call Interop.TizenCoreWl.Shutdown() if it fails.
public TizenCoreWlDisplay() { Tizen.Log.Debug("TIZEN_CORE_WL", "TizenCoreWlDisplay: calling tizen_core_wl_init()"); int initRet = Interop.TizenCoreWl.Init(); if (initRet != (int)Interop.TizenCoreWl.ErrorCode.None) { Tizen.Log.Error("TIZEN_CORE_WL", $"TizenCoreWlDisplay: tizen_core_wl_init() failed with error={initRet}"); ErrorCodeThrow((Interop.TizenCoreWl.ErrorCode)initRet); } try { int ret = Interop.TizenCoreWl.DisplayCreate(out _display); if (ret != (int)Interop.TizenCoreWl.ErrorCode.None) { Tizen.Log.Error("TIZEN_CORE_WL", $"TizenCoreWlDisplay: DisplayCreate() failed with error={ret}"); ErrorCodeThrow((Interop.TizenCoreWl.ErrorCode)ret); } } catch { Interop.TizenCoreWl.Shutdown(); throw; } Tizen.Log.Debug("TIZEN_CORE_WL", $"TizenCoreWlDisplay: DisplayCreate() succeeded, handle=0x{_display:X}"); }
Comment on lines
+85
to
+95
public void Connect(string name = null)
{
Tizen.Log.Debug("TIZEN_CORE_WL", $"TizenCoreWlDisplay: Connect(name={name ?? "null"}) called");
int ret = Interop.TizenCoreWl.DisplayConnect(_display, name);
if (ret != (int)Interop.TizenCoreWl.ErrorCode.None)
{
Tizen.Log.Error("TIZEN_CORE_WL", $"TizenCoreWlDisplay: DisplayConnect() failed with error={ret}");
ErrorCodeThrow((Interop.TizenCoreWl.ErrorCode)ret);
}
Tizen.Log.Debug("TIZEN_CORE_WL", "TizenCoreWlDisplay: DisplayConnect() succeeded");
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should check if the display is already disposed or if _display is IntPtr.Zero before calling DisplayConnect to avoid passing an invalid handle to native code.
public void Connect(string name = null) { if (disposed || _display == IntPtr.Zero) { throw new ObjectDisposedException(nameof(TizenCoreWlDisplay)); } Tizen.Log.Debug("TIZEN_CORE_WL", $"TizenCoreWlDisplay: Connect(name={name ?? "null"}) called"); int ret = Interop.TizenCoreWl.DisplayConnect(_display, name); if (ret != (int)Interop.TizenCoreWl.ErrorCode.None) { Tizen.Log.Error("TIZEN_CORE_WL", $"TizenCoreWlDisplay: DisplayConnect() failed with error={ret}"); ErrorCodeThrow((Interop.TizenCoreWl.ErrorCode)ret); } Tizen.Log.Debug("TIZEN_CORE_WL", "TizenCoreWlDisplay: DisplayConnect() succeeded"); }
Comment on lines
+155
to
177
public InputGenerator(TizenCoreWlDisplay display, DeviceType devType)
{
if (display == null)
{
Tizen.Log.Error("InputGenerator", "InputGenerator: display is null");
throw new ArgumentNullException("display is null.");
}
if (devType == DeviceType.None)
{
Tizen.Log.Error("InputGenerator", "InputGenerator: Invalid device type (None)");
throw new ArgumentException("Invalid device type");
}
_handler = Interop.InputGenerator.Init((int)devType);
_display = display;
Tizen.Log.Debug("InputGenerator", $"InputGenerator: Creating with devType={devType}");
int ret = Interop.InputGenerator.Create(display.GetNativeHandle(), (int)devType, out _handler);
if (ret != (int)Interop.InputGenerator.ErrorCode.None)
{
Tizen.Log.Error("InputGenerator", $"InputGenerator: Create() failed with error={ret}");
ErrorCodeThrow((Interop.InputGenerator.ErrorCode)ret);
}
Tizen.Log.Debug("InputGenerator", $"InputGenerator: Create() succeeded, handler=0x{_handler:X}");
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should check if display.GetNativeHandle() is IntPtr.Zero before passing it to Interop.InputGenerator.Create to prevent passing an invalid display handle.
public InputGenerator(TizenCoreWlDisplay display, DeviceType devType) { if (display == null) { Tizen.Log.Error("InputGenerator", "InputGenerator: display is null"); throw new ArgumentNullException("display is null."); } if (display.GetNativeHandle() == IntPtr.Zero) { Tizen.Log.Error("InputGenerator", "InputGenerator: display handle is invalid"); throw new ArgumentException("display handle is invalid."); } if (devType == DeviceType.None) { Tizen.Log.Error("InputGenerator", "InputGenerator: Invalid device type (None)"); throw new ArgumentException("Invalid device type"); } _display = display; Tizen.Log.Debug("InputGenerator", $"InputGenerator: Creating with devType={devType}"); int ret = Interop.InputGenerator.Create(display.GetNativeHandle(), (int)devType, out _handler); if (ret != (int)Interop.InputGenerator.ErrorCode.None) { Tizen.Log.Error("InputGenerator", $"InputGenerator: Create() failed with error={ret}"); ErrorCodeThrow((Interop.InputGenerator.ErrorCode)ret); } Tizen.Log.Debug("InputGenerator", $"InputGenerator: Create() succeeded, handler=0x{_handler:X}"); }
Comment on lines
+188
to
215
public InputGenerator(TizenCoreWlDisplay display, DeviceType devType, string name, bool sync = false)
{
if (display == null)
{
Tizen.Log.Error("InputGenerator", "InputGenerator: display is null");
throw new ArgumentNullException("display is null.");
}
if (devType == DeviceType.None)
{
Tizen.Log.Error("InputGenerator", "InputGenerator: Invalid device type (None)");
throw new ArgumentException("Invalid device type");
}
_display = display;
Tizen.Log.Debug("InputGenerator", $"InputGenerator: Creating with devType={devType}, name={name}, sync={sync}");
int ret;
if (sync)
_handler = Interop.InputGenerator.SyncInit((int)devType, name);
ret = Interop.InputGenerator.CreateWithSync(display.GetNativeHandle(), (int)devType, name, out _handler);
else
_handler = Interop.InputGenerator.InitWithName((int)devType, name);
ret = Interop.InputGenerator.CreateWithName(display.GetNativeHandle(), (int)devType, name, out _handler);
if (ret != (int)Interop.InputGenerator.ErrorCode.None)
{
Tizen.Log.Error("InputGenerator", $"InputGenerator: Create with name failed, error={ret}");
ErrorCodeThrow((Interop.InputGenerator.ErrorCode)ret);
}
Tizen.Log.Debug("InputGenerator", $"InputGenerator: Create with name succeeded, handler=0x{_handler:X}");
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should check if display.GetNativeHandle() is IntPtr.Zero before passing it to Interop.InputGenerator.CreateWithSync or CreateWithName to prevent passing an invalid display handle.
public InputGenerator(TizenCoreWlDisplay display, DeviceType devType, string name, bool sync = false) { if (display == null) { Tizen.Log.Error("InputGenerator", "InputGenerator: display is null"); throw new ArgumentNullException("display is null."); } if (display.GetNativeHandle() == IntPtr.Zero) { Tizen.Log.Error("InputGenerator", "InputGenerator: display handle is invalid"); throw new ArgumentException("display handle is invalid."); } if (devType == DeviceType.None) { Tizen.Log.Error("InputGenerator", "InputGenerator: Invalid device type (None)"); throw new ArgumentException("Invalid device type"); } _display = display; Tizen.Log.Debug("InputGenerator", $"InputGenerator: Creating with devType={devType}, name={name}, sync={sync}"); int ret; if (sync) ret = Interop.InputGenerator.CreateWithSync(display.GetNativeHandle(), (int)devType, name, out _handler); else ret = Interop.InputGenerator.CreateWithName(display.GetNativeHandle(), (int)devType, name, out _handler); if (ret != (int)Interop.InputGenerator.ErrorCode.None) { Tizen.Log.Error("InputGenerator", $"InputGenerator: Create with name failed, error={ret}"); ErrorCodeThrow((Interop.InputGenerator.ErrorCode)ret); } Tizen.Log.Debug("InputGenerator", $"InputGenerator: Create with name succeeded, handler=0x{_handler:X}"); }
Comment on lines
+191
to
205
public InputGesture(TizenCoreWlDisplay display)
{
_handler = Interop.InputGesture.Initialize();
if (_handler == IntPtr.Zero)
if (display == null)
{
int err = Tizen.Internals.Errors.ErrorFacts.GetLastResult();
ErrorCodeThrow((Interop.InputGesture.ErrorCode)err);
throw new ArgumentNullException("display is null.");
}
Log.Debug(LogTag, "InputGesture Created");
_display = display;
int ret = Interop.InputGesture.Create(display.GetNativeHandle(), out _handler);
if (ret != (int)Interop.InputGesture.ErrorCode.None)
{
ErrorCodeThrow((Interop.InputGesture.ErrorCode)ret);
}
Log.Debug(Interop.InputGesture.LogTag, "InputGesture Created");
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should check if display.GetNativeHandle() is IntPtr.Zero before passing it to Interop.InputGesture.Create to prevent passing an invalid display handle.
public InputGesture(TizenCoreWlDisplay display) { if (display == null) { throw new ArgumentNullException("display is null."); } if (display.GetNativeHandle() == IntPtr.Zero) { throw new ArgumentException("display handle is invalid."); } _display = display; int ret = Interop.InputGesture.Create(display.GetNativeHandle(), out _handler); if (ret != (int)Interop.InputGesture.ErrorCode.None) { ErrorCodeThrow((Interop.InputGesture.ErrorCode)ret); } Log.Debug(Interop.InputGesture.LogTag, "InputGesture Created"); }
TizenAPI-Bot
commented
Jun 12, 2026
Collaborator
Internal API Changed
Added: 10, Removed: 5, Changed: 3
Added
+ /// <since_tizen>none</since_tizen + [EditorBrowsable(EditorBrowsableState.Never)] + System.Void Tizen.NUI.WindowSystem.InputGenerator::.ctor(Tizen.NUI.WindowSystem.TizenCoreWlDisplay,Tizen.NUI.WindowSystem.InputGenerator/DeviceType,System.String,System.Boolean) + /// <since_tizen>none</since_tizen + [EditorBrowsable(EditorBrowsableState.Never)] + System.Void Tizen.NUI.WindowSystem.InputGenerator::.ctor(Tizen.NUI.WindowSystem.TizenCoreWlDisplay,Tizen.NUI.WindowSystem.InputGenerator/DeviceType) + /// <since_tizen>none</since_tizen + [EditorBrowsable(EditorBrowsableState.Never)] + System.Void Tizen.NUI.WindowSystem.InputGenerator::GenerateKey(System.String,System.Boolean) + /// <since_tizen>none</since_tizen + [EditorBrowsable(EditorBrowsableState.Never)] + System.Void Tizen.NUI.WindowSystem.InputGesture::.ctor(Tizen.NUI.WindowSystem.TizenCoreWlDisplay) + /// <since_tizen>none</since_tizen + [EditorBrowsable(EditorBrowsableState.Never)] + Tizen.NUI.WindowSystem.TizenCoreWlDisplay + /// <since_tizen>none</since_tizen + [EditorBrowsable(EditorBrowsableState.Never)] + System.Void Tizen.NUI.WindowSystem.TizenCoreWlDisplay::.ctor() + /// <since_tizen>none</since_tizen + [EditorBrowsable(EditorBrowsableState.Never)] + System.Void Tizen.NUI.WindowSystem.TizenCoreWlDisplay::Connect(System.String) + /// <since_tizen>none</since_tizen + [EditorBrowsable(EditorBrowsableState.Never)] + System.Void Tizen.NUI.WindowSystem.TizenCoreWlDisplay::Dispose() + /// <since_tizen>none</since_tizen + [EditorBrowsable(EditorBrowsableState.Never)] + System.Void Tizen.NUI.WindowSystem.TizenCoreWlDisplay::Dispose(Tizen.NUI.DisposeTypes) + /// <since_tizen>none</since_tizen + [EditorBrowsable(EditorBrowsableState.Never)] + System.Void Tizen.NUI.WindowSystem.TizenCoreWlDisplay::Finalize()
Removed
- /// <since_tizen>none</since_tizen - [EditorBrowsable(EditorBrowsableState.Never)] - System.Void Tizen.NUI.WindowSystem.InputGenerator::.ctor(Tizen.NUI.WindowSystem.InputGenerator/DeviceType,System.String,System.Boolean) - /// <since_tizen>none</since_tizen - [EditorBrowsable(EditorBrowsableState.Never)] - System.Void Tizen.NUI.WindowSystem.InputGenerator::.ctor(Tizen.NUI.WindowSystem.InputGenerator/DeviceType) - /// <since_tizen>none</since_tizen - [EditorBrowsable(EditorBrowsableState.Never)] - System.Void Tizen.NUI.WindowSystem.InputGenerator::GenerateKey(System.String,System.Int32) - /// <since_tizen>none</since_tizen - [EditorBrowsable(EditorBrowsableState.Never)] - static Tizen.NUI.WindowSystem.InputGenerator/TouchType Tizen.NUI.WindowSystem.InputGenerator/TouchType::None - /// <since_tizen>none</since_tizen - [EditorBrowsable(EditorBrowsableState.Never)] - System.Void Tizen.NUI.WindowSystem.InputGesture::.ctor()
Changed
/// <since_tizen>none</since_tizen [EditorBrowsable(EditorBrowsableState.Never)] static Tizen.NUI.WindowSystem.InputGenerator/TouchType Tizen.NUI.WindowSystem.InputGenerator/TouchType::Begin /// <since_tizen>none</since_tizen [EditorBrowsable(EditorBrowsableState.Never)] static Tizen.NUI.WindowSystem.InputGenerator/TouchType Tizen.NUI.WindowSystem.InputGenerator/TouchType::End /// <since_tizen>none</since_tizen [EditorBrowsable(EditorBrowsableState.Never)] static Tizen.NUI.WindowSystem.InputGenerator/TouchType Tizen.NUI.WindowSystem.InputGenerator/TouchType::Update
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of Change
API Changes