devtools: Fix source contents tests and fix a race (#38359)

test_sources_list() relied on <https://servo.org/js/load-table.js> to
test scripts loaded from multiple origins, but that file was deleted a
couple of weeks ago. this patch adds a second internal web server, then
replaces that load with scripts loaded from two distinct origins:
<http://127.0.0.1:10000> and <http://127.0.0.1:10001>.

fixing that test revealed an impl bug where inline module scripts
containing `import` statements may never get their source contents
populated. this is because the logic for populating source contents for
inline scripts only applied to source actors created *before* we
finished parsing the page. we assumed that inline scripts always block
the parser, but this is not the case. this patch ensures that inline
source contents can be populated for source actors created after parse.

Testing: adds a new test,
test_source_content_with_inline_module_import_external()
Fixes: part of #36325

Signed-off-by: Delan Azabani <dazabani@igalia.com>
Co-authored-by: atbrakhi <atbrakhi@igalia.com>
This commit is contained in:
shuppy 2025-07-30 19:29:24 +08:00 committed by GitHub
parent 900dd8d191
commit a671c50bc9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 35 additions and 1 deletions

View file

@ -84,6 +84,8 @@ pub struct ActorRegistry {
/// Lookup table for SourceActor names associated with a given PipelineId.
source_actor_names: RefCell<HashMap<PipelineId, Vec<String>>>,
/// Lookup table for inline source content associated with a given PipelineId.
inline_source_content: RefCell<HashMap<PipelineId, String>>,
shareable: Option<Arc<Mutex<ActorRegistry>>>,
next: Cell<u32>,
@ -99,6 +101,7 @@ impl ActorRegistry {
old_actors: RefCell::new(vec![]),
script_actors: RefCell::new(HashMap::new()),
source_actor_names: RefCell::new(HashMap::new()),
inline_source_content: RefCell::new(HashMap::new()),
shareable: None,
next: Cell::new(0),
start_stamp: CrossProcessInstant::now(),
@ -262,4 +265,20 @@ impl ActorRegistry {
vec![]
}
pub fn set_inline_source_content(&mut self, pipeline_id: PipelineId, content: String) {
assert!(
self.inline_source_content
.borrow_mut()
.insert(pipeline_id, content)
.is_none()
);
}
pub fn inline_source_content(&mut self, pipeline_id: PipelineId) -> Option<String> {
self.inline_source_content
.borrow()
.get(&pipeline_id)
.cloned()
}
}

View file

@ -543,11 +543,14 @@ impl DevtoolsInstance {
fn handle_create_source_actor(&mut self, pipeline_id: PipelineId, source_info: SourceInfo) {
let mut actors = self.actors.lock().unwrap();
let source_content = source_info
.content
.or_else(|| actors.inline_source_content(pipeline_id));
let source_actor = SourceActor::new_registered(
&mut actors,
pipeline_id,
source_info.url,
source_info.content,
source_content,
source_info.content_type,
);
let source_actor_name = source_actor.name.clone();
@ -613,6 +616,10 @@ impl DevtoolsInstance {
source_actor.content = Some(source_content.clone());
}
}
// Store the source content separately for any future source actors that get created *after* we finish parsing
// the HTML. For example, adding an `import` to an inline module script can delay it until after parsing.
actors.set_inline_source_content(pipeline_id, source_content);
}
}

View file

@ -112,6 +112,14 @@ class DevtoolsTests(unittest.IsolatedAsyncioTestCase):
expected_content = open(self.get_test_path("sources/test.html")).read()
self.assert_source_content(f"{self.base_url}/test.html", expected_content)
def test_source_content_with_inline_module_import_external(self):
self.start_web_server(test_dir=self.get_test_path("sources_content_with_inline_module_import_external"))
self.run_servoshell()
expected_content = open(
self.get_test_path("sources_content_with_inline_module_import_external/test.html")
).read()
self.assert_source_content(f"{self.base_urls[0]}/test.html", expected_content)
# Test case that uses innerHTML and would actually need the HTML parser
# (innerHTML has a fast path for values that dont contain b'&' | b'\0' | b'<' | b'\r')
def test_source_content_inline_script_with_inner_html(self):