Added Async HTML Tokenizer #17037

cynicaldevil · 2017-05-25T15:05:34Z

Design: I realized having two different parsers for sync and async was wrong, because the API for both was fundamentally the same. All I needed to do was create another Tokenizer, because ParseNode ( representation for nodes which are yet to be created) is used by the TreeBuilder and the Sink, and the Tokenizer is the 'lowermost' type concerned with these two types.

Therefore, I created one and placed it in async_html.rs, and also created a new Sink which deals with ParseNodes. I changed the methods in ServoParser to take an async argument too, which decides which Tokenizer will be used. The Tokenizer isn't exactly async for now, but this PR separates action creation from execution, which allows the async behaviour to be implemented later. Right now, all actions are executed soon after they are created.

The Sink consists of two Hashmaps, nodes, which contains the actual nodes, with the key being their corresponding ParseNode's id, and parse_node_data, which contains metadata about the nodes.

It's still a bit rough, (I can't figure out how to deal with complete_script and is_mathml_annotation_xml_integration_point, along with some other parts I wrote in a hurry), but I believe the overall design is sound. I'd like to hear what you think about it.

This change is

highfive · 2017-05-25T15:05:38Z

Heads up! This PR modifies the following files:

@fitzgen: components/script/dom/document.rs, components/script/dom/servoparser/async_html.rs, components/script/dom/element.rs, components/script/dom/xmlhttprequest.rs, components/script/dom/servoparser/mod.rs and 2 more
@KiChjang: components/script/dom/document.rs, components/script/dom/servoparser/async_html.rs, components/script/dom/element.rs, components/script/dom/xmlhttprequest.rs, components/script/dom/servoparser/mod.rs and 2 more

highfive · 2017-05-25T15:05:40Z

Warning

These commits modify unsafe code. Please review it carefully!
These commits modify script code, but no tests are modified. Please consider adding a test!

cynicaldevil · 2017-05-25T15:05:59Z

cc @jdm @nox

bors-servo · 2017-05-27T10:41:23Z

☔ The latest upstream changes (presumably #16989) made this pull request unmergeable. Please resolve the merge conflicts.

nox

Initial partial review.

nox · 2017-05-31T08:39:22Z

components/script/dom/servoparser/mod.rs

@@ -91,6 +92,7 @@ pub struct ServoParser {
    aborted: Cell<bool>,
    /// https://html.spec.whatwg.org/multipage/#script-created-parser
    script_created_parser: bool,
+    async: bool,


You don't need this, the provided tokeniser already lets you know whether it's async or not, please remove.

nox · 2017-05-31T08:39:22Z

components/script/dom/servoparser/mod.rs

@@ -54,6 +54,7 @@ use std::mem;
 use style::context::QuirksMode as ServoQuirksMode;

 mod html;
+mod async_html;


Nit: alphabetic order.

nox · 2017-05-31T08:39:22Z

components/script/dom/element.rs

@@ -1364,7 +1364,7 @@ impl Element {
        // Steps 1-2.
        let context_document = document_from_node(self);
        // TODO(#11995): XML case.
-        let new_children = ServoParser::parse_html_fragment(self, markup);
+        let new_children = ServoParser::parse_html_fragment(self, markup, true);


Why async? The only input here comes from document.write.

nox · 2017-05-31T08:39:22Z

components/script/dom/servoparser/async_html.rs

+}
+
+#[allow(unsafe_code)]
+unsafe impl JSTraceable for HtmlTokenizer<TreeBuilder<ParseNode, Sink>> {


This impl can be shared with the sync tokeniser.

Forgot about this one, trying it now.

This is not possible, I tried to make the outer impl generic, but it seems that the inner impl (the one inside trace) cannot pick up the generic type.

nox · 2017-05-31T08:39:22Z

components/script/dom/servoparser/async_html.rs

+    id: usize,
+}
+
+enum ParserNodeOrText {


Why do you need this new enum?

nox · 2017-05-31T08:39:22Z

components/script/dom/servoparser/async_html.rs

+                node_data.data = Some(String::from(data));
+                self.enqueue(ParseOperation::CreatePI(node.id));
+            },
+            None => panic!("Element was just created using new_parse_node() and should exist!"),


What is this for?

nox · 2017-05-31T08:39:22Z

components/script/dom/servoparser/async_html.rs

+    }
+
+    fn has_parent_node(&self, node: &Self::Handle) -> bool {
+        match self.parse_node_data.borrow().get(&node.id) {


Use expect on that, instead of matching explicitly.

nox · 2017-05-31T08:39:22Z

components/script/dom/servoparser/mod.rs

        parser.parse_chunk(String::from(input));
    }

    // https://html.spec.whatwg.org/multipage/#parsing-html-fragments
-    pub fn parse_html_fragment(context: &Element, input: DOMString) -> FragmentParsingResult {
+    pub fn parse_html_fragment(context: &Element, input: DOMString, async: bool) -> FragmentParsingResult {


Why would you parse a fragment in async mode?

nox · 2017-05-31T08:39:22Z

components/script/dom/servoparser/mod.rs

@@ -157,11 +171,18 @@ impl ServoParser {
        }
    }

-    pub fn parse_html_script_input(document: &Document, url: ServoUrl, type_: &str) {
+    pub fn parse_html_script_input(document: &Document, url: ServoUrl, type_: &str, async: bool) {


Why would you parse script input in async mode?

nox · 2017-05-31T08:39:22Z

components/script/dom/servoparser/mod.rs

-    pub fn parse_html_document(document: &Document, input: DOMString, url: ServoUrl) {
-        let parser = ServoParser::new(document,
-                                      Tokenizer::Html(self::html::Tokenizer::new(document, url, None)),
+    pub fn parse_html_document(document: &Document, input: DOMString, url: ServoUrl, async: bool) {


No boolean argument for that please, and I think there is no reason an HTML document should be parsed in a sync way.

cynicaldevil

@nox Right! I changed most of the things you requested, and got the wikipedia main page and my github profile page (also a few pages I selected from tests) to run successfully at last! I'm still not sure how to implement a couple of methods from TreeSink, though.

cynicaldevil · 2017-06-01T20:49:09Z

components/script/dom/servoparser/async_html.rs

+        let mut parse_node_data = self.parse_node_data.borrow_mut();
+        let mut data = parse_node_data.get_mut(&target.id).expect("Expected data of parse node");
+        let contents = match data.contents {
+            Some(ref node) => node.clone(),


IIUC, we can safely clone the ParseNode to be returned from this function. I am not able to see the problem with it, but I would like your advice on this one.

cynicaldevil · 2017-06-01T20:49:09Z

components/script/dom/servoparser/async_html.rs

+#[derive(JSTraceable, Clone, HeapSizeOf)]
+pub struct ParseNode {
+    id: usize,
+    qual_name: Option<QualName>,


I decided to move this field here from ParseNodeData, because that was the only way I could get elem_name() to work, given its lifetime constraints.

cynicaldevil · 2017-06-01T20:49:09Z

components/script/dom/servoparser/async_html.rs

+    }
+
+    // TODO: Handle this
+    fn complete_script(&mut self, node: &Self::Handle) -> NextParserState {


I'm still not exactly sure how to handle this.

cynicaldevil · 2017-06-01T20:49:09Z

components/script/dom/servoparser/async_html.rs

+
+    /// https://html.spec.whatwg.org/multipage/#html-integration-point
+    /// Specifically, the <annotation-xml> cases.
+    fn is_mathml_annotation_xml_integration_point(&self, handle: &Self::Handle) -> bool {


This one too.

Fixed this one today.

cynicaldevil · 2017-06-04T07:14:27Z

@bors-servo try

bors-servo · 2017-06-04T07:14:33Z

⌛ Trying commit 438f421 with merge f3ad8d8...


        Auto merge of #17037 - cynicaldevil:impl-Sink, r=<try>

[WIP] Added Async HTML Tokenizer Design: I realized having two different parsers for sync and async was wrong, because the API for both was fundamentally the same. All I needed to do was create another Tokenizer, because `ParseNode` ( representation for nodes which are yet to be created) is used by the TreeBuilder and the Sink, and the `Tokenizer` is the 'lowermost' type concerned with these two types. Therefore, I created one and placed it in `async_html.rs`, and also created a new Sink which deals with `ParseNode`s. I changed the methods in ServoParser to take an `async` argument too, which decides which Tokenizer will be used. The Tokenizer isn't exactly *async* for now, but this PR separates action creation from execution, which allows the async behaviour to be implemented later. Right now, all actions are executed soon after they are created. The Sink consists of two Hashmaps, `nodes`, which contains the actual nodes, with the key being their corresponding `ParseNode`'s id, and `parse_node_data`, which contains metadata about the nodes. It's still a bit rough, (I can't figure out how to deal with `complete_script` and `is_mathml_annotation_xml_integration_point`, along with some other parts I wrote in a hurry), but I believe the overall design is sound. I'd like to hear what you think about it.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17037)

bors-servo · 2017-06-04T07:44:08Z

💔 Test failed - linux-rel-wpt

jdm · 2017-06-05T16:19:01Z

components/script/dom/servoparser/async_html.rs

+    }
+
+    fn set_current_line(&mut self, line_number: u64) {
+        self.current_line = line_number;


This won't work as written - the code that creates elements relies on the line number that's stored, and it will be incorrect when elements are created after the synchronous section of the tokenizer.

This stills stands, right @cynicaldevil?

I misread the changes. Currently this works fine, but when we stop calling process_operation immediately after creating parse nodes it will no longer work.

cynicaldevil · 2017-06-07T11:41:36Z

@bors-servo try

bors-servo · 2017-06-07T11:41:39Z

⌛ Trying commit 24093e9 with merge 5ffd5c1...


        Auto merge of #17037 - cynicaldevil:impl-Sink, r=<try>

[WIP] Added Async HTML Tokenizer Design: I realized having two different parsers for sync and async was wrong, because the API for both was fundamentally the same. All I needed to do was create another Tokenizer, because `ParseNode` ( representation for nodes which are yet to be created) is used by the TreeBuilder and the Sink, and the `Tokenizer` is the 'lowermost' type concerned with these two types. Therefore, I created one and placed it in `async_html.rs`, and also created a new Sink which deals with `ParseNode`s. I changed the methods in ServoParser to take an `async` argument too, which decides which Tokenizer will be used. The Tokenizer isn't exactly *async* for now, but this PR separates action creation from execution, which allows the async behaviour to be implemented later. Right now, all actions are executed soon after they are created. The Sink consists of two Hashmaps, `nodes`, which contains the actual nodes, with the key being their corresponding `ParseNode`'s id, and `parse_node_data`, which contains metadata about the nodes. It's still a bit rough, (I can't figure out how to deal with `complete_script` and `is_mathml_annotation_xml_integration_point`, along with some other parts I wrote in a hurry), but I believe the overall design is sound. I'd like to hear what you think about it.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17037)

bors-servo · 2017-06-07T12:24:32Z

💔 Test failed - linux-rel-wpt

cynicaldevil · 2017-06-07T14:42:56Z

@bors-servo try

bors-servo · 2017-06-07T14:43:00Z

⌛ Trying commit 725965f with merge 3598f78...


        Auto merge of #17037 - cynicaldevil:impl-Sink, r=<try>

Added Async HTML Tokenizer Design: I realized having two different parsers for sync and async was wrong, because the API for both was fundamentally the same. All I needed to do was create another Tokenizer, because `ParseNode` ( representation for nodes which are yet to be created) is used by the TreeBuilder and the Sink, and the `Tokenizer` is the 'lowermost' type concerned with these two types. Therefore, I created one and placed it in `async_html.rs`, and also created a new Sink which deals with `ParseNode`s. I changed the methods in ServoParser to take an `async` argument too, which decides which Tokenizer will be used. The Tokenizer isn't exactly *async* for now, but this PR separates action creation from execution, which allows the async behaviour to be implemented later. Right now, all actions are executed soon after they are created. The Sink consists of two Hashmaps, `nodes`, which contains the actual nodes, with the key being their corresponding `ParseNode`'s id, and `parse_node_data`, which contains metadata about the nodes. It's still a bit rough, (I can't figure out how to deal with `complete_script` and `is_mathml_annotation_xml_integration_point`, along with some other parts I wrote in a hurry), but I believe the overall design is sound. I'd like to hear what you think about it.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17037)

cynicaldevil · 2017-06-08T09:59:04Z

@nox I think you can review it in-depth now, it's mostly complete aside from a few warnings which I'll remove later. I am still unsure of what to do with complete_script, though.

nox · 2017-06-13T12:07:43Z

components/script/dom/servoparser/async_html.rs

+            ParseOperation::Insert(parent, reference_child, new_node) => {
+                let nodes = self.nodes.borrow();
+                let parent = nodes.get(&parent).unwrap();
+                let reference_child = reference_child.map(|n| nodes.get(&n).unwrap().deref());


Why the explicit deref call?

reference_child.map(|n| nodes.get(&n))

nox

You are doing great!

nox · 2017-06-13T12:42:37Z

components/script/dom/servoparser/async_html.rs

+}
+
+enum ParseOperation {
+    GetTemplateContents(usize, usize),


Please use named fields or document what each usize is. Named fields would be better of course.

nox · 2017-06-13T12:42:37Z

components/script/dom/servoparser/async_html.rs

+
+    fn new_parse_node(&self) -> ParseNode {
+        let id = self.next_parse_node_id.get();
+        let data: ParseNodeData = Default::default();


Nit: that could be ParseNodeData::default().

nox · 2017-06-13T12:42:37Z

components/script/dom/servoparser/async_html.rs

+    document: JS<Document>,
+    current_line: u64,
+    script: MutNullableJS<HTMLScriptElement>,
+    parse_node_data: DOMRefCell<HashMap<usize, ParseNodeData>>,


Why does this need to be in a DOMRefCell given all TreeSink methods take a &mut self?

This is a tricky one, parse_node_data needs to be DOMRefCell because I want to use it mutably in the same_tree method, where I update the tree-related info based on the data I get from the updated Nodes (same tree borrows self immutably).

The nodes field needs it because it is used in process_operation, which takes &self. If process_operation begins taking &mut self, it would cause problems with the TreeSink methods, as many methods already immutably borrow parse_node_data there, when they call process_operation. Therefore, the process_operation must borrow immutably too, to satisfy the borrow checker.

However, I might be able to make nodes a normal Hashmap in the next PR, because it will exist in main thread altogether, and therefore there will be no conflicts with Sink then.

nox · 2017-06-13T12:42:37Z

components/script/dom/servoparser/async_html.rs

+        }
+    }
+
+    fn enqueue(&self, op: ParseOperation) {


I think you can remove this method or process_operation. :)

nox · 2017-06-13T12:42:37Z

components/script/dom/servoparser/async_html.rs

+    }
+
+    fn process_operation(&self, op: ParseOperation) {
+        let document = Root::from_ref(&**self.nodes.borrow().get(&0).expect("document node should exist!"));


I wonder if the data for the document node shouldn't live outside the nodes map.

nox · 2017-06-13T12:42:37Z

components/script/dom/servoparser/async_html.rs

+        let mut node_data = parse_node_data.get_mut(&node.id).expect("Element does not exist!");
+        node_data.is_integration_point = attrs.iter()
+        .any(|attr| {
+            let attr_value = &String::from(attr.value.clone());


Why is this needed?

I found that I could compute this value once and store it when I'm creating a new element (the value's needed in is_mathml_annotation_xml_integration_point). But now that I think about it, this could be a problem if the element's attributes related to this value are changed at any point of time after the element's created. Do you think I should change it to be synchronous ? (i.e, get the actual Node from the nodes map and compute the value from it).

nox · 2017-06-13T12:42:37Z

components/script/dom/servoparser/async_html.rs

+
+    fn create_comment(&mut self, text: StrTendril) -> Self::Handle {
+        let node = self.new_parse_node();
+        self.enqueue(ParseOperation::CreateComment(String::from(text), node.id));


Why aren't we just storing the StrTendril value here?

nox · 2017-06-13T12:42:37Z

components/script/dom/servoparser/async_html.rs

+            QuirksMode::Quirks => ServoQuirksMode::Quirks,
+            QuirksMode::LimitedQuirks => ServoQuirksMode::LimitedQuirks,
+            QuirksMode::NoQuirks => ServoQuirksMode::NoQuirks,
+        };


Why do we have 2 quirks mode enum again?

nox · 2017-06-13T12:42:37Z

components/script/dom/servoparser/async_html.rs

+    }
+
+    fn complete_script(&mut self, _: &Self::Handle) -> NextParserState {
+        panic!("complete_script should not be called here!");


Please file a follow-up about killing this altogether.

nox · 2017-06-13T12:42:37Z

components/script/dom/servoparser/async_html.rs

+    }
+
+    fn set_current_line(&mut self, line_number: u64) {
+        self.current_line = line_number;


This stills stands, right @cynicaldevil?

cynicaldevil · 2017-06-14T17:08:39Z

components/script/dom/servoparser/async_html.rs

+
+    fn create_pi(&mut self, target: StrTendril, data: StrTendril) -> ParseNode {
+        let node = self.new_parse_node();
+        {


Quite a few of these scopes appear in the TreeSink methods. It's needed because self needs to be borrowed for parse_node_data operations, and then in the end needs to be borrowed mutably for process_operation.

nox · 2017-06-16T20:46:51Z

@bors-servo try

This is looking good.

@jdm Do we want to make the use of the async tokenizer settable through some config?

bors-servo · 2017-06-16T20:50:02Z

⌛ Trying commit 1361216 with merge 212b0b2...


        Auto merge of #17037 - cynicaldevil:impl-Sink, r=<try>

Added Async HTML Tokenizer Design: I realized having two different parsers for sync and async was wrong, because the API for both was fundamentally the same. All I needed to do was create another Tokenizer, because `ParseNode` ( representation for nodes which are yet to be created) is used by the TreeBuilder and the Sink, and the `Tokenizer` is the 'lowermost' type concerned with these two types. Therefore, I created one and placed it in `async_html.rs`, and also created a new Sink which deals with `ParseNode`s. I changed the methods in ServoParser to take an `async` argument too, which decides which Tokenizer will be used. The Tokenizer isn't exactly *async* for now, but this PR separates action creation from execution, which allows the async behaviour to be implemented later. Right now, all actions are executed soon after they are created. The Sink consists of two Hashmaps, `nodes`, which contains the actual nodes, with the key being their corresponding `ParseNode`'s id, and `parse_node_data`, which contains metadata about the nodes. It's still a bit rough, (I can't figure out how to deal with `complete_script` and `is_mathml_annotation_xml_integration_point`, along with some other parts I wrote in a hurry), but I believe the overall design is sound. I'd like to hear what you think about it.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17037)

jdm · 2017-06-16T21:50:42Z

A preference probably makes the most sense.

cynicaldevil · 2017-06-16T22:34:08Z

How should I implement it?

bors-servo · 2017-06-16T22:39:01Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
State: approved= try=True

jdm · 2017-06-16T22:55:34Z

Probably check the pref in parse_html_document and disregard Async if the preference is disabled.


        Added Async HTML Tokenizer


        Added preference to enable/disable async html tokenizer

cynicaldevil · 2017-06-17T08:11:41Z

Added preference.

nox · 2017-06-17T09:21:17Z

@bors-servo r+

bors-servo · 2017-06-17T09:21:18Z

📌 Commit ad649bb has been approved by nox

bors-servo · 2017-06-17T09:21:22Z

⌛ Testing commit ad649bb with merge 9c2dffd...


        Auto merge of #17037 - cynicaldevil:impl-Sink, r=nox

Added Async HTML Tokenizer Design: I realized having two different parsers for sync and async was wrong, because the API for both was fundamentally the same. All I needed to do was create another Tokenizer, because `ParseNode` ( representation for nodes which are yet to be created) is used by the TreeBuilder and the Sink, and the `Tokenizer` is the 'lowermost' type concerned with these two types. Therefore, I created one and placed it in `async_html.rs`, and also created a new Sink which deals with `ParseNode`s. I changed the methods in ServoParser to take an `async` argument too, which decides which Tokenizer will be used. The Tokenizer isn't exactly *async* for now, but this PR separates action creation from execution, which allows the async behaviour to be implemented later. Right now, all actions are executed soon after they are created. The Sink consists of two Hashmaps, `nodes`, which contains the actual nodes, with the key being their corresponding `ParseNode`'s id, and `parse_node_data`, which contains metadata about the nodes. It's still a bit rough, (I can't figure out how to deal with `complete_script` and `is_mathml_annotation_xml_integration_point`, along with some other parts I wrote in a hurry), but I believe the overall design is sound. I'd like to hear what you think about it.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17037)

bors-servo · 2017-06-17T10:20:20Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
Approved by: nox
Pushing 9c2dffd to master...

highfive assigned nox May 25, 2017

highfive added the S-awaiting-review label May 25, 2017

highfive added the S-needs-rebase label May 27, 2017

nox requested changes May 31, 2017

View changes

cynicaldevil force-pushed the cynicaldevil:impl-Sink branch 2 times, most recently from 0dee74f to 22627fa Jun 1, 2017

cynicaldevil reviewed Jun 1, 2017

View changes

nox removed the S-needs-rebase label Jun 2, 2017

highfive added the S-tests-failed label Jun 4, 2017

jdm reviewed Jun 5, 2017

View changes

cynicaldevil force-pushed the cynicaldevil:impl-Sink branch from 438f421 to 24093e9 Jun 7, 2017

highfive removed the S-tests-failed label Jun 7, 2017

highfive added the S-tests-failed label Jun 7, 2017

cynicaldevil changed the title ~~[WIP] Added Async HTML Tokenizer~~ Added Async HTML Tokenizer Jun 7, 2017

highfive removed the S-tests-failed label Jun 7, 2017

cynicaldevil force-pushed the cynicaldevil:impl-Sink branch from 8af96fb to 8775737 Jun 12, 2017

nox reviewed Jun 13, 2017

View changes

nox requested changes Jun 13, 2017

View changes

cynicaldevil force-pushed the cynicaldevil:impl-Sink branch from 8775737 to 79ffa13 Jun 14, 2017

cynicaldevil reviewed Jun 14, 2017

View changes

cynicaldevil force-pushed the cynicaldevil:impl-Sink branch 2 times, most recently from db900e8 to 1361216 Jun 15, 2017

cynicaldevil added 2 commits May 21, 2017

Added Async HTML Tokenizer

161ff15

Added preference to enable/disable async html tokenizer

Loading status checks…

ad649bb

cynicaldevil force-pushed the cynicaldevil:impl-Sink branch from 1361216 to ad649bb Jun 17, 2017

nox approved these changes Jun 17, 2017

View changes

highfive added S-awaiting-merge and removed S-awaiting-review labels Jun 17, 2017

bors-servo merged commit ad649bb into servo:master Jun 17, 2017
3 checks passed

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

continuous-integration/travis-ci/pr The Travis CI build passed
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Jun 17, 2017

cynicaldevil deleted the cynicaldevil:impl-Sink branch Jun 17, 2017

servo / servo

Join GitHub today

Added Async HTML Tokenizer #17037

Added Async HTML Tokenizer #17037

Conversation

cynicaldevil commented May 25, 2017 • edited

highfive commented May 25, 2017

highfive commented May 25, 2017

cynicaldevil commented May 25, 2017

bors-servo commented May 27, 2017

nox left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

cynicaldevil left a comment

This comment has been minimized.

cynicaldevil Jun 1, 2017 • edited

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

cynicaldevil commented Jun 4, 2017

bors-servo commented Jun 4, 2017

bors-servo commented Jun 4, 2017

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

cynicaldevil commented Jun 7, 2017

bors-servo commented Jun 7, 2017

bors-servo commented Jun 7, 2017

cynicaldevil commented Jun 7, 2017

bors-servo commented Jun 7, 2017

cynicaldevil commented Jun 8, 2017 • edited

This comment has been minimized.

nox Jun 13, 2017 • edited

nox left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

nox commented Jun 16, 2017

bors-servo commented Jun 16, 2017

jdm commented Jun 16, 2017

cynicaldevil commented Jun 16, 2017

bors-servo commented Jun 16, 2017

jdm commented Jun 16, 2017

cynicaldevil commented Jun 17, 2017

nox commented Jun 17, 2017

bors-servo commented Jun 17, 2017

bors-servo commented Jun 17, 2017

bors-servo commented Jun 17, 2017

cynicaldevil commented May 25, 2017 •

edited

cynicaldevil Jun 1, 2017 •

edited

cynicaldevil commented Jun 8, 2017 •

edited

nox Jun 13, 2017 •

edited