perf(fast-browser): bound readiness polling and verify asynchronous tab close

2026-09-21 13:42:57 -04:00
parent fe643e2d8d
commit 2dc359c48d
2 changed files with 225 additions and 9 deletions
@@ -824,7 +824,18 @@ class RealBrowserBackend:
        return self._helpers.cdp("Page.reload")

    def wait_ready(self, timeout: float = NAVIGATION_TIMEOUT_SECONDS) -> bool:
-        return bool(self._helpers.wait_for_load(timeout=timeout))
+        # Keep the harness' exact readyState query, but avoid its coarse 300ms
+        # sleep. This is only a readiness read; it never dispatches navigation.
+        if timeout <= 0:
+            return False
+        deadline = time.monotonic() + timeout
+        while True:
+            if self._helpers.js("document.readyState") == "complete":
+                return True
+            remaining = deadline - time.monotonic()
+            if remaining <= 0:
+                return False
+            time.sleep(min(0.025, remaining))

    def visible_text_content(self) -> str:
        # This is a fixed, worker-owned read operation.  No caller expression,
@@ -1708,13 +1719,20 @@ class BrowserRuntimeWorker(BrowserWorker):
            raise self._fail("closeTab", "aborted", "close", "tab close was cancelled",
                             mutation_state="unknown" if self._mutation_dispatched else "not_attempted")
        # Do not assume a close acknowledgement means the target disappeared;
-        # verify the current target inventory before reporting a verified close.
-        try:
-            self._refresh_tabs()
-        except BrowserOperationError:
-            raise self._fail("closeTab", "worker_failure", "verify", "tab close could not be verified", mutation_state="unknown")
-        if tab.target_id in self._target_to_tab:
-            raise self._fail("closeTab", "worker_failure", "verify", "tab close could not be verified", mutation_state="unknown")
+        # verify inventories until the target is gone or the existing action
+        # budget expires. The close mutation itself is dispatched exactly once.
+        deadline = time.monotonic() + ACTION_TIMEOUT_SECONDS
+        while True:
+            try:
+                self._refresh_tabs()
+            except BrowserOperationError:
+                raise self._fail("closeTab", "worker_failure", "verify", "tab close could not be verified", mutation_state="unknown")
+            if tab.target_id not in self._target_to_tab:
+                break
+            remaining = deadline - time.monotonic()
+            if remaining <= 0:
+                raise self._fail("closeTab", "worker_failure", "verify", "tab close could not be verified", mutation_state="unknown")
+            time.sleep(min(0.025, remaining))
        self._target_to_tab.pop(tab.target_id, None)
        self._tabs.pop(tab.tab_id, None)
        if self._selected_tab_id == tab.tab_id:
@@ -8,7 +8,7 @@ from unittest.mock import Mock, patch

 from browser_worker import (
    BrowserWorker, BrowserRuntimeWorker, ElementMetadata, WorkerError, BrowserOperationError,
-    RealBrowserBackend, MAX_LINE_BYTES, MAX_READ_TEXT, process_request, run_stream,
+    RealBrowserBackend, ACTION_TIMEOUT_SECONDS, MAX_LINE_BYTES, MAX_READ_TEXT, process_request, run_stream,
    validate_cdp_url, main,
 )

@@ -190,6 +190,46 @@ class RuntimeFakeBackend(FakeBackend):
        return b"\\x89PNG" + (b"x" * 32)


+class FakeClock:
+    def __init__(self):
+        self.now = 0.0
+        self.sleeps = []
+
+    def monotonic(self):
+        return self.now
+
+    def sleep(self, duration):
+        self.sleeps.append(duration)
+        self.now += duration
+
+
+class DelayedCloseBackend(RuntimeFakeBackend):
+    def __init__(self, disappear_after=None):
+        super().__init__()
+        self.tabs = [self.tabs[0]]
+        self.disappear_after = disappear_after
+        self.inventory_reads = 0
+        self.post_close_reads = 0
+        self.pending_close = None
+        self.close_dispatches = []
+
+    def list_tabs(self):
+        self.inventory_reads += 1
+        rows = [dict(tab) for tab in self.tabs]
+        if self.pending_close is not None:
+            self.post_close_reads += 1
+            if self.disappear_after is not None and self.post_close_reads >= self.disappear_after:
+                target_id = self.pending_close
+                self.tabs[:] = [tab for tab in self.tabs if tab["targetId"] != target_id]
+                self.pending_close = None
+                rows = [dict(tab) for tab in self.tabs]
+        return rows
+
+    def close_tab(self, target_id):
+        self.close_dispatches.append(target_id)
+        self.pending_close = target_id
+
+
 class RuntimeTests(unittest.TestCase):
    def make_runtime(self, backend=None, origins="*"):
        backend = backend or RuntimeFakeBackend()
@@ -404,6 +444,102 @@ class RuntimeTests(unittest.TestCase):
        with self.assertRaisesRegex(BrowserOperationError, "stale|unavailable"):
            runtime.select_tab({"tabId": tab_id})

+    def test_close_waits_for_delayed_target_disappearance_without_redispatching(self):
+        backend = DelayedCloseBackend(disappear_after=2)
+        runtime = self.make_runtime(backend)
+        opener = runtime.list_tabs()[0]["tabId"]
+        backend.tabs.append({"targetId": "popup", "url": "http://fixture.test/popup",
+                             "title": "Popup", "openerTargetId": "tab-a"})
+        popup = runtime.wait_popup({"openerTabId": opener})["tab"]
+        self.assertIsNotNone(popup)
+        popup_id = popup["tabId"]
+        clock = FakeClock()
+
+        with patch("browser_worker.time.monotonic", side_effect=clock.monotonic), \
+             patch("browser_worker.time.sleep", side_effect=clock.sleep):
+            result = runtime.close_tab({"tabId": popup_id})
+
+        self.assertEqual(result["mutationState"], "verified")
+        self.assertEqual(backend.close_dispatches, ["popup"])
+        self.assertGreaterEqual(backend.post_close_reads, 2)
+        self.assertGreater(clock.now, 0)
+        tabs = runtime.list_tabs()
+        self.assertEqual([tab["tabId"] for tab in tabs], [opener])
+        self.assertTrue(tabs[0]["selected"])
+        with self.assertRaisesRegex(BrowserOperationError, "stale|unavailable"):
+            runtime.select_tab({"tabId": popup_id})
+
+    def test_close_deadline_reports_unknown_without_retry_when_target_persists(self):
+        backend = DelayedCloseBackend(disappear_after=None)
+        runtime = self.make_runtime(backend)
+        opener = runtime.list_tabs()[0]["tabId"]
+        backend.tabs.append({"targetId": "popup", "url": "http://fixture.test/popup",
+                             "title": "Popup", "openerTargetId": "tab-a"})
+        popup = runtime.wait_popup({"openerTabId": opener})["tab"]
+        self.assertIsNotNone(popup)
+        clock = FakeClock()
+
+        with patch("browser_worker.time.monotonic", side_effect=clock.monotonic), \
+             patch("browser_worker.time.sleep", side_effect=clock.sleep), \
+             self.assertRaises(BrowserOperationError) as caught:
+            runtime.close_tab({"tabId": popup["tabId"]})
+
+        details = caught.exception.details
+        self.assertEqual(details["code"], "worker_failure")
+        self.assertEqual(details["mutationState"], "unknown")
+        self.assertEqual(details["retry"], "never")
+        self.assertEqual(backend.close_dispatches, ["popup"])
+        self.assertGreaterEqual(backend.post_close_reads, 2)
+        self.assertGreaterEqual(clock.now, ACTION_TIMEOUT_SECONDS)
+
+    def test_close_inventory_failure_preserves_unknown_state_without_retry(self):
+        backend = DelayedCloseBackend(disappear_after=None)
+        runtime = self.make_runtime(backend)
+        opener = runtime.list_tabs()[0]["tabId"]
+        backend.tabs.append({"targetId": "popup", "url": "http://fixture.test/popup",
+                             "title": "Popup", "openerTargetId": "tab-a"})
+        popup = runtime.wait_popup({"openerTabId": opener})["tab"]
+        self.assertIsNotNone(popup)
+        original_list_tabs = backend.list_tabs
+        def fail_after_close():
+            if backend.pending_close is not None:
+                raise RuntimeError("inventory unavailable")
+            return original_list_tabs()
+        backend.list_tabs = fail_after_close
+
+        with self.assertRaises(BrowserOperationError) as caught:
+            runtime.close_tab({"tabId": popup["tabId"]})
+
+        details = caught.exception.details
+        self.assertEqual(details["code"], "worker_failure")
+        self.assertEqual(details["mutationState"], "unknown")
+        self.assertEqual(details["retry"], "never")
+        self.assertEqual(backend.close_dispatches, ["popup"])
+
+    def test_close_cancellation_preserves_unknown_state_without_retry(self):
+        backend = DelayedCloseBackend(disappear_after=2)
+        runtime = self.make_runtime(backend)
+        opener = runtime.list_tabs()[0]["tabId"]
+        backend.tabs.append({"targetId": "popup", "url": "http://fixture.test/popup",
+                             "title": "Popup", "openerTargetId": "tab-a"})
+        popup = runtime.wait_popup({"openerTabId": opener})["tab"]
+        self.assertIsNotNone(popup)
+        class Cancelled(BaseException):
+            pass
+        def cancel(_target_id):
+            backend.close_dispatches.append(_target_id)
+            raise Cancelled()
+        backend.close_tab = cancel
+
+        with self.assertRaises(BrowserOperationError) as caught:
+            runtime.close_tab({"tabId": popup["tabId"]})
+
+        details = caught.exception.details
+        self.assertEqual(details["code"], "aborted")
+        self.assertEqual(details["mutationState"], "unknown")
+        self.assertEqual(details["retry"], "never")
+        self.assertEqual(backend.close_dispatches, ["popup"])
+
    def test_transport_wrapper_context_restores_base_and_uses_exact_result_shape(self):
        backend = RuntimeFakeBackend()
        runtime = self.make_runtime(backend)
@@ -921,6 +1057,68 @@ class BridgeRegressionTests(unittest.TestCase):
                    if call.args[0] == "Input.dispatchKeyEvent" and call.kwargs["type"] == "rawKeyDown"]
        self.assertEqual([call.kwargs["modifiers"] for call in keydowns], [4, 4])

+    def test_wait_ready_returns_immediately_for_complete_document(self):
+        backend = RealBrowserBackend.__new__(RealBrowserBackend)
+        helper = Mock()
+        helper.js.return_value = "complete"
+        backend._helpers = helper
+        clock = FakeClock()
+
+        with patch("browser_worker.time.monotonic", side_effect=clock.monotonic), \
+             patch("browser_worker.time.sleep", side_effect=clock.sleep):
+            self.assertTrue(backend.wait_ready(timeout=3.0))
+
+        helper.js.assert_called_once_with("document.readyState")
+        helper.wait_for_load.assert_not_called()
+        self.assertEqual(clock.sleeps, [])
+
+    def test_wait_ready_polls_loading_document_at_short_bounded_intervals(self):
+        backend = RealBrowserBackend.__new__(RealBrowserBackend)
+        helper = Mock()
+        helper.js.side_effect = ["loading", "complete"]
+        backend._helpers = helper
+        clock = FakeClock()
+
+        with patch("browser_worker.time.monotonic", side_effect=clock.monotonic), \
+             patch("browser_worker.time.sleep", side_effect=clock.sleep):
+            self.assertTrue(backend.wait_ready(timeout=3.0))
+
+        self.assertEqual(helper.js.call_count, 2)
+        self.assertEqual(len(clock.sleeps), 1)
+        self.assertGreater(clock.sleeps[0], 0)
+        self.assertLessEqual(clock.sleeps[0], 0.025)
+        helper.wait_for_load.assert_not_called()
+
+    def test_wait_ready_times_out_without_sleeping_past_deadline(self):
+        backend = RealBrowserBackend.__new__(RealBrowserBackend)
+        helper = Mock()
+        helper.js.return_value = "loading"
+        backend._helpers = helper
+        clock = FakeClock()
+
+        with patch("browser_worker.time.monotonic", side_effect=clock.monotonic), \
+             patch("browser_worker.time.sleep", side_effect=clock.sleep):
+            self.assertFalse(backend.wait_ready(timeout=0.075))
+
+        self.assertEqual(clock.now, 0.075)
+        self.assertTrue(all(duration <= 0.025 for duration in clock.sleeps))
+        helper.wait_for_load.assert_not_called()
+
+    def test_wait_ready_preserves_failed_ready_state_query(self):
+        backend = RealBrowserBackend.__new__(RealBrowserBackend)
+        helper = Mock()
+        helper.js.side_effect = RuntimeError("ready state query failed")
+        backend._helpers = helper
+        clock = FakeClock()
+
+        with patch("browser_worker.time.monotonic", side_effect=clock.monotonic), \
+             patch("browser_worker.time.sleep", side_effect=clock.sleep):
+            with self.assertRaisesRegex(RuntimeError, "ready state query failed"):
+                backend.wait_ready(timeout=3.0)
+
+        self.assertEqual(clock.sleeps, [])
+        helper.wait_for_load.assert_not_called()
+
    def test_real_backend_unrestricted_policy_still_requires_one_web_tab(self):
        helper = Mock()
        helper.list_tabs.return_value = [