Skip to content

/ servo

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add StreamConsumer wrapper and methods to response #24977

Merged
merged 1 commit into from Dec 3, 2019
+51 −90
Merged

Add StreamConsumer wrapper and methods to response #24977

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

Loading status checks…

Add StreamConsumer wrapper and methods to response

  • Loading branch information
@ridhimrastogi @jdm
ridhimrastogi authored and jdm committed Dec 3, 2019
commit a5469f871073c4cf7912719c8fd47e233805e4e8
2 components/script/dom/bindings/trace.rs
View file
Open in desktop
@@ -40,6 +40,7 @@ use crate::dom::document::PendingRestyle;
use crate::dom::htmlimageelement::SourceSet;
use crate::dom::htmlmediaelement::{HTMLMediaElementFetchContext, MediaFrameRenderer};
use crate::dom::identityhub::Identities;
use crate::script_runtime::StreamConsumer;
use crate::task::TaskBox;
use app_units::Au;
use canvas_traits::canvas::{
@@ -543,6 +544,7 @@ unsafe_no_jsmanaged_fields!(Arc<Mutex<dyn AudioRenderer>>);
unsafe_no_jsmanaged_fields!(MediaSessionActionType);
unsafe_no_jsmanaged_fields!(MediaMetadata);
unsafe_no_jsmanaged_fields!(WebrenderIpcSender);
unsafe_no_jsmanaged_fields!(StreamConsumer);

unsafe impl<'a> JSTraceable for &'a str {
#[inline]
17 components/script/dom/response.rs
View file
Open in desktop
@@ -19,6 +19,7 @@ use crate::dom::headers::{is_obs_text, is_vchar};
use crate::dom::headers::{Guard, Headers};
use crate::dom::promise::Promise;
use crate::dom::xmlhttprequest::Extractable;
use crate::script_runtime::StreamConsumer;
use dom_struct::dom_struct;
use http::header::HeaderMap as HyperHeaders;
use hyper::StatusCode;
@@ -48,6 +49,8 @@ pub struct Response {
body: DomRefCell<NetTraitsResponseBody>,
#[ignore_malloc_size_of = "Rc"]
body_promise: DomRefCell<Option<(Rc<Promise>, BodyType)>>,
#[ignore_malloc_size_of = "StreamConsumer"]
stream_consumer: DomRefCell<Option<StreamConsumer>>,
}

impl Response {
@@ -64,6 +67,7 @@ impl Response {
url_list: DomRefCell::new(vec![]),
body: DomRefCell::new(NetTraitsResponseBody::Empty),
body_promise: DomRefCell::new(None),
stream_consumer: DomRefCell::new(None),
}
}

@@ -441,11 +445,24 @@ impl Response {
}
}

pub fn set_stream_consumer(&self, sc: Option<StreamConsumer>) {
*self.stream_consumer.borrow_mut() = sc;
}

pub fn stream_chunk(&self, stream: &[u8]) {
if let Some(stream_consumer) = self.stream_consumer.borrow_mut().as_ref() {
stream_consumer.consume_chunk(stream);
}
}

#[allow(unrooted_must_root)]
pub fn finish(&self, body: Vec<u8>) {
*self.body.borrow_mut() = NetTraitsResponseBody::Done(body);
if let Some((p, body_type)) = self.body_promise.borrow_mut().take() {
consume_body_with_promise(self, body_type, &p);
}
if let Some(stream_consumer) = self.stream_consumer.borrow_mut().take() {
stream_consumer.stream_end();
}
}
}
1 components/script/fetch.rs
View file
Open in desktop
@@ -260,6 +260,7 @@ impl FetchResponseListener for FetchContext {
}

fn process_response_chunk(&mut self, mut chunk: Vec<u8>) {
self.response_object.root().stream_chunk(chunk.as_slice());

This comment has been minimized.

Sign in to view
Copy link
@jdm

jdm Dec 2, 2019

Member

It turns out that self.body.append(&mut chunk); empties the passed vector, which is why a bunch of tests that I would expect to pass are failing with these changes. If we reverse the order of these lines, then the compileStreaming tests from /wasm/webapi/contenttype.any.html (and likely other test files as well) should start passing.

This comment has been minimized.

Sign in to view
Copy link
@ridhimrastogi

ridhimrastogi Dec 3, 2019

Author Contributor

I have made the changes. Now the mimeType related tests are passing. However, some of the response status tests in /tests/wpt/metadata/wasm/webapi/status.any.js.ini are failing, which I believe should have passed. Is this expected?
self.body.append(&mut chunk);
}

13 components/script/script_runtime.rs
View file
Open in desktop
@@ -853,26 +853,30 @@ pub struct StreamConsumer(*mut JSStreamConsumer);

#[allow(unsafe_code)]
impl StreamConsumer {
fn consume_chunk(&self, stream: &[u8]) -> bool {
pub fn consume_chunk(&self, stream: &[u8]) -> bool {
unsafe {
let stream_ptr = stream.as_ptr();
return StreamConsumerConsumeChunk(self.0, stream_ptr, stream.len());
}
}

fn stream_end(&self) {
pub fn stream_end(&self) {
unsafe {
StreamConsumerStreamEnd(self.0);
}
}

fn stream_error(&self, error_code: usize) {
pub fn stream_error(&self, error_code: usize) {
unsafe {
StreamConsumerStreamError(self.0, error_code);
}
}

fn note_response_urls(&self, maybe_url: Option<String>, maybe_source_map_url: Option<String>) {
pub fn note_response_urls(
&self,
maybe_url: Option<String>,
maybe_source_map_url: Option<String>,
) {
unsafe {
let maybe_url = maybe_url.map(|url| CString::new(url).unwrap());
let maybe_source_map_url = maybe_source_map_url.map(|url| CString::new(url).unwrap());
@@ -965,6 +969,7 @@ unsafe extern "C" fn consume_stream(
);
return false;
}
unwrapped_source.set_stream_consumer(Some(StreamConsumer(_consumer)));
} else {
//Step 3 Upon rejection of source, return with reason.
throw_dom_exception(
32 tests/wpt/metadata/wasm/webapi/contenttype.any.js.ini
View file
Open in desktop
@@ -1,22 +1,12 @@
[contenttype.any.worker.html]
expected: TIMEOUT
[Response with Content-Type "application/wasm": compileStreaming]
expected: TIMEOUT

[Response with Content-Type "APPLICATION/wasm": compileStreaming]
expected: NOTRUN

[Response with Content-Type "APPLICATION/wasm": instantiateStreaming]
expected: NOTRUN
expected: FAIL

[Response with Content-Type "APPLICATION/WASM": instantiateStreaming]
expected: NOTRUN
expected: FAIL

[Response with Content-Type "application/wasm": instantiateStreaming]
expected: NOTRUN

[Response with Content-Type "APPLICATION/WASM": compileStreaming]
expected: NOTRUN
expected: FAIL


[contenttype.any.sharedworker.html]
@@ -26,24 +16,14 @@


[contenttype.any.html]
expected: TIMEOUT
[Response with Content-Type "application/wasm": compileStreaming]
expected: TIMEOUT

[Response with Content-Type "APPLICATION/wasm": compileStreaming]
expected: NOTRUN

[Response with Content-Type "APPLICATION/wasm": instantiateStreaming]
expected: NOTRUN
expected: FAIL

[Response with Content-Type "APPLICATION/WASM": instantiateStreaming]
expected: NOTRUN
expected: FAIL

[Response with Content-Type "application/wasm": instantiateStreaming]
expected: NOTRUN

[Response with Content-Type "APPLICATION/WASM": compileStreaming]
expected: NOTRUN
expected: FAIL


[contenttype.any.serviceworker.html]
76 tests/wpt/metadata/wasm/webapi/status.any.js.ini
View file
Open in desktop
@@ -1,49 +1,27 @@
[status.any.html]
expected: TIMEOUT
[Response with status 300: instantiateStreaming]
expected: NOTRUN
expected: FAIL

[Response with status 500: instantiateStreaming]
expected: NOTRUN

[Response with status 700: instantiateStreaming]
expected: NOTRUN

[Response with status 700: compileStreaming]
expected: NOTRUN

[Response with status 600: instantiateStreaming]
expected: NOTRUN

[Response with status 0: instantiateStreaming]
expected: NOTRUN
expected: FAIL

[Response with status 404: compileStreaming]
expected: NOTRUN
expected: FAIL

[Response with status 500: compileStreaming]
expected: NOTRUN

[Response with status 600: compileStreaming]
expected: NOTRUN
expected: FAIL

[Response with status 300: compileStreaming]
expected: TIMEOUT
expected: FAIL

[Response with status 400: compileStreaming]
expected: NOTRUN
expected: FAIL

[Response with status 400: instantiateStreaming]
expected: NOTRUN

[Response with status 999: instantiateStreaming]
expected: NOTRUN

[Response with status 999: compileStreaming]
expected: NOTRUN
expected: FAIL

[Response with status 404: instantiateStreaming]
expected: NOTRUN
expected: FAIL


[status.any.serviceworker.html]
@@ -59,49 +37,27 @@


[status.any.worker.html]
expected: TIMEOUT
[Response with status 300: instantiateStreaming]
expected: NOTRUN
expected: FAIL

[Response with status 500: instantiateStreaming]
expected: NOTRUN

[Response with status 700: instantiateStreaming]
expected: NOTRUN

[Response with status 700: compileStreaming]
expected: NOTRUN

[Response with status 600: instantiateStreaming]
expected: NOTRUN

[Response with status 0: instantiateStreaming]
expected: NOTRUN
expected: FAIL

[Response with status 404: compileStreaming]
expected: NOTRUN
expected: FAIL

[Response with status 500: compileStreaming]
expected: NOTRUN

[Response with status 600: compileStreaming]
expected: NOTRUN
expected: FAIL

[Response with status 300: compileStreaming]
expected: TIMEOUT
expected: FAIL

[Response with status 400: compileStreaming]
expected: NOTRUN
expected: FAIL

[Response with status 400: instantiateStreaming]
expected: NOTRUN

[Response with status 999: instantiateStreaming]
expected: NOTRUN

[Response with status 999: compileStreaming]
expected: NOTRUN
expected: FAIL

[Response with status 404: instantiateStreaming]
expected: NOTRUN
expected: FAIL

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.