fix(page): use runBeforeUnload channel method

Author: Skn0tt

playwright/_impl/_page.py

@@ -852,14 +852,19 @@ async def aria_snapshot(
 
     async def close(self, runBeforeUnload: bool = None, reason: str = None) -> None:
         self._close_reason = reason
-        self._close_was_called = True
+        if not runBeforeUnload:
+            self._close_was_called = True
         try:
-            await self._channel.send("close", None, locals_to_params(locals()))
             if self._owned_context:
                 await self._owned_context.close()
+            elif runBeforeUnload:
+                await self._channel.send("runBeforeUnload", None)
+            else:
+                await self._channel.send("close", None, {"reason": reason})
         except Exception as e:
-            if not is_target_closed_error(e) and not runBeforeUnload:
-                raise e
+            if is_target_closed_error(e) and not runBeforeUnload:
+                return
+            raise e
 
     def is_closed(self) -> bool:
         return self._is_closed

tests/async/test_screencast.py

@@ -35,71 +35,81 @@ def on_frame(frame: ScreencastFrame) -> None:
         event.set()
 
     await page.screencast.start(on_frame=on_frame)
-    await page.goto(server.EMPTY_PAGE)
-    await page.evaluate("() => document.body.style.backgroundColor = 'red'")
-    # Force a couple of paint cycles so engines that only emit on visual change
-    # still produce a frame. Mirrors upstream `ensureSomeFrames`.
-    for _ in range(3):
-        await page.evaluate(
-            "() => new Promise(f => requestAnimationFrame(() => requestAnimationFrame(f)))"
-        )
-    await page.screenshot()
-    await asyncio.wait_for(event.wait(), timeout=10)
-    await page.screencast.stop()
+    try:
+        await page.goto(server.EMPTY_PAGE)
+        await page.evaluate("() => document.body.style.backgroundColor = 'red'")
+        # Force a couple of paint cycles so engines that only emit on visual change
+        # still produce a frame. Mirrors upstream `ensureSomeFrames`.
+        for _ in range(3):
+            await page.evaluate(
+                "() => new Promise(f => requestAnimationFrame(() => requestAnimationFrame(f)))"
+            )
+        await page.screenshot()
+        await asyncio.wait_for(event.wait(), timeout=10)
+    finally:
+        await page.screencast.stop()
     assert len(received) >= 1
     assert all(isinstance(d, bytes) and len(d) > 0 for d in received)
 
 
 async def test_starting_twice_should_throw(page: Page) -> None:
     await page.screencast.start(on_frame=lambda f: None)
-    with pytest.raises(Exception, match="already started"):
-        await page.screencast.start(on_frame=lambda f: None)
-    await page.screencast.stop()
+    try:
+        with pytest.raises(Exception, match="already started"):
+            await page.screencast.start(on_frame=lambda f: None)
+    finally:
+        await page.screencast.stop()
 
 
 async def test_show_overlays_and_overlay_apis_should_not_throw(page: Page) -> None:
     await page.screencast.start(on_frame=lambda f: None)
-    await page.screencast.show_overlay("<div>hello</div>", duration=100)
-    await page.screencast.show_chapter("ch", description="desc", duration=100)
-    await page.screencast.hide_overlays()
-    await page.screencast.show_overlays()
-    await page.screencast.show_actions(duration=100, position="top-right")
-    await page.screencast.hide_actions()
-    await page.screencast.stop()
+    try:
+        await page.screencast.show_overlay("<div>hello</div>", duration=100)
+        await page.screencast.show_chapter("ch", description="desc", duration=100)
+        await page.screencast.hide_overlays()
+        await page.screencast.show_overlays()
+        await page.screencast.show_actions(duration=100, position="top-right")
+        await page.screencast.hide_actions()
+    finally:
+        await page.screencast.stop()
 
 
 async def test_on_frame_receives_viewport_size(
     browser: Browser, server: Server
 ) -> None:
     context = await browser.new_context(viewport={"width": 1000, "height": 400})
-    page = await context.new_page()
-    received: list = []
-
-    def on_frame(frame: ScreencastFrame) -> None:
-        received.append(frame)
-
-    size: ScreencastSize = {"width": 500, "height": 400}
-    await page.screencast.start(on_frame=on_frame, size=size)
-    await page.goto(server.EMPTY_PAGE)
-    await page.evaluate("() => document.body.style.backgroundColor = 'red'")
-    for _ in range(100):
-        await page.evaluate(
-            "() => new Promise(f => requestAnimationFrame(() => requestAnimationFrame(f)))"
-        )
-    await page.screenshot()
-    await page.screencast.stop()
-    assert len(received) >= 1
-    assert any(frame["viewportWidth"] == 1000 for frame in received)
-    for frame in received:
-        assert frame["viewportHeight"] == 400
-        assert isinstance(frame["timestamp"], (int, float))
-    await context.close()
+    async with context:
+        page = await context.new_page()
+        received: list = []
+
+        def on_frame(frame: ScreencastFrame) -> None:
+            received.append(frame)
+
+        size: ScreencastSize = {"width": 500, "height": 400}
+        await page.screencast.start(on_frame=on_frame, size=size)
+        try:
+            await page.goto(server.EMPTY_PAGE)
+            await page.evaluate("() => document.body.style.backgroundColor = 'red'")
+            for _ in range(100):
+                await page.evaluate(
+                    "() => new Promise(f => requestAnimationFrame(() => requestAnimationFrame(f)))"
+                )
+            await page.screenshot()
+        finally:
+            await page.screencast.stop()
+        assert len(received) >= 1
+        assert any(frame["viewportWidth"] == 1000 for frame in received)
+        for frame in received:
+            assert frame["viewportHeight"] == 400
+            assert isinstance(frame["timestamp"], (int, float))
 
 
 async def test_show_actions_should_accept_cursor_param(page: Page) -> None:
     await page.screencast.start(on_frame=lambda f: None)
-    async with await page.screencast.show_actions(duration=100, cursor="pointer"):
-        pass
-    async with await page.screencast.show_actions(duration=100, cursor="none"):
-        pass
-    await page.screencast.stop()
+    try:
+        async with await page.screencast.show_actions(duration=100, cursor="pointer"):
+            pass
+        async with await page.screencast.show_actions(duration=100, cursor="none"):
+            pass
+    finally:
+        await page.screencast.stop()

tests/sync/test_screencast.py

@@ -27,56 +27,64 @@ def test_should_expose_screencast_property(page: Page) -> None:
 def test_start_should_deliver_frames_via_callback(page: Page, server: Server) -> None:
     received: list = []
     page.screencast.start(on_frame=lambda f: received.append(f["data"]))
-    page.goto(server.EMPTY_PAGE)
-    page.evaluate("() => document.body.style.backgroundColor = 'red'")
-    # Force a couple of paint cycles so engines that only emit on visual change
-    # still produce a frame. Mirrors upstream `ensureSomeFrames`.
-    for _ in range(3):
-        page.evaluate(
-            "() => new Promise(f => requestAnimationFrame(() => requestAnimationFrame(f)))"
-        )
-    page.screenshot()
-    deadline = time.time() + 10
-    while not received and time.time() < deadline:
-        page.wait_for_timeout(100)
-    page.screencast.stop()
+    try:
+        page.goto(server.EMPTY_PAGE)
+        page.evaluate("() => document.body.style.backgroundColor = 'red'")
+        # Force a couple of paint cycles so engines that only emit on visual change
+        # still produce a frame. Mirrors upstream `ensureSomeFrames`.
+        for _ in range(3):
+            page.evaluate(
+                "() => new Promise(f => requestAnimationFrame(() => requestAnimationFrame(f)))"
+            )
+        page.screenshot()
+        deadline = time.time() + 10
+        while not received and time.time() < deadline:
+            page.wait_for_timeout(100)
+    finally:
+        page.screencast.stop()
     assert len(received) >= 1
     assert all(isinstance(d, bytes) and len(d) > 0 for d in received)
 
 
 def test_starting_twice_should_throw(page: Page) -> None:
     page.screencast.start(on_frame=lambda f: None)
-    with pytest.raises(Exception, match="already started"):
-        page.screencast.start(on_frame=lambda f: None)
-    page.screencast.stop()
+    try:
+        with pytest.raises(Exception, match="already started"):
+            page.screencast.start(on_frame=lambda f: None)
+    finally:
+        page.screencast.stop()
 
 
 def test_on_frame_receives_viewport_size(browser: Browser, server: Server) -> None:
     context = browser.new_context(viewport={"width": 1000, "height": 400})
-    page = context.new_page()
-    received: list = []
-    size: ScreencastSize = {"width": 500, "height": 400}
-    page.screencast.start(on_frame=lambda f: received.append(f), size=size)
-    page.goto(server.EMPTY_PAGE)
-    page.evaluate("() => document.body.style.backgroundColor = 'red'")
-    for _ in range(100):
-        page.evaluate(
-            "() => new Promise(f => requestAnimationFrame(() => requestAnimationFrame(f)))"
-        )
-    page.screenshot()
-    page.screencast.stop()
-    assert len(received) >= 1
-    assert any(frame["viewportWidth"] == 1000 for frame in received)
-    for frame in received:
-        assert frame["viewportHeight"] == 400
-        assert isinstance(frame["timestamp"], (int, float))
-    context.close()
+    with context:
+        page = context.new_page()
+        received: list = []
+        size: ScreencastSize = {"width": 500, "height": 400}
+        page.screencast.start(on_frame=lambda f: received.append(f), size=size)
+        try:
+            page.goto(server.EMPTY_PAGE)
+            page.evaluate("() => document.body.style.backgroundColor = 'red'")
+            for _ in range(100):
+                page.evaluate(
+                    "() => new Promise(f => requestAnimationFrame(() => requestAnimationFrame(f)))"
+                )
+            page.screenshot()
+        finally:
+            page.screencast.stop()
+        assert len(received) >= 1
+        assert any(frame["viewportWidth"] == 1000 for frame in received)
+        for frame in received:
+            assert frame["viewportHeight"] == 400
+            assert isinstance(frame["timestamp"], (int, float))
 
 
 def test_show_actions_should_accept_cursor_param(page: Page) -> None:
     page.screencast.start(on_frame=lambda f: None)
-    with page.screencast.show_actions(duration=100, cursor="pointer"):
-        pass
-    with page.screencast.show_actions(duration=100, cursor="none"):
-        pass
-    page.screencast.stop()
+    try:
+        with page.screencast.show_actions(duration=100, cursor="pointer"):
+            pass
+        with page.screencast.show_actions(duration=100, cursor="none"):
+            pass
+    finally:
+        page.screencast.stop()