Implemented paint worklets invoking worklet scripts. #17239

asajeffrey · 2017-06-08T17:42:21Z

Implemented the "invoke a paint callback" functionality of paint worklets (https://drafts.css-houdini.org/css-paint-api/#invoke-a-paint-callback).

This PR does not implement the 2D rendering context, and just generates a placeholder image.

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes do not require tests because we can't write reftests until we have 2D rendering contexts implemented.

This change is

highfive · 2017-06-08T17:42:26Z

Heads up! This PR modifies the following files:

@fitzgen: components/script/dom/paintrenderingcontext2d.rs, components/script/dom/webidls/PaintWorkletGlobalScope.webidl, components/script/dom/webidls/PaintRenderingContext2D.webidl, components/script/dom/bindings/conversions.rs, components/script/dom/mod.rs and 4 more
@KiChjang: components/script/dom/paintrenderingcontext2d.rs, components/script/dom/webidls/PaintWorkletGlobalScope.webidl, components/script/dom/webidls/PaintRenderingContext2D.webidl, components/script/dom/bindings/conversions.rs, components/script/dom/mod.rs and 4 more

highfive · 2017-06-08T17:42:27Z

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!

asajeffrey · 2017-06-08T17:42:58Z

cc @jdm @metajack @tschneidereit

cbrewster · 2017-06-09T05:30:38Z

components/script/dom/paintworkletglobalscope.rs

+        debug!("Got {:?}.", alpha);
+
+        // Step 14
+        if unsafe { !IsConstructor(paint_obj.get()) } {


Is paint_ctor allowed to be an arrow function? If not, you may need to unwrap paint_obj first before using IsConstructor.

Step 14 of https://drafts.css-houdini.org/css-paint-api/#dom-paintworkletglobalscope-registerpaint implies no, since IsConstructor of an arrow function is false?

@asajeffrey IsConstructor returns true for any wrapper that is callable which includes wrapped arrow functions. If you use UnwrapObject and then use IsConstructor, it will return false for the arrow function.

I tried it, and ran:

registerPaint("foo", (x => "arrow fn"));

and got:

$ ./mach run -d ~/tmp/test.html ERROR:script::dom::bindings::error: Error at :5:1 Not a constructor. ERROR:script::dom::bindings::error: Error at :5:1 Not a constructor. ERROR:script::dom::bindings::error: Error at :5:1 Not a constructor. ...

So it looks like it's doing the right thing? (Which reminds me, this needs some test cases...)

Looks like the right thing, quite odd since IsConstructor always returned true for an arrow fn unless I unwrapped it. Maybe it has to do with Function vs VoidFunction

asajeffrey · 2017-06-14T20:49:07Z

r? @jdm

jdm

Generally straightforward implementation of the spec. Unfortunately our unsafe SpiderMonkey bindings keep getting in the way.

jdm · 2017-06-16T15:39:45Z

components/script/dom/paintworkletglobalscope.rs

+    /// https://drafts.css-houdini.org/css-paint-api/#paint-definitions
+    paint_definitions: DOMRefCell<HashMap<Atom, PaintDefinition>>,
+    /// https://drafts.css-houdini.org/css-paint-api/#paint-class-instances
+    paint_class_instances: DOMRefCell<HashMap<Atom, Heap<JSVal>>>,


We will need Box<Heap<JSVal>> here.

jdm · 2017-06-16T15:39:45Z

components/script/dom/paintworkletglobalscope.rs

+/// A paint definition
+/// https://drafts.css-houdini.org/css-paint-api/#paint-definition
+#[derive(Clone, JSTraceable, HeapSizeOf)]
+struct PaintDefinition {


Mark this as #[must_root].

jdm · 2017-06-16T15:39:45Z

components/script/dom/paintworkletglobalscope.rs

+    class_constructor: Heap<JSVal>,
+    paint_function: Heap<JSVal>,
+    #[ignore_heap_size_of = "Rc"]
+    constructor_valid_flag: Rc<Cell<bool>>,


It's not clear to me why this is inside an Rc.

It was because I was cloning the definition. I'm not now, so the Rc is gone.

jdm · 2017-06-16T15:39:45Z

components/script/dom/paintworkletglobalscope.rs

+        debug!("Getting alpha.");
+        let alpha: bool =
+            unsafe { get_property(cx, paint_obj.handle(), "alpha", ()) }?
+            .unwrap_or_default();


This looks like alpha will be false if the alpha property is undefined.

Good point. Fixed.

jdm · 2017-06-16T15:39:45Z

components/script/dom/paintworkletglobalscope.rs

+        // Step 19.
+        let definition = PaintDefinition {
+            class_constructor: Heap::new(ObjectValue(paint_obj.get())),
+            paint_function: Heap::new(paint_function),


Heap::new is dangerous. Please use Heap::default and box the heap values.

jdm · 2017-06-16T15:39:45Z

components/script/dom/paintworkletglobalscope.rs

+        // TODO: Steps 1-2.1.
+        // Step 3.
+        let definition = match self.paint_definitions.borrow().get(&name) {
+            Some(defn) => defn.clone(),


This is scary when a PaintDefinition contains Heap values. We need to find something else to do here.

OK, I rewrote this to copy out the values needed (into rooted locations) rather than clone the paint definition.

jdm · 2017-06-16T15:39:45Z

components/script/dom/paintworkletglobalscope.rs

+                    return;
+                }
+                // Step 5.4
+                entry.insert(Heap::new(paint_instance.get()));


We'll need Heap::default.

jdm · 2017-06-16T15:39:45Z

components/script/dom/paintworkletglobalscope.rs

+        // TODO: Step 10
+        // Steps 11-12
+        debug!("Invoking paint function {}.", name);
+        let args = unsafe { HandleValueArray::from_rooted_slice(&[


Unfortunately from_rooted_slice is easy to misuse. Please create a RootedVec (rooted_vec!) containing these values and pass its slice as the argument here.

Well I tried, but I hit:

--> /home/ajeffrey/github/asajeffrey/servo/components/script/dom/macros.rs:549:25 | 549 | let mut $name = $crate::dom::bindings::trace::RootedVec::new(&mut root); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `dom::bindings::trace::JSTraceable` is not implemented for `js::jsapi::Value` | ::: /home/ajeffrey/github/asajeffrey/servo/components/script/dom/paintworkletglobalscope.rs | 171 | rooted_vec!(let mut args); | -------------------------- in this macro invocation | = note: required by `<dom::bindings::trace::RootedVec<'a, T>>::new`

The problem being that I need to construct a RootedVec<JSVal>.

There's a similar type error with rooted!(in(cx) let args = ....):

error[E0277]: the trait bound `js::jsapi::HandleValueArray: js::rust::RootKind` is not satisfied --> /home/ajeffrey/github/asajeffrey/servo/components/script/dom/paintworkletglobalscope.rs:171:9 | 171 | / rooted!(in(cx) let args = unsafe { HandleValueArray::from_rooted_slice(&[ 172 | | ObjectValue(rendering_context.reflector().get_jsobject().get()), 173 | | ObjectValue(paint_size.reflector().get_jsobject().get()), 174 | | ]) }); | |______________^ the trait `js::rust::RootKind` is not implemented for `js::jsapi::HandleValueArray` | = note: required by `<js::rust::RootedGuard<'a, T>>::new` = note: this error originates in a macro outside of the current crate

I'm not sure this is needed anyway, all the objects in the array are already rooted, the only thing is the array itself, but I believe Call manages the lifetime of the arguments object?

HandleValueArray is a type meaning "this is a pointer to an array of values that are rooted." This is literally storing a pointer to an array that goes out of scope immediately after returning.

I am ok with the current code as long as we store the slice in a binding that outlives the call to from_rooted_slice, otherwise we are storing a pointer to a temporary stack value that could be overwritten before we actually reach Call.

asajeffrey

OK, I fought the JS bindings, and I think it's in better shape now!

asajeffrey · 2017-06-16T20:34:33Z

components/script/dom/paintworkletglobalscope.rs

+    /// https://drafts.css-houdini.org/css-paint-api/#paint-definitions
+    paint_definitions: DOMRefCell<HashMap<Atom, PaintDefinition>>,
+    /// https://drafts.css-houdini.org/css-paint-api/#paint-class-instances
+    paint_class_instances: DOMRefCell<HashMap<Atom, Heap<JSVal>>>,


asajeffrey · 2017-06-16T20:34:33Z

components/script/dom/paintworkletglobalscope.rs

+        // TODO: Steps 1-2.1.
+        // Step 3.
+        let definition = match self.paint_definitions.borrow().get(&name) {
+            Some(defn) => defn.clone(),


OK, I rewrote this to copy out the values needed (into rooted locations) rather than clone the paint definition.

asajeffrey · 2017-06-16T20:34:33Z

components/script/dom/paintworkletglobalscope.rs

+                    return;
+                }
+                // Step 5.4
+                entry.insert(Heap::new(paint_instance.get()));


asajeffrey · 2017-06-16T20:34:33Z

components/script/dom/paintworkletglobalscope.rs

+        // TODO: Step 10
+        // Steps 11-12
+        debug!("Invoking paint function {}.", name);
+        let args = unsafe { HandleValueArray::from_rooted_slice(&[


Well I tried, but I hit:

--> /home/ajeffrey/github/asajeffrey/servo/components/script/dom/macros.rs:549:25 | 549 | let mut $name = $crate::dom::bindings::trace::RootedVec::new(&mut root); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `dom::bindings::trace::JSTraceable` is not implemented for `js::jsapi::Value` | ::: /home/ajeffrey/github/asajeffrey/servo/components/script/dom/paintworkletglobalscope.rs | 171 | rooted_vec!(let mut args); | -------------------------- in this macro invocation | = note: required by `<dom::bindings::trace::RootedVec<'a, T>>::new`

The problem being that I need to construct a RootedVec<JSVal>.

There's a similar type error with rooted!(in(cx) let args = ....):

error[E0277]: the trait bound `js::jsapi::HandleValueArray: js::rust::RootKind` is not satisfied --> /home/ajeffrey/github/asajeffrey/servo/components/script/dom/paintworkletglobalscope.rs:171:9 | 171 | / rooted!(in(cx) let args = unsafe { HandleValueArray::from_rooted_slice(&[ 172 | | ObjectValue(rendering_context.reflector().get_jsobject().get()), 173 | | ObjectValue(paint_size.reflector().get_jsobject().get()), 174 | | ]) }); | |______________^ the trait `js::rust::RootKind` is not implemented for `js::jsapi::HandleValueArray` | = note: required by `<js::rust::RootedGuard<'a, T>>::new` = note: this error originates in a macro outside of the current crate

I'm not sure this is needed anyway, all the objects in the array are already rooted, the only thing is the array itself, but I believe Call manages the lifetime of the arguments object?

asajeffrey · 2017-06-16T20:34:33Z

components/script/dom/paintworkletglobalscope.rs

+        debug!("Getting alpha.");
+        let alpha: bool =
+            unsafe { get_property(cx, paint_obj.handle(), "alpha", ()) }?
+            .unwrap_or_default();


Good point. Fixed.

asajeffrey · 2017-06-16T20:34:33Z

components/script/dom/paintworkletglobalscope.rs

+        // Step 19.
+        let definition = PaintDefinition {
+            class_constructor: Heap::new(ObjectValue(paint_obj.get())),
+            paint_function: Heap::new(paint_function),


asajeffrey · 2017-06-16T20:34:33Z

components/script/dom/paintworkletglobalscope.rs

+/// A paint definition
+/// https://drafts.css-houdini.org/css-paint-api/#paint-definition
+#[derive(Clone, JSTraceable, HeapSizeOf)]
+struct PaintDefinition {


asajeffrey · 2017-06-16T20:34:33Z

components/script/dom/paintworkletglobalscope.rs

+    class_constructor: Heap<JSVal>,
+    paint_function: Heap<JSVal>,
+    #[ignore_heap_size_of = "Rc"]
+    constructor_valid_flag: Rc<Cell<bool>>,


It was because I was cloning the definition. I'm not now, so the Rc is gone.

jdm · 2017-06-20T21:08:55Z

components/script/dom/paintworkletglobalscope.rs

-            Entry::Vacant(entry) => {
+            Some(definition) => {
+                class_constructor.set(definition.class_constructor.get());
+                paint_function.set(definition.paint_function.get());


Move these after the code for 5.1.

jdm · 2017-06-20T21:08:55Z

components/script/dom/paintworkletglobalscope.rs

@@ -208,6 +215,7 @@ impl PaintWorkletGlobalScopeMethods for PaintWorkletGlobalScope {
        let name = Atom::from(name);
        let cx = self.worklet_global.get_cx();
        rooted!(in(cx) let paint_obj = paint_ctor.callback_holder().get());
+        let paint_val = ObjectValue(paint_obj.get());


Unrooted values are scary.

OK, I rooted it, somewhat unnecessarily because the line above roots it too, but better safe than sorry.

jdm · 2017-06-20T21:08:55Z

components/script/dom/paintworkletglobalscope.rs

+
+impl PaintDefinition {
+    fn new(class_constructor: JSVal,
+           paint_function: JSVal,


Let's take HandleValue arguments instead.

jdm · 2017-06-20T21:08:55Z

components/script/dom/paintworkletglobalscope.rs

+           paint_function: JSVal,
+           input_properties: Vec<DOMString>,
+           alpha: bool)
+           -> Box<PaintDefinition>


I am actually quite unnerved that this does not trigger our must_root compiler check :(

jdm · 2017-06-20T21:08:55Z

tests/wpt/mozilla/tests/mozilla/worklets/test_paint_worklet_properties_throws.js

+registerPaint("propertiesThrows", class {
+    static get inputProperties() { throw new TypeError(); }
+    paint(ctx, size) { }
+});


These tests may not be needed if #17416 merges first.

jdm · 2017-06-20T21:08:55Z

components/script/dom/paintworkletglobalscope.rs

+        // TODO: Step 10
+        // Steps 11-12
+        debug!("Invoking paint function {}.", name);
+        let args = unsafe { HandleValueArray::from_rooted_slice(&[


HandleValueArray is a type meaning "this is a pointer to an array of values that are rooted." This is literally storing a pointer to an array that goes out of scope immediately after returning.

asajeffrey

Some more fighting with GC rooting.

asajeffrey · 2017-06-21T22:10:29Z

components/script/dom/paintworkletglobalscope.rs

-            Entry::Vacant(entry) => {
+            Some(definition) => {
+                class_constructor.set(definition.class_constructor.get());
+                paint_function.set(definition.paint_function.get());


asajeffrey · 2017-06-21T22:10:29Z

components/script/dom/paintworkletglobalscope.rs

@@ -208,6 +215,7 @@ impl PaintWorkletGlobalScopeMethods for PaintWorkletGlobalScope {
        let name = Atom::from(name);
        let cx = self.worklet_global.get_cx();
        rooted!(in(cx) let paint_obj = paint_ctor.callback_holder().get());
+        let paint_val = ObjectValue(paint_obj.get());


OK, I rooted it, somewhat unnecessarily because the line above roots it too, but better safe than sorry.

asajeffrey · 2017-06-21T22:10:29Z

components/script/dom/paintworkletglobalscope.rs

+
+impl PaintDefinition {
+    fn new(class_constructor: JSVal,
+           paint_function: JSVal,


asajeffrey · 2017-06-21T22:10:29Z

components/script/dom/paintworkletglobalscope.rs

+           paint_function: JSVal,
+           input_properties: Vec<DOMString>,
+           alpha: bool)
+           -> Box<PaintDefinition>


bors-servo · 2017-06-22T16:09:24Z

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


        PR servo#17239


        PR servo#17239

asajeffrey

Can I r=you?

asajeffrey · 2017-06-29T17:04:19Z

components/script/dom/paintworkletglobalscope.rs

+        // TODO: Step 10
+        // Steps 11-12
+        debug!("Invoking paint function {}.", name);
+        let args = unsafe { HandleValueArray::from_rooted_slice(&[



        Implemented paint worklets invoking worklet scripts.

asajeffrey · 2017-06-29T22:33:22Z

Squashed. @bors-servo r=jdm IRL.

bors-servo · 2017-06-29T22:33:22Z

📌 Commit 3db4761 has been approved by jdm

bors-servo · 2017-06-29T22:33:23Z

🌲 The tree is currently closed for pull requests below priority 9000, this pull request will be tested once the tree is reopened

jdm · 2017-06-30T08:14:59Z

@bors-servo: treeclosed-

bors-servo · 2017-06-30T08:16:12Z

⌛ Testing commit 3db4761 with merge 39da6d4...


        Auto merge of #17239 - asajeffrey:script-paint-worklets-invoke-workle…

…t-scripts, r=jdm Implemented paint worklets invoking worklet scripts.  Implemented the "invoke a paint callback" functionality of paint worklets (https://drafts.css-houdini.org/css-paint-api/#invoke-a-paint-callback). This PR does not implement the 2D rendering context, and just generates a placeholder image. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes do not require tests because we can't write reftests until we have 2D rendering contexts implemented.    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17239)

bors-servo · 2017-06-30T09:21:29Z

☀️ 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: jdm
Pushing 39da6d4 to master...


        Auto merge of #17364 - asajeffrey:script-paint-worklets-properties, r…

…=jdm Implement paint worklet properties  This is the final PR to get basic paint worklet support. It adds support for paint worklet properties (https://drafts.css-houdini.org/css-paint-api/#paint-definition-input-properties). When a paint worklet is registered, it specifies a list of CSS properties, and is provided with their computed values when it is invoked. This is a dependent PR: * "Implemented paint worklets invoking worklet scripts" is #17239. * "Implemented paint worklets rendering contexts" is #17326. There should be tests added for this, hopefully the existing wpt houdini tests. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #16839 - [x] There are tests for these changes    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17364)


        Auto merge of #17364 - asajeffrey:script-paint-worklets-properties, r…

…=jdm Implement paint worklet properties  This is the final PR to get basic paint worklet support. It adds support for paint worklet properties (https://drafts.css-houdini.org/css-paint-api/#paint-definition-input-properties). When a paint worklet is registered, it specifies a list of CSS properties, and is provided with their computed values when it is invoked. This is a dependent PR: * "Implemented paint worklets invoking worklet scripts" is #17239. * "Implemented paint worklets rendering contexts" is #17326. There should be tests added for this, hopefully the existing wpt houdini tests. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #16839 - [x] There are tests for these changes    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17364)


        Auto merge of #17499 - asajeffrey:script-paint-worklets-zoom, r=glennw

Fixed scaling artefacts in paint worklets caused by zoom and hidpi  This PR renders paint worklet canvases at the device pixel resolution, rather than the CSS pixel resolution. It's a dependent PR, building on #17239, #17326 and #17364. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #17454 - [X] These changes do not require tests because we don't run reftests with zoom enabled    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17499)


        Auto merge of #17499 - asajeffrey:script-paint-worklets-zoom, r=glennw

Fixed scaling artefacts in paint worklets caused by zoom and hidpi  This PR renders paint worklet canvases at the device pixel resolution, rather than the CSS pixel resolution. It's a dependent PR, building on #17239, #17326 and #17364. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #17454 - [X] These changes do not require tests because we don't run reftests with zoom enabled    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17499)

asajeffrey added the A-content/script label Jun 8, 2017

highfive assigned glennw Jun 8, 2017

highfive added the S-awaiting-review label Jun 8, 2017

cbrewster reviewed Jun 9, 2017

View changes

asajeffrey added the S-needs-tests label Jun 12, 2017

asajeffrey mentioned this pull request Jun 14, 2017

Implement paint worklets rendering context #17326

Merged

3 of 3 tasks complete

highfive assigned jdm and unassigned glennw Jun 14, 2017

jdm requested changes Jun 16, 2017

View changes

jdm added S-needs-code-changes and removed S-awaiting-review labels Jun 16, 2017

asajeffrey mentioned this pull request Jun 16, 2017

Implement paint worklet properties #17364

Merged

4 of 4 tasks complete

asajeffrey reviewed Jun 16, 2017

View changes

highfive added S-awaiting-review and removed S-needs-code-changes labels Jun 16, 2017

asajeffrey removed the S-needs-tests label Jun 19, 2017

jdm requested changes Jun 20, 2017

View changes

jdm added S-needs-code-changes and removed S-awaiting-review labels Jun 20, 2017

highfive added S-awaiting-review and removed S-needs-code-changes labels Jun 21, 2017

asajeffrey reviewed Jun 21, 2017

View changes

highfive added the S-needs-rebase label Jun 22, 2017

asajeffrey force-pushed the asajeffrey:script-paint-worklets-invoke-worklet-scripts branch from db5165a to d89851c Jun 22, 2017

asajeffrey removed the S-needs-rebase label Jun 22, 2017

asajeffrey added the S-needs-squash label Jun 22, 2017

asajeffrey added a commit to asajeffrey/servo that referenced this pull request Jun 23, 2017

PR servo#17239

089480c

asajeffrey mentioned this pull request Jun 23, 2017

Fixed scaling artefacts in paint worklets caused by zoom and hidpi #17499

Merged

4 of 4 tasks complete

asajeffrey added a commit to asajeffrey/servo that referenced this pull request Jun 28, 2017

PR servo#17239

696948a

asajeffrey reviewed Jun 29, 2017

View changes

components/script/dom/paintworkletglobalscope.rs Outdated

// TODO: Step 10

// Steps 11-12

debug!("Invoking paint function {}.", name);

let args = unsafe { HandleValueArray::from_rooted_slice(&[

This comment has been minimized.

Sign in to view

asajeffrey Jun 29, 2017
Author Member

Cool.

Implemented paint worklets invoking worklet scripts.

Loading status checks…

3db4761

asajeffrey force-pushed the asajeffrey:script-paint-worklets-invoke-worklet-scripts branch from 9c9fb82 to 3db4761 Jun 29, 2017

highfive added S-awaiting-merge and removed S-awaiting-review S-needs-squash labels Jun 29, 2017

bors-servo merged commit 3db4761 into servo:master Jun 30, 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 30, 2017

bors-servo mentioned this pull request Jun 30, 2017

Script paint worklets arguments #17546

Merged

4 of 4 tasks complete

servo / servo

Join GitHub today

Implemented paint worklets invoking worklet scripts. #17239

Implemented paint worklets invoking worklet scripts. #17239

Conversation

asajeffrey commented Jun 8, 2017 • edited by larsbergstrom

highfive commented Jun 8, 2017

highfive commented Jun 8, 2017

asajeffrey commented Jun 8, 2017

This comment has been minimized.

cbrewster Jun 9, 2017 • edited

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.

asajeffrey commented Jun 14, 2017

jdm 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.

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.

asajeffrey 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.

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.

asajeffrey left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bors-servo commented Jun 22, 2017

asajeffrey left a comment

This comment has been minimized.

asajeffrey commented Jun 29, 2017

bors-servo commented Jun 29, 2017

bors-servo commented Jun 29, 2017

jdm commented Jun 30, 2017

bors-servo commented Jun 30, 2017

bors-servo commented Jun 30, 2017

asajeffrey commented Jun 8, 2017 •

edited by larsbergstrom

cbrewster Jun 9, 2017 •

edited