Unify the task source and task canceller API #21804

AgustinCB · 2018-09-24T23:42:32Z

To do so, I created a struct TaskManagement(TaskSource, TaskCanceller) and made *_task_source return that instead of just
the task source.

Next, I refactored all places in which task_canceller by basically
removing them in favour of a previously called *_task_source.

I tried to make task_canceller a private method in Window, with the
hope of enforcing the use of *_task_source. However, it's used in
components/script/dom/globalscope.rs:575 in such a way that will make it
harder to avoid. I decided to leave it that way.

It'd be possible to unify *_task_source in such a way that we would
have only one method. However, I decided not to do it because one of the
TaskSource implementations is special:
history_traversal_task_source. Not wanting to over complicate things,
I decided to leave the structure this way.

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix #21154 (github issue number if applicable).
These changes do not require tests because it's refactoring code that should already be tested.

This change is

highfive · 2018-09-24T23:42:36Z

Heads up! This PR modifies the following files:

@asajeffrey: components/script/dom/textcontrol.rs, components/script/layout_image.rs, components/script/dom/audioscheduledsourcenode.rs, components/script/dom/htmlscriptelement.rs, components/script/dom/baseaudiocontext.rs and 17 more
@KiChjang: components/script/dom/textcontrol.rs, components/script/layout_image.rs, components/script/dom/audioscheduledsourcenode.rs, components/script/dom/htmlscriptelement.rs, components/script/dom/baseaudiocontext.rs and 17 more

highfive · 2018-09-24T23:42:38Z

Warning

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

gterzian · 2018-09-26T04:17:05Z

Thanks! I will look at this later this week!

gterzian · 2018-09-26T07:04:43Z

Is it just me, or is the HistoryTraversalTaskSource not actually used anywhere(anymore)? It seems that History communicates directly via the constellation, and the constellation will "queue" a task to update or remove the history state on a script-thread via a message back to it? This seems to match the concept of a https://html.spec.whatwg.org/multipage/history.html#session-history-traversal-queue, which is tied to a specific 'session history event loop', which runs "in parallel" to the script event-loop to which the other task sources are tied.

Should we perhaps remove the HistoryTraversalTaskSource?

cc @cbrewster @jdm

gterzian

Looks like a very good step in the right direction. I have one question on the overall design...

gterzian · 2018-09-26T12:28:59Z

components/script/dom/window.rs

@@ -1207,6 +1214,8 @@ impl WindowMethods for Window {
    }
 }

+pub struct TaskManagement<T: TaskSource>(pub T, pub TaskCanceller);


It'd be possible to unify *_task_source in such a way that we would
have only one method.

Have you considered making this a struct that would completely take over the task management from a Window?

Perhaps something like:

struct TaskManager { task_cancellers: DomRefCell<HashMap<TaskSourceName, Arc<AtomicBool>>>, task_sources: DomRefCell<HashMap<TaskSourceName, Box<TaskSource>>> } impl TaskManager { fn new(task_sources: HashMap<TaskSourceName, Box<TaskSource>>) -> Self; fn task_source_with_canceller(&self, name: TaskSourceName) -> (Box<TaskSource>, TaskCanceller); fn task_source(&self, name: TaskSourceName)-> Box<TaskSource>; fn task_canceller(&self, name: TaskSourceName)-> TaskCanceller; }

Perhaps it could be initialized when a new page loads, passed to Window::new, and basically replace all the task source related methods and fields on Window with one public task_manager field?

one of the
TaskSource implementations is special:
history_traversal_task_source. Not wanting to over complicate things,
I decided to leave the structure this way.

Perhaps if you were to implement TaskSource for HistoryTraversalTaskSource(or remove it if others confirm this is a good idea, see my other comment), it could work without requiring too much complication?

I thought of that, but not knowing what to do with HistoryTraversalTaskSource made me chose something simpler. Truth being, I'd love to move responsibilities outside of Window (it's the most overwhelming file in the source code) :). Your proposal of adding a "fake" implementation for HistoryTraversalTaskSource sounds good to me and addresses the concern I had. Will address your feedback and push the new version in the next days.

Thanks for the suggestion.

Sounds good, you could also do an actually 'real' implementation, since we might actually need it later for #21815

I think basically a copy/paste from any of the other task-source, and adding a new ScriptThreadEventCategory, should do the trick...

Unfortunately, TaskSource can't be made a trait object :(.

I do have the HistoryTraversalTaskSource implementation. Should I just do that? Or maybe we can somehow modify TaskSource to allow it to make a trait object?

I guess I could also make a less elegant...

struct TaskManager { task_cancellers: DomRefCell<HashMap<TaskSourceName, Arc<AtomicBool>>>, *_task_sources: DomRefCell<*TaskSource>, }

Although we would still have a lot of methods.

Of course! Here it's:

error[E0038]: the trait `task_source::TaskSource` cannot be made into an object --> components/script/task_manager.rs:119:5 | 119 | pub fn task_source(&self, name: TaskSourceName) -> Box<TaskSource> { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `task_source::TaskSource` cannot be made into an object | = note: method `queue_with_canceller` has generic type parameters = note: method `queue` has generic type parameters = note: the trait cannot contain associated consts like `NAME

I see, so that's not going to work :)

I haven't found another way to avoid the repetition of methods, have you?

In any case, introducing a TaskManager that will lighten up Window on methods and fields is probably progress.

Thanks for pushing to additional commits, I will review them as is unless we come up with a better design...

I tried to trick the compiler in a couple naive ways (i.e. using & mut instead of Box, wrapping the TaskSource in a structure that is object safe, etc.). As expected, the compiler outsmarted me.

Another thing I considered was just living with the duplication, but create a couple macros so it's less apparent. I didn't want to go through that route, because I didn't see any macro in the rest of the code and I know some projects are (rightfully) cautious about them. If this were a personal project, though, and the choice depended only on me, I'd probably do that.

We do use macros when it makes sense in Servo's codebase.

Good to know! Will push (yet another) commit with how it would look like with macros. Thanks!

cbrewster · 2018-09-26T13:25:25Z

We don’t use the history task source, I tried using it so we would line up with the spec a bit more, but it ends up queuing a message on the task source to send a message to the constellation, which doesn’t make much sense. I would be okay with removing the history task source, but it really depends on how much we care about history spec compliance.

gterzian · 2018-09-26T14:24:11Z

@cbrewster Thanks for the info!

I'll open a separate issue to discuss this...

@AgustinCB I think in the context of this PR, we could implement a ~~blank~~ TaskSource for HistoryTraversalTaskSource ~~that always returns Ok(()) just~~ so that it can be used along with all the other task sources as part of the 'task management'.

EDIT: I think an actual task source implementation similar to the others is probably better, since I just realized the history-traversal task queue and task source aren't completely related, see #21815

Could please do this implementation of TaskSource for HistoryTraversalTaskSource in a separate commit from the rest of this PR, yet include it in this one(using the task-source as mentioned in the issue is probably out of scope of this PR, yet could be done in another)?

ghost · 2018-09-28T01:30:45Z

No Taskcluster jobs started for this pull request

The `allowPullRequests` configuration for this repository (in `.taskcluster.yml` on the
default branch) does not allow starting tasks for this pull request.

ghost · 2018-09-28T01:48:43Z

No Taskcluster jobs started for this pull request

The `allowPullRequests` configuration for this repository (in `.taskcluster.yml` on the
default branch) does not allow starting tasks for this pull request.

gterzian

Looks great! Just a few nits really

gterzian · 2018-09-29T06:04:49Z

components/script/dom/audiocontext.rs

@@ -137,7 +137,7 @@ impl AudioContextMethods for AudioContext {
                    if base_context.State() != AudioContextState::Suspended {
                        base_context.set_state_attribute(AudioContextState::Suspended);
                        let window = DomRoot::downcast::<Window>(context.global()).unwrap();
-                        window.dom_manipulation_task_source().queue_simple_event(
+                        window.task_manager().dom_manipulation_task_source().queue_simple_event(


nit: I'm wondering if task_source could be used instead here.

Not really, since task_source is borrowed in line 131.

gterzian · 2018-09-29T06:04:49Z

components/script/dom/audiocontext.rs

@@ -199,7 +199,7 @@ impl AudioContextMethods for AudioContext {
                    if base_context.State() != AudioContextState::Closed {
                        base_context.set_state_attribute(AudioContextState::Closed);
                        let window = DomRoot::downcast::<Window>(context.global()).unwrap();
-                        window.dom_manipulation_task_source().queue_simple_event(
+                        window.task_manager().dom_manipulation_task_source().queue_simple_event(


In here, it's borrowed in line 193.

gterzian · 2018-09-29T06:04:49Z

components/script/dom/audioscheduledsourcenode.rs

        let callback = OnEndedCallback::new(move || {
            let _ = task_source.queue_with_canceller(
                task!(ended: move || {
                    let this = this.root();
                    let global = this.global();
                    let window = global.as_window();
-                    window.dom_manipulation_task_source().queue_simple_event(
+                    window.task_manager().dom_manipulation_task_source().queue_simple_event(


Similar, is borrowed before.

gterzian · 2018-09-29T06:04:49Z

components/script/dom/baseaudiocontext.rs

@@ -225,7 +225,7 @@ impl BaseAudioContext {
                            if this.state.get() != AudioContextState::Running {
                                this.state.set(AudioContextState::Running);
                                let window = DomRoot::downcast::<Window>(this.global()).unwrap();
-                                window.dom_manipulation_task_source().queue_simple_event(
+                                window.task_manager().dom_manipulation_task_source().queue_simple_event(


It's also borrowed before.

To be fair, this parts also bothered me. I suspect that there's some way in which this code can be refactor to call less clones. But I'm not sure it's immediately obvious.

Maybe I can offer you to open an issue to track the tech debt?

Ok, thanks for the info. I'm not sure at this point how to make a potential issue actionable, so I will not open one. If you happen to have a clearer idea, it could be useful if you were to open one indeed...

No, not really.

gterzian · 2018-09-29T06:04:49Z

components/script/dom/htmldialogelement.rs

-        win.dom_manipulation_task_source()
-            .queue_simple_event(target, atom!("close"), &win);
+        win.task_manager().dom_manipulation_task_source()
+                          .queue_simple_event(target, atom!("close"), &win);


nit: This is what rustfmt did with it(actually it appears the indent here is slightly different, the idea is 4 space ident I believe, starting at win.task_manager()):

// Step 5 win.task_manager() .dom_manipulation_task_source() .queue_simple_event(target, atom!("close"), &win);

gterzian · 2018-09-29T06:04:49Z

components/script/dom/htmlmediaelement.rs

@@ -588,11 +589,11 @@ impl HTMLMediaElement {

                let context = Arc::new(Mutex::new(HTMLMediaElementContext::new(self)));
                let (action_sender, action_receiver) = ipc::channel().unwrap();
-                let window = window_from_node(self);
+                let (task_source, canceller) = document.window().task_manager().networking_task_source_with_canceller();


nit: this one looks a little too long(you can check with ./mach test-tidy)

gterzian · 2018-09-29T06:04:49Z

components/script/dom/window.rs

-                        self.task_canceller(TaskSourceName::DOMManipulation)
-                            .wrap_task(task),
+                        self.task_manager.task_canceller(TaskSourceName::DOMManipulation)
+                                         .wrap_task(task),


nit: looks like too much indent like above

gterzian · 2018-09-29T06:04:49Z

components/script/dom/window.rs

-                self.task_canceller(TaskSourceName::DOMManipulation)
-                    .wrap_task(task),
+                self.task_manager.task_canceller(TaskSourceName::DOMManipulation)
+                                 .wrap_task(task),


nit: too much indent like above

gterzian · 2018-09-29T06:04:49Z

components/script/task_manager.rs

@@ -0,0 +1,130 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public


Since this is a completely new file, you can safely run ./mach rustfmt -d components/script/task_manager.rs and apply the changes to the commit (requires installing first if you haven't, see https://github.com/rust-lang-nursery/rustfmt)

gterzian · 2018-09-29T06:04:49Z

components/script/task_source/history_traversal.rs

+        T: TaskOnce + 'static,
+    {
+        let msg = MainThreadScriptMsg::Common(CommonScriptMsg::Task(
+            ScriptThreadEventCategory::DocumentEvent,


I think you want to add a new ScriptThreadEventCategory::HistoryEvent(in the same commit)

gterzian · 2018-10-01T14:06:27Z

Ok, looks good to me. I think you can squash it into two commits, one fro the history task source changes, and one for the rest. Thanks for tackling this!

@jdm r?

jdm

I agree with all of gterzian's earlier comments, so thank you for addressing them. Nice work on this!

jdm · 2018-10-01T14:58:20Z

components/script/dom/window.rs

@@ -173,21 +165,7 @@ pub struct Window {
    #[ignore_malloc_size_of = "trait objects are hard"]
    script_chan: MainThreadScriptChan,
    #[ignore_malloc_size_of = "task sources are hard"]


Let's remove this annotation and add any required annotations inside of TaskManager instead.

Sorry, I'm blocked in this I dunno how to get past: What do I have to do to have ignore_malloc_size_of in the TaskManager? I keep getting this error:

error[E0658]: The attribute `ignore_malloc_size_of` is currently unknown to the compiler and may have meaning added to it in the future (see issue #29642) --> components/script/task_manager.rs:50:5 | 50 | #[ignore_malloc_size_of = "task sources are hard"] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: add #![feature(custom_attribute)] to the crate attributes to enable

And I don't know how to solve it (I already tried the suggestion).

Thanks!

Did you add #[derive(MallocSizeOf)] to the TaskManager struct as well?

That work, thanks!

jdm · 2018-10-03T23:45:10Z

@bors-servo r+

bors-servo · 2018-10-03T23:45:11Z

📌 Commit 5954474 has been approved by jdm

gterzian

I think the problem we've experienced where what was previously an intermittent problem suddenly failed consistently, with tasks not being cancelled when clear_js_runtime has been called, is because despite the latest changes in this PR, that function(via ignore_all_events) is still using the ignore_further_async_events field of Window, which isn't tied to scheduled tasks anymore, since those are now schedule via the TaskManager which has it's own task_cancellers: DomRefCell<HashMap<TaskSourceName, Arc<AtomicBool>>>.

Fixing the above should put the situation back to the previous 'intermittent test failure'...

gterzian · 2018-11-06T02:47:52Z

components/script/dom/window.rs

@@ -1227,14 +1169,6 @@ impl Window {
        self.paint_worklet.or_init(|| self.new_paint_worklet())
    }

-    pub fn task_canceller(&self, name: TaskSourceName) -> TaskCanceller {
-        let mut flags = self.ignore_further_async_events.borrow_mut();


The ignore_further_async_events field of Window is still used in a few places, such as ignore_all_events, this makes the task cancelling inside ignore_all_events not have effect on tasks that have been scheduled via the new TaskManager, since the Arc<AtomicBool> are not shared between ignore_further_async_events and TaskManager.task_cancellers.

So we need to remove ignore_further_async_events from Window, and make methods like Window.ignore_all_events used the TaskManager instead.

Wow, there's no way I'd have seen that. Thank you so much.

One question, before I implement that, is there an (easy) way to properly fix this? I.e. no failure at all. Or would it fall outside the scope of this pr?

You're welcome.

I'm not sure if there is an easy way to fix the intermittent failure, although once the above is fixed in one commit, you could re-try, in a separate commit, the suggestion I made at #21804 (comment) (although since it is intermittent, you might have a hard time actually knowing for sure whether it helped or not).

bors-servo · 2018-11-06T21:10:22Z

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


        Implement TaskSource for HistoryTraversal

And remove the method in window that returns it, because it isn't used so far.

AgustinCB · 2018-11-13T20:18:44Z

Arf, this is finally fixed.

Sorry for taking so long with it.

jdm · 2018-11-13T22:17:14Z

@bors-servo try=wpt

bors-servo · 2018-11-13T22:17:19Z

⌛ Trying commit 3fb8d4d with merge 8b47358...


        Auto merge of #21804 - AgustinCB:unify-task-source-canceller-api, r=<…

…try> Unify the task source and task canceller API To do so, I created a struct `TaskManagement(TaskSource, TaskCanceller)` and made `*_task_source` return that instead of just the task source. Next, I refactored all places in which `task_canceller` by basically removing them in favour of a previously called `*_task_source`. I tried to make `task_canceller` a private method in `Window`, with the hope of enforcing the use of `*_task_source`. However, it's used in components/script/dom/globalscope.rs:575 in such a way that will make it harder to avoid. I decided to leave it that way. It'd be possible to unify `*_task_source` in such a way that we would have only one method. However, I decided not to do it because one of the `TaskSource` implementations is special: `history_traversal_task_source`. Not wanting to over complicate things, I decided to leave the structure this way. --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #21154 (github issue number if applicable). - [x] These changes do not require tests because it's refactoring code that should already be tested.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21804)

bors-servo · 2018-11-13T22:44:31Z

☀️ Test successful - linux-rel-css, linux-rel-wpt
State: approved= try=True

components/script/dom/htmlscriptelement.rs

CYBAI · 2018-11-14T03:01:16Z

Some tidy errors from rustfmt in Travis. You can run ./mach test-tidy to see these errors and fix them with ./mach fmt!


        Unify the task source and task canceller API

I moved away from the `Window` struct all the logic to handle task sources, into a new struct called `TaskManager`. In a happy world, I'd be able to just have there two functions, of the types: ```rust fn task_source<T: TaskSource>(&self, name: TaskSourceName) -> Box<T> fn task_source_with_canceller<T: TaskSource>(&self, name: TaskSourceName) -> (Box<T>, TaskSourceCanceller) ``` And not so much duplicated code. However, because TaskSource can't be a trait object (because it has generic type parameters), that's not possible. Instead, I decided to reduce duplicated logic through macros. For reasons[1], I have to pass both the name of the function with canceller and the name of the function without, as I'm not able to concatenate them in the macro itself. I could probably use `concat_idents` to create both types already defined and reduce the amount of arguments by one, but that macro is nightly only. At the same time, not being able to declare macros inside `impl` forces me to pass `self` as an argument. All this makes this solution more verbose than it would be ideally. It does reduce duplication, but it doesn't reduce the size of the file. [1](rust-lang/rust#29599)

AgustinCB · 2018-11-14T11:37:54Z

Thanks!

I addressed both issues and rebased.

jdm · 2018-11-16T14:04:20Z

@bors-servo r=gterzian

bors-servo · 2018-11-16T14:04:21Z

📌 Commit 75eb94a has been approved by gterzian

bors-servo · 2018-11-16T14:54:42Z

⌛ Testing commit 75eb94a with merge a2937d9...


        Auto merge of #21804 - AgustinCB:unify-task-source-canceller-api, r=g…

…terzian Unify the task source and task canceller API To do so, I created a struct `TaskManagement(TaskSource, TaskCanceller)` and made `*_task_source` return that instead of just the task source. Next, I refactored all places in which `task_canceller` by basically removing them in favour of a previously called `*_task_source`. I tried to make `task_canceller` a private method in `Window`, with the hope of enforcing the use of `*_task_source`. However, it's used in components/script/dom/globalscope.rs:575 in such a way that will make it harder to avoid. I decided to leave it that way. It'd be possible to unify `*_task_source` in such a way that we would have only one method. However, I decided not to do it because one of the `TaskSource` implementations is special: `history_traversal_task_source`. Not wanting to over complicate things, I decided to leave the structure this way. --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #21154 (github issue number if applicable). - [x] These changes do not require tests because it's refactoring code that should already be tested.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21804)

bors-servo · 2018-11-16T16:10:25Z

☀️ Test successful - android, android-mac, android-x86, 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, magicleap, status-taskcluster, windows-msvc-dev
Approved by: gterzian
Pushing a2937d9 to master...

highfive assigned paulrouget Sep 24, 2018

highfive added the S-awaiting-review label Sep 24, 2018

paulrouget removed their assignment Sep 25, 2018

gterzian suggested changes Sep 26, 2018

View changes

AgustinCB force-pushed the AgustinCB:unify-task-source-canceller-api branch from fe67621 to abe1a44 Sep 28, 2018

AgustinCB force-pushed the AgustinCB:unify-task-source-canceller-api branch from abe1a44 to abdcefa Sep 28, 2018

gterzian suggested changes Sep 29, 2018

View changes

AgustinCB force-pushed the AgustinCB:unify-task-source-canceller-api branch from 4ebc260 to c8268f6 Sep 29, 2018

jdm requested changes Oct 1, 2018

View changes

jdm added S-needs-code-changes S-needs-squash and removed S-awaiting-review labels Oct 1, 2018

AgustinCB force-pushed the AgustinCB:unify-task-source-canceller-api branch from c8268f6 to 5954474 Oct 3, 2018

highfive added S-awaiting-review and removed S-needs-code-changes labels Oct 3, 2018

highfive assigned jdm Oct 3, 2018

highfive added S-awaiting-merge and removed S-awaiting-review S-needs-squash labels Oct 3, 2018

gterzian suggested changes Nov 6, 2018

View changes

mandreyel mentioned this pull request Nov 11, 2018

Fully implement HistoryTravesalTaskSource #10994

Open

Implement TaskSource for HistoryTraversal

14bc8ab

And remove the method in window that returns it, because it isn't used so far.

AgustinCB force-pushed the AgustinCB:unify-task-source-canceller-api branch from 5079c12 to 3fb8d4d Nov 13, 2018

CYBAI reviewed Nov 14, 2018

View changes

components/script/dom/htmlscriptelement.rs Outdated Show resolved Hide resolved

AgustinCB force-pushed the AgustinCB:unify-task-source-canceller-api branch from 3fb8d4d to 75eb94a Nov 14, 2018

gterzian approved these changes Nov 14, 2018

View changes

highfive assigned gterzian and unassigned jdm Nov 16, 2018

highfive added S-awaiting-merge and removed S-awaiting-review S-needs-rebase S-awaiting-answer labels Nov 16, 2018

bors-servo merged commit 75eb94a into servo:master Nov 16, 2018
2 of 3 checks passed

2 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details

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

homu Test successful
Details

highfive removed the S-awaiting-merge label Nov 16, 2018

bors-servo mentioned this pull request Nov 16, 2018

Implement MessagePort and MessageChannel #16622

Closed

		@@ -0,0 +1,130 @@
		/* This Source Code Form is subject to the terms of the Mozilla Public

servo / servo

Join GitHub today

Unify the task source and task canceller API #21804

Unify the task source and task canceller API #21804

Conversation

AgustinCB commented Sep 24, 2018 • edited by SimonSapin

highfive commented Sep 24, 2018

highfive commented Sep 24, 2018

gterzian commented Sep 26, 2018

gterzian commented Sep 26, 2018 • edited

gterzian left a comment • edited

This comment has been minimized.

gterzian Sep 26, 2018 • edited

This comment has been minimized.

This comment has been minimized.

gterzian Sep 26, 2018 • edited

This comment has been minimized.

This comment has been minimized.

AgustinCB Sep 27, 2018 • 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.

cbrewster commented Sep 26, 2018

gterzian commented Sep 26, 2018 • edited

ghost commented Sep 28, 2018

ghost commented Sep 28, 2018

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

gterzian Oct 1, 2018 • edited

This comment has been minimized.

This comment has been minimized.

gterzian Sep 29, 2018 • 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.

gterzian Sep 29, 2018 • edited

gterzian commented Oct 1, 2018 • edited

jdm left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

jdm commented Oct 3, 2018

bors-servo commented Oct 3, 2018

gterzian left a comment • edited

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

gterzian Nov 6, 2018 • edited

bors-servo commented Nov 6, 2018

AgustinCB commented Nov 13, 2018

jdm commented Nov 13, 2018

bors-servo commented Nov 13, 2018

bors-servo commented Nov 13, 2018

CYBAI commented Nov 14, 2018

AgustinCB commented Nov 14, 2018

jdm commented Nov 16, 2018

bors-servo commented Nov 16, 2018

bors-servo commented Nov 16, 2018

bors-servo commented Nov 16, 2018

AgustinCB commented Sep 24, 2018 •

edited by SimonSapin

gterzian commented Sep 26, 2018 •

edited

gterzian left a comment •

edited

gterzian Sep 26, 2018 •

edited

gterzian Sep 26, 2018 •

edited

AgustinCB Sep 27, 2018 •

edited

gterzian commented Sep 26, 2018 •

edited

gterzian Oct 1, 2018 •

edited

gterzian Sep 29, 2018 •

edited

gterzian Sep 29, 2018 •

edited

gterzian commented Oct 1, 2018 •

edited

gterzian left a comment •

edited

gterzian Nov 6, 2018 •

edited