Skip to content

Commit f2d84dd

Browse files
committed
fix(identity): prove refs in the coordinates allocation writes
The capture proof looked refs up by native child index while allocation writes positions in the projected tree, so after a walker skipped a sibling a unique ref was published unproven and failed with identity_not_unique. The proof now walks the allocation path through the observation's own child lists, and projection keeps those lists instead of moving them out.
1 parent d29aa1d commit f2d84dd

4 files changed

Lines changed: 85 additions & 15 deletions

File tree

‎crates/core/src/locator/live_locator/observed_tree.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ impl ObservedTree {
255255
.known()
256256
.cloned()
257257
.unwrap_or_else(|| "unknown".into());
258-
let child_indices = std::mem::take(&mut node.children);
258+
let child_indices = node.children.clone();
259259
let projected = AccessibilityNode {
260260
ref_id: node.ref_id.take(),
261261
role,

‎crates/core/src/refs/ref_capture_proof.rs‎

Lines changed: 44 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ const MASKS: usize = (NATIVE_ID | NAME | DESCRIPTION) as usize + 1;
2323
/// will count later.
2424
pub(crate) struct CaptureProof<'a> {
2525
observed: &'a ObservedTree,
26-
paths: OnceLock<HashMap<&'a [usize], usize>>,
26+
start: Option<usize>,
2727
buckets: [OnceLock<HashMap<LiveKey<'a>, usize>>; MASKS],
2828
unread: Vec<(u8, LivePredicate<'a>)>,
2929
whole_window: bool,
@@ -36,7 +36,7 @@ impl<'a> CaptureProof<'a> {
3636
Self {
3737
observed,
3838
whole_window,
39-
paths: OnceLock::new(),
39+
start: observed.roots.first().map(|root| *root as usize),
4040
buckets: std::array::from_fn(|_| OnceLock::new()),
4141
unread: observed
4242
.nodes
@@ -49,18 +49,49 @@ impl<'a> CaptureProof<'a> {
4949
}
5050
}
5151

52+
/// Starts allocation paths at the node whose observed path is `prefix`, as a
53+
/// drill does: the prefix is that node's own path, and the rest of an
54+
/// allocation path counts positions in the projected tree below it.
55+
pub(crate) fn rooted_at(mut self, prefix: &[usize]) -> Self {
56+
self.start = self
57+
.observed
58+
.nodes
59+
.iter()
60+
.position(|node| node.path.as_slice() == prefix);
61+
self
62+
}
63+
64+
/// Whether `entry`, allocated at `path`, is proven. The path is read the way
65+
/// allocation wrote it: the start node's own path, then child positions in
66+
/// the projection, which skips no child the observation kept even when the
67+
/// native indices have gaps.
5268
pub(crate) fn proves(&self, entry: &RefEntry, path: &[usize]) -> bool {
53-
self.paths
54-
.get_or_init(|| {
55-
self.observed
56-
.nodes
57-
.iter()
58-
.enumerate()
59-
.map(|(index, node)| (node.path.as_slice(), index))
60-
.collect()
61-
})
62-
.get(path)
63-
.is_some_and(|index| self.proves_node(entry, *index))
69+
let Some(mut index) = self.start else {
70+
return false;
71+
};
72+
let Some(prefix) = self
73+
.observed
74+
.nodes
75+
.get(index)
76+
.map(|node| node.path.as_slice())
77+
else {
78+
return false;
79+
};
80+
let Some(steps) = path.strip_prefix(prefix) else {
81+
return false;
82+
};
83+
for step in steps {
84+
match self
85+
.observed
86+
.nodes
87+
.get(index)
88+
.and_then(|node| node.children.get(*step))
89+
{
90+
Some(child) => index = *child as usize,
91+
None => return false,
92+
}
93+
}
94+
self.proves_node(entry, index)
6495
}
6596

6697
/// Whether `entry` was proven unique at the node it was built from: the

‎crates/core/src/refs/ref_capture_proof_tests.rs‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,3 +302,41 @@ fn a_published_entry_is_proven_by_its_own_predicate_not_the_preferred_identifier
302302
assert_eq!(named.matches.len(), 1);
303303
assert!(named.matches[0].entry.identity.unique_at_capture);
304304
}
305+
306+
#[test]
307+
fn a_skipped_native_sibling_does_not_unprove_the_refs_after_it() {
308+
let observed = ObservedTree::from_roots(
309+
vec![ObservedSubtree::new(
310+
evidence("window", "Fixture", Vec::new(), 0),
311+
vec![
312+
ObservedSubtree::new(
313+
evidence("button", "Cancel", Vec::new(), 1),
314+
Vec::new(),
315+
true,
316+
None,
317+
)
318+
.with_source_child_index(0),
319+
ObservedSubtree::new(
320+
evidence("button", "Save", Vec::new(), 1),
321+
Vec::new(),
322+
true,
323+
None,
324+
)
325+
.with_source_child_index(2),
326+
],
327+
true,
328+
None,
329+
)],
330+
ObservationSource::Window {
331+
window: fixture_window(),
332+
surface: crate::SnapshotSurface::Window,
333+
renderer_accessibility_forced: false,
334+
},
335+
LocatorStats::default(),
336+
true,
337+
)
338+
.expect("the fixture observes");
339+
let root = window(vec![named("button", "Cancel"), named("button", "Save")]);
340+
341+
assert_eq!(flags(root, &observed), [true, true]);
342+
}

‎crates/core/src/snapshot/snapshot_ref.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,8 @@ pub fn run_from_ref_with_context(
8080
path_prefix,
8181
observed,
8282
} = capture;
83-
let capture_proof = crate::refs::ref_capture_proof::CaptureProof::new(&observed);
83+
let capture_proof =
84+
crate::refs::ref_capture_proof::CaptureProof::new(&observed).rooted_at(&path_prefix);
8485

8586
let source_app = entry.source.source_app.as_deref();
8687
let source_window_id = entry.source.source_window_id.as_deref();

0 commit comments

Comments
 (0)