[py][bidi]: Implement BiDi browser module

[navin772]

User description

🔗 Related Issues

💥 What does this PR do?

Implements the BiDi browser module for the python bindings - https://w3c.github.io/webdriver-bidi/#module-browser.

These are the commands that have been added:

1. create_user_context
2. get_user_contexts
3. remove_user_context
4. get_client_windows

The PR also adds a new common.py file in the bidi/ directory that will contain the commonly used functions like command_builder to avoid repetition.

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

• New feature (non-breaking change which adds functionality and tests!)

---

PR Type

Enhancement, Tests

---

Description

• Implemented BiDi browser module for Python bindings.

• Added commands for user context and client window management.

• Refactored common BiDi command builder to avoid duplication.

• Introduced comprehensive tests for BiDi browser functionality.

---

Changes walkthrough 📝

	Relevant files
Enhancement	browser.py Added BiDi browser module implementation                                  py/selenium/webdriver/common/bidi/browser.py Added BiDi browser module implementation. Introduced user context and client window management commands. Defined ClientWindowInfo and ClientWindowState classes. +190/-0  common.py Added reusable BiDi command builder                                            py/selenium/webdriver/common/bidi/common.py Added a reusable command_builder function for BiDi commands. Simplified command construction for BiDi modules. +38/-0    network.py Refactored BiDi network commands to use common builder      py/selenium/webdriver/common/bidi/network.py Replaced inline command builders with the common command_builder. Simplified network-related BiDi command execution. +11/-33  webdriver.py Integrated BiDi browser module into WebDriver                        py/selenium/webdriver/remote/webdriver.py Integrated BiDi browser module into WebDriver. Added browser property for accessing BiDi browser commands. +25/-0
Miscellaneous	__init__.py Added init file for BiDi tests                                                      py/test/selenium/webdriver/common/bidi/init.py Added initialization file for BiDi tests. +17/-0
Tests	bidi_browser_tests.py Added tests for BiDi browser module                                            py/test/selenium/webdriver/common/bidi_browser_tests.py Added tests for BiDi browser module functionality. Covered user context and client window management. Validated ClientWindowState constants. +99/-0

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ❌ 1234 - Not compliant Non-compliant requirements: • Fix issue where JavaScript in link's href is not triggered on click() in Firefox 42.0 with Selenium 2.48.0/2.48.2 • Ensure click() properly executes JavaScript in href attributes • Maintain backward compatibility with previous working behavior in Selenium 2.47.1 5678 - Not compliant Non-compliant requirements: • Fix "Error: ConnectFailure (Connection refused)" when instantiating ChromeDriver multiple times • Address issue specific to Ubuntu 16.04.4 with Chrome 65.0.3325.181 and ChromeDriver 2.35 • Ensure subsequent ChromeDriver instances work properly after first instance

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Error Handling The create_user_context and get_client_windows methods don't include error handling for potential BiDi protocol errors or connection issues. Consider adding try/except blocks to handle potential exceptions. def create_user_context(self) -> str: """Creates a new user context. Returns: ------- str: The ID of the created user context. """ result = self.conn.execute(command_builder("browser.createUserContext", {})) return result["userContext"] def get_user_contexts(self) -> List[str]: """Gets all user contexts. Returns: ------- List[str]: A list of user context IDs. """ result = self.conn.execute(command_builder("browser.getUserContexts", {})) return [context_info["userContext"] for context_info in result["userContexts"]] def remove_user_context(self, user_context_id: str) -> None: """Removes a user context. Parameters: ----------- user_context_id: The ID of the user context to remove. Raises: ------ Exception: If the user context ID is "default" or does not exist. """ params = {"userContext": user_context_id} self.conn.execute(command_builder("browser.removeUserContext", params)) def get_client_windows(self) -> List[ClientWindowInfo]: """Gets all client windows. Returns: ------- List[ClientWindowInfo]: A list of client window information. """ result = self.conn.execute(command_builder("browser.getClientWindows", {})) return [ClientWindowInfo.from_dict(window) for window in result["clientWindows"]] Command Builder Return The command_builder function uses yield to create a generator but the docstring indicates it returns a response. This may cause confusion as the actual response comes from the yield statement, not the function return. def command_builder(method: str, params: Dict = None) -> Dict: """Build a command iterator to send to the BiDi protocol. Parameters: ----------- method: The method to execute. params: The parameters to pass to the method. Default is None. Returns: -------- The response from the command execution. """ if params is None: params = {} command = {"method": method, "params": params} cmd = yield command return cmd Test Assertions The test_get_client_windows has assertions that may fail on some browser configurations where window coordinates could be zero or negative. Consider making these assertions more flexible. def test_get_client_windows(driver): """Test getting client windows.""" client_windows = driver.browser.get_client_windows() assert client_windows is not None assert len(client_windows) > 0 window_info = client_windows[0] assert isinstance(window_info, ClientWindowInfo) assert window_info.get_client_window() is not None assert window_info.get_state() is not None assert isinstance(window_info.get_state(), str) assert window_info.get_width() > 0 assert window_info.get_height() > 0 assert isinstance(window_info.is_active(), bool) assert window_info.get_x() > 0 assert window_info.get_y() > 0

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	✅ Prevent removing default context Suggestion Impact: The suggestion was directly implemented in the commit. The code now checks if the user_context_id is 'default' and raises an exception if it is, preventing the removal of the default user context. code diff: + if user_context_id == "default": + raise Exception("Cannot remove the default user context") + Add validation to prevent removing the "default" user context. The docstring mentions that an exception will be raised if the user context ID is "default", but there's no actual validation in the code to prevent this operation. py/selenium/webdriver/common/bidi/browser.py [168-180] def remove_user_context(self, user_context_id: str) -> None: """Removes a user context. Parameters: ----------- user_context_id: The ID of the user context to remove. Raises: ------ Exception: If the user context ID is "default" or does not exist. """ + if user_context_id == "default": + raise Exception("Cannot remove the default user context") + params = {"userContext": user_context_id} self.conn.execute(command_builder("browser.removeUserContext", params)) [Suggestion has been applied] Suggestion importance[1-10]: 9 __ Why: The suggestion adds critical validation to prevent removing the "default" user context, which aligns with the documented behavior in the docstring. Without this check, users could attempt to remove the default context, potentially causing system instability or unexpected behavior.	High
	Validate required fields Add validation for required fields in the from_dict method. Currently, if any of the expected keys are missing in the input dictionary, the method will silently pass None values to the constructor, which could lead to unexpected behavior or errors later. py/selenium/webdriver/common/bidi/browser.py [117-137] @classmethod def from_dict(cls, data: Dict) -> "ClientWindowInfo": """Creates a ClientWindowInfo instance from a dictionary. Parameters: ----------- data: A dictionary containing the client window information. Returns: ------- ClientWindowInfo: A new instance of ClientWindowInfo. + + Raises: + ------ + KeyError: If any required field is missing from the data dictionary. """ + required_fields = ["clientWindow", "state", "width", "height", "x", "y", "active"] + for field in required_fields: + if field not in data: + raise KeyError(f"Required field '{field}' missing from client window data") + return cls( - client_window=data.get("clientWindow"), - state=data.get("state"), - width=data.get("width"), - height=data.get("height"), - x=data.get("x"), - y=data.get("y"), - active=data.get("active"), + client_window=data["clientWindow"], + state=data["state"], + width=data["width"], + height=data["height"], + x=data["x"], + y=data["y"], + active=data["active"], ) Apply this suggestion Suggestion importance[1-10]: 8 __ Why: This suggestion adds important validation for required fields in the from_dict method, preventing silent failures when dictionary keys are missing. This would catch potential bugs early and provide clear error messages instead of allowing None values to propagate through the system.	Medium
Update

[navin772]

@shbenzer thank you for implementing the Network module in python, I have implemented the Browser module taking inspiration from it! Feel free to review this.

[shbenzer]

This looks great!

Could we add the browser.SetClientWindowState browser command too?

[shbenzer]

Great work @navin772!