mirror of
https://github.com/servo/servo
synced 2026-04-26 09:35:26 +02:00
script: Have FetchResponseListener::process_response_eof consume the listener (#40556)
The goal of this change is to prevent having to copy so much data out of listeners when a fetch completes, which will be particularly important for off-the-main thread parsing of CSS (see #22478). This change has pros and cons: Pros: - This makes the design of the `FetchResponseListener` a great deal simpler. They no longer individually store a dummy `ResourceFetchTiming` that is only replaced right before `process_response_eof`. - The creation of the `Arc<Mutex<FetchResponseListener>>` in the `NetworkListener` is abstracted away from clients and now they just pass the `FetchResponseListener` to the fetch methods in the global. Cons: - Now each `FetchResponseListener` must explicitly call `submit_timing` instead of having the `NetworkListener` do it. This is arguably a bit easier to follow in the code. - Since the internal data of the `NetworkListener` is now an `Arc<Mutex<Option<FetchResponseListener>>>`, when the fetching code needs to share state with the `NetworkListener` it either needs to share an `Option` or some sort of internal state. In one case I've stored the `Option` and in another case, I've stored a new inner shared value. Testing: This should not change observable behavior and is thus covered by existing tests. Fixes: #22550 --------- Signed-off-by: Martin Robinson <mrobinson@igalia.com>
This commit is contained in:
@@ -19,7 +19,7 @@ use net_traits::request::{
|
||||
CorsSettings, CredentialsMode, Destination, InsecureRequestsPolicy, ParserMetadata,
|
||||
RequestBuilder, RequestId,
|
||||
};
|
||||
use net_traits::{FetchMetadata, Metadata, NetworkError, ResourceFetchTiming, ResourceTimingType};
|
||||
use net_traits::{FetchMetadata, Metadata, NetworkError, ResourceFetchTiming};
|
||||
use script_bindings::domstring::BytesView;
|
||||
use servo_url::{ImmutableOrigin, ServoUrl};
|
||||
use style::attr::AttrValue;
|
||||
@@ -340,8 +340,6 @@ struct ClassicContext {
|
||||
status: Result<(), NetworkError>,
|
||||
/// The fetch options of the script
|
||||
fetch_options: ScriptFetchOptions,
|
||||
/// Timing object for this resource
|
||||
resource_timing: ResourceFetchTiming,
|
||||
}
|
||||
|
||||
impl FetchResponseListener for ClassicContext {
|
||||
@@ -388,37 +386,42 @@ impl FetchResponseListener for ClassicContext {
|
||||
/// <https://html.spec.whatwg.org/multipage/#fetch-a-classic-script>
|
||||
/// step 4-9
|
||||
fn process_response_eof(
|
||||
&mut self,
|
||||
mut self,
|
||||
_: RequestId,
|
||||
response: Result<ResourceFetchTiming, NetworkError>,
|
||||
) {
|
||||
let (source_text, final_url) = match (response.as_ref(), self.status.as_ref()) {
|
||||
(Err(err), _) | (_, Err(err)) => {
|
||||
match (response.as_ref(), self.status.as_ref()) {
|
||||
(Err(error), _) | (_, Err(error)) => {
|
||||
// Step 6, response is an error.
|
||||
finish_fetching_a_classic_script(
|
||||
&self.elem.root(),
|
||||
self.kind,
|
||||
self.url.clone(),
|
||||
Err(NoTrace(err.clone())),
|
||||
Err(NoTrace(error.clone())),
|
||||
CanGc::note(),
|
||||
);
|
||||
|
||||
// Resource timing is expected to be available before "error" or "load" events are fired.
|
||||
if let Ok(response) = &response {
|
||||
network_listener::submit_timing(&self, response, CanGc::note());
|
||||
}
|
||||
return;
|
||||
},
|
||||
(Ok(_), Ok(_)) => {
|
||||
let metadata = self.metadata.take().unwrap();
|
||||
|
||||
// Step 7.
|
||||
let encoding = metadata
|
||||
.charset
|
||||
.and_then(|encoding| Encoding::for_label(encoding.as_bytes()))
|
||||
.unwrap_or(self.character_encoding);
|
||||
|
||||
// Step 8.
|
||||
let (source_text, _, _) = encoding.decode(&self.data);
|
||||
(source_text, metadata.final_url)
|
||||
},
|
||||
_ => {},
|
||||
};
|
||||
|
||||
let metadata = self.metadata.take().unwrap();
|
||||
let final_url = metadata.final_url;
|
||||
|
||||
// Step 7.
|
||||
let encoding = metadata
|
||||
.charset
|
||||
.and_then(|encoding| Encoding::for_label(encoding.as_bytes()))
|
||||
.unwrap_or(self.character_encoding);
|
||||
|
||||
// Step 8.
|
||||
let (source_text, _, _) = encoding.decode(&self.data);
|
||||
|
||||
let elem = self.elem.root();
|
||||
let global = elem.global();
|
||||
// let cx = GlobalScope::get_cx();
|
||||
@@ -469,18 +472,10 @@ impl FetchResponseListener for ClassicContext {
|
||||
CanGc::note(),
|
||||
);
|
||||
// }
|
||||
}
|
||||
|
||||
fn resource_timing_mut(&mut self) -> &mut ResourceFetchTiming {
|
||||
&mut self.resource_timing
|
||||
}
|
||||
|
||||
fn resource_timing(&self) -> &ResourceFetchTiming {
|
||||
&self.resource_timing
|
||||
}
|
||||
|
||||
fn submit_resource_timing(&mut self) {
|
||||
network_listener::submit_timing(self, CanGc::note())
|
||||
if let Ok(response) = response {
|
||||
network_listener::submit_timing(&self, &response, CanGc::note());
|
||||
}
|
||||
}
|
||||
|
||||
fn process_csp_violations(&mut self, _request_id: RequestId, violations: Vec<Violation>) {
|
||||
@@ -581,7 +576,6 @@ fn fetch_a_classic_script(
|
||||
url: url.clone(),
|
||||
status: Ok(()),
|
||||
fetch_options: options,
|
||||
resource_timing: ResourceFetchTiming::new(ResourceTimingType::Resource),
|
||||
};
|
||||
doc.fetch(LoadType::Script(url), request, context);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user