From 2dc359c48d042a2982de8c972008b76685cbe105 Mon Sep 17 00:00:00 2001 From: JB Date: Mon, 21 Sep 2026 13:42:57 -0400 Subject: [PATCH] perf(fast-browser): bound readiness polling and verify asynchronous tab close --- extensions/fast-browser/browser_worker.py | 34 ++- .../fast-browser/test_browser_worker.py | 200 +++++++++++++++++- 2 files changed, 225 insertions(+), 9 deletions(-) diff --git a/extensions/fast-browser/browser_worker.py b/extensions/fast-browser/browser_worker.py index c98ca98..8310a41 100644 --- a/extensions/fast-browser/browser_worker.py +++ b/extensions/fast-browser/browser_worker.py @@ -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: diff --git a/extensions/fast-browser/test_browser_worker.py b/extensions/fast-browser/test_browser_worker.py index d3752bb..2bd8d46 100644 --- a/extensions/fast-browser/test_browser_worker.py +++ b/extensions/fast-browser/test_browser_worker.py @@ -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 = [