perf(fast-browser): eliminate redundant browser protocol calls
This commit is contained in:
@@ -53,6 +53,9 @@ NAVIGATION_TIMEOUT_SECONDS = 10.0
|
|||||||
# timeout returns a structured result instead of racing the parent deadline.
|
# timeout returns a structured result instead of racing the parent deadline.
|
||||||
POPUP_WAIT_SECONDS = 4.75
|
POPUP_WAIT_SECONDS = 4.75
|
||||||
EXPECTED_BROWSER_HARNESS_VERSION = "0.1.13"
|
EXPECTED_BROWSER_HARNESS_VERSION = "0.1.13"
|
||||||
|
_INTERNAL_TARGET_PREFIXES = (
|
||||||
|
"chrome://", "chrome-untrusted://", "devtools://", "chrome-extension://", "about:",
|
||||||
|
)
|
||||||
_TEXT_INPUT_TYPES = {"", "text", "search", "email", "url", "tel", "number", "password"}
|
_TEXT_INPUT_TYPES = {"", "text", "search", "email", "url", "tel", "number", "password"}
|
||||||
|
|
||||||
_FILL_ROLES = {"textbox", "searchbox", "spinbutton"}
|
_FILL_ROLES = {"textbox", "searchbox", "spinbutton"}
|
||||||
@@ -493,12 +496,12 @@ class BrowserWorker:
|
|||||||
self.backend.fill(candidate.backend_id, text)
|
self.backend.fill(candidate.backend_id, text)
|
||||||
deadline = time.monotonic() + ACTION_TIMEOUT_SECONDS
|
deadline = time.monotonic() + ACTION_TIMEOUT_SECONDS
|
||||||
while time.monotonic() < deadline:
|
while time.monotonic() < deadline:
|
||||||
self._post_action_state(snapshot, same_document=True)
|
tab = self._post_action_state(snapshot, same_document=True)
|
||||||
value_verified = self.backend.value_matches(candidate.backend_id, text)
|
value_verified = self.backend.value_matches(candidate.backend_id, text)
|
||||||
condition_verified = True
|
condition_verified = True
|
||||||
if postcondition is not None:
|
if postcondition is not None:
|
||||||
if postcondition["kind"] == "url":
|
if postcondition["kind"] == "url":
|
||||||
condition_verified = self.backend.current_tab().get("url") == postcondition["value"]
|
condition_verified = tab.get("url") == postcondition["value"]
|
||||||
else:
|
else:
|
||||||
condition_verified = self.backend.visible_text(postcondition["value"])
|
condition_verified = self.backend.visible_text(postcondition["value"])
|
||||||
if value_verified and condition_verified:
|
if value_verified and condition_verified:
|
||||||
@@ -560,6 +563,7 @@ class RealBrowserBackend:
|
|||||||
self._daemon_ready = False
|
self._daemon_ready = False
|
||||||
self._process: subprocess.Popen | None = None
|
self._process: subprocess.Popen | None = None
|
||||||
self._attached_target: str | None = None
|
self._attached_target: str | None = None
|
||||||
|
self._select_all_modifier: int | None = None
|
||||||
self._configure_environment(cdp_url, self._sandbox)
|
self._configure_environment(cdp_url, self._sandbox)
|
||||||
try:
|
try:
|
||||||
self._load_harness()
|
self._load_harness()
|
||||||
@@ -703,45 +707,46 @@ class RealBrowserBackend:
|
|||||||
def switch_tab(self, target_id: str) -> None:
|
def switch_tab(self, target_id: str) -> None:
|
||||||
if not isinstance(target_id, str) or not target_id:
|
if not isinstance(target_id, str) or not target_id:
|
||||||
raise WorkerError("tab identity is unavailable")
|
raise WorkerError("tab identity is unavailable")
|
||||||
|
# browser-harness 0.1.13 enables Page/DOM/Runtime/Network as part of
|
||||||
|
# its switch_tab set_session path. Re-enabling them here duplicates
|
||||||
|
# four CDP calls and can race the daemon's session setup.
|
||||||
self._session_id = self._helpers.switch_tab(target_id, activate=False)
|
self._session_id = self._helpers.switch_tab(target_id, activate=False)
|
||||||
self._attached_target = target_id
|
self._attached_target = target_id
|
||||||
# The pinned helper does not enable these domains on every attach. Do
|
|
||||||
# so here once per target; drain_events is then a truthful source for
|
|
||||||
# the bounded diagnostics surface.
|
|
||||||
for method in ("Page.enable", "DOM.enable", "Runtime.enable", "Network.enable"):
|
|
||||||
try:
|
|
||||||
self._helpers.cdp(method)
|
|
||||||
except Exception:
|
|
||||||
# Diagnostics are best effort and never block a browser action.
|
|
||||||
pass
|
|
||||||
|
|
||||||
def list_tabs(self) -> list[dict[str, Any]]:
|
def list_tabs(self) -> list[dict[str, Any]]:
|
||||||
tabs = self._helpers.list_tabs(include_chrome=False)
|
# helpers.list_tabs() and this opener lookup used to issue two identical
|
||||||
opener_by_target: dict[str, str] = {}
|
# Target.getTargets requests. Keep its page/internal filtering locally
|
||||||
try:
|
# while deriving opener metadata from the same target inventory.
|
||||||
raw = self._helpers.cdp("Target.getTargets")
|
raw = self._helpers.cdp("Target.getTargets")
|
||||||
for info in raw.get("targetInfos", []) if isinstance(raw, dict) else []:
|
target_infos = raw.get("targetInfos", []) if isinstance(raw, dict) else []
|
||||||
|
if not isinstance(target_infos, list):
|
||||||
|
return []
|
||||||
|
result: list[dict[str, Any]] = []
|
||||||
|
for info in target_infos:
|
||||||
if not isinstance(info, dict) or info.get("type") != "page":
|
if not isinstance(info, dict) or info.get("type") != "page":
|
||||||
continue
|
continue
|
||||||
target = info.get("targetId")
|
url = info.get("url", "")
|
||||||
opener = info.get("openerId")
|
title = info.get("title", "")
|
||||||
if isinstance(target, str) and isinstance(opener, str) and opener:
|
if not isinstance(url, str) or not isinstance(title, str):
|
||||||
opener_by_target[target] = opener
|
|
||||||
except Exception:
|
|
||||||
pass
|
|
||||||
result: list[dict[str, Any]] = []
|
|
||||||
for tab in tabs:
|
|
||||||
if not isinstance(tab, dict) or not isinstance(tab.get("url"), str):
|
|
||||||
continue
|
continue
|
||||||
target = tab.get("targetId") or tab.get("target_id")
|
# Match browser-harness helpers.list_tabs(include_chrome=False),
|
||||||
|
# including its startup-placeholder suppression.
|
||||||
|
if title.startswith("Starting agent ") and (
|
||||||
|
url in ("", "about:blank") or url.startswith("about:blank#")
|
||||||
|
):
|
||||||
|
continue
|
||||||
|
if url.startswith(_INTERNAL_TARGET_PREFIXES):
|
||||||
|
continue
|
||||||
|
target = info.get("targetId")
|
||||||
if not isinstance(target, str) or not target:
|
if not isinstance(target, str) or not target:
|
||||||
continue
|
continue
|
||||||
|
opener = info.get("openerId")
|
||||||
result.append({
|
result.append({
|
||||||
"targetId": target,
|
"targetId": target,
|
||||||
"target_id": target,
|
"target_id": target,
|
||||||
"url": tab.get("url", ""),
|
"url": url,
|
||||||
"title": tab.get("title", ""),
|
"title": title,
|
||||||
"openerTargetId": opener_by_target.get(target),
|
"openerTargetId": opener if isinstance(opener, str) and opener else None,
|
||||||
})
|
})
|
||||||
return result
|
return result
|
||||||
|
|
||||||
@@ -948,9 +953,12 @@ class RealBrowserBackend:
|
|||||||
raise WorkerError("candidate could not be focused")
|
raise WorkerError("candidate could not be focused")
|
||||||
# Select all without an arbitrary page script. Input.insertText is a
|
# Select all without an arbitrary page script. Input.insertText is a
|
||||||
# single bounded browser operation and is substantially faster than
|
# single bounded browser operation and is substantially faster than
|
||||||
# dispatching up to 8,000 individual key events.
|
# dispatching up to 8,000 individual key events. Browser.getVersion is
|
||||||
|
# stable for this backend, so cache the platform modifier for its life.
|
||||||
|
if getattr(self, "_select_all_modifier", None) is None:
|
||||||
user_agent = self._helpers.cdp("Browser.getVersion").get("userAgent", "")
|
user_agent = self._helpers.cdp("Browser.getVersion").get("userAgent", "")
|
||||||
modifiers = 4 if "Mac OS X" in user_agent or "Macintosh" in user_agent else 2
|
self._select_all_modifier = 4 if "Mac OS X" in user_agent or "Macintosh" in user_agent else 2
|
||||||
|
modifiers = self._select_all_modifier
|
||||||
select_all = {"key": "a", "code": "KeyA", "modifiers": modifiers,
|
select_all = {"key": "a", "code": "KeyA", "modifiers": modifiers,
|
||||||
"windowsVirtualKeyCode": 65, "nativeVirtualKeyCode": 65,
|
"windowsVirtualKeyCode": 65, "nativeVirtualKeyCode": 65,
|
||||||
"commands": ["SelectAll"]}
|
"commands": ["SelectAll"]}
|
||||||
|
|||||||
@@ -721,6 +721,26 @@ class BridgeRegressionTests(unittest.TestCase):
|
|||||||
with self.assertRaisesRegex(WorkerError, "outside"):
|
with self.assertRaisesRegex(WorkerError, "outside"):
|
||||||
worker.act({"observationId": obs["observationId"], "candidateId": "c-0", "action": "fill", "text": "Ada"})
|
worker.act({"observationId": obs["observationId"], "candidateId": "c-0", "action": "fill", "text": "Ada"})
|
||||||
|
|
||||||
|
def test_fill_url_postcondition_reuses_post_action_tab_state(self):
|
||||||
|
backend = FakeBackend()
|
||||||
|
original_fill = backend.fill
|
||||||
|
def fill_and_navigate(node, text):
|
||||||
|
original_fill(node, text)
|
||||||
|
backend.url = "http://fixture.test/done"
|
||||||
|
backend.fill = fill_and_navigate
|
||||||
|
current_tab = backend.current_tab
|
||||||
|
backend.current_tab = Mock(side_effect=current_tab)
|
||||||
|
worker = BrowserWorker(backend, ["http://fixture.test"])
|
||||||
|
observation = worker.observe()
|
||||||
|
result = worker.act({
|
||||||
|
"observationId": observation["observationId"], "candidateId": "c-0",
|
||||||
|
"action": "fill", "text": "Ada",
|
||||||
|
"postcondition": {"kind": "url", "value": "http://fixture.test/done"},
|
||||||
|
})
|
||||||
|
self.assertTrue(result["verified"])
|
||||||
|
# observe, fresh pre-mutation revalidation, and one post-action read.
|
||||||
|
self.assertEqual(backend.current_tab.call_count, 3)
|
||||||
|
|
||||||
def test_oversized_input_and_sensitive_error_are_not_echoed(self):
|
def test_oversized_input_and_sensitive_error_are_not_echoed(self):
|
||||||
worker = BrowserWorker(FakeBackend(), ["http://fixture.test"])
|
worker = BrowserWorker(FakeBackend(), ["http://fixture.test"])
|
||||||
out = io.BytesIO()
|
out = io.BytesIO()
|
||||||
@@ -738,6 +758,74 @@ class BridgeRegressionTests(unittest.TestCase):
|
|||||||
backend._load_harness()
|
backend._load_harness()
|
||||||
spawn.assert_not_called()
|
spawn.assert_not_called()
|
||||||
|
|
||||||
|
def test_real_backend_list_tabs_uses_one_inventory_with_helper_filtering_and_openers(self):
|
||||||
|
helper = Mock()
|
||||||
|
helper.cdp.return_value = {"targetInfos": [
|
||||||
|
{"type": "page", "targetId": "page", "url": "http://fixture.test/form", "title": "Fixture",
|
||||||
|
"openerId": "opener"},
|
||||||
|
{"type": "page", "targetId": "internal", "url": "chrome://settings", "title": "Settings"},
|
||||||
|
{"type": "page", "targetId": "devtools", "url": "devtools://devtools", "title": "DevTools"},
|
||||||
|
{"type": "page", "targetId": "placeholder", "url": "about:blank#startup", "title": "Starting agent 1"},
|
||||||
|
{"type": "service_worker", "targetId": "worker", "url": "http://fixture.test/sw", "title": "Worker"},
|
||||||
|
{"type": "page", "targetId": "plain", "url": "https://other.test/", "title": "Other", "openerId": ""},
|
||||||
|
{"type": "page", "targetId": "malformed-opener", "url": "http://fixture.test/other", "title": "Other",
|
||||||
|
"openerId": 42},
|
||||||
|
]}
|
||||||
|
backend = RealBrowserBackend.__new__(RealBrowserBackend)
|
||||||
|
backend._helpers = helper
|
||||||
|
|
||||||
|
tabs = backend.list_tabs()
|
||||||
|
|
||||||
|
self.assertEqual([tab["targetId"] for tab in tabs], ["page", "plain", "malformed-opener"])
|
||||||
|
self.assertEqual(tabs[0]["openerTargetId"], "opener")
|
||||||
|
self.assertIsNone(tabs[1]["openerTargetId"])
|
||||||
|
self.assertIsNone(tabs[2]["openerTargetId"])
|
||||||
|
helper.cdp.assert_called_once_with("Target.getTargets")
|
||||||
|
helper.list_tabs.assert_not_called()
|
||||||
|
|
||||||
|
def test_real_backend_list_tabs_propagates_inventory_errors_without_fallback(self):
|
||||||
|
helper = Mock()
|
||||||
|
helper.cdp.side_effect = RuntimeError("inventory unavailable")
|
||||||
|
backend = RealBrowserBackend.__new__(RealBrowserBackend)
|
||||||
|
backend._helpers = helper
|
||||||
|
|
||||||
|
with self.assertRaisesRegex(RuntimeError, "inventory unavailable"):
|
||||||
|
backend.list_tabs()
|
||||||
|
helper.cdp.assert_called_once_with("Target.getTargets")
|
||||||
|
helper.list_tabs.assert_not_called()
|
||||||
|
|
||||||
|
def test_real_backend_switch_tab_does_not_reenable_harness_domains(self):
|
||||||
|
helper = Mock()
|
||||||
|
helper.switch_tab.return_value = "session-1"
|
||||||
|
backend = RealBrowserBackend.__new__(RealBrowserBackend)
|
||||||
|
backend._helpers = helper
|
||||||
|
|
||||||
|
backend.switch_tab("target-1")
|
||||||
|
|
||||||
|
self.assertEqual(backend._session_id, "session-1")
|
||||||
|
self.assertEqual(backend._attached_target, "target-1")
|
||||||
|
helper.switch_tab.assert_called_once_with("target-1", activate=False)
|
||||||
|
helper.cdp.assert_not_called()
|
||||||
|
|
||||||
|
def test_real_backend_caches_select_all_modifier_for_backend_lifetime(self):
|
||||||
|
helper = Mock()
|
||||||
|
calls = []
|
||||||
|
def cdp(method, **_kwargs):
|
||||||
|
calls.append(method)
|
||||||
|
return {"userAgent": "Macintosh"} if method == "Browser.getVersion" else {}
|
||||||
|
helper.cdp.side_effect = cdp
|
||||||
|
backend = RealBrowserBackend.__new__(RealBrowserBackend)
|
||||||
|
backend._helpers = helper
|
||||||
|
backend._call = Mock(return_value=True)
|
||||||
|
|
||||||
|
backend.fill(1, "first")
|
||||||
|
backend.fill(1, "second")
|
||||||
|
|
||||||
|
self.assertEqual(calls.count("Browser.getVersion"), 1)
|
||||||
|
keydowns = [call for call in helper.cdp.call_args_list
|
||||||
|
if call.args[0] == "Input.dispatchKeyEvent" and call.kwargs["type"] == "rawKeyDown"]
|
||||||
|
self.assertEqual([call.kwargs["modifiers"] for call in keydowns], [4, 4])
|
||||||
|
|
||||||
def test_real_backend_unrestricted_policy_still_requires_one_web_tab(self):
|
def test_real_backend_unrestricted_policy_still_requires_one_web_tab(self):
|
||||||
helper = Mock()
|
helper = Mock()
|
||||||
helper.list_tabs.return_value = [
|
helper.list_tabs.return_value = [
|
||||||
|
|||||||
Reference in New Issue
Block a user