Skip to content

[NUI][API14] Add WebView APIs for video playback notification.#7722

Open
huayongxu wants to merge 1 commit into
Samsung:API14from
huayongxu:da-video-webview-api14-1
Open

[NUI][API14] Add WebView APIs for video playback notification.#7722
huayongxu wants to merge 1 commit into
Samsung:API14from
huayongxu:da-video-webview-api14-1

Conversation

@huayongxu

@huayongxu huayongxu commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Original-Author: yehyeon.kim@samsung.com

Description of Change

the related patch at binder side:
https://review.tizen.org/gerrit/c/platform/core/uifw/dali-csharp-binder/+/346690

API Changes

  • ACR:

Original-Author: yehyeon.kim@samsung.com
Co-Authored-By: Cline SR <GaussO3.4>
@github-actions github-actions Bot added the API14 Platform : Tizen 11.0 / TFM: net8.0-tizen11.0 label Jun 26, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

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 introduces several new events to the WebView class to monitor video playback states, including Ready, Started, Finished, Stopped, and Paused, along with their corresponding native interop callback registrations. The review feedback suggests optimizing memory usage by replacing "new EventArgs()" with the cached singleton "EventArgs.Empty" when raising these events to avoid unnecessary allocations.

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 +3525 to +3548
private void OnPlaybackVideoReady()
{
playbackVideoReadyEventHandler?.Invoke(this, new EventArgs());
}

private void OnPlaybackVideoStarted()
{
playbackVideoStartedEventHandler?.Invoke(this, new EventArgs());
}

private void OnPlaybackVideoFinished()
{
playbackVideoFinishedEventHandler?.Invoke(this, new EventArgs());
}

private void OnPlaybackVideoStopped()
{
playbackVideoStoppedEventHandler?.Invoke(this, new EventArgs());
}

private void OnPlaybackVideoPaused()
{
playbackVideoPausedEventHandler?.Invoke(this, new EventArgs());
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using new EventArgs() allocates a new instance of EventArgs every time the event is raised. Since video playback events can be triggered frequently, it is highly recommended to use the cached singleton EventArgs.Empty instead to avoid unnecessary memory allocations and reduce garbage collection pressure.

        private void OnPlaybackVideoReady()
        {
            playbackVideoReadyEventHandler?.Invoke(this, EventArgs.Empty);
        }

        private void OnPlaybackVideoStarted()
        {
            playbackVideoStartedEventHandler?.Invoke(this, EventArgs.Empty);
        }

        private void OnPlaybackVideoFinished()
        {
            playbackVideoFinishedEventHandler?.Invoke(this, EventArgs.Empty);
        }

        private void OnPlaybackVideoStopped()
        {
            playbackVideoStoppedEventHandler?.Invoke(this, EventArgs.Empty);
        }

        private void OnPlaybackVideoPaused()
        {
            playbackVideoPausedEventHandler?.Invoke(this, EventArgs.Empty);
        }

@TizenAPI-Bot

Copy link
Copy Markdown
Collaborator

Internal API Changed

Added: 5, Removed: 0, Changed: 0

Added

+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.EventHandler Tizen.NUI.BaseComponents.WebView::PlaybackVideoFinished

+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.EventHandler Tizen.NUI.BaseComponents.WebView::PlaybackVideoPaused

+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.EventHandler Tizen.NUI.BaseComponents.WebView::PlaybackVideoReady

+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.EventHandler Tizen.NUI.BaseComponents.WebView::PlaybackVideoStarted

+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.EventHandler Tizen.NUI.BaseComponents.WebView::PlaybackVideoStopped

@huayongxu huayongxu marked this pull request as draft June 26, 2026 09:13
@huayongxu huayongxu marked this pull request as ready for review June 29, 2026 07:36
@JoonghyunCho

Copy link
Copy Markdown
Member

🤖 [AI Review]

Reviewed — no findings.

Scope checked:

  • 5 new public events (PlaybackVideoReady/Started/Finished/Stopped/Paused) all carry a and are marked [EditorBrowsable(EditorBrowsableState.Never)] — IntelliSense-hidden per TizenFX convention, so the public-doc surface requirement is satisfied.
  • add/remove accessors follow the existing WebView callback idiom: managed delegate fields are retained to prevent GC while registered, and IntPtr.Zero is passed to the native Register* call on full unsubscribe — no leak, no double-register.
  • The 5 Interop P/Invoke declarations (HandleRef, IntPtr callbackRef) line up with the new Register*Callback native entry points; internal, so no XML doc required.
  • new EventArgs() in the 5 On* handlers matches the 6 pre-existing identical usages already in this file — left untouched to keep the file consistent (not flagged as a nitpick).
  • Change is purely additive — no existing public API signature was modified, so no breaking change.

No 🔴 critical issues, no 🟡 suggestions to flag.


Automated review — final merge decision rests with human reviewers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API14 Platform : Tizen 11.0 / TFM: net8.0-tizen11.0 Internal API Changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants