Skip to content
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

Implement XMLHttpRequest.send(Document) #10604

Closed
wants to merge 3 commits into from
Closed
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

Next

Re-applying patch to avoid old stuff to be included

  • Loading branch information
jaysonsantos committed Sep 20, 2016
commit 5e02bee29df12e9e7c3fd19c0bcf8696d95888fc
@@ -88,6 +88,9 @@ use dom::window::{ReflowReason, Window};
use encoding::EncodingRef;
use encoding::all::UTF_8;
use euclid::point::Point2D;
use html5ever::serialize;
use html5ever::serialize::SerializeOpts;
use html5ever::serialize::TraversalScope::ChildrenOnly;
use html5ever::tree_builder::{LimitedQuirks, NoQuirks, Quirks, QuirksMode};
use ipc_channel::ipc::{self, IpcSender};
use js::jsapi::{JSContext, JSObject, JSRuntime};
@@ -1975,6 +1978,16 @@ impl Document {
ReflowQueryType::NoQuery,
ReflowReason::ElementStateChanged);
}

pub fn serialize(&self) -> Fallible<DOMString> {

This comment has been minimized.

@KiChjang

KiChjang Sep 15, 2016

Member

I don't think this method belongs here, since it's only used in XHR.send().

This comment has been minimized.

@jaysonsantos

jaysonsantos Sep 16, 2016

Author

What would you suggest? I put here because it was meant to serialize a Document.

This comment has been minimized.

@KiChjang

KiChjang Sep 16, 2016

Member

Putting it inline in xmlhttprequest.rs? There's no need for a function altogether.

let mut writer = vec![];
if let Ok(()) = serialize(&mut writer, &self.upcast::<Node>(),
SerializeOpts { traversal_scope: ChildrenOnly, ..Default::default() }) {

This comment has been minimized.

@KiChjang

KiChjang Sep 15, 2016

Member

Why ChildrenOnly?

This comment has been minimized.

@jaysonsantos

jaysonsantos Sep 16, 2016

Author

I don't quite remember, If I am not wrong, it was generating a parent tag that for the tests it shouldn't be there. But, I am having trouble running the wpt tests on linux, I tried to check on IRC if someone had a thought about it but I had no success. If I download nightly build for linux it can run all the tests and report it.
I started the discussion here there I put the log output and screenshot of the browser.
Tomorrow I will go to rustfest and I will try a bit more to fix it from there.

This comment has been minimized.

@jaysonsantos

jaysonsantos Sep 20, 2016

Author

Just answering this. If I don't use it, the serializer will break with this message.

  ▶ CRASH [expected OK] /XMLHttpRequest/send-entity-body-document.htm
  │ 
  │ Can't serialize Document node itself (thread ScriptThread PipelineId { namespace_id: PipelineNamespaceId(0), index: PipelineIndex(0) }, at /home/jayson/p/servo/components/script/parse/html.rs:244)
  │ stack backtrace:
  │    0:     0x559b20a9ed9d - backtrace::backtrace::trace::h0e60ef08c7c34e9f
  │    1:     0x559b20a9f422 - backtrace::capture::Backtrace::new::h8bf319c36d8f5d1b
  │    2:     0x559b1f1dc19d - servo::main::_{{closure}}::hd8627710ce441e59
  │    3:     0x559b20cf21e5 - std::panicking::rust_panic_with_hook::hb1322e5f2588b4db
  │    4:     0x559b1f5d54bf - std::panicking::begin_panic::h0914615a412ba184
  │    5:     0x559b1fb71014 - script::parse::html::_<impl html5ever..serialize..Serializable for &'a script..dom..node..Node>::serialize::hb19cf3d1a8128106
  │    6:     0x559b1f827c4e - html5ever::serialize::serialize::h5ca983e95371f52f
  │    7:     0x559b1f9e2530 - script::dom::document::Document::serialize::h29b3a36f0c434859
  │    8:     0x559b1fb63be0 - _<script..dom..bindings..codegen..UnionTypes..DocumentOrBodyInit as script..dom..xmlhttprequest..Extractable>::extract::h6808393b4e7b8052
  │    9:     0x559b1fb5bcd3 - _<script..dom..xmlhttprequest..XMLHttpRequest as script..dom..bindings..codegen..Bindings..XMLHttpRequestBinding..XMLHttpRequestBinding..XMLHttpRequestMethods>::Send::h337db624ae2e3ae8
  │   10:     0x559b1f5db6f3 - std::panicking::try::do_call::h0608cafe54224437
  │   11:     0x559b20cfa456 - __rust_maybe_catch_panic
  │   12:     0x559b1f999600 - script::dom::bindings::codegen::Bindings::XMLHttpRequestBinding::XMLHttpRequestBinding::send::hf8d32e9e3f3c3781
  │   13:     0x559b1fd1d184 - CallJitMethodOp
  │   14:     0x559b1f862854 - script::dom::bindings::utils::generic_call::hc239be2aefa6c7d4
  │   15:     0x559b200e2942 - 2js12CallJSNativeEP9JSContextPFbS1_jPN2JS5ValueEERKNS2_8CallArgs
  │                         at /home/jayson/p/servo/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/master/mozjs/js/src/jscntxtinlines.h:232
  │                          - 2js23InternalCallOrConstructEP9JSContextRKN2JS8CallArgsENS_14MaybeConstruct
  │                         at /home/jayson/p/servo/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/master/mozjs/js/src/vm/Interpreter.cpp:453
  │   16:     0x559b200de0f1 - 2js13CallFromStackEP9JSContextRKN2JS8CallArgs
  │                         at /home/jayson/p/servo/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/master/mozjs/js/src/vm/Interpreter.cpp:504
  │                          - Interpret
  │                         at /home/jayson/p/servo/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/master/mozjs/js/src/vm/Interpreter.cpp:2873
  │   17:     0x559b200e265f - 2js9RunScriptEP9JSContextRNS_8RunState
  │                         at /home/jayson/p/servo/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/master/mozjs/js/src/vm/Interpreter.cpp:399
  │   18:     0x559b200e2880 - 2js23InternalCallOrConstructEP9JSContextRKN2JS8CallArgsENS_14MaybeConstruct
  │                         at /home/jayson/p/servo/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/master/mozjs/js/src/vm/Interpreter.cpp:471
  │   19:     0x559b200e2b47 - 2js4CallEP9JSContextN2JS6HandleINS2_5ValueEEES5_RKNS_13AnyInvokeArgsENS2_13MutableHandleIS4_E
  │                         at /home/jayson/p/servo/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/master/mozjs/js/src/vm/Interpreter.cpp:517
  │   20:     0x559b1ffd0edc - 2js9fun_applyEP9JSContextjPN2JS5Value
  │                         at /home/jayson/p/servo/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/master/mozjs/js/src/jsfun.cpp:1325
  │   21:     0x559b200e2942 - 2js12CallJSNativeEP9JSContextPFbS1_jPN2JS5ValueEERKNS2_8CallArgs
  │                         at /home/jayson/p/servo/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/master/mozjs/js/src/jscntxtinlines.h:232
  │                          - 2js23InternalCallOrConstructEP9JSContextRKN2JS8CallArgsENS_14MaybeConstruct
  │                         at /home/jayson/p/servo/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/master/mozjs/js/src/vm/Interpreter.cpp:453
  │   22:     0x559b200de0f1 - 2js13CallFromStackEP9JSContextRKN2JS8CallArgs
  │                         at /home/jayson/p/servo/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/master/mozjs/js/src/vm/Interpreter.cpp:504
  │                          - Interpret
  │                         at /home/jayson/p/servo/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/master/mozjs/js/src/vm/Interpreter.cpp:2873
  │   23:     0x559b200e265f - 2js9RunScriptEP9JSContextRNS_8RunState
  │                         at /home/jayson/p/servo/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/master/mozjs/js/src/vm/Interpreter.cpp:399
  │   24:     0x559b200e2880 - 2js23InternalCallOrConstructEP9JSContextRKN2JS8CallArgsENS_14MaybeConstruct
  │                         at /home/jayson/p/servo/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/master/mozjs/js/src/vm/Interpreter.cpp:471
  │   25:     0x559b200e2b47 - 2js4CallEP9JSContextN2JS6HandleINS2_5ValueEEES5_RKNS_13AnyInvokeArgsENS2_13MutableHandleIS4_E
  │                         at /home/jayson/p/servo/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/master/mozjs/js/src/vm/Interpreter.cpp:517
  │   26:     0x559b1ff6650a - _Z20JS_CallFunctionValueP9JSContextN2JS6HandleIP8JSObjectEENS2_INS1_5ValueEEERKNS1_16HandleValueArrayENS1_13MutableHandleIS6_EE
  │                         at /home/jayson/p/servo/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/master/mozjs/js/src/jsapi.cpp:2781
  │   27:     0x559b1f8b8c1e - script::dom::bindings::codegen::Bindings::EventHandlerBinding::EventHandlerNonNull::Call::hfff992aab4ab8b4e
  │   28:     0x559b1f8b88c5 - script::dom::bindings::codegen::Bindings::EventHandlerBinding::EventHandlerNonNull::Call_::h86d2e00a2172acc3
  │   29:     0x559b1fa1790a - script::dom::eventdispatcher::handle_event::h16053516d9ef050b
  │   30:     0x559b1fa19c95 - script::dom::eventdispatcher::invoke::h1eae724394b1a36d
  │   31:     0x559b1fa18fef - script::dom::eventdispatcher::dispatch_event::h44cedf88bf8e65be
  │   32:     0x559b1fa1c9f2 - script::dom::eventtarget::EventTarget::fire_event::hc088fba0976b8daa
  │   33:     0x559b1fa622dc - script::dom::htmliframeelement::HTMLIFrameElement::iframe_load_event_steps::hb6a68ea29c7054a6
  │   34:     0x559b1fb8d5d2 - script::script_thread::ScriptThread::handle_msg_from_constellation::h4e39a1d4f2af6997
  │   35:     0x559b1fbd8b01 - script::script_thread::ScriptThread::handle_msgs::_{{closure}}::had1422a31c223a9c
  │   36:     0x559b1fb86de4 - script::script_thread::ScriptThread::handle_msgs::he0c72778ea94b501
  │   37:     0x559b1f631517 - std::panicking::try::do_call::h4c43a18df829bea5
  │   38:     0x559b20cfa456 - __rust_maybe_catch_panic
  │   39:     0x559b1f7943e4 - _<F as alloc..boxed..FnBox<A>>::call_box::h9939d2c8ad214242
  │   40:     0x559b20cf0820 - std::sys::thread::Thread::new::thread_start::h022e3887023c6290
  │   41:     0x7f9b8a9b36f9 - start_thread
  │   42:     0x7f9b8a4d3b5c - clone
  │   43:                0x0 - <unknown>
  │ ERROR:servo: Can't serialize Document node itself
  └ Pipeline failed in hard-fail mode.  Crashing!
Ok(DOMString::from(String::from_utf8(writer).unwrap()))
} else {
Err(Error::InvalidState)
}
}
}


@@ -53,7 +53,7 @@ interface XMLHttpRequest : XMLHttpRequestEventTarget {
attribute boolean withCredentials;
readonly attribute XMLHttpRequestUpload upload;
[Throws]
void send(optional /*Document or*/ BodyInit? data = null);
void send(optional (Document or Blob or DOMString or URLSearchParams)? data = null);

This comment has been minimized.

@KiChjang

KiChjang Sep 15, 2016

Member

This is not necessary anymore. Using Document or BodyInit should work now.

void abort();

// response
@@ -5,12 +5,13 @@
use document_loader::DocumentLoader;
use dom::bindings::cell::DOMRefCell;
use dom::bindings::codegen::Bindings::BlobBinding::BlobMethods;
use dom::bindings::codegen::Bindings::DocumentBinding::DocumentMethods;
use dom::bindings::codegen::Bindings::EventHandlerBinding::EventHandlerNonNull;
use dom::bindings::codegen::Bindings::WindowBinding::WindowMethods;
use dom::bindings::codegen::Bindings::XMLHttpRequestBinding;
use dom::bindings::codegen::Bindings::XMLHttpRequestBinding::BodyInit;
use dom::bindings::codegen::Bindings::XMLHttpRequestBinding::XMLHttpRequestMethods;
use dom::bindings::codegen::Bindings::XMLHttpRequestBinding::XMLHttpRequestResponseType;
use dom::bindings::codegen::UnionTypes::BodyInit;
use dom::bindings::conversions::ToJSValConvertible;
use dom::bindings::error::{Error, ErrorResult, Fallible};
use dom::bindings::global::{GlobalRef, GlobalRoot};
@@ -35,6 +36,7 @@ use encoding::label::encoding_from_whatwg_label;
use encoding::types::{DecoderTrap, EncoderTrap, Encoding, EncodingRef};
use euclid::length::Length;
use hyper::header::{ContentLength, ContentType};
use html5ever::serialize::Serializable;
use hyper::header::Headers;
use hyper::method::Method;
use hyper::mime::{self, Attr as MimeAttr, Mime, Value as MimeValue};
@@ -1354,6 +1356,7 @@ impl XHRTimeoutCallback {
trait Extractable {
fn extract(&self) -> (Vec<u8>, Option<DOMString>);
}

impl Extractable for BodyInit {
// https://fetch.spec.whatwg.org/#concept-bodyinit-extract
fn extract(&self) -> (Vec<u8>, Option<DOMString>) {
@@ -1382,6 +1385,29 @@ impl Extractable for BodyInit {
let bytes = encode_multipart_form_data(&mut formdata.datums(), boundary.clone(),
UTF_8 as EncodingRef);
(bytes, Some(DOMString::from(format!("multipart/form-data;boundary={}", boundary))))
(data.get_bytes().to_vec(), content_type)

This comment has been minimized.

@KiChjang

KiChjang Sep 15, 2016

Member

Looks like a rebasing error.

},
BodyInit::Document(ref d) => {

This comment has been minimized.

@KiChjang

KiChjang Sep 15, 2016

Member

I'm surprised if this code even compiled at all.

This comment has been minimized.

@jaysonsantos

jaysonsantos Sep 16, 2016

Author

It didn't, I did the rebase and pushed because I would change from one computer to another.

This comment has been minimized.

@jaysonsantos

jaysonsantos Sep 16, 2016

Author

I've managed to make it compilable, if you want I can push the wip.

let data: Vec<u8> = d.serialize().unwrap().into();
let decoded_data: Vec<u8> = match &*d.CharacterSet() {
"UTF-8" => {
debug!("Document is already utf-8, skipping conversion {:?}", d.url());
data
},
document_charset => {
debug!("Document is {:?} we have to decode", document_charset);
let charset = encoding_from_whatwg_label(&*d.CharacterSet()).unwrap_or(UTF_8);
charset.decode(&*data, DecoderTrap::Replace).unwrap().into_bytes()
}
};
let mut content_type = String::new();
if d.is_html_document() {
content_type.push_str("text/html");
} else {
content_type.push_str("application/xml");
};
content_type.push_str(";charset=UTF-8");
(decoded_data, Some(DOMString::from(content_type)))
}
}
}
@@ -2,22 +2,6 @@
type: testharness
[XML document, windows-1252]
expected: FAIL

[HTML document, invalid UTF-8]
expected: FAIL

[HTML document, shift-jis]
expected: FAIL

[plain text file]
expected: FAIL

[image file]
expected: FAIL

[img tag]
expected: FAIL

[empty div]
expected: FAIL

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