From ac902163e702751ed88b443c194259bb9a1f25f2 Mon Sep 17 00:00:00 2001 From: "microsoft-playwright-automation[bot]" <203992400+microsoft-playwright-automation[bot]@users.noreply.github.com> Date: Fri, 13 Nov 2026 17:53:34 +0000 Subject: [PATCH] fix(handlers): support immutable and built-in callbacks Fixes: https://github.com/microsoft/playwright-python/issues/3340 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- playwright/_impl/_impl_to_api_mapping.py | 77 ++++++++++++++---------- tests/sync/python/test_sync.py | 17 +++++- 2 files changed, 58 insertions(+), 36 deletions(-) diff --git a/playwright/_impl/_impl_to_api_mapping.py b/playwright/_impl/_impl_to_api_mapping.py index 0e2e16f5e..a44d2ff3b 100644 --- a/playwright/_impl/_impl_to_api_mapping.py +++ b/playwright/_impl/_impl_to_api_mapping.py @@ -13,14 +13,15 @@ # limitations under the License. import inspect -from typing import Any, Callable, Dict, List, Optional, Sequence, Union +from types import BuiltinMethodType, MethodWrapperType +from typing import Any, Callable, Dict, Hashable, List, Optional, Sequence, Union +from weakref import WeakValueDictionary from playwright._impl._errors import Error from playwright._impl._map import Map from playwright._impl._signature import signature API_ATTR = "_pw_api_instance_" -IMPL_ATTR = "_pw_impl_instance_" class ImplWrapper: @@ -34,6 +35,9 @@ def __repr__(self) -> str: class ImplToApiMapping: def __init__(self) -> None: self._mapping: Dict[type, type] = {} + self._wrapped_handlers: WeakValueDictionary[Hashable, Callable[..., None]] = ( + WeakValueDictionary() + ) def register(self, impl_class: type, api_class: type) -> None: self._mapping[impl_class] = api_class @@ -120,41 +124,48 @@ def to_impl( raise Error("Maximum argument depth exceeded") def wrap_handler(self, handler: Callable[..., Any]) -> Callable[..., None]: + # Cache without modifying user objects or keeping unused handlers alive. + # A bound method can be a new object on each attribute lookup. + key: Hashable = id(handler) + if inspect.ismethod(handler): + key = (id(handler.__self__), id(handler.__func__)) + elif ( + isinstance(handler, (BuiltinMethodType, MethodWrapperType)) + and handler.__self__ is not None + ): + key = (id(handler.__self__), type(handler), handler.__qualname__) + wrapper = self._wrapped_handlers.get(key) + if wrapper is not None: + return wrapper + def wrapper_func(*args: Any) -> Any: - parameters = signature(handler).parameters - has_varargs = any( - parameter.kind == inspect.Parameter.VAR_POSITIONAL - for parameter in parameters.values() - ) - arg_count = ( - len(args) - if has_varargs - else sum( - parameter.kind - in ( - inspect.Parameter.POSITIONAL_ONLY, - inspect.Parameter.POSITIONAL_OR_KEYWORD, - ) + try: + parameters = signature(handler).parameters + except ValueError: + if not isinstance(handler, (BuiltinMethodType, MethodWrapperType)): + raise + # Some built-ins, such as set.add, have no inspectable signature. + arg_count = len(args) + else: + has_varargs = any( + parameter.kind == inspect.Parameter.VAR_POSITIONAL for parameter in parameters.values() ) - ) + arg_count = ( + len(args) + if has_varargs + else sum( + parameter.kind + in ( + inspect.Parameter.POSITIONAL_ONLY, + inspect.Parameter.POSITIONAL_OR_KEYWORD, + ) + for parameter in parameters.values() + ) + ) return handler( *list(map(lambda a: self.from_maybe_impl(a), args))[:arg_count] ) - if inspect.ismethod(handler): - wrapper = getattr(handler.__self__, IMPL_ATTR + handler.__name__, None) - if not wrapper: - wrapper = wrapper_func - setattr( - handler.__self__, - IMPL_ATTR + handler.__name__, - wrapper, - ) - return wrapper - - wrapper = getattr(handler, IMPL_ATTR, None) - if not wrapper: - wrapper = wrapper_func - setattr(handler, IMPL_ATTR, wrapper) - return wrapper + self._wrapped_handlers[key] = wrapper_func + return wrapper_func diff --git a/tests/sync/python/test_sync.py b/tests/sync/python/test_sync.py index 965724c1a..c6bec2941 100644 --- a/tests/sync/python/test_sync.py +++ b/tests/sync/python/test_sync.py @@ -23,7 +23,7 @@ import time from datetime import timedelta from pathlib import Path -from typing import Any, Dict, Generator, List +from typing import Any, Dict, Generator, List, Set import pytest from greenlet import greenlet @@ -87,12 +87,23 @@ def on_dialog(dialog: Dialog) -> None: def test_console_repr(page: Page) -> None: - messages = [] - page.on("console", lambda m: messages.append(m)) + # Regression test for https://github.com/microsoft/playwright-python/issues/3340. + messages: List[ConsoleMessage] = [] + page.on("console", messages.append) page.evaluate('() => console.log("Hello world")') message = messages[0] assert repr(message) == f"" + page.remove_listener("console", messages.append) + page.evaluate('() => console.log("removed")') + assert messages == [message] + + once_messages: Set[ConsoleMessage] = set() + page.once("console", once_messages.add) + page.evaluate('() => { console.log("once"); console.log("ignored"); }') + assert [m.text for m in once_messages] == ["once"] + assert messages == [message] + def test_sync_wait_for_event_raise(page: Page) -> None: with pytest.raises(AssertionError):