-
Notifications
You must be signed in to change notification settings - Fork 275
[Tizen.WindowSystem] change to use tizen_core_wl instead of efl_util#7694
[Tizen.WindowSystem] change to use tizen_core_wl instead of efl_util #7694Inhong wants to merge 1 commit into
Conversation
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 transitions the input generation and gesture modules from the EFL-based utility library to the new Tizen Core Wayland library (libtizen-core-wl.so.0). It introduces the TizenCoreWlDisplay class to manage the Wayland display connection and updates InputGenerator and InputGesture to require a display instance upon initialization. The review feedback highlights several important improvements: ensuring TizenCoreWlDisplay properly shuts down on creation failure to avoid resource leaks, implementing a finalizer for unmanaged resource cleanup, throwing an ObjectDisposedException if Connect is called after disposal, validating that the name parameter is not null in InputGenerator to prevent native crashes, and correcting the usage of ArgumentNullException to pass the parameter name instead of a descriptive string.
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.
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 DisplayCreate fails, Interop.TizenCoreWl.Init() has already succeeded. We must call Interop.TizenCoreWl.Shutdown() to decrement the initialization reference count and avoid resource leaks before throwing the exception.
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}"); Interop.TizenCoreWl.Shutdown(); ErrorCodeThrow((Interop.TizenCoreWl.ErrorCode)ret); }
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.
In addition to correcting the ArgumentNullException parameter name for display, we should also validate that name is not null. Passing a null name to CreateWithName or CreateWithSync can cause native crashes.
public InputGenerator(TizenCoreWlDisplay display, InputGeneratorDevices deviceType, string name, bool sync = false) { if (display == null) { throw new ArgumentNullException(nameof(display)); } if (name == null) { throw new ArgumentNullException(nameof(name)); }
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.
Calling Connect after the display has been disposed will pass IntPtr.Zero to the native library, which can cause undefined behavior or crashes. We should throw an ObjectDisposedException if the instance is already disposed.
public void Connect(string name = null) { if (disposed) { throw new ObjectDisposedException(nameof(TizenCoreWlDisplay)); } Tizen.Log.Debug("TIZEN_CORE_WL", $"TizenCoreWlDisplay: Connect(name={name ?? "null"}) called"); int ret = Interop.TizenCoreWl.DisplayConnect(_display, name);
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.
Since TizenCoreWlDisplay directly manages an unmanaged resource (IntPtr _display) and implements IDisposable, it should implement a finalizer to ensure that the native resources are released if the user forgets to call Dispose().
/// <summary> /// Finalizes an instance of the <see cref="TizenCoreWlDisplay"/> class. /// </summary> ~TizenCoreWlDisplay() { Dispose(false); } /// <summary> /// Dispose. /// </summary> public void Dispose() { Dispose(true); GC.SuppressFinalize(this); }
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.
The first parameter of ArgumentNullException should be the parameter name (nameof(display)), not the error message. Passing a descriptive message as the first argument results in a confusing exception message like Parameter 'display is null.' cannot be null.
public InputGenerator(TizenCoreWlDisplay display, InputGeneratorDevices deviceType) { if (display == null) { throw new ArgumentNullException(nameof(display)); }
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.
The first parameter of ArgumentNullException should be the parameter name (nameof(display)), not the error message.
public InputGesture(TizenCoreWlDisplay display) { if (display == null) { throw new ArgumentNullException(nameof(display)); }
TizenAPI-Bot
commented
Jun 12, 2026
Internal API Changed
Added: 8, Removed: 3, Changed: 0
Added
+ /// <since_tizen>none</since_tizen + [EditorBrowsable(EditorBrowsableState.Never)] + System.Void Tizen.WindowSystem.InputGenerator::.ctor(Tizen.WindowSystem.TizenCoreWlDisplay,Tizen.WindowSystem.InputGeneratorDevices,System.String,System.Boolean) + /// <since_tizen>none</since_tizen + [EditorBrowsable(EditorBrowsableState.Never)] + System.Void Tizen.WindowSystem.InputGenerator::.ctor(Tizen.WindowSystem.TizenCoreWlDisplay,Tizen.WindowSystem.InputGeneratorDevices) + /// <since_tizen>none</since_tizen + [EditorBrowsable(EditorBrowsableState.Never)] + System.Void Tizen.WindowSystem.InputGesture::.ctor(Tizen.WindowSystem.TizenCoreWlDisplay) + /// <since_tizen>none</since_tizen + [EditorBrowsable(EditorBrowsableState.Never)] + Tizen.WindowSystem.TizenCoreWlDisplay + /// <since_tizen>none</since_tizen + [EditorBrowsable(EditorBrowsableState.Never)] + System.Void Tizen.WindowSystem.TizenCoreWlDisplay::.ctor() + /// <since_tizen>none</since_tizen + [EditorBrowsable(EditorBrowsableState.Never)] + System.Void Tizen.WindowSystem.TizenCoreWlDisplay::Connect(System.String) + /// <since_tizen>none</since_tizen + [EditorBrowsable(EditorBrowsableState.Never)] + System.Void Tizen.WindowSystem.TizenCoreWlDisplay::Dispose() + /// <since_tizen>none</since_tizen + [EditorBrowsable(EditorBrowsableState.Never)] + System.Void Tizen.WindowSystem.TizenCoreWlDisplay::Dispose(System.Boolean)
Removed
- /// <since_tizen>none</since_tizen - [EditorBrowsable(EditorBrowsableState.Never)] - System.Void Tizen.WindowSystem.InputGenerator::.ctor(System.String,System.Boolean) - /// <since_tizen>none</since_tizen - [EditorBrowsable(EditorBrowsableState.Never)] - System.Void Tizen.WindowSystem.InputGenerator::.ctor(Tizen.WindowSystem.InputGeneratorDevices,System.String,System.Boolean) - /// <since_tizen>none</since_tizen - [EditorBrowsable(EditorBrowsableState.Never)] - System.Void Tizen.WindowSystem.InputGesture::.ctor()
Description of Change
API Changes