script: Upgrade node_ids to pipeline_to_node_ids to track the owner pipeline of the node (#37213)

Upgrade `ScriptThread::node_ids` to `pipeline_to_node_ids` to track the
owner pipeline of the node
This will enable webdriver to know if it is requesting element from
other origins and properly distinguish "stale element reference" from
"no such element".

Testing: [Action
run](1538599490), no
regression. We can now pass WebDriver "cross origin" tests.

Fixes: #35749

---------

Signed-off-by: Euclid Ye <yezhizhenjiakang@gmail.com>
This commit is contained in:
Euclid Ye 2025-06-02 20:26:45 +08:00 committed by GitHub
parent 2a07ef01f5
commit c28394f476
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 61 additions and 43 deletions

View file

@ -132,7 +132,7 @@ fn find_node_by_unique_id(
document document
.upcast::<Node>() .upcast::<Node>()
.traverse_preorder(ShadowIncluding::Yes) .traverse_preorder(ShadowIncluding::Yes)
.find(|candidate| candidate.unique_id() == node_id) .find(|candidate| candidate.unique_id(pipeline) == node_id)
}) })
} }

View file

@ -1261,13 +1261,13 @@ impl Node {
} }
} }
pub(crate) fn unique_id(&self) -> String { pub(crate) fn unique_id(&self, pipeline: PipelineId) -> String {
let mut rare_data = self.ensure_rare_data(); let mut rare_data = self.ensure_rare_data();
if rare_data.unique_id.is_none() { if rare_data.unique_id.is_none() {
let id = UniqueId::new(); let node_id = UniqueId::new();
ScriptThread::save_node_id(id.borrow().simple().to_string()); ScriptThread::save_node_id(pipeline, node_id.borrow().simple().to_string());
rare_data.unique_id = Some(id); rare_data.unique_id = Some(node_id);
} }
rare_data rare_data
.unique_id .unique_id
@ -1281,6 +1281,7 @@ impl Node {
pub(crate) fn summarize(&self, can_gc: CanGc) -> NodeInfo { pub(crate) fn summarize(&self, can_gc: CanGc) -> NodeInfo {
let USVString(base_uri) = self.BaseURI(); let USVString(base_uri) = self.BaseURI();
let node_type = self.NodeType(); let node_type = self.NodeType();
let pipeline = self.owner_document().window().pipeline_id();
let maybe_shadow_root = self.downcast::<ShadowRoot>(); let maybe_shadow_root = self.downcast::<ShadowRoot>();
let shadow_root_mode = maybe_shadow_root let shadow_root_mode = maybe_shadow_root
@ -1288,7 +1289,7 @@ impl Node {
.map(ShadowRootMode::convert); .map(ShadowRootMode::convert);
let host = maybe_shadow_root let host = maybe_shadow_root
.map(ShadowRoot::Host) .map(ShadowRoot::Host)
.map(|host| host.upcast::<Node>().unique_id()); .map(|host| host.upcast::<Node>().unique_id(pipeline));
let is_shadow_host = self.downcast::<Element>().is_some_and(|potential_host| { let is_shadow_host = self.downcast::<Element>().is_some_and(|potential_host| {
let Some(root) = potential_host.shadow_root() else { let Some(root) = potential_host.shadow_root() else {
return false; return false;
@ -1310,12 +1311,12 @@ impl Node {
.map(|style| style.Display().into()); .map(|style| style.Display().into());
NodeInfo { NodeInfo {
unique_id: self.unique_id(), unique_id: self.unique_id(pipeline),
host, host,
base_uri, base_uri,
parent: self parent: self
.GetParentNode() .GetParentNode()
.map_or("".to_owned(), |node| node.unique_id()), .map_or("".to_owned(), |node| node.unique_id(pipeline)),
node_type, node_type,
is_top_level_document: node_type == NodeConstants::DOCUMENT_NODE, is_top_level_document: node_type == NodeConstants::DOCUMENT_NODE,
node_name: String::from(self.NodeName()), node_name: String::from(self.NodeName()),

View file

@ -194,6 +194,8 @@ pub(crate) struct IncompleteParserContexts(RefCell<Vec<(PipelineId, ParserContex
unsafe_no_jsmanaged_fields!(TaskQueue<MainThreadScriptMsg>); unsafe_no_jsmanaged_fields!(TaskQueue<MainThreadScriptMsg>);
type NodeIdSet = HashSet<String>;
#[derive(JSTraceable)] #[derive(JSTraceable)]
// ScriptThread instances are rooted on creation, so this is okay // ScriptThread instances are rooted on creation, so this is okay
#[cfg_attr(crown, allow(crown::unrooted_must_root))] #[cfg_attr(crown, allow(crown::unrooted_must_root))]
@ -315,8 +317,9 @@ pub struct ScriptThread {
#[no_trace] #[no_trace]
player_context: WindowGLContext, player_context: WindowGLContext,
/// A set of all nodes ever created in this script thread /// A map from pipelines to all owned nodes ever created in this script thread
node_ids: DomRefCell<HashSet<String>>, #[no_trace]
pipeline_to_node_ids: DomRefCell<HashMap<PipelineId, NodeIdSet>>,
/// Code is running as a consequence of a user interaction /// Code is running as a consequence of a user interaction
is_user_interacting: Cell<bool>, is_user_interacting: Cell<bool>,
@ -818,14 +821,25 @@ impl ScriptThread {
}) })
} }
pub(crate) fn save_node_id(node_id: String) { pub(crate) fn save_node_id(pipeline: PipelineId, node_id: String) {
with_script_thread(|script_thread| { with_script_thread(|script_thread| {
script_thread.node_ids.borrow_mut().insert(node_id); script_thread
.pipeline_to_node_ids
.borrow_mut()
.entry(pipeline)
.or_default()
.insert(node_id);
}) })
} }
pub(crate) fn has_node_id(node_id: &str) -> bool { pub(crate) fn has_node_id(pipeline: PipelineId, node_id: &str) -> bool {
with_script_thread(|script_thread| script_thread.node_ids.borrow().contains(node_id)) with_script_thread(|script_thread| {
script_thread
.pipeline_to_node_ids
.borrow()
.get(&pipeline)
.is_some_and(|node_ids| node_ids.contains(node_id))
})
} }
/// Creates a new script thread. /// Creates a new script thread.
@ -945,7 +959,7 @@ impl ScriptThread {
unminify_css: opts.unminify_css, unminify_css: opts.unminify_css,
user_content_manager: state.user_content_manager, user_content_manager: state.user_content_manager,
player_context: state.player_context, player_context: state.player_context,
node_ids: Default::default(), pipeline_to_node_ids: Default::default(),
is_user_interacting: Cell::new(false), is_user_interacting: Cell::new(false),
#[cfg(feature = "webgpu")] #[cfg(feature = "webgpu")]
gpu_id_hub: Arc::new(IdentityHub::default()), gpu_id_hub: Arc::new(IdentityHub::default()),

View file

@ -81,7 +81,7 @@ fn find_node_by_unique_id(
match documents.find_document(pipeline) { match documents.find_document(pipeline) {
Some(doc) => find_node_by_unique_id_in_document(&doc, node_id), Some(doc) => find_node_by_unique_id_in_document(&doc, node_id),
None => { None => {
if ScriptThread::has_node_id(&node_id) { if ScriptThread::has_node_id(pipeline, &node_id) {
Err(ErrorStatus::StaleElementReference) Err(ErrorStatus::StaleElementReference)
} else { } else {
Err(ErrorStatus::NoSuchElement) Err(ErrorStatus::NoSuchElement)
@ -94,14 +94,15 @@ pub(crate) fn find_node_by_unique_id_in_document(
document: &Document, document: &Document,
node_id: String, node_id: String,
) -> Result<DomRoot<Node>, ErrorStatus> { ) -> Result<DomRoot<Node>, ErrorStatus> {
let pipeline = document.window().pipeline_id();
match document match document
.upcast::<Node>() .upcast::<Node>()
.traverse_preorder(ShadowIncluding::Yes) .traverse_preorder(ShadowIncluding::Yes)
.find(|node| node.unique_id() == node_id) .find(|node| node.unique_id(pipeline) == node_id)
{ {
Some(node) => Ok(node), Some(node) => Ok(node),
None => { None => {
if ScriptThread::has_node_id(&node_id) { if ScriptThread::has_node_id(pipeline, &node_id) {
Err(ErrorStatus::StaleElementReference) Err(ErrorStatus::StaleElementReference)
} else { } else {
Err(ErrorStatus::NoSuchElement) Err(ErrorStatus::NoSuchElement)
@ -129,7 +130,10 @@ fn matching_links(
content == link_text content == link_text
} }
}) })
.map(|node| node.upcast::<Node>().unique_id()) .map(|node| {
node.upcast::<Node>()
.unique_id(node.owner_doc().window().pipeline_id())
})
} }
fn all_matching_links( fn all_matching_links(
@ -329,20 +333,25 @@ unsafe fn jsval_to_webdriver_inner(
Ok(WebDriverJSValue::ArrayLike(result)) Ok(WebDriverJSValue::ArrayLike(result))
} else if let Ok(element) = root_from_object::<Element>(*object, cx) { } else if let Ok(element) = root_from_object::<Element>(*object, cx) {
Ok(WebDriverJSValue::Element(WebElement( Ok(WebDriverJSValue::Element(WebElement(
element.upcast::<Node>().unique_id(), element
.upcast::<Node>()
.unique_id(element.owner_document().window().pipeline_id()),
))) )))
} else if let Ok(window) = root_from_object::<Window>(*object, cx) { } else if let Ok(window) = root_from_object::<Window>(*object, cx) {
let window_proxy = window.window_proxy(); let window_proxy = window.window_proxy();
if window_proxy.is_browsing_context_discarded() { if window_proxy.is_browsing_context_discarded() {
return Err(WebDriverJSError::StaleElementReference); return Err(WebDriverJSError::StaleElementReference);
} else if window_proxy.browsing_context_id() == window_proxy.webview_id() {
Ok(WebDriverJSValue::Window(WebWindow(
window.Document().upcast::<Node>().unique_id(),
)))
} else { } else {
Ok(WebDriverJSValue::Frame(WebFrame( let pipeline = window.pipeline_id();
window.Document().upcast::<Node>().unique_id(), if window_proxy.browsing_context_id() == window_proxy.webview_id() {
))) Ok(WebDriverJSValue::Window(WebWindow(
window.Document().upcast::<Node>().unique_id(pipeline),
)))
} else {
Ok(WebDriverJSValue::Frame(WebFrame(
window.Document().upcast::<Node>().unique_id(pipeline),
)))
}
} }
} else if object_has_to_json_property(cx, global_scope, object.handle()) { } else if object_has_to_json_property(cx, global_scope, object.handle()) {
let name = CString::new("toJSON").unwrap(); let name = CString::new("toJSON").unwrap();
@ -598,7 +607,7 @@ pub(crate) fn handle_find_element_css(
.QuerySelector(DOMString::from(selector)) .QuerySelector(DOMString::from(selector))
.map_err(|_| ErrorStatus::InvalidSelector) .map_err(|_| ErrorStatus::InvalidSelector)
}) })
.map(|node| node.map(|x| x.upcast::<Node>().unique_id())), .map(|node| node.map(|x| x.upcast::<Node>().unique_id(pipeline))),
) )
.unwrap(); .unwrap();
} }
@ -640,7 +649,7 @@ pub(crate) fn handle_find_element_tag_name(
.elements_iter() .elements_iter()
.next() .next()
}) })
.map(|node| node.map(|x| x.upcast::<Node>().unique_id())), .map(|node| node.map(|x| x.upcast::<Node>().unique_id(pipeline))),
) )
.unwrap(); .unwrap();
} }
@ -664,7 +673,7 @@ pub(crate) fn handle_find_elements_css(
.map(|nodes| { .map(|nodes| {
nodes nodes
.iter() .iter()
.map(|x| x.upcast::<Node>().unique_id()) .map(|x| x.upcast::<Node>().unique_id(pipeline))
.collect() .collect()
}), }),
) )
@ -706,7 +715,7 @@ pub(crate) fn handle_find_elements_tag_name(
.map(|nodes| { .map(|nodes| {
nodes nodes
.elements_iter() .elements_iter()
.map(|x| x.upcast::<Node>().unique_id()) .map(|x| x.upcast::<Node>().unique_id(pipeline))
.collect::<Vec<String>>() .collect::<Vec<String>>()
}), }),
) )
@ -725,7 +734,7 @@ pub(crate) fn handle_find_element_element_css(
find_node_by_unique_id(documents, pipeline, element_id).and_then(|node| { find_node_by_unique_id(documents, pipeline, element_id).and_then(|node| {
node.query_selector(DOMString::from(selector)) node.query_selector(DOMString::from(selector))
.map_err(|_| ErrorStatus::InvalidSelector) .map_err(|_| ErrorStatus::InvalidSelector)
.map(|node| node.map(|x| x.upcast::<Node>().unique_id())) .map(|node| node.map(|x| x.upcast::<Node>().unique_id(pipeline)))
}), }),
) )
.unwrap(); .unwrap();
@ -764,7 +773,7 @@ pub(crate) fn handle_find_element_element_tag_name(
.GetElementsByTagName(DOMString::from(selector), can_gc) .GetElementsByTagName(DOMString::from(selector), can_gc)
.elements_iter() .elements_iter()
.next() .next()
.map(|x| x.upcast::<Node>().unique_id())), .map(|x| x.upcast::<Node>().unique_id(pipeline))),
None => Err(ErrorStatus::UnknownError), None => Err(ErrorStatus::UnknownError),
}), }),
) )
@ -786,7 +795,7 @@ pub(crate) fn handle_find_element_elements_css(
.map(|nodes| { .map(|nodes| {
nodes nodes
.iter() .iter()
.map(|x| x.upcast::<Node>().unique_id()) .map(|x| x.upcast::<Node>().unique_id(pipeline))
.collect() .collect()
}) })
}), }),
@ -826,7 +835,7 @@ pub(crate) fn handle_find_element_elements_tag_name(
Some(element) => Ok(element Some(element) => Ok(element
.GetElementsByTagName(DOMString::from(selector), can_gc) .GetElementsByTagName(DOMString::from(selector), can_gc)
.elements_iter() .elements_iter()
.map(|x| x.upcast::<Node>().unique_id()) .map(|x| x.upcast::<Node>().unique_id(pipeline))
.collect::<Vec<String>>()), .collect::<Vec<String>>()),
None => Err(ErrorStatus::UnknownError), None => Err(ErrorStatus::UnknownError),
}), }),
@ -867,7 +876,7 @@ pub(crate) fn handle_get_active_element(
documents documents
.find_document(pipeline) .find_document(pipeline)
.and_then(|document| document.GetActiveElement()) .and_then(|document| document.GetActiveElement())
.map(|element| element.upcast::<Node>().unique_id()), .map(|element| element.upcast::<Node>().unique_id(pipeline)),
) )
.unwrap(); .unwrap();
} }
@ -1395,7 +1404,7 @@ pub(crate) fn handle_element_click(
Ok(None) Ok(None)
}, },
None => Ok(Some(node.unique_id())), None => Ok(Some(node.unique_id(pipeline))),
} }
}), }),
) )

View file

@ -1,9 +1,3 @@
[iframe.py] [iframe.py]
[test_frame_element] [test_frame_element]
expected: FAIL expected: FAIL
[test_source_origin[same_origin\]]
expected: FAIL
[test_source_origin[cross_origin\]]
expected: FAIL