Improve navigate for service worker hooks #1776

jungkees · 2016-09-13T12:03:29Z

Improve navigate for service worker hooks

Before this patch, there has been no concept that holds reserved
client's information referenced from a service worker during a
navigation (more precisely before the actual global object and its
relevant environment settings object for the resulting resource is
created).

This patch defines an "environment" concept (a supertype object of an
environment settings object) that has an id, a creation URL, a target
browsing context, and an active service worker such that service
workers' FetchEvent hanlders can reference this client information
before the corresponding environmet setings object is actually
created.

The changes include:

Create an environment (a reserved client) during a navigation; for
worker creation requests, create an environment settings object and
set the fields correspondingly
Set request's reserved client to the created environment or
environment settings object such that fetch (and Handle Fetch) can
use it
Set request's target client id such that fetch (and Handle Fetch)
can use it
Note: The reserved client's active service worker is set, if
available, during the fetch (in service worker's Handle Fetch
algorithm).

Related issue: w3c/ServiceWorker#870
Related change: whatwg/fetch#383

This patch fixes the client matching flow and exposes reserved client and target client information by adding necessary hooks to fetch and HTML. Related issue: #870 Call flow: #870 (comment) Expected behavior: #870 (comment) Related changes: - HTML: whatwg/html#1776 - Fetch: whatwg/fetch#383 * Work on Handle Fetch is done. Work on the client API is in progress.

jungkees · 2016-09-13T12:14:47Z

@annevk, I put together the changes based on the outline here:

Call flow: Inconsistencies due to when clients are created w3c/ServiceWorker#870 (comment)
Expected behavior: Inconsistencies due to when clients are created w3c/ServiceWorker#870 (comment)

I'd appreciate your review.

/cc @domenic @jakearchibald

annevk · 2016-09-15T10:56:58Z

Note for others interested in this PR, we're discussing the general design in whatwg/fetch#383.

This patch defines request's reserved client id and target client id that are set to a reserved environment's id and the target browsing context's active document's settings object's id respectively during a navigation. The given values are primarily used to provide the reserved client's id and target client's id to service workers' fetch event handlers. Related issue: w3c/ServiceWorker#870 Call flow: w3c/ServiceWorker#870 (comment) Expected behavior: w3c/ServiceWorker#870 (comment) Related changes: - HTML: whatwg/html#1776 - SW (WIP): w3c/ServiceWorker@df39d89

jungkees · 2016-09-28T13:38:27Z

@annevk, I basically regarded the environment concept and the environment settings object concept are covariant. So, I defined it as "an environment settings object is an environment." and later in the service worker, I'll use the environment to enumerate all the clients there. Please take a look.

jungkees · 2016-09-28T13:42:37Z

Self-note: ~~destroy the environment object after when its values are set up to the resulting environment settings object. (missed it.)~~

annevk

This looks like an improvement though the editorial changes are a little distracting.

I'm also a little unsure about naming. We have "environment settings object" which effectively means global object. This adds "environment" which means something like "future/reserved global object".

It's also a little weird still that we grab some state from request to create something new and then grab something from the newly created thing and tack it on request. That just doesn't seem quite right.

annevk · 2016-09-28T17:18:43Z

source

+  optional opaque string <var>reservedId</var>, it must run the following steps:</p>
+
+  <ol>
+   <li><p>If <var>reservedId</var> is present, set <var>settingsObject</var>'s <span>id</span> to


<span>id</span> probably needs data-x?

annevk · 2016-09-28T17:19:24Z

source

-      <p>Return <var>execution context</var>.</p>
-
-     </dd>
+     <dd><p>Return <var>executionContext</var>.</p></dd>


I like this kind of cleanup, but @domenic needs to approve it. Also, it might be better to do that in a separate editorial-changes-only commit.

I'll follow what you and @domenic guide here.

Yes, let's please remove the reflowing of paragraphs/deleting of blank lines/renaming of variables from this commit.

annevk · 2016-09-28T17:21:59Z

source

+
+   <li><p>Set <var>request</var>'s <span data-x="concept-request-reserved-client-id">reserved
+   client id</span> to <var>reservedEnvironment</var>'s <span
+   data-x="concept-environment-id">id</span>.</p></li>


I don't really understand this. If we create the environment here, doesn't it need to be passed along with the request? Or is the idea that we later find it again through its id?

That'd be an option, but I think fetch already has the other states on the request already: the url and the target browsing context.

What service worker needs during dispatching the fetch event is the reserved environment's id to set FetchEvent.reservedClientId. And yes, the idea is to let the fetch event handler get the client using e.g. clients.get(e.reservedClientId).

In the similar reason, I proposed adding a target client id internal slot to request, too: whatwg/fetch@4511e2e As discussed during the f2f, we wanted to capture the target client's id here instead of crossing the process border later when the service worker needs to access the target client in the fetch event dispatch steps.

annevk · 2016-09-28T17:22:15Z

source

@@ -82253,15 +82263,14 @@ State: &lt;OUTPUT NAME=I>1&lt;/OUTPUT> &lt;INPUT VALUE="Increment" TYPE=BUTTON O
   </li>

   <li><p>Run <span>process a navigate response</span> given <var>request</var>,
-   <var>response</var>, <var>navigationType</var>, the <span>source browsing context</span>, and
-   <var>browsingContext</var>.</p></li>
+   <var>response</var>, <var>navigationType</var>, the <span>source browsing context</span>, <var>browsingContext</var>, and <var>reservedEnvironment</var>.</p></li>


The wrapping here seems wrong.

jungkees · 2016-09-29T02:45:33Z

I'm also a little unsure about naming. We have "environment settings object" which effectively means global object. This adds "environment" which means something like "future/reserved global object".

Open to renaming it. I absolutely want a better name for it if we can get one.

It's also a little weird still that we grab some state from request to create something new and then grab something from the newly created thing and tack it on request. That just doesn't seem quite right.

I don't think I can help here. The only state I copy from request to environment is the request's url which the newly created environment has to have. (Note that I tried to factor the creation URL out to environment as both environment and environment settings object need this info.) And the only state I copy from the created environment to request is its id, which is the whole idea this patch suggests. I think it makes sense as a navigation works in this way.

annevk · 2016-09-29T10:19:11Z

Are you saying that this is also how Chrome/Samsung Internet would engineer this setup? @wanderview would we use this setup in Firefox?

jungkees · 2016-09-29T11:24:33Z

I'll dig into it again, but this is what I found sometime ago: #322 (comment)

Basically, Chromium will have to implement something similar to the reserved id, store it somewhere like in the service worker provider and later relate the id to the actual frame id when the frame and the document are created. So, I think, in order to expose the FetchEvent.reservedClientId and FetchEvent.targetClientId, we'll have to implement the similar logic. It'll be great if @wanderview and @mattto cross-checks if this setup is good to implement. Please check out this simplified flow to help your understanding: w3c/ServiceWorker#870 (comment)

Also, I'd appreciate your comments on calling the match service worker registration algorithm twice: once for main resource fetch and the other for attaching a controller after the global is created. This is due to the fact that it's not easy to pass the captured state around between navigate/fetch/SW.

jungkees · 2016-09-29T13:26:41Z

Having thought a bit more on this, matching the registration twice is awkward. I think we can do better as we now have the environment concept where we can store the matched registration. For this, we'd have to pass this environment along with the request as @annevk asked above.

So, the idea is:

Match a service worker registration with request's url
Create an environment and set all their internal slots properly including the service worker registration internal slot to the matched registration
Check (in Handle Fetch) if the reserved client's service worker registration is not null and its active worker is not null to dispatch fetch event. (For subresource fetch, client's registration should be used instead of the reserved client's registration.)

(With this change, we don't need to store the active worker on the global objects. Also, by passing the environment along with a request, we can remove request's target browsing context internal slot. We can use reserved client's target browsing context instead.)

For worker clients, we can basically do the same and will pass the entire environment settings object (instead of an environment) that is set with the matched registration to fetch. This means the type of "reserved client" internal slot to request should be "null or an environment or an environment settings object".

I think this matches the implementations more precisely, at least Chromium. @wanderview, would it make sense for Gecko? (Note that this doesn't change the originally proposed behavior in this PR. Both matching the registration twice and matching it once / storing in an environment will use the latest active worker always.)

jungkees · 2016-09-30T00:52:40Z

@mattto, @kinu I'd appreciate your comments on this.

jungkees · 2016-10-04T07:55:58Z

Having reviewed Chromium's navigation part of the code again, I believe the structure I'm trying to spec in this PR would be implementable.

The current implementation is explained in #322 (comment) The gist is when it initializes the request handler for a navigation request (ServiceWorkerRequestHandler::InitializeForNavigation - largely corresponding to initializing a request to pass to Handle Fetch) it already sets up the |provider_host|'s client (request's client in the spec) to the new reserved client - which the spec will do with the change. The current implementation just ignores this value by setting the FetchEvent's clientId to null if it's a navigation request.

So for navigation requests, Chromium will need to change to set the request handler's provider_host to the source client instead of the reserved client. And add a reserved_provider_host or similar state to grab the pre-created (i.e. reserved) client and the target_provider_host's client_id.

Still would like to check what Gecko does and whether the proposed structure is implementable there.

domenic

This needs to be rebased and also the build fails due to a parsing error, so I am not quite able to review the output. Reviewing will be much easier after the editorial changes are removed too.

Overall I am pretty happy with this direction though. The environment + environment settings object concept makes sense. We should think harder to see if there is a better name, but we can leave that for a bit later.

domenic · 2016-10-04T18:15:05Z

source

@@ -77994,12 +78001,31 @@ console.assert(iframeWindow.frameElement === null);

  <h4>Script settings for browsing contexts</h4>

+  <p>When the user agent is required to <dfn>set up an environment</dfn>, given an


Since this is browsing context specific, then it should probably have that in the name, like "set up a browsing context environment settings object" does. So "set up a browsing context environment".

Indeed this algorithm isn't browsing context specific. It's invoked from both set up a browsing context environment settings object and set up a worker environment settings object. I think this algorithm definition needs to move to a better place?

domenic · 2016-10-04T18:15:42Z

source

+  optional opaque string <var>reservedId</var>, it must run the following steps:</p>
+
+  <ol>
+   <li><p>If <var>reservedId</var> is present, set <var>settingsObject</var>'s <span


s/is present/is given/

domenic · 2016-10-04T18:16:48Z

source

-      <p>Return <var>execution context</var>.</p>
-
-     </dd>
+     <dd><p>Return <var>executionContext</var>.</p></dd>


Yes, let's please remove the reflowing of paragraphs/deleting of blank lines/renaming of variables from this commit.

domenic · 2016-10-04T18:22:59Z

source

@@ -78487,6 +78483,16 @@ callback <dfn>FrameRequestCallback</dfn> = void (<span>DOMHighResTimeStamp</span
 http://lxr.mozilla.org/mozilla/source/dom/public/idl/base/nsIDOMWindowInternal.idl - DOM level 0
 -->

+  <p>A <code>Window</code> object has an <dfn data-export=""


Why move this? I think it should stay where it is (in the w-nodev div) and the active worker should also go there.

I thought it would've been more consistent to other part of the spec such as WorkerGlobalScope definition where the internal slots are defined before the method/attribute description section. But I found it's not very consistent across the document. Reverted this line. (Note that active worker isn't defined in the global object any long, so it's removed here.)

domenic · 2016-10-04T18:23:17Z

source

+
+  <p>A <code>Window</code> object has an associated <dfn data-export="" data-dfn-for="Window"
+  data-x="concept-Window-active-worker">active worker</dfn> (null or a <span
+  data-x="dfn-service-worker">service worker</span>). It is initially null.</p>


We should call this "active service worker"; it is quite confusing to me otherwise.

This state has been moved to the environment concept. So, it's called environment's service worker now. Not sure whether "active service worker" would be a better name though. This internal slot basically holds the value of either null or a service worker in "active" state only.

domenic · 2016-10-04T18:27:34Z

source

+      <ol>
+       <li><p>Let <var>registration</var> be the result of running <span
+       data-x="scope-match-algorithm">match service worker registration</span> with the
+       <code>Document</code>’s <span>URL</span>.</p></li>


No curly quotes

Addressed. Thanks for spotting.

jungkees · 2016-10-05T08:11:28Z

Thanks @domenic. I'll address your comments and this change (#1776 (comment)) after checking with @wanderview if Gecko has any concerns.

jungkees · 2016-10-05T08:35:09Z

@wanderview, I'd like to check with you this spec change is feasible with Gecko. I'd appreciate your comments on the below questions.

Gecko's using a document id to identify a client. It generates a new document id in nsDocShell::ChannelIntercepted and passes it to the fetch event dispatch procedure where it seems to be planing to use this document id to set FetcheEvent.reservedClientId according to the comments in the code. I'd like to ask and discuss around the possibilities below:

It seems the current client getter methods (e.g. clients.get(id)) are not able to grab a reserved client in fetch event's handlers as only the already-being-created documents (i.e. those set with AddObserver(.."service-worker-get-client"..) ) are observable in the client list. So, this needs changes I guess.
Can Gecko define and use a new object that has the id, creation URL, reference to target browsing context and matched service worker registration instead of using a document id?
(As an aside, I think this environment concept associated with a global is a better fit to represent what's controlled than a document as we have both window client and worker client concepts.)
Can we do the registration matching at the beginning of a navigation (before calling fetch) as proposed in Improve navigate for service worker hooks #1776 (comment)?
Gecko will need to capture the target DocShell's associated Window's client id during a navigation. (Chromium will need to add this as well.)

This patch fixes the client matching flow and exposes reserved client and target client information by adding necessary hooks to fetch and HTML. Related issue: #870 Call flow: #870 (comment) Expected behavior: #870 (comment) Related changes: - HTML: whatwg/html#1776 - Fetch: whatwg/fetch#383 * Work on Handle Fetch is done. Work on the client API is in progress.

jungkees · 2016-10-11T15:21:34Z

Had a chat with @wanderview. We agreed to proceed with this PR. I'll address the comments given so far and the changes outlined in #1776 (comment), and ask review.

Before this patch, we didn't have a concept that holds reserved client's information referenced from a service worker during a navigation (more precisely before the actual global object and its relevant environment settings object for the resulting resource is created). This patch defines an "environment" concept (a supertype object of an environment settings object) that has its id, creation URL and target browsing context such that service workers' FetchEvent hanlder can reference this client information before the corresponding environmet setings object is actually created. The changes include: - Create an environment (a reserved client) during a navigation - Set request's reserved client id such that fetch (and Handle Fetch) can use it - Set request's target client id such that fetch (and Handle Fetch) can use it - Add Window object's active worker internal slot - Add WorkerGlobalObject's active worker internal slot - Add "match service worker registration" logic to attach controller Related issue: w3c/ServiceWorker#870 Call flow: w3c/ServiceWorker#870 (comment) Expected behavior: w3c/ServiceWorker#870 (comment) Related changes: - SW (WIP): w3c/ServiceWorker@df39d89 - Fetch: whatwg/fetch#383

Destroy the reserved environment after setting its states to the corresponding environment settings object that is created and used for the resulting resource.

annevk · 2016-10-19T06:45:40Z

source

+   </li>
+
+   <li><p>Otherwise, set <var>settings object</var>'s <span
+   data-x="concept-environment-id">id</span> to a new unique opaque string, <var>settings


Do we expose these strings somewhere? Perhaps we should define the "opaque" format in that case. Not having that defined turned out to be a problem before (with blob URLs).

Yes, we do. FetchEvent's clientId/reservedClientId/targetClientId and Client's id should return the values of this type. We used the UUID at the beginning but changed to not mandate the UUID type. Here's a discussion on this: w3c/ServiceWorker#647 And also I think we're now doing what you filed here: w3c/ServiceWorker#643

By the way, is there any reference that I can base the work on?

I don't think so. What might be problematic is if folks start assuming the string is UTF-8-safe or even ASCII-safe, depending on what user agents do. Is it an option to return something other than a string that can only be used to check for object identity? E.g., a symbol?

only be used to check for object identity

FWIW, I think this is the only purpose of the id itself. I don't think I'm familiar with what you meant by a symbol, though. Could you explain more? Would it be shown as just a string to JS?

As a Symbol: https://tc39.github.io/ecma262/#sec-symbol-objects. They're somewhat more opaque than strings so might be more suitable here. Not a 100% sure though and it also depends a bit on what happens with the ID.

Symbol doesn't work great across realms, without basically giving it a string name (like "Symbol.iterator") in which case we're back to strings. String is probably the best we'll be able to do.

If these IDs are meant to work across realms, that makes sense.

Yes, IDs are values that can be shared across realms. Symbol.for("stringkey") wouldn't help much here.

annevk · 2016-10-19T06:48:37Z

source

+    <p class="note">The created environment's <span
+    data-x="concept-environment-active-service-worker">active service worker</span> is set in the
+    <span data-x="on-fetch-request-algorithm">handle fetch</span> algorithm during the subsequent
+    fetch flow if its <span data-x="concept-environment-creation-url">creation URL</span> matches a


I'm not sure what subsequent fetch flow is. Do you mean "during the fetch"?

Yes, I meant "during the fetch". Will change it to "during the fetch".

annevk · 2016-10-21T12:31:48Z

source

@@ -3866,10 +3868,14 @@ a.setAttribute('href', 'http://example.com/'); // change the content attribute d
    <p>The following terms are defined in <cite>Service Workers</cite>: <ref spec="SW"></p>

    <ul class="brief">
+     <li><dfn data-x="dfn-active-worker" data-x-href="https://w3c.github.io/ServiceWorker/#dfn-active-worker">active worker</dfn></li>
     <li><dfn data-x="dfn-client-message-queue" data-x-href="https://slightlyoff.github.io/ServiceWorker/spec/service_worker/#dfn-client-message-queue">client message queue</dfn></li>


If we land this PR first before the other PR I wrote where I update links we should also update this link here.

annevk · 2016-10-21T12:34:20Z

source

+  current or potential execution environment. An <span>environment</span> has the following
+  fields:</p>
+
+  <dl>


Since this is a new <dl> I feel like we should remove a whole bunch of whitespace. E.g., when <dd> only contains a single <p> it can be all together. No newline between <dt> and <dd>, only between <dd> and <dt>. What do you think?

Referencing the contributing.md,

If a "block" element contains a single "block" element, do not put it on a newline. Do not indent for anything except a new "block" element.

I think that makes sense for new works and changed them as such.

annevk · 2016-10-21T12:34:54Z

source

+   <dd>
+
+    <p>null or a target <span>browsing context</span> for a <span
+    data-x="navigation-request">navigation request</span>.</p>


This should say "Null" I think. We don't start with a lowercase letter in these <dd>s anywhere else either. Same for below.

annevk · 2016-10-21T12:38:51Z

LGTM with those last few nits fixed. I assume we don't have a testing plan here because this is is about new stuff? That's probably okay.

jungkees · 2016-10-24T01:55:50Z

@annevk, I uploaded a new snapshot having addressed those comments. Please take a look.

annevk · 2016-10-24T07:43:45Z

I'm going to merge this now and then work with @jungkees today/tomorrow to get the corresponding change to Fetch landed as soon as possible. Either way there'll be some broken URLs.

annevk · 2016-10-24T07:46:27Z

Actually, @jungkees could you write a new commit message perhaps?

jungkees · 2016-10-24T10:49:59Z

@annevk, I'm not sure but presume you'll squash all the commits to a single commit, right? I have no idea if I can amend any commit message myself, so I updated the commit message in the OP. Let me know if I can help in another way.

This patch defines request's reserved client id and target client id that are set to a reserved environment's id and the target browsing context's active document's settings object's id respectively during a navigation. The given values are primarily used to provide the reserved client's id and target client's id to service workers' fetch event handlers. Related issue: w3c/ServiceWorker#870 Call flow: w3c/ServiceWorker#870 (comment) Expected behavior: w3c/ServiceWorker#870 (comment) Related changes: - HTML: whatwg/html#1776 - SW (WIP): w3c/ServiceWorker@df39d89

This patch defines the request's reserved client and the target client id. The reserved client of a request is set to an environment or an environment settings object during a navigation or a worker creation request, respectively. The target client id of a request is set to the target browsing context's active document's environment settings object's id during a navigation. The given values are primarily used to provide the reserved client's id and target client's id to service workers' fetch event handlers. This removes the request's target browsing context as the reserved client provides the same information. Related issue: w3c/ServiceWorker#870 Related change: whatwg/html#1776

jungkees · 2016-10-24T12:04:09Z

@annevk @domenic, thanks for having reviewed this PR.

annevk · 2016-10-24T12:05:53Z

Thanks for writing it! 😊

This patch defines the request's reserved client and the target client id. The reserved client of a request is set to an environment or an environment settings object during a navigation or a worker creation request, respectively. The target client id of a request is set to the target browsing context's active document's environment settings object's id during a navigation. The given values are primarily used to provide the reserved client's id and target client's id to service workers' fetch event handlers. This removes the request's target browsing context as the reserved client provides the same information. Related issue: w3c/ServiceWorker#870 Related change: whatwg/html#1776

2b93f9e (PR #1776) introduced a minor markup error (an `ol` nested inside an `ol`). This fixes it.

This makes browsing context's active document a getter, returning browsing context's WindowProxy object's [[Window]] internal slot's associated Document. This removes some service worker text that was added in #1776. Reinstating that in some manner is #2687. Fixes #2657, fixes #2676.

2b93f9e (PR whatwg#1776) introduced a minor markup error (an `ol` nested inside an `ol`). This fixes it.

This makes browsing context's active document a getter, returning browsing context's WindowProxy object's [[Window]] internal slot's associated Document. This removes some service worker text that was added in whatwg#1776. Reinstating that in some manner is whatwg#2687. Fixes whatwg#2657, fixes whatwg#2676.

jungkees mentioned this pull request Sep 13, 2016

Add request's reserved client and target client id whatwg/fetch#383

Merged

domenic assigned annevk Sep 14, 2016

jungkees force-pushed the navigate-fix branch from df5262b to 8019b32 Compare September 27, 2016 02:53

annevk removed their assignment Sep 28, 2016

jungkees force-pushed the navigate-fix branch from 8019b32 to adda326 Compare September 28, 2016 13:26

annevk reviewed Sep 28, 2016

View reviewed changes

domenic requested changes Oct 4, 2016

View reviewed changes

domenic added topic: navigation topic: workers labels Oct 4, 2016

domenic added the do not merge yet Pull request must not be merged per rationale in comment label Oct 7, 2016

domenic assigned jungkees Oct 7, 2016

jungkees added 3 commits October 13, 2016 11:59

Add data-x and fix a wrapping

f33307c

Destroy the reserved environment

f585688

Destroy the reserved environment after setting its states to the corresponding environment settings object that is created and used for the resulting resource.

annevk reviewed Oct 19, 2016

View reviewed changes

Replace destroy language with resetting the id

a3e7049

annevk reviewed Oct 21, 2016

View reviewed changes

Update link; remove whitespace; capitalize letter

0483230

ghost approved these changes Oct 24, 2016

View reviewed changes

annevk merged commit 2b93f9e into whatwg:master Oct 24, 2016

jungkees deleted the navigate-fix branch October 24, 2016 12:04

sideshowbarker added a commit that referenced this pull request Nov 24, 2016

Fix markup in “Initializing a new Document object”

2b36655

2b93f9e (PR #1776) introduced a minor markup error (an `ol` nested inside an `ol`). This fixes it.

sideshowbarker mentioned this pull request Nov 24, 2016

Fix markup in “Initializing a new Document object” #2094

Merged

zcorpan pushed a commit that referenced this pull request Nov 25, 2016

Editorial: Fix markup in “Initializing a new Document object”

0495be1

2b93f9e (PR #1776) introduced a minor markup error (an `ol` nested inside an `ol`). This fixes it.

jungkees mentioned this pull request Mar 24, 2017

clarify creation URL when initial about:blank is replaced #2456

Open

annevk mentioned this pull request May 12, 2017

Centralize setting the active document #2671

Merged

alice pushed a commit to alice/html that referenced this pull request Jan 8, 2019

Editorial: Fix markup in “Initializing a new Document object”

f9fe526

2b93f9e (PR whatwg#1776) introduced a minor markup error (an `ol` nested inside an `ol`). This fixes it.

		@@ -77994,12 +78001,31 @@ console.assert(iframeWindow.frameElement === null);

		<h4>Script settings for browsing contexts</h4>

		<p>When the user agent is required to <dfn>set up an environment</dfn>, given an

Improve navigate for service worker hooks #1776

Improve navigate for service worker hooks #1776

Conversation

jungkees commented Sep 13, 2016 • edited Loading

jungkees commented Sep 13, 2016 • edited Loading

annevk commented Sep 15, 2016

jungkees commented Sep 28, 2016

jungkees commented Sep 28, 2016 • edited Loading

annevk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domenic Oct 4, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jungkees commented Sep 29, 2016

annevk commented Sep 29, 2016 • edited Loading

jungkees commented Sep 29, 2016

jungkees commented Sep 29, 2016 • edited Loading

jungkees commented Sep 30, 2016

jungkees commented Oct 4, 2016

domenic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domenic Oct 4, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jungkees commented Oct 5, 2016 • edited Loading

jungkees commented Oct 5, 2016 • edited Loading

jungkees commented Oct 11, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

annevk commented Oct 21, 2016

jungkees commented Oct 24, 2016

annevk commented Oct 24, 2016

annevk commented Oct 24, 2016

jungkees commented Oct 24, 2016

jungkees commented Oct 24, 2016

annevk commented Oct 24, 2016

jungkees commented Sep 13, 2016 •

edited

Loading

jungkees commented Sep 13, 2016 •

edited

Loading

jungkees commented Sep 28, 2016 •

edited

Loading

domenic Oct 4, 2016 •

edited

Loading

annevk commented Sep 29, 2016 •

edited

Loading

jungkees commented Sep 29, 2016 •

edited

Loading

domenic Oct 4, 2016 •

edited

Loading

jungkees commented Oct 5, 2016 •

edited

Loading

jungkees commented Oct 5, 2016 •

edited

Loading