[bidi][js] Add methods to add/remove handlers in Script module

[pujagani]

User description

Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

Precursor to adding methods for Dom Mutations handling.

Motivation and Context

Adding a way to add callbacks and remove the callbacks for script module related events.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

---

PR Type

Enhancement

---

Description

• Added ScriptEvent constants to define event types (MESSAGE, REALM_CREATED, REALM_DESTROYED).
• Introduced addCallback and removeCallback methods to manage event callbacks.
• Implemented invokeCallbacks method to trigger all registered callbacks for a specific event.
• Updated onMessage, onRealmCreated, and onRealmDestroyed methods to use ScriptEvent constants.
• Modified subscribeAndHandleEvent to register callbacks and handle WebSocket messages.

---

Changes walkthrough 📝

	Relevant files
Enhancement	scriptManager.js Add callback management and event handling in ScriptManager javascript/node/selenium-webdriver/bidi/scriptManager.js Added ScriptEvent constants for event types. Introduced methods to add and remove callbacks. Implemented invokeCallbacks to trigger callbacks. Updated event subscription methods to use ScriptEvent constants. +62/-9

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[codiumai-pr-agent-pro]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review Error Handling: The removeCallback method throws an error if the callback ID is not found. This could be handled more gracefully, perhaps by returning a boolean indicating success or failure, rather than throwing an error which might need to be caught by the caller. Memory Management: There is no apparent mechanism to prevent the #listener map from growing indefinitely as callbacks are added and not removed. This could potentially lead to memory leaks if not managed properly.

[codiumai-pr-agent-pro]

PR Code Suggestions ✨

Category	Suggestion	Score
Performance	Improve the performance of the removeCallback method by storing a mapping of callback IDs to their respective event types In the removeCallback method, instead of iterating over all event types to find the callback ID, you can store a mapping of callback IDs to their respective event types. This will improve the performance of the removeCallback method. javascript/node/selenium-webdriver/bidi/scriptManager.js [62-71] -let hasId = false -for (const [, callbacks] of this.#listener) { - if (callbacks.has(id)) { - callbacks.delete(id) - hasId = true - } -} - -if (!hasId) { +const eventType = this.#callbackIdToEventType.get(id) +if (!eventType) { throw Error(`Callback with id ${id} not found`) } +this.#listener.get(eventType).delete(id) +this.#callbackIdToEventType.delete(id) Apply this suggestion Suggestion importance[1-10]: 8 Why: The suggestion correctly identifies a performance issue in the removeCallback method and provides a valid solution to optimize the callback management by mapping IDs to event types, which can significantly reduce the time complexity of the method.	8
Possible issue	Add a check in the addCallback method to ensure the eventType is valid before adding the callback In the addCallback method, add a check to ensure that the eventType is valid before adding the callback. This will prevent potential issues if an invalid event type is passed. javascript/node/selenium-webdriver/bidi/scriptManager.js [54-58] +if (!this.#listener.has(eventType)) { + throw new Error(`Invalid event type: ${eventType}`) +} const id = ++this.#callbackId const eventCallbackMap = this.#listener.get(eventType) eventCallbackMap.set(id, callback) return id Apply this suggestion Suggestion importance[1-10]: 7 Why: This suggestion correctly addresses a potential issue where an invalid eventType could be passed to the addCallback method. Adding a check enhances the robustness of the method.	7
Robustness	Add error handling in the invokeCallbacks method to catch exceptions thrown by callback functions In the invokeCallbacks method, add error handling to catch any exceptions thrown by the callback functions to prevent the entire process from failing. javascript/node/selenium-webdriver/bidi/scriptManager.js [78-79] for (const [, callback] of callbacks) { - callback(data) + try { + callback(data) + } catch (error) { + console.error(`Error invoking callback: ${error}`) + } } Apply this suggestion Suggestion importance[1-10]: 7 Why: Adding error handling in the invokeCallbacks method is a good practice to ensure that exceptions in callbacks do not interrupt the overall execution flow, thus improving the robustness of the event handling system.	7
Best practice	Remove the redundant await keyword before the return statement in the subscribeAndHandleEvent method In the subscribeAndHandleEvent method, remove the redundant await keyword before the return statement as it is not necessary. javascript/node/selenium-webdriver/bidi/scriptManager.js [488] -return await this.subscribeAndHandleEvent(ScriptEvent.MESSAGE, callback) +return this.subscribeAndHandleEvent(ScriptEvent.MESSAGE, callback) Apply this suggestion Suggestion importance[1-10]: 5 Why: The suggestion to remove the redundant await is correct as it simplifies the code without affecting functionality. However, it is a minor improvement in terms of code cleanliness and does not impact performance or functionality significantly.	5