Fix WebDriver activation boundaries and share network cleanup rules

This commit is contained in:
kevin
2026-09-24 18:09:19 +08:00
committed by lanyue-llk
parent e5205a28e0
commit 3e66eaa4bc
8 changed files with 116 additions and 114 deletions
+32 -17
View File
@@ -70,23 +70,7 @@ where
/// longer fit. Retained entries keep their original insertion order.
pub fn set_limits(&mut self, limits: ByteLimits) -> Vec<(K, V)> {
self.limits = limits;
let mut evicted = Vec::new();
// Filter the order once. Calling remove for every oversized entry
// would rescan the whole queue on each removal when budgets shrink.
self.insertion_order.retain(|key| {
if self
.entries
.get(key)
.is_none_or(|entry| entry.byte_len <= limits.max_entry_bytes)
{
return true;
}
if let Some(entry) = self.entries.remove(key) {
self.used_bytes -= entry.byte_len;
evicted.push((key.clone(), entry.value));
}
false
});
let mut evicted = self.retain_entries(|_, entry| entry.byte_len <= limits.max_entry_bytes);
while self.used_bytes > limits.max_total_bytes {
if let Some(entry) = self.pop_oldest() {
evicted.push(entry);
@@ -182,12 +166,43 @@ where
}
/// Removes all entries and releases all byte charges.
///
/// This does not return removed entries; use [`Self::retain`] to take ownership.
pub fn clear(&mut self) {
self.entries.clear();
self.insertion_order.clear();
self.used_bytes = 0;
}
/// Visits each value once in insertion order, returning removed entries in
/// that order and releasing their byte charges. Mutating a value does not
/// change its recorded charge; use [`Self::insert`] to replace its charge.
pub fn retain(&mut self, mut keep: impl FnMut(&K, &mut V) -> bool) -> Vec<(K, V)> {
self.retain_entries(|key, entry| keep(key, &mut entry.value))
}
fn retain_entries(
&mut self,
mut keep: impl FnMut(&K, &mut BufferedValue<V>) -> bool,
) -> Vec<(K, V)> {
let mut removed = Vec::new();
self.insertion_order.retain(|key| {
if self
.entries
.get_mut(key)
.is_some_and(|entry| keep(key, entry))
{
return true;
}
if let Some(entry) = self.entries.remove(key) {
self.used_bytes -= entry.byte_len;
removed.push((key.clone(), entry.value));
}
false
});
removed
}
fn pop_oldest(&mut self) -> Option<(K, V)> {
while let Some(key) = self.insertion_order.pop_front() {
let Some(entry) = self.entries.remove(&key) else {
+34
View File
@@ -154,3 +154,37 @@ fn remove_and_clear_return_all_byte_charges() {
assert!(buffer.is_empty());
assert_eq!(buffer.used_bytes(), 0);
}
#[test]
fn retain_visits_once_preserves_fifo_and_releases_only_removed_charges() {
let mut buffer = BoundedByteBuffer::new(ByteLimits::new(10, 10));
for key in 0..4 {
let _ = buffer.insert(key, key * 10, 2);
}
let mut visited = Vec::new();
let removed = buffer.retain(|key, value| {
visited.push(*key);
*value += 1;
key % 2 == 0
});
assert_eq!(visited, vec![0, 1, 2, 3]);
assert_eq!(removed, vec![(1, 11), (3, 31)]);
assert_eq!(buffer.used_bytes(), 4);
assert_eq!(buffer.get(&2), Some(&21));
assert_eq!(
buffer.insert(4, 40, 8),
InsertOutcome::Stored {
evicted: vec![(0, 1)]
}
);
assert!(buffer.retain(|_, _| true).is_empty());
assert_eq!(buffer.used_bytes(), 10);
assert_eq!(buffer.retain(|_, _| false), vec![(2, 21), (4, 40)]);
assert_eq!(buffer.used_bytes(), 0);
assert!(buffer.is_empty());
assert!(
buffer
.retain(|_, _| panic!("empty buffer must not visit"))
.is_empty()
);
}
+2 -1
View File
@@ -72,7 +72,8 @@ pub use request::{
RequestAuthScheme, RequestAuthTarget, RequestCacheMode, RequestCredentialsMode,
RequestHeaderOverride, RequestMode, RequestPriorityHints, RequestRedirectMode,
RequestResourceType, ResourceLoadPriority, ScriptFetchRequestMetadata,
ScriptFetchSchedulerPriority, SubresourceRequestMetadata,
ScriptFetchSchedulerPriority, SubresourceRequestMetadata, is_request_body_header_name,
redirect_status_rewrites_to_get,
};
pub use request_policy::{is_bad_port, should_request_be_blocked_due_to_bad_port};
pub use response::{
+4 -2
View File
@@ -1122,12 +1122,14 @@ impl Request {
}
}
fn redirect_status_rewrites_to_get(status: u16, method: &str) -> bool {
/// Whether following this HTTP redirect replaces the request method with GET.
pub fn redirect_status_rewrites_to_get(status: u16, method: &str) -> bool {
status == 303 && !method.eq_ignore_ascii_case("GET") && !method.eq_ignore_ascii_case("HEAD")
|| matches!(status, 301 | 302) && method.eq_ignore_ascii_case("POST")
}
fn is_request_body_header_name(name: &str) -> bool {
/// Whether a header must be removed when a redirect discards the request body.
pub fn is_request_body_header_name(name: &str) -> bool {
matches!(
name.to_ascii_lowercase().as_str(),
"content-encoding"
@@ -11559,7 +11559,6 @@ async fn webdriver_classic_click_uses_top_level_pointer_coordinates_inside_offse
}
#[tokio::test]
#[ignore = "Pending iframe overlay click fix; expectations verified against Chromium"]
async fn webdriver_classic_frame_target_click_is_invariant_under_neutral_frame_wrapping() {
for depth in 0..=2 {
for selector in ["#target", "#container"] {
@@ -11646,7 +11645,6 @@ async fn webdriver_classic_frame_target_click_is_invariant_under_neutral_frame_w
}
#[tokio::test]
#[ignore = "Pending ancestor overlay click fix; expectations verified against Chromium"]
async fn webdriver_classic_frame_click_rejects_ancestor_overlays_without_dispatch() {
for (depth, blocked_level) in [(1, 0), (2, 0), (2, 1)] {
let app = build_router(test_state());
+17 -37
View File
@@ -1402,18 +1402,12 @@ impl CapturedResponseBodyStore {
fn revoke_retained_visibility(&mut self, session_id: Option<&str>) {
// Revocation affects only old-document claims, not ordinary access to
// responses from the current document.
for request_id in &self.retained_request_ids {
if let Some(body) = self.bodies.get_mut(request_id)
&& !body.remove_session_visibility(session_id)
{
self.bodies.remove(request_id);
}
if let Some(body) = self.buffered_bodies.get_mut(request_id)
&& !body.remove_session_visibility(session_id)
{
self.buffered_bodies.remove(request_id);
}
}
let keep = |request_id: &String, body: &mut CapturedResponseBody| {
!self.retained_request_ids.contains(request_id)
|| body.remove_session_visibility(session_id)
};
self.bodies.retain(keep);
self.buffered_bodies.retain(keep);
}
fn effective_durable_limits(&self) -> ByteLimits {
@@ -1462,13 +1456,20 @@ impl CapturedResponseBodyStore {
}
fn trim_durable_entries(&mut self) {
let mut expired_payloads = HashSet::new();
while self.durable_entry_order.len() > DURABLE_RESPONSE_BODY_MAX_ENTRIES {
if let Some(id) = self.durable_entry_order.pop_front() {
self.bodies.remove(&id);
self.buffered_bodies.remove(&id);
self.retained_request_ids.remove(&id);
if self.buffered_bodies.contains_key(&id) {
expired_payloads.insert(id);
}
}
}
if !expired_payloads.is_empty() {
self.buffered_bodies
.retain(|id, _| !expired_payloads.contains(id));
}
}
fn prepare_navigation(&mut self) {
@@ -1485,16 +1486,7 @@ impl CapturedResponseBodyStore {
&& !matches!(body.state, CapturedResponseBodyState::Pending)
};
self.bodies.retain(|_, body| retain(body));
let ids = self
.buffered_bodies
.iter()
.map(|(id, _)| id.clone())
.collect::<Vec<_>>();
for id in ids {
if !self.buffered_bodies.get_mut(&id).is_some_and(retain) {
self.buffered_bodies.remove(&id);
}
}
self.buffered_bodies.retain(|_, body| retain(body));
self.retained_request_ids = self
.bodies
.keys()
@@ -1638,20 +1630,8 @@ impl CapturedResponseBodyStore {
self.bodies
.retain(|_, body| body.remove_session_visibility(session_id));
let buffered_request_ids = self
.buffered_bodies
.iter()
.map(|(request_id, _)| request_id.clone())
.collect::<Vec<_>>();
for request_id in buffered_request_ids {
let retain = self
.buffered_bodies
.get_mut(request_id.as_str())
.is_some_and(|body| body.remove_session_visibility(session_id));
if !retain {
self.buffered_bodies.remove(request_id.as_str());
}
}
self.buffered_bodies
.retain(|_, body| body.remove_session_visibility(session_id));
}
#[cfg(test)]
@@ -1,6 +1,6 @@
/// Project the request metadata for successive followed redirect hops.
///
/// This mirrors FetchRequest::apply_redirect_status: the emitted CDP request
/// This shares FetchRequest::apply_redirect_status rules: the emitted CDP request
/// must describe the new hop, not repeat the original POST after it became GET.
pub(super) struct RedirectRequest<'a> {
pub(super) method: &'a str,
@@ -22,23 +22,11 @@ impl<'a> RedirectRequest<'a> {
}
pub(super) fn follow(&mut self, status: u16) {
let becomes_get = matches!(status, 301 | 302) && self.method.eq_ignore_ascii_case("POST")
|| status == 303
&& !self.method.eq_ignore_ascii_case("GET")
&& !self.method.eq_ignore_ascii_case("HEAD");
if becomes_get {
if moli_fetch::redirect_status_rewrites_to_get(status, self.method) {
self.method = "GET";
self.body = None;
self.headers.retain(|(name, _)| {
!matches!(
name.to_ascii_lowercase().as_str(),
"content-encoding"
| "content-language"
| "content-length"
| "content-location"
| "content-type"
)
});
self.headers
.retain(|(name, _)| !moli_fetch::is_request_body_header_name(name));
}
}
}
+23 -39
View File
@@ -40,9 +40,6 @@ mod tests {
struct SharedWriterGuard(Arc<Mutex<Vec<u8>>>);
#[derive(Clone, Copy)]
struct BrokenWriter;
impl Write for SharedWriterGuard {
fn write(&mut self, buffer: &[u8]) -> io::Result<usize> {
self.0.lock().extend_from_slice(buffer);
@@ -54,30 +51,6 @@ mod tests {
}
}
impl Write for BrokenWriter {
fn write(&mut self, _buffer: &[u8]) -> io::Result<usize> {
Err(io::Error::new(
io::ErrorKind::BrokenPipe,
"diagnostic sink is gone",
))
}
fn flush(&mut self) -> io::Result<()> {
Err(io::Error::new(
io::ErrorKind::BrokenPipe,
"diagnostic sink is gone",
))
}
}
impl<'writer> MakeWriter<'writer> for BrokenWriter {
type Writer = Self;
fn make_writer(&'writer self) -> Self::Writer {
*self
}
}
impl<'writer> MakeWriter<'writer> for SharedWriter {
type Writer = SharedWriterGuard;
@@ -138,17 +111,28 @@ mod tests {
#[test]
fn broken_diagnostic_writer_does_not_panic() {
let subscriber = tracing_subscriber::fmt()
.without_time()
.with_ansi(false)
.with_target(false)
.with_writer(BrokenWriter)
.with_env_filter(first_party_filter("error"))
.log_internal_errors(false)
.finish();
tracing::subscriber::with_default(subscriber, || {
tracing::error!("broken diagnostic writer regression");
});
const CHILD: &str = "MOLI_TEST_BROKEN_DIAGNOSTIC_SINK";
if std::env::var_os(CHILD).is_some() {
super::init("error");
tracing::error!("the diagnostic receiver has already exited");
return;
}
let (reader, writer) = std::io::pipe().unwrap();
drop(reader);
let status = std::process::Command::new(std::env::current_exe().unwrap())
.args([
"--exact",
"telemetry::tests::broken_diagnostic_writer_does_not_panic",
"--nocapture",
])
.env(CHILD, "1")
.stdout(std::process::Stdio::null())
.stderr(writer)
.status()
.unwrap();
assert!(
status.success(),
"logging to a closed diagnostic pipe terminated the process: {status}"
);
}
}