Skip to content

RDKDEV-1386 Add rdkNativescript Documentation - #133

Merged
vjain008 merged 10 commits into
rdkcentral:developfrom
gourivarma3:feature/RDKDEV-1386
Sep 18, 2026
Merged

vjain008 merged 10 commits into
rdkcentral:developfrom
gourivarma3:feature/RDKDEV-1386

Conversation

@gourivarma3

@gourivarma3 gourivarma3 commented May 25, 2026 •

Copy link
Copy Markdown
Contributor

RDKDEV-1386
Added component documentation for rdkNativeScript .

@gourivarma3
gourivarma3 requested a review from a team as a code owner May 25, 2026 15:41
@gourivarma3

Copy link
Copy Markdown
Contributor Author

I have read the CLA Document and I hereby sign the CLA

Copilot AI lite review requested due to automatic review settings June 9, 2026 10:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI review requested due to automatic review settings July 24, 2026 05:39
@gourivarma3
gourivarma3 force-pushed the feature/RDKDEV-1386 branch from 4bc78f6 to 3ba3ba4 Compare July 24, 2026 05:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comment thread docs/README.md Outdated
Comment thread docs/README.md Outdated
Copilot AI review requested due to automatic review settings July 24, 2026 07:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

docs/README.md:363

  • NATIVEJS_LOG_LEVEL parsing is case-insensitive (strcasecmp()), but the table suggests only lowercase values are accepted. Clarify that the accepted values are case-insensitive to match the implementation.
| `NATIVEJS_LOG_LEVEL`          | string (env)   | `INFO`          | Sets the logging verbosity. Accepted values: `debug`, `info`, `warn`, `error`, `fatal`.                        |

docs/README.md:365

  • This row implies the remote inspector can be enabled purely via NATIVEJS_INSPECTOR_SERVER, but the inspector code is compiled only when the REMOTE_INSPECTOR_ENABLE CMake option is ON (-DREMOTE_INSPECTOR_ENABLE). Also, the implementation listens on all interfaces (soup_server_listen_all) and uses the env var mainly to extract the starting port and for logging.
| `NATIVEJS_INSPECTOR_SERVER`   | string (env)   | _(not set)_     | Activates the remote JavaScript inspector. Format: `host:port` (e.g., `0.0.0.0:9226`).                        |

Comment thread docs/README.md Outdated
Copilot AI review requested due to automatic review settings July 24, 2026 07:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

vjain008
vjain008 previously approved these changes Aug 30, 2026
Copilot AI review requested due to automatic review settings August 30, 2026 14:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Suppressed comments (3)

docs/README.md:157

  • This bullet states that both Thunder JSON-RPC and WebSocket use an options parameter, but the WebSocket server implementation reads module tokens from the moduleSettings param (see src/JSRuntimeServer.cpp / src/JSRuntimeContainer.cpp). Clarify the parameter name to match the implementation.
- **Via Thunder JSON-RPC or WebSocket command**: The `options` parameter of `launchApplication` is a comma-separated string of token names (e.g., `"player,xhr,ws"`). `ModuleSettings::fromString()` parses this string and sets the corresponding boolean flags.

docs/README.md:315

  • In the WebSocket sequence diagram, the request example uses options, but the server parses moduleSettings for module tokens (see src/JSRuntimeServer.cpp). Update the example so it matches what clients must send.
    ExtTool->>SRV: JSON: {"method": "launchApplication", "params": {"url": "...", "options": "player,xhr"}}

docs/README.md:320

  • The WebSocket server wraps responses as { "result": "..." } (see src/JSRuntimeServer.cpp), not as {success, id}. The launchApplication result string is formatted like ID : <id>, so the diagram should reflect that.
    SRV-->>ExtTool: JSON response {success, id}

Comment thread docs/README.md Outdated
Comment thread docs/README.md Outdated
Copilot AI review requested due to automatic review settings August 31, 2026 12:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (2)

docs/README.md:157

  • This bullet claims the WebSocket path uses the options parameter, but JSRuntimeServer reads module tokens from the moduleSettings field (see src/JSRuntimeServer.cpp:253). This can mislead users trying to launch apps over WebSocket.
- **Via Thunder JSON-RPC or WebSocket command**: The `options` parameter of `launchApplication` is a comma-separated string of token names (e.g., `"player,xhr,ws"`). `ModuleSettings::fromString()` parses this string and sets the corresponding boolean flags.

docs/README.md:315

  • The WebSocket launch example uses options, but the server implementation expects the module token string under params.moduleSettings (see src/JSRuntimeServer.cpp:253). The example should match the actual wire format.
    ExtTool->>SRV: JSON: {"method": "launchApplication", "params": {"url": "...", "options": "player,xhr"}}

gourivarma3 and others added 6 commits September 14, 2026 10:58
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 14, 2026 05:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The documentation contains multiple implementation mismatches that should be corrected before approval.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (19)

docs/README.md:77

  • Application URLs and module settings are removed with their application context, so they are not held for the lifetime of the process as stated here. Only process-level runtime flags persist for that lifetime.
Module settings, application URLs, and runtime flags are held in memory for the lifetime of the process. Runtime configuration is managed through environment variables or sentinel files in `/tmp`.

docs/README.md:158

  • There is no jsruntime-launcher that reads /package/app.config and converts features to command-line flags in this repository. The container launcher parses app.config features into the WebSocket moduleSettings string, while the standalone jsruntime executable accepts --enable... flags directly.
- **Via standalone launcher (jsruntime-launcher)**: The launcher reads an `app.config` JSON file at `/package/app.config` and maps `features` array entries (e.g., `{"name": "player", "enable": true}`) to command-line flags (`--enablePlayer`, `--enableXHR`, etc.), which are then parsed into `ModuleSettings` fields before the application context is created.

docs/README.md:208

  • The console thread reads stdin and queues snippets, but processDevConsoleRequests() evaluates them from the renderer loop. Calling this evaluation a console-thread operation misstates the synchronization model.
- In developer console mode, the console thread continuously reads from a deque of queued scripts and evaluates them in a dedicated console context, separate from launched application contexts.

docs/README.md:270

  • The WebSocket server does not accept a terminateApplication method. Its destroy command is named destroyApplication and then calls NativeJSRenderer::terminateApplication() internally; clients using the documented method name will receive an error response.
| `JSRuntimeServer`       | WebSocket server (singleton) that listens on a configurable port and dispatches JSON-encoded `launchApplication`, `createApplication`, `runApplication`, `runJavaScript`, and `terminateApplication` commands to `NativeJSRenderer`. Uses websocketpp with Asio transport.                                             | `src/JSRuntimeServer.cpp`, `include/JSRuntimeServer.h`             |

docs/README.md:271

  • JSRuntimeContainer does not use JSRuntimeClient; its connectAndSend() method creates its own websocketpp client. JSRuntimeClient is a separate singleton client, so this usage relationship is misleading for integrators.
| `JSRuntimeClient`       | WebSocket client (singleton) that connects to a `JSRuntimeServer` instance and provides a synchronous `sendCommand` interface with a 5-second response timeout. Used by `JSRuntimeContainer` to relay launch commands into a container.                                                                                | `src/JSRuntimeClient.cpp`, `include/JSRuntimeClient.h`             |

docs/README.md:132

  • dobby is not referenced by any container target in this repository; container support uses JSRuntimeContainer with cgroup lookup and setns. Listing it as an additional required build dependency would send integrators looking for an unnecessary package unless an external build specifically requires it.
- **Build Dependencies**: westeros, essos, rapidjson, rtcore, libuv, gstreamer1.0, uwebsockets, JavaScriptCore, websocketpp, cjson, boost, virtual/egl. When container widget support is enabled, dobby is additionally required.

docs/README.md:189

  • Initialization checks the NATIVEJS_EMBED_THUNDERJS and NATIVEJS_ENABLE_WEBSOCKET_SERVER environment variables before falling back to sentinel files; the diagram currently documents only the file checks and omits the supported environment-variable path.
    Note over NR: Check /tmp sentinel files for ThunderJS, WebBridge, WS server

docs/README.md:295

  • JSRemoteInspectorStart() is not defined or called in this repository. The JSC implementation starts the custom inspector through InspectorHTTPServer::singleton().start(...) when REMOTE_INSPECTOR_ENABLE is enabled.
| Remote Inspector         | JavaScript debugger connection over network                            | `JSRemoteInspectorStart()`, env `NATIVEJS_INSPECTOR_SERVER`                               |

docs/README.md:341

  • JSSynchronousGarbageCollectForDebugging() is called by JavaScriptContext::~JavaScriptContext() only when the context being destroyed is the top-level context, not on every context release as this row states.
| `JSSynchronousGarbageCollectForDebugging()` | Forces synchronous GC on context release                               | `src/jsc/JavaScriptContext.cpp` |

docs/README.md:160

  • The minijsdom precedence guarantee only applies to ModuleSettings::fromString(), which uses an else if. Standalone command-line parsing can set both flags, and JavaScriptContext then selects full JSDOM before MiniJSDOM. Please scope this statement to the options-string parser.
All flags default to `false`; only tokens present in the options string activate the corresponding module. `minijsdom` and `jsdom` are mutually exclusive — `minijsdom` takes precedence when both tokens appear.

docs/README.md:201

  • For the WebSocket server, .html and .htm URLs are routed to mExternalHandler->runExternalApplication() and do not call runApplication() to download/evaluate the script. The blanket state transition here is incorrect for that supported input.
- A `launchApplication` JSON-RPC call (or equivalent WebSocket command) causes `NativeJSRenderer` to allocate a new application ID, create a `JavaScriptContext` with the specified module settings, download the script via libcurl if it is a remote URL, and evaluate it in the context.

docs/README.md:291

  • The dynamic AAMP path does not export AAMPPlayer_LoadJS/AAMPPlayer_UnloadJS; it dlopens libaampjsbindings.so and resolves aamp_LoadJSController and aamp_UnloadJSController. The current row conflates static and dynamic symbols.
| AAMP JS Bindings         | Media playback control exposed as `AAMPMediaPlayer` in JavaScript      | `AAMPPlayer_LoadJS()`, `AAMPPlayer_UnloadJS()` (dynamic: `libaampjsbindings.so`)          |

docs/README.md:365

  • The inspector environment variable is only consumed inside #ifdef REMOTE_INSPECTOR_ENABLE; setting it on the default build does not activate an inspector. The configuration entry should state this compile-time prerequisite.
| `NATIVEJS_INSPECTOR_SERVER`   | string (env)   | _(not set)_     | Activates the remote JavaScript inspector. Format: `host:port` (e.g., `0.0.0.0:9226`).                        |

docs/README.md:371

  • WS_SERVER_PORT is used by the external JSRuntimeServer, JSRuntimeClient, and container client, while the JavaScript rtWebSocketServer binding takes its port from the JavaScript options object. Calling this simply the WebSocket server port conflates two different servers.
| `WS_SERVER_PORT`              | int (build)    | `5000`          | WebSocket server listen port. Defined at build time via `-DWS_SERVER_PORT=5000`.                               |

docs/README.md:268

  • crypto is not a general binding registered for every context; the JSC crypto shim is assigned by linkedjsdomwrapper.js when full JSDOM is enabled. Listing it alongside unconditional bindings makes the module behavior inaccurate.
| `JavaScriptContext`     | Per-application JSC global context. Registers all enabled module bindings (setTimeout, XHR, WebSocket, HTTP, Fetch, JSDOM, crypto, player). Tracks performance metrics (context creation time, execution time, playback start time) and network metrics via `NetworkMetricsListener`. Implements `IJavaScriptContext`. | `src/jsc/JavaScriptContext.cpp`, `include/jsc/JavaScriptContext.h` |

docs/README.md:247

  • The constructor flow shown here does not match NativeJSRenderer::createApplicationInternal(): it constructs JavaScriptContext with JavaScriptContextFeatures and an empty URL, then the later run request sets the application URL. Please show the actual ordering and arguments.
    NR->>CTX: new JavaScriptContext(moduleSettings, url, engine)

docs/README.md:254

  • Remote content is evaluated with runScript(chunk.contentsBuffer, ...); only local paths use runFile(url.c_str(), ...). The call flow currently labels every execution as runFile and therefore describes the remote URL path incorrectly.
    NR->>CTX: runFile(scriptPath, args, isApplication=true)

docs/README.md:75

  • JSRuntimeContainer uses setns only while obtaining the container IP address; connectAndSend() then creates the WebSocket client in the caller's namespace and connects to that IP. This does not connect from inside the container namespace as the current description implies.
IPC is handled through two mechanisms. Within the device, Thunder JSON-RPC is the primary channel for application control. The WebSocket server and client provide an alternative channel used for remote control and container-bridged delivery. Container-side delivery is implemented in `JSRuntimeContainer`, which reads the container process PID from the cgroup filesystem, enters the container's network namespace using `setns`, and connects a WebSocket client to the in-container server endpoint.

docs/README.md:49

  • The contexts are independent, but application execution is not concurrent in this implementation: NativeJSRenderer::run() holds mUserMutex while it processes each pending request and evaluates scripts sequentially. Please avoid promising concurrently running applications.
- **Multi-context JavaScript Execution**: Hosts multiple independent JavaScript application contexts within a single process. Each application receives its own JSC global context with its own module bindings, allowing widgets and lightweight apps to run concurrently without interference.
  • Files reviewed: 1/1 changed files
  • Comments generated: 9
  • Review effort level: Lite

Comment thread docs/README.md Outdated
Comment thread docs/README.md Outdated
Comment thread docs/README.md Outdated
Comment thread docs/README.md Outdated
Comment thread docs/README.md Outdated
Comment thread docs/README.md Outdated
Comment thread docs/README.md Outdated
Comment thread docs/README.md Outdated
Comment thread docs/README.md Outdated
Copilot AI review requested due to automatic review settings September 14, 2026 06:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The documentation contains multiple inaccuracies about runtime behavior, configuration, integrations, security, and endpoint behavior.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (17)

docs/README.md:59

  • EssosInstance has a single mJavaScriptKeyListener slot, and each newly constructed JavaScriptContextBase registers itself into that slot. Input is therefore delivered to the most recently registered context, not a managed “active” context; please make this limitation explicit here.
- **Wayland Display and Input Handling**: Integrates with the Essos compositor abstraction layer to set up a Wayland display surface and route keyboard input events from the compositor into the active JavaScript context.

docs/README.md:73

  • JSContextGroupRef and the JSC global GC behavior apply only to the JSC implementation. QuickJS creates JSContext objects on a shared JSRuntime, and its collectGarbage() is empty, so this engine-agnostic paragraph is inaccurate for the documented alternate engine.
The design isolates per-application state (URL, JS context, module flags, performance metrics) inside `JavaScriptContext` and uses a shared `JSContextGroupRef` across all contexts in the process. This allows garbage collection to be coordinated globally through a periodic GLib timer while individual contexts can be released independently. The main GLib event loop processes both JSC internal events and timer callbacks on the main thread; applications are created and run from separate per-application threads that call into the renderer.

docs/README.md:271

  • JSRuntimeContainer does not use JSRuntimeClient; its connectAndSend() creates a separate websocketpp client (SimpleClient) in src/JSRuntimeContainer.cpp. JSRuntimeClient is the standalone interactive client executable, so this row describes the relationship incorrectly.
| `JSRuntimeClient`       | WebSocket client (singleton) that connects to a `JSRuntimeServer` instance and provides a synchronous `sendCommand` interface with a 5-second response timeout. Used by `JSRuntimeContainer` to relay launch commands into a container.                                                                                | `src/JSRuntimeClient.cpp`, `include/JSRuntimeClient.h`             |

docs/README.md:295

  • JSRemoteInspectorStart() is not defined or used in this repository. The inspector is started through InspectorHTTPServer::singleton().start(...) when REMOTE_INSPECTOR_ENABLE is compiled, so the matrix should reference the actual API and feature guard.
| Remote Inspector         | JavaScript debugger connection over network                            | `JSRemoteInspectorStart()`, env `NATIVEJS_INSPECTOR_SERVER`                             |

docs/README.md:320

  • The WebSocket handler calls createApplication() and runApplication() only to append requests to gPendingRequests, then immediately sends the response from onMessage. It does not wait for context creation or script execution, so this sequence incorrectly makes the response follow CTX-->>NR: Done.
    SRV->>NR: createApplication(moduleSettings)
    SRV->>NR: runApplication(id, url)
    NR->>CTX: Script execution
    CTX-->>NR: Done
    SRV-->>ExtTool: JSON response {"result":"ID : <id>"}

docs/README.md:365

  • The implementation does not bind to the host in this host:port value: it parses the port and calls soup_server_listen_all, while the address is only logged. Thus 127.0.0.1:9226 would not restrict the inspector to loopback; document the actual binding behavior or change the implementation.
| `NATIVEJS_INSPECTOR_SERVER`        | string (env)    | _(not set)_     | Activates the remote JavaScript inspector. Format: `host:port` (e.g., `0.0.0.0:9226`).                                                                                                                                                                                                          |

docs/README.md:160

  • ModuleSettings::fromString() uses std::string::find() for each name rather than parsing comma-delimited tokens. For example, notplayer enables player (and wsenhanced also matches ws), so “only tokens present” is not the actual behavior. Either tighten the parser or qualify this documentation.
All flags default to `false`; only tokens present in the options string activate the corresponding module. `minijsdom` and `jsdom` are mutually exclusive — `minijsdom` takes precedence when both tokens appear.

docs/README.md:55

  • The player option does not by itself guarantee that AAMPMediaPlayer exists: the registration is compiled only under ENABLE_JSRUNTIME_PLAYER (and dynamic builds additionally depend on the AAMP binding library; see src/jsc/JavaScriptContext.cpp:113-134 and :420-440). Please document the build-time prerequisite instead of describing this as an always-available DAC module.
- **AAMP Media Player Integration**: Exposes `AAMPMediaPlayer` as a JavaScript global when the player module is enabled. AAMP bindings are deployed as a DAC (Downloadable Application Container) module, decoupling the player library from the runtime binary.

docs/README.md:189

  • The sentinel files checked during renderer initialization enable the JavaScript webSocketServer binding in each context; they do not start the external JSRuntimeServer. The external server is started separately by the standalone --server option, as the configuration table later explains. Please make this diagram note unambiguous.
    Note over NR: Check /tmp sentinel files for ThunderJS, WebBridge, WS server

docs/README.md:365

  • NATIVEJS_INSPECTOR_SERVER is read only inside #ifdef REMOTE_INSPECTOR_ENABLE (src/jsc/JavaScriptEngine.cpp:111-133), and that build option defaults to OFF in CMakeLists.txt:35. Setting this environment variable alone therefore does not activate the inspector; the compile-time prerequisite should be documented.
| `NATIVEJS_INSPECTOR_SERVER`        | string (env)    | _(not set)_     | Activates the remote JavaScript inspector. Format: `host:port` (e.g., `0.0.0.0:9226`).                                                                                                                                                                                                          |

docs/README.md:132

  • The repository has no Dobby option, include, link, or code path; container support is implemented with cgroup PID lookup and setns in JSRuntimeContainer.cpp. Listing Dobby as a required dependency for an unspecified “container widget support” mode is unsupported by this component documentation and can send builders looking for a nonexistent requirement.
- **Build Dependencies**: westeros, essos, rapidjson, rtcore, libuv, gstreamer1.0, uwebsockets, JavaScriptCore, websocketpp, cjson, boost, virtual/egl. When container widget support is enabled, dobby is additionally required.

docs/README.md:135

  • This is not a Systemd service requirement: the implementation directly initializes Essos and requires a reachable Wayland compositor only when Essos integration is enabled and a display is requested. The current heading implies a systemd unit or dependency that this repository neither defines nor uses.
- **Systemd Services**: A running Wayland compositor (Westeros) must be available when Essos integration is enabled and a display is requested.

docs/README.md:341

  • JSSynchronousGarbageCollectForDebugging() is not run on every context release. The destructor invokes it only when the context being destroyed is gTopLevelContext (src/jsc/JavaScriptContext.cpp:136-147); other contexts are released without that synchronous GC. Please narrow this description to avoid implying a per-context guarantee.
| `JSSynchronousGarbageCollectForDebugging()` | Forces synchronous GC on context release                               | `src/jsc/JavaScriptContext.cpp` |

docs/README.md:53

  • This module list is not valid for both engines described above. The QuickJS context implementation only registers XHR, basic WebSocket, Window, and JSDOM from these settings; it does not register HTTP, Fetch, WebSocketEnhanced, or MiniJSDOM. Please scope this list to JSC or document the engine-specific subset.
- **Per-Application Module Configuration**: Each application context is configured at creation time with a set of optional module bindings. Supported modules include HTTP, XHR, WebSocket, WebSocketEnhanced, Fetch, JSDOM, MiniJSDOM, Window, and media Player. Modules are activated by passing an options string to `launchApplication`.

docs/README.md:254

  • Remote URLs are downloaded into memory and evaluated with JavaScriptContext::runScript; only local URLs use runFile. This flow currently labels both cases as runFile, which makes the documented remote execution path inaccurate.
    NR->>CTX: runFile(scriptPath, args, isApplication=true)

docs/README.md:365

  • The example 0.0.0.0:9226 is unsafe to recommend: the inspector has no authentication, accepts Runtime.evaluate, and the current server calls soup_server_listen_all, so it is reachable on all interfaces. Please warn that this is a privileged debugging endpoint and should be protected with network isolation/firewalling or a secure tunnel.
| `NATIVEJS_INSPECTOR_SERVER`        | string (env)    | _(not set)_     | Activates the remote JavaScript inspector. Format: `host:port` (e.g., `0.0.0.0:9226`).                                                                                                                                                                                                          |

docs/README.md:73

  • The implementation does not use a GLib timer for garbage collection: JavaScriptEngine::initialize() calls installTimeout(), and JavaScriptEngine::run() dispatches the custom timeout queue. Calling this a GLib timer gives an incorrect description of the scheduling model.
The design isolates per-application state (URL, JS context, module flags, performance metrics) inside `JavaScriptContext` and uses a shared `JSContextGroupRef` across all contexts in the process. This allows garbage collection to be coordinated globally through a periodic GLib timer while individual contexts can be released independently. The main GLib event loop processes both JSC internal events and timer callbacks on the main thread; applications are created and run from separate per-application threads that call into the renderer.
  • Files reviewed: 1/1 changed files
  • Comments generated: 4
  • Review effort level: Lite

Comment thread docs/README.md Outdated
Comment thread docs/README.md Outdated
Comment thread docs/README.md Outdated
Comment thread docs/README.md Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 14, 2026 06:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Documentation accuracy, scope, configuration, and security details need correction.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (22)

Previously missed (3) — in code that hasn't changed since the last review.

docs/README.md:59

  • EssosInstance does not select an active context: it stores one listener pointer, and each JavaScriptContextBase constructor overwrites it. Describing keyboard routing as going to the active JavaScript context is therefore inaccurate when multiple contexts exist; document that events go only to the most recently registered listener.

This issue also appears in the following locations of the same file:

  • line 73
  • line 208
    docs/README.md:201
  • JSRuntimeServer::launchApplication does not always download and evaluate the URL: URLs containing .html or .htm are sent to mExternalHandler, and with no handler they are not run at all. Qualify this trigger so WebSocket clients are not promised JavaScript execution for HTML launches.

This issue also appears on line 295 of the same file.
docs/README.md:341

  • The API table overstates this as garbage collection on every context release. JavaScriptContext::~JavaScriptContext() calls JSSynchronousGarbageCollectForDebugging() only when the context being destroyed is the process-wide top-level context; other contexts are released without this synchronous collection.

docs/README.md:73

  • The final threading sentence says applications are created and run from separate per-application threads, but those threads only enqueue ApplicationRequests; NativeJSRenderer::run() performs context creation and script evaluation on the renderer loop. This contradicts the more precise threading bullets below and can lead callers to assume script execution is concurrent.
The design isolates per-application state (URL, JS context, module flags, performance metrics) inside `JavaScriptContext` and uses a shared `JSContextGroupRef` across all contexts in the process. This allows garbage collection to be coordinated globally through a periodic GLib timer while individual contexts can be released independently. The main GLib event loop processes both JSC internal events and timer callbacks on the main thread; applications are created and run from separate per-application threads that call into the renderer.

docs/README.md:208

  • The console thread reads interactive input and places snippets on the queue; processDevConsoleRequests() on the renderer loop drains that queue and evaluates the code. This says the console thread reads the queued scripts and evaluates them, which misstates both the producer/consumer roles and the execution thread.
- In developer console mode, the console thread continuously reads from a deque of queued scripts and evaluates them in a dedicated console context, separate from launched application contexts.

docs/README.md:291

  • The AAMP interaction row names only the static entry points and identifies dynamic mode only by its library. Dynamic mode actually resolves aamp_LoadJSController and aamp_UnloadJSController; without those names, this integration table is misleading for dynamic AAMP deployments.
| AAMP JS Bindings         | Media playback control exposed as `AAMPMediaPlayer` in JavaScript      | `AAMPPlayer_LoadJS()`, `AAMPPlayer_UnloadJS()` (dynamic: `libaampjsbindings.so`)        |

docs/README.md:295

  • JSRemoteInspectorStart() is not an implementation symbol in this repository. The inspector is started with InspectorHTTPServer::singleton().start(inspectorServer, port) from JavaScriptEngine::initialize(), so this API reference cannot be used to trace the documented integration.
| Remote Inspector         | JavaScript debugger connection over network                            | `JSRemoteInspectorStart()`, env `NATIVEJS_INSPECTOR_SERVER`                             |

docs/README.md:132

  • The container-support dependency claim is not backed by this component: neither the CMake files nor the namespace/WebSocket launcher reference Dobby. The implementation uses setns, cgroup files, and websocketpp, so listing an undefined 'container widget support' Dobby requirement can cause unnecessary or impossible build setup.
- **Build Dependencies**: westeros, essos, rapidjson, rtcore, libuv, gstreamer1.0, uwebsockets, JavaScriptCore, websocketpp, cjson, boost, virtual/egl. When container widget support is enabled, dobby is additionally required.

docs/README.md:365

  • The host:port description implies that the host controls where the inspector binds, but the implementation parses only the port and calls soup_server_listen_all(), ignoring the supplied address. A value such as 127.0.0.1:9226 therefore does not limit exposure to loopback; document the all-interface behavior or change the binding implementation.
| `NATIVEJS_INSPECTOR_SERVER`        | string (env)    | _(not set)_     | Activates the remote JavaScript inspector. Format: `host:port` (e.g., `0.0.0.0:9226`).                                                                                                                                                                                                          |

docs/README.md:49

  • JavaScriptContext is not always a JSC global context: the supported QuickJS build creates JSContext objects on a JSRuntime instead. Since the next bullet advertises QuickJS support, this feature description should use engine-neutral terminology or explicitly scope the JSC wording.
- **Multi-context JavaScript Execution**: Hosts multiple JavaScript application contexts within a single process, each with its own JSC global context and module bindings. Application requests are evaluated serially by the renderer loop, and shared process resources are not isolated per context.

docs/README.md:53

  • The module list is not shared by both advertised engines: the QuickJS context currently consumes only the XHR, WebSocket, Window, and JSDOM flags, while HTTP, Fetch, enhanced WebSocket, and MiniJSDOM are not registered from ModuleSettings (and the player interface is registered unconditionally). Please qualify this list as JSC-specific or document the QuickJS differences.
- **Per-Application Module Configuration**: Each application context is configured at creation time with a set of optional module bindings. Supported modules include HTTP, XHR, WebSocket, WebSocketEnhanced, Fetch, JSDOM, MiniJSDOM, Window, and media Player. Modules are activated by passing an options string to `launchApplication`.

docs/README.md:61

  • The inspector code is compiled only when REMOTE_INSPECTOR_ENABLE is enabled, and QuickJS has no corresponding implementation. Setting NATIVEJS_INSPECTOR_SERVER alone has no effect in the default build, so this feature description should include the build-time prerequisite.
- **Remote JavaScript Inspector**: Optionally starts a JSC remote inspector server to enable JavaScript debugging over a network connection, controlled via the `NATIVEJS_INSPECTOR_SERVER` environment variable.

docs/README.md:371

  • The documented build-time configurability is not exposed by the current build: CMakeLists.txt unconditionally adds -DWS_SERVER_PORT=5000 when the server is enabled, with no CMake option or substitution for another value. Please describe this as a fixed compile definition or add/document the actual override mechanism.
| `WS_SERVER_PORT`                   | int (build)     | `5000`          | WebSocket server listen port. Defined at build time via `-DWS_SERVER_PORT=5000`.                                                                                                                                                                                                                |

docs/README.md:55

  • The DAC/dynamic-library description is not true for every supported build: static JSC bindings call AAMPPlayer_LoadJS() directly, the non-binding JSC path uses the in-tree PlayerWrapper, and the QuickJS build links AAMP directly. Please scope the DAC/decoupling claim to ENABLE_AAMP_JSBINDINGS_DYNAMIC rather than presenting it as the universal player deployment model.
- **AAMP Media Player Integration**: Exposes `AAMPMediaPlayer` as a JavaScript global when the player module is enabled. AAMP bindings are deployed as a DAC (Downloadable Application Container) module, decoupling the player library from the runtime binary.

docs/README.md:132

  • This dependency list is presented as universal even though the document advertises QuickJS as an alternate engine. The QuickJS build links quickjs.lto and does not use JavaScriptCore, while the JSC build uses libJavaScriptCore; split the common, JSC, and QuickJS dependencies or qualify this list as JSC-specific.
- **Build Dependencies**: westeros, essos, rapidjson, rtcore, libuv, gstreamer1.0, uwebsockets, JavaScriptCore, websocketpp, cjson, boost, virtual/egl. When container widget support is enabled, dobby is additionally required.

docs/README.md:128

  • The timer and deferred-work paths do not schedule callbacks through GLib: installTimeout() stores timers in gTimeoutQueue, dispatchOnMainLoop() stores functions in gPendingFun, and JavaScriptEngine::run() dispatches both queues before/after GLib/uv processing. Please distinguish these internal queues from the GLib event loop.
- **Async / Event Dispatch**: Essos key events are dispatched synchronously to the single listener stored by `EssosInstance` (constructing another context replaces that listener). Timer callbacks and deferred JS execution are scheduled through the GLib main loop rather than blocking caller threads.

docs/README.md:69

  • NativeJSRenderer is not a process-wide singleton: its constructor allocates a new JavaScriptEngine, and each cloned plugin instance can own its own renderer. Saying initialization happens once per process conflicts with the documented cloneable runtime model; scope this to each renderer/plugin instance.
rdkNativeScript is structured around a layered separation between engine management, context lifecycle, and application dispatch. The runtime is initialized once per process through `NativeJSRenderer`, which owns the `IJavaScriptEngine` instance and manages the map of active application contexts. Each application is represented by a numeric identifier mapped to a `JavaScriptContext` instance. The `JavaScriptContextBase` class provides engine-agnostic operations — file loading, script evaluation delegation, key event routing, and ThunderJS / RDK WebBridge code injection — while engine-specific implementations extend it for JSC or QuickJS.

docs/README.md:71

  • This design paragraph describes the component as consuming the JavaScriptCore C API, but the build system explicitly supports JSRUNTIME_ENGINE_NAME=quickjs. The southbound list should be scoped to the JSC implementation or explain the corresponding QuickJS path.
Northbound, the component exposes its API through the Thunder plugin mechanism: clients issue a JSON-RPC `launchApplication` call to a cloned plugin instance. The server path, when enabled, accepts a similar command set over a WebSocket connection on the port defined by `WS_SERVER_PORT` using messages of the form `{ "method": "…", "params": { … } }` (module tokens are passed as `moduleSettings`). The `JSRuntimeServer` dispatches incoming messages to the same `NativeJSRenderer` methods used by the Thunder plugin path. Southbound, the component consumes the JavaScriptCore C API for script evaluation, GStreamer for media pipeline initialization, libcurl for script download from remote URLs, and the Essos API for Wayland compositor setup and keyboard event delivery.

docs/README.md:182

  • Essos is initialized only when a non-empty --display value is supplied and the build includes ENABLE_JSRUNTIME_ESSOS; the initialization flow currently shows it as unconditional. Mark this interaction as optional.
    NR->>Essos: EssosInstance.initialize useWayland
    Essos-->>NR: Essos context ready

docs/README.md:255

  • For a remote URL, runApplicationInternal() downloads the source into memory and calls context->runScript(...); runFile() is used only for local paths. The call flow should distinguish these branches instead of showing runFile(scriptPath) after every download.
    NR->>CURL: downloadFile(url) [if remote URL]
    CURL-->>NR: Script content
    NR->>CTX: runFile(scriptPath, args, isApplication=true)
    CTX-->>NR: Execution complete

docs/README.md:320

  • The WebSocket server enqueues both renderer requests and sends the ID : <id> response immediately; script execution happens later on the renderer loop. The diagram currently shows the response after execution, which gives clients the wrong timing contract.
    SRV->>NR: createApplication(moduleSettings)
    SRV->>NR: runApplication(id, url)
    NR->>CTX: Script execution
    CTX-->>NR: Done
    SRV-->>ExtTool: JSON response {"result":"ID : <id>"}

docs/README.md:268

  • This row describes the JSC implementation as the universal JavaScriptContext, but the supported QuickJS implementation uses JSContext, does not implement NetworkMetricsListener, and registers a different set of bindings. Mark the row JSC-specific or add a separate QuickJS description.
| `JavaScriptContext`     | Per-application JSC global context. Registers all enabled module bindings (setTimeout, XHR, WebSocket, HTTP, Fetch, JSDOM, crypto, player). Tracks performance metrics (context creation time, execution time, playback start time) and network metrics via `NetworkMetricsListener`. Implements `IJavaScriptContext`. | `src/jsc/JavaScriptContext.cpp`, `include/jsc/JavaScriptContext.h` |
  • Files reviewed: 1/1 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread docs/README.md Outdated
Comment thread docs/README.md Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 14, 2026 07:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The documentation contains unresolved inaccuracies in runtime behavior, threading, integrations, and configuration.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (17)

Previously missed (4) — in code that hasn't changed since the last review.

docs/README.md:53

  • The document advertises QuickJS as an alternate engine, but this module list is presented as generally supported. src/quickjs/JavaScriptContext.cpp only conditionally loads XHR, WebSocket, Window, and JSDOM; the documented HTTP, WebSocketEnhanced, Fetch, and MiniJSDOM flags are not consumed by that implementation. Scope this list to JSC or document the engine-specific support.

This issue also appears on line 295 of the same file.
docs/README.md:73

  • This paragraph still assigns context creation/evaluation to per-application threads and describes a GLib timer, but NativeJSRenderer::run() performs the queued work on the renderer loop and JavaScriptEngine drains its own timeout queue before GLib polling. Application threads only enqueue requests, so this description should match the threading model below.

This issue also appears on line 128 of the same file.
docs/README.md:124

  • The dedicated console thread does not process queued snippets; it reads interactive input and pushes snippets into codeToExecute. NativeJSRenderer::processDevConsoleRequests() on the renderer loop evaluates them, so this worker-thread description is reversed.

This issue also appears on line 254 of the same file.
docs/README.md:201

  • The state-flow description treats every launchApplication as a script download/evaluation. The WebSocket implementation has a separate .html/.htm branch that delegates to IExternalApplicationHandler::runExternalApplication() and does not call runApplication() itself, so this flow needs to document that exception.

This issue also appears on line 349 of the same file.

docs/README.md:59

  • EssosInstance does not track an active context: it stores one mJavaScriptKeyListener, and each new JavaScriptContextBase overwrites it. Saying input is routed to the active context is misleading for multiple apps; describe delivery to the single registered listener instead.
- **Wayland Display and Input Handling**: Integrates with the Essos compositor abstraction layer to set up a Wayland display surface and route keyboard input events from the compositor into the active JavaScript context.

docs/README.md:61

  • The implementation is not a JSC remote-inspector server: JavaScriptEngine starts the project's custom InspectorHTTPServer (libsoup HTTP/WebSocket/CDP) when REMOTE_INSPECTOR_ENABLE is compiled in. Calling it a JSC remote inspector can lead integrators to expect different endpoints and activation behavior.
- **Remote JavaScript Inspector**: Optionally starts a JSC remote inspector server to enable JavaScript debugging over a network connection, controlled via the `NATIVEJS_INSPECTOR_SERVER` environment variable. The endpoint is unauthenticated and should be restricted to a trusted network.

docs/README.md:128

  • The event-dispatch description says timers and deferred work are scheduled through GLib, but the implementation stores deferred callbacks in a private queue and drains both that queue and the timeout heap from JavaScriptEngine::run(). GLib/uv polling is performed alongside that work on the renderer loop.
- **Async / Event Dispatch**: Essos key events are dispatched synchronously to the single listener stored by `EssosInstance` (constructing another context replaces that listener). Timer callbacks and deferred JS execution are scheduled through the GLib main loop rather than blocking caller threads.

docs/README.md:254

  • For a remote URL, runApplicationInternal() calls downloadFile() and then evaluates the returned buffer with context->runScript(...); it does not call runFile() or pass a downloaded script path. The flow should show the actual remote execution method.
    NR->>CTX: runFile(scriptPath, args, isApplication=true)

docs/README.md:43

  • The graph reverses the display/input path: NativeJSRenderer calls EssosInstance, and Essos creates the Wayland context; there is no direct runtime-to-Westeros edge. The Westeros -> Essos edge therefore contradicts the implementation. Route the runtime through Essos and then to the compositor.
    JSRuntime -->|Wayland display + key input| Westeros
    AAMP -->|Pipeline control| GST
    Westeros -->|Compositor abstraction| Essos

docs/README.md:73

  • The GC callback is not installed as a GLib timer. JavaScriptEngine::initialize() registers it with installTimeout(), which stores it in the engine's gTimeoutQueue and is serviced by JavaScriptEngine::run(). Calling this a GLib timer misstates which loop controls collection.
The design isolates per-application state (URL, JS context, module flags, performance metrics) inside `JavaScriptContext` and uses a shared `JSContextGroupRef` across all contexts in the process. This allows garbage collection to be coordinated globally through a periodic GLib timer while individual contexts can be released independently. The main GLib event loop processes both JSC internal events and timer callbacks on the main thread; applications are created and run from separate per-application threads that call into the renderer.

docs/README.md:132

  • This dependency list does not match the build configuration: the CMake files link libcurl, OpenSSL, libffi, GLib/GIO, uWebSockets, libuv, zlib, and engine-specific libraries, while this list names westeros, boost, virtual/egl, and dobby even though they are not referenced by the build. Please derive the base and feature-conditional dependencies from the CMake options so integrators are not given missing or spurious packages.
- **Build Dependencies**: westeros, essos, rapidjson, rtcore, libuv, gstreamer1.0, uwebsockets, JavaScriptCore, websocketpp, cjson, boost, virtual/egl. When container widget support is enabled, dobby is additionally required.

docs/README.md:291

  • The dynamic AAMP implementation does not use the AAMPPlayer_* entry points listed here. It loads libaampjsbindings.so and resolves aamp_LoadJSController/aamp_UnloadJSController; reserve the AAMPPlayer_* names for the static path so the integration matrix identifies the callable symbols correctly.
| AAMP JS Bindings         | Media playback control exposed as `AAMPMediaPlayer` in JavaScript      | `AAMPPlayer_LoadJS()`, `AAMPPlayer_UnloadJS()` (dynamic: `libaampjsbindings.so`)        |

docs/README.md:295

  • JSRemoteInspectorStart() is not an API in this repository. The JSC engine starts the inspector through InspectorHTTPServer::singleton().start(...); documenting the nonexistent symbol sends integrators to an unusable entry point.
| Remote Inspector         | JavaScript debugger connection over network                            | `JSRemoteInspectorStart()`, env `NATIVEJS_INSPECTOR_SERVER`                             |

docs/README.md:341

  • JSSynchronousGarbageCollectForDebugging() is not run whenever any context is released. The destructor invokes it only when the released context is the process-wide gTopLevelContext; other context releases skip this call. Qualify the API description accordingly.
| `JSSynchronousGarbageCollectForDebugging()` | Forces synchronous GC on context release                               | `src/jsc/JavaScriptContext.cpp` |

docs/README.md:349

  • The JSC key handler creates an rtMapObject and passes it to jsruntime.onKeyDown/onKeyUp; it does not create a DOM KeyboardEvent or other DOM event instance. Calling it a synthetic DOM event overstates the contract and may lead consumers to rely on missing DOM properties.
- **Event Processing**: Keyboard events from Essos flow through `EssosInstance` static callbacks, which call `onKeyPress` / `onKeyRelease` on the registered `JavaScriptKeyListener`. `JavaScriptContextBase` implements that listener and forwards events to the engine-specific `processKeyEvent()` implementation. JSC contexts then inject a synthetic DOM key event object into the executing script context.

docs/README.md:247

  • createApplicationInternal() constructs the context with an empty URL (new JavaScriptContext(features, "", mEngine)); runApplicationInternal() sets the application URL immediately before executing the script. The diagram's url constructor argument does not match that lifecycle.
    NR->>CTX: new JavaScriptContext(moduleSettings, url, engine)

docs/README.md:132

  • The dependency list omits QuickJS even though the document advertises it as an alternate engine. JSRUNTIME_ENGINE_NAME=quickjs selects src/quickjs/include.cmake, which links libquickjs.lto instead of JavaScriptCore; the current list can therefore give QuickJS builders the wrong prerequisites.
- **Build Dependencies**: westeros, essos, rapidjson, rtcore, libuv, gstreamer1.0, uwebsockets, JavaScriptCore, websocketpp, cjson, boost, virtual/egl. When container widget support is enabled, dobby is additionally required.
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread docs/README.md Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 14, 2026 08:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The documentation contains unresolved moderate inaccuracies in threading, event delivery, and lifecycle behavior.

Review details

Suppressed comments (19)

Previously missed (5) — in code that hasn't changed since the last review.

docs/README.md:124

  • The developer-console thread reads stdin and queues snippets; processDevConsoleRequests() on the renderer loop consumes and evaluates them. Calling the console thread the processor misstates which thread executes JavaScript.

This issue also appears on line 208 of the same file.
docs/README.md:128

  • installTimeout()/dispatchTimeouts() and dispatchOnMainLoop() queues are drained by JavaScriptEngine::run(); they are not scheduled through the GLib main loop. This sentence should identify the engine/renderer loop as the dispatch point for timers and deferred JavaScript work.

This issue also appears on line 295 of the same file.
docs/README.md:201

  • JSRuntimeServer::onMessage() treats .html/.htm launch URLs specially: it delegates them to mExternalHandler->runExternalApplication() and does not call runApplication(). This lifecycle description is therefore not true for every equivalent WebSocket command.

This issue also appears on line 254 of the same file.
docs/README.md:269

  • EssosInstance forwards events only to its single registered listener; constructing another context replaces that pointer. JavaScriptContextBase therefore cannot route events to an independently selected active context as stated here.
    docs/README.md:291
  • The dynamic AAMP path does not call AAMPPlayer_LoadJS()/AAMPPlayer_UnloadJS(). It dlopens libaampjsbindings.so and resolves aamp_LoadJSController()/aamp_UnloadJSController(), so the current row does not accurately identify the dynamic entry points.

docs/README.md:59

  • EssosInstance stores only one JavaScriptKeyListener, and each new JavaScriptContextBase overwrites that pointer (src/JavaScriptContextBase.cpp:59-61). There is no active-context selection, so this wording can promise key delivery to an app that is no longer registered.
- **Wayland Display and Input Handling**: Integrates with the Essos compositor abstraction layer to set up a Wayland display surface and route keyboard input events from the compositor into the active JavaScript context.

docs/README.md:73

  • This contradicts the threading model below: NativeJSRenderer::run() drains gPendingRequests and performs context creation and script evaluation on the renderer loop, while application/IPC threads only enqueue requests. Describing per-application threads as doing the work is misleading.
The design isolates per-application state (URL, JS context, module flags, performance metrics) inside `JavaScriptContext` and uses a shared `JSContextGroupRef` across all contexts in the process. This allows garbage collection to be coordinated globally through a periodic GLib timer while individual contexts can be released independently. The main GLib event loop processes both JSC internal events and timer callbacks on the main thread; applications are created and run from separate per-application threads that call into the renderer.

docs/README.md:160

  • This precedence is enforced only by ModuleSettings::fromString(). Standalone argument parsing can set enableJSDOM and enableMiniJSDOM independently, and JavaScriptContext::registerUtils() checks JSDOM first, so passing both --enableJSDOM and --enableMiniJSDOM selects full JSDOM rather than MiniJSDOM.
All flags default to `false`; only tokens present in the options string activate the corresponding module. `minijsdom` and `jsdom` are mutually exclusive — `minijsdom` takes precedence when both tokens appear.

docs/README.md:202

  • The renderer method is terminateApplication, but the WebSocket-facing command is destroyApplication. Leaving this unqualified in the state-transition API list conflicts with the WebSocket contract documented elsewhere and can lead clients to send an unsupported method.
- A `terminateApplication` call releases the `JavaScriptContext` for the given ID, runs synchronous garbage collection on the global context if it was the top-level context, and removes the entry from the context map.

docs/README.md:208

  • The console worker does not read from or evaluate the deque: runDeveloperConsole() queues stdin lines, while the renderer loop's processDevConsoleRequests() consumes and evaluates them. This repeats the incorrect thread attribution above.
- In developer console mode, the console thread continuously reads from a deque of queued scripts and evaluates them in a dedicated console context, separate from launched application contexts.

docs/README.md:254

  • For remote URLs, NativeJSRenderer downloads into a MemoryStruct and calls runScript(chunk.contentsBuffer, ...); runFile(scriptPath, ...) is used for local paths. The diagram currently shows a remote script-path call that the implementation does not make.
    NR->>CTX: runFile(scriptPath, args, isApplication=true)

docs/README.md:295

  • JSRemoteInspectorStart() is not a symbol used by this repository. Under REMOTE_INSPECTOR_ENABLE, JavaScriptEngine starts InspectorHTTPServer::singleton().start(), so this reference sends integrators to a nonexistent API.
| Remote Inspector         | JavaScript debugger connection over network                            | `JSRemoteInspectorStart()`, env `NATIVEJS_INSPECTOR_SERVER`                             |

docs/README.md:349

  • Essos callbacks call only the single mJavaScriptKeyListener; constructing another context replaces it rather than selecting an active context. Please describe delivery to the registered listener instead.
- **Event Processing**: Keyboard events from Essos flow through `EssosInstance` static callbacks, which call `onKeyPress` / `onKeyRelease` on the registered `JavaScriptKeyListener`. `JavaScriptContextBase` implements that listener and forwards events to the engine-specific `processKeyEvent()` implementation. JSC contexts then inject a synthetic DOM key event object into the executing script context.

docs/README.md:73

  • The GC interval is registered with the custom installTimeout() queue and executed by dispatchTimeouts() inside JavaScriptEngine::run(); it is not a GLib timer. Please describe the actual renderer-loop timeout mechanism so operators do not look for a GLib timer that is never installed.
The design isolates per-application state (URL, JS context, module flags, performance metrics) inside `JavaScriptContext` and uses a shared `JSContextGroupRef` across all contexts in the process. This allows garbage collection to be coordinated globally through a periodic GLib timer while individual contexts can be released independently. The main GLib event loop processes both JSC internal events and timer callbacks on the main thread; applications are created and run from separate per-application threads that call into the renderer.

docs/README.md:53

  • Because JSRUNTIME_ENGINE_NAME can select QuickJS, this list is not valid for every supported build: src/quickjs/JavaScriptContext.cpp only loads XHR, WebSocket, Window, and JSDOM from these settings, while HTTP, Fetch, WebSocketEnhanced, and MiniJSDOM are ignored. Please qualify the list as JSC-specific or document the QuickJS-specific behavior.
- **Per-Application Module Configuration**: Each application context is configured at creation time with a set of optional module bindings. Supported modules include HTTP, XHR, WebSocket, WebSocketEnhanced, Fetch, JSDOM, MiniJSDOM, Window, and media Player. Modules are activated by passing an options string to `launchApplication`.

docs/README.md:189

  • The WebSocket sentinel enables the in-context webSocketServer binding; it does not start the external JSRuntimeServer (that requires the standalone --server option). This diagram note is ambiguous and can send operators to the wrong configuration mechanism.
    Note over NR: Check /tmp sentinel files for ThunderJS, WebBridge, WS server

docs/README.md:341

  • JSSynchronousGarbageCollectForDebugging() is called in the destructor only when the context being released is the current gTopLevelContext; other application-context releases do not invoke it. Qualify this row so it does not imply synchronous GC for every context.
| `JSSynchronousGarbageCollectForDebugging()` | Forces synchronous GC on context release                               | `src/jsc/JavaScriptContext.cpp` |

docs/README.md:353

  • EthanLog routing is compiled only when USE_ETHANLOG is enabled; setting ETHAN_LOGGING_PIPE alone leaves the stdout path active in a normal build. Please document the compile-time guard along with the environment variable.
- **Logging & Diagnostics**: Console logs are emitted as `[LEVEL] JsRuntime Thread-<tid>: <message>`; when `ETHAN_LOGGING_PIPE` is set (EthanLog enabled), the message is prefixed with `JSRuntime [Thread-<tid>]`. The log level is controlled by the `NATIVEJS_LOG_LEVEL` environment variable (DEBUG, INFO, WARN, ERROR, FATAL); the default level is INFO. When `ETHAN_LOGGING_PIPE` is set, output is redirected to the EthanLog daemon using mapped severity levels. Key log points include engine initialization, GC timer installation, remote inspector startup, Essos context creation, application creation and termination, script download status, and WebSocket server events.

docs/README.md:132

  • There is no dobby option, include, link, or implementation in this repository, and BUILD_JSRUNTIME_CONTAINER builds the container utility without it. This unsupported dependency claim should be removed or tied to a concrete external build configuration.
- **Build Dependencies**: westeros, essos, rapidjson, rtcore, libuv, gstreamer1.0, uwebsockets, JavaScriptCore, websocketpp, cjson, boost, virtual/egl. When container widget support is enabled, dobby is additionally required.
  • Files reviewed: 1/1 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@vjain008
vjain008 merged commit 323f72e into rdkcentral:develop Sep 18, 2026
4 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 18, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants