diff --git a/playwright/_impl/_browser_context.py b/playwright/_impl/_browser_context.py index 844955322..cd226164e 100644 --- a/playwright/_impl/_browser_context.py +++ b/playwright/_impl/_browser_context.py @@ -48,6 +48,7 @@ from playwright._impl._frame import Frame from playwright._impl._har_router import HarRouter from playwright._impl._helper import ( + BackgroundTaskTracker, HarRecordingMetadata, RouteFromHarNotFoundPolicy, RouteHandler, @@ -103,6 +104,7 @@ def __init__( self._request: APIRequestContext = from_channel( initializer["APIRequestContext"] ) + self._background_task_tracker: BackgroundTaskTracker = BackgroundTaskTracker() self._channel.on( "bindingCall", lambda params: self._on_binding(from_channel(params["binding"])), @@ -113,7 +115,7 @@ def __init__( ) self._channel.on( "route", - lambda params: asyncio.create_task( + lambda params: self._background_task_tracker.create_task( self._on_route( from_channel(params.get("route")), from_channel(params.get("request")), @@ -163,8 +165,14 @@ def __init__( ), ) self._closed_future: asyncio.Future = asyncio.Future() + + def _on_close(_: Any) -> None: + self._background_task_tracker.close() + self._closed_future.set_result(True) + self.once( - self.Events.Close, lambda context: self._closed_future.set_result(True) + self.Events.Close, + _on_close, ) def __repr__(self) -> str: @@ -187,7 +195,10 @@ async def _on_route(self, route: Route, request: Request) -> None: handled = await route_handler.handle(route, request) finally: if len(self._routes) == 0: - asyncio.create_task(self._disable_interception()) + try: + await self._disable_interception() + except Exception: + pass if handled: return await route._internal_continue(is_internal=True) diff --git a/playwright/_impl/_helper.py b/playwright/_impl/_helper.py index fb2295298..70cbee8b7 100644 --- a/playwright/_impl/_helper.py +++ b/playwright/_impl/_helper.py @@ -362,3 +362,19 @@ def is_file_payload(value: Optional[Any]) -> bool: and "mimeType" in value and "buffer" in value ) + + +class BackgroundTaskTracker: + def __init__(self) -> None: + self._pending_tasks: List[asyncio.Task] = [] + + def create_task(self, coro: Coroutine) -> asyncio.Task: + task = asyncio.create_task(coro) + task.add_done_callback(lambda task: self._pending_tasks.remove(task)) + self._pending_tasks.append(task) + return task + + def close(self) -> None: + for task in self._pending_tasks: + if not task.done(): + task.cancel() diff --git a/playwright/_impl/_network.py b/playwright/_impl/_network.py index 5fc408042..ca142e5d7 100644 --- a/playwright/_impl/_network.py +++ b/playwright/_impl/_network.py @@ -223,7 +223,8 @@ def _report_handled(self, done: bool) -> None: chain = self._handling_future assert chain self._handling_future = None - chain.set_result(done) + if not chain.done(): + chain.set_result(done) def _check_not_handled(self) -> None: if not self._handling_future: diff --git a/playwright/_impl/_page.py b/playwright/_impl/_page.py index ddb41aa12..5128acbcb 100644 --- a/playwright/_impl/_page.py +++ b/playwright/_impl/_page.py @@ -55,6 +55,7 @@ from playwright._impl._frame import Frame from playwright._impl._har_router import HarRouter from playwright._impl._helper import ( + BackgroundTaskTracker, ColorScheme, DocumentLoadState, ForcedColors, @@ -151,6 +152,7 @@ def __init__( self._browser_context._timeout_settings ) self._video: Optional[Video] = None + self._background_task_tracker = BackgroundTaskTracker() self._opener = cast("Page", from_nullable_channel(initializer.get("opener"))) self._channel.on( @@ -192,7 +194,7 @@ def __init__( ) self._channel.on( "route", - lambda params: asyncio.create_task( + lambda params: self._browser_context._background_task_tracker.create_task( self._on_route( from_channel(params["route"]), from_channel(params["request"]) ) @@ -246,7 +248,10 @@ async def _on_route(self, route: Route, request: Request) -> None: handled = await route_handler.handle(route, request) finally: if len(self._routes) == 0: - asyncio.create_task(self._disable_interception()) + try: + await self._disable_interception() + except Exception: + pass if handled: return await self._browser_context._on_route(route, request) diff --git a/tests/async/test_browsercontext_request_intercept.py b/tests/async/test_browsercontext_request_intercept.py index 763073df0..e4ea30bef 100644 --- a/tests/async/test_browsercontext_request_intercept.py +++ b/tests/async/test_browsercontext_request_intercept.py @@ -174,3 +174,20 @@ async def test_should_give_access_to_the_intercepted_response_body( route.fulfill(response=response), eval_task, ) + + +async def test_should_cleanup_route_handlers_after_context_close( + context: BrowserContext, page: Page +) -> None: + async def handle(r: Route): + pass + + await context.route("**", handle) + try: + await page.goto("https://example.com", timeout=700) + except Exception: + pass + await context.close() + + for task in asyncio.all_tasks(): + assert "_on_route" not in str(task) diff --git a/tests/async/test_har.py b/tests/async/test_har.py index cd1c871a6..fb820232d 100644 --- a/tests/async/test_har.py +++ b/tests/async/test_har.py @@ -503,6 +503,7 @@ async def test_should_round_trip_har_zip( await expect(page_2.locator("body")).to_have_css( "background-color", "rgb(255, 192, 203)" ) + await context_2.close() async def test_should_round_trip_har_with_post_data( @@ -536,6 +537,7 @@ async def test_should_round_trip_har_with_post_data( assert await page_2.evaluate(fetch_function, "3") == "3" with pytest.raises(Exception): await page_2.evaluate(fetch_function, "4") + await context_2.close() async def test_should_disambiguate_by_header( @@ -578,6 +580,7 @@ async def test_should_disambiguate_by_header( assert await page_2.evaluate(fetch_function, "baz2") == "baz2" assert await page_2.evaluate(fetch_function, "baz3") == "baz3" assert await page_2.evaluate(fetch_function, "baz4") == "baz1" + await context_2.close() async def test_should_produce_extracted_zip( @@ -605,6 +608,7 @@ async def test_should_produce_extracted_zip( await expect(page_2.locator("body")).to_have_css( "background-color", "rgb(255, 192, 203)" ) + await context_2.close() async def test_should_update_har_zip_for_context( @@ -627,6 +631,7 @@ async def test_should_update_har_zip_for_context( await expect(page_2.locator("body")).to_have_css( "background-color", "rgb(255, 192, 203)" ) + await context_2.close() async def test_should_update_har_zip_for_page( @@ -649,6 +654,7 @@ async def test_should_update_har_zip_for_page( await expect(page_2.locator("body")).to_have_css( "background-color", "rgb(255, 192, 203)" ) + await context_2.close() async def test_should_update_extracted_har_zip_for_page( @@ -675,3 +681,4 @@ async def test_should_update_extracted_har_zip_for_page( await expect(page_2.locator("body")).to_have_css( "background-color", "rgb(255, 192, 203)" ) + await context_2.close() diff --git a/tests/async/test_request_intercept.py b/tests/async/test_request_intercept.py index 39ccf3d3f..30d8941c1 100644 --- a/tests/async/test_request_intercept.py +++ b/tests/async/test_request_intercept.py @@ -17,7 +17,7 @@ from twisted.web import http -from playwright.async_api import Page, Route +from playwright.async_api import BrowserContext, Page, Route from tests.server import Server @@ -168,3 +168,20 @@ async def test_should_give_access_to_the_intercepted_response_body( route.fulfill(response=response), eval_task, ) + + +async def test_should_cleanup_route_handlers_after_context_close( + context: BrowserContext, page: Page +) -> None: + async def handle(r: Route): + pass + + await page.route("**", handle) + try: + await page.goto("https://example.com", timeout=700) + except Exception: + pass + await context.close() + + for task in asyncio.all_tasks(): + assert "_on_route" not in str(task) diff --git a/tests/sync/test_browsercontext_request_intercept.py b/tests/sync/test_browsercontext_request_intercept.py index b136038ec..acc021659 100644 --- a/tests/sync/test_browsercontext_request_intercept.py +++ b/tests/sync/test_browsercontext_request_intercept.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import asyncio from pathlib import Path from twisted.web import http @@ -121,3 +122,17 @@ def handle_route(route: Route) -> None: assert request.uri.decode() == "/title.html" original = (assetdir / "title.html").read_text() assert response.text() == original + + +def test_should_cleanup_route_handlers_after_context_close( + context: BrowserContext, page: Page +) -> None: + context.route("**", lambda r: None) + try: + page.goto("https://example.com", timeout=700) + except Exception: + pass + context.close() + + for task in asyncio.all_tasks(): + assert "_on_route" not in str(task) diff --git a/tests/sync/test_har.py b/tests/sync/test_har.py index 81452c9de..f313ae257 100644 --- a/tests/sync/test_har.py +++ b/tests/sync/test_har.py @@ -471,6 +471,7 @@ def test_should_round_trip_har_with_post_data( assert page_2.evaluate(fetch_function, "3") == "3" with pytest.raises(Exception): page_2.evaluate(fetch_function, "4") + context_2.close() def test_should_disambiguate_by_header( @@ -512,6 +513,7 @@ def test_should_disambiguate_by_header( assert page_2.evaluate(fetch_function, "baz2") == "baz2" assert page_2.evaluate(fetch_function, "baz3") == "baz3" assert page_2.evaluate(fetch_function, "baz4") == "baz1" + context_2.close() def test_should_produce_extracted_zip( @@ -537,6 +539,7 @@ def test_should_produce_extracted_zip( page_2.goto(server.PREFIX + "/one-style.html") assert "hello, world!" in page_2.content() expect(page_2.locator("body")).to_have_css("background-color", "rgb(255, 192, 203)") + context_2.close() def test_should_update_har_zip_for_context( @@ -557,6 +560,7 @@ def test_should_update_har_zip_for_context( page_2.goto(server.PREFIX + "/one-style.html") assert "hello, world!" in page_2.content() expect(page_2.locator("body")).to_have_css("background-color", "rgb(255, 192, 203)") + context_2.close() def test_should_update_har_zip_for_page( @@ -577,6 +581,7 @@ def test_should_update_har_zip_for_page( page_2.goto(server.PREFIX + "/one-style.html") assert "hello, world!" in page_2.content() expect(page_2.locator("body")).to_have_css("background-color", "rgb(255, 192, 203)") + context_2.close() def test_should_update_extracted_har_zip_for_page( @@ -601,3 +606,4 @@ def test_should_update_extracted_har_zip_for_page( page_2.goto(server.PREFIX + "/one-style.html") assert "hello, world!" in page_2.content() expect(page_2.locator("body")).to_have_css("background-color", "rgb(255, 192, 203)") + context_2.close() diff --git a/tests/sync/test_request_intercept.py b/tests/sync/test_request_intercept.py index dc714e832..ab90fc079 100644 --- a/tests/sync/test_request_intercept.py +++ b/tests/sync/test_request_intercept.py @@ -12,11 +12,12 @@ # See the License for the specific language governing permissions and # limitations under the License. +import asyncio from pathlib import Path from twisted.web import http -from playwright.sync_api import Page, Route +from playwright.sync_api import BrowserContext, Page, Route from tests.server import Server @@ -115,3 +116,17 @@ def handle_route(route: Route) -> None: assert request.uri.decode() == "/title.html" original = (assetdir / "title.html").read_text() assert response.text() == original + + +def test_should_cleanup_route_handlers_after_context_close( + context: BrowserContext, page: Page +) -> None: + page.route("**", lambda r: None) + try: + page.goto("https://example.com", timeout=700) + except Exception: + pass + context.close() + + for task in asyncio.all_tasks(): + assert "_on_route" not in str(task)