Skip to content

Commit

Permalink
Avoid sending a new chunk when the body is already done
Browse files Browse the repository at this point in the history
As discussed in #26807 (comment), we'd like to
add a new flag, `in_memory_done`, to `TransmitBodyConnectHandler` so
that we can correctly finish and drop the sender correctly.

When we send the bytes, we will mark the body as done and we can
recognize it's already done in next tick so that we can send a Done
request to finish the sender.

Also, when there comes a redirect request, it will go to `re-extract`
route, we can set the `done` flag to `false` which means we won't
stop the IPC routers yet. Then, if the re-extract sent its bytes, we
will be marked as done again so that we can finish with stopping the IPC
routes when Chunk request comes.
  • Loading branch information
CYBAI committed Jun 13, 2020
1 parent 9798373 commit 63aab1b
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 8 deletions.
7 changes: 2 additions & 5 deletions components/net_traits/request.rs
Expand Up @@ -117,7 +117,7 @@ pub enum ParserMetadata {
}

/// <https://fetch.spec.whatwg.org/#concept-body-source>
#[derive(Clone, Debug, Deserialize, MallocSizeOf, Serialize)]
#[derive(Clone, Debug, Deserialize, MallocSizeOf, PartialEq, Serialize)]
pub enum BodySource {
Null,
Object,
Expand Down Expand Up @@ -178,10 +178,7 @@ impl RequestBody {
}

pub fn source_is_null(&self) -> bool {
if let BodySource::Null = self.source {
return true;
}
false
self.source == BodySource::Null
}

pub fn len(&self) -> Option<usize> {
Expand Down
33 changes: 30 additions & 3 deletions components/script/body.rs
Expand Up @@ -55,7 +55,7 @@ pub enum BodySource {
Null,
/// Another Dom object as source,
/// TODO: store the actual object
/// and re-exctact a stream on re-direct.
/// and re-extract a stream on re-direct.
Object,
}

Expand All @@ -72,6 +72,7 @@ struct TransmitBodyConnectHandler {
bytes_sender: Option<IpcSender<Vec<u8>>>,
control_sender: IpcSender<BodyChunkRequest>,
in_memory: Option<Vec<u8>>,
in_memory_done: bool,
source: BodySource,
}

Expand All @@ -91,14 +92,22 @@ impl TransmitBodyConnectHandler {
bytes_sender: None,
control_sender,
in_memory,
in_memory_done: false,
source,
}
}

/// Reset `in_memory_done`, called when a stream is
/// re-extracted from the source to support a re-direct.
pub fn reset_in_memory_done(&mut self) {
self.in_memory_done = false;
}

/// Re-extract the source to support streaming it again for a re-direct.
/// TODO: actually re-extract the source, instead of just cloning data, to support Blob.
fn re_extract(&self, chunk_request_receiver: IpcReceiver<BodyChunkRequest>) {
fn re_extract(&mut self, chunk_request_receiver: IpcReceiver<BodyChunkRequest>) {
let mut body_handler = self.clone();
body_handler.reset_in_memory_done();

ROUTER.add_route(
chunk_request_receiver.to_opaque(),
Expand Down Expand Up @@ -126,11 +135,20 @@ impl TransmitBodyConnectHandler {
/// TODO: this method should be deprecated
/// in favor of making `re_extract` actually re-extract a stream from the source.
/// See #26686
fn transmit_source(&self) {
fn transmit_source(&mut self) {
if self.in_memory_done {
// Step 5.1.3
self.stop_reading();
return;
}

if let BodySource::Null = self.source {
panic!("ReadableStream(Null) sources should not re-direct.");
}

if let Some(bytes) = self.in_memory.clone() {
// The memoized bytes are sent so we mark it as done again
self.in_memory_done = true;
let _ = self
.bytes_sender
.as_ref()
Expand All @@ -155,6 +173,12 @@ impl TransmitBodyConnectHandler {

/// The entry point to <https://fetch.spec.whatwg.org/#concept-request-transmit-body>
fn transmit_body_chunk(&mut self) {
if self.in_memory_done {
// Step 5.1.3
self.stop_reading();
return;
}

let stream = self.stream.clone();
let control_sender = self.control_sender.clone();
let bytes_sender = self
Expand All @@ -165,6 +189,9 @@ impl TransmitBodyConnectHandler {
// In case of the data being in-memory, send everything in one chunk, by-passing SpiderMonkey.
if let Some(bytes) = self.in_memory.clone() {
let _ = bytes_sender.send(bytes);
// Mark this body as `done` so that we can stop reading in the next tick,
// matching the behavior of the promise-based flow
self.in_memory_done = true;
return;
}

Expand Down

0 comments on commit 63aab1b

Please sign in to comment.