Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make addEventListener throw for service worker optimization #155

Closed
wants to merge 1 commit into from

Conversation

jungkees
Copy link
Contributor

@jungkees jungkees commented Feb 2, 2016

To optimize the service worker execution behavior, this change makes
addEventListener throw when it is called after the very first
evaluation of the service worker script.

SW issue: w3c/ServiceWorker#718

@jungkees
Copy link
Contributor Author

jungkees commented Feb 2, 2016

Ah.. I have to change a bit. The has ever been evaluated flag is not on service worker but on its script resource. Will push the change.

@@ -1065,6 +1065,15 @@ seen from the definition above, an <a>event listener</a> is a more broad concept
method, when invoked, must run these steps:

<ol>
<li>
<p>If the global object is a {{ServiceWorkerGlobalScope}} object and its associated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be context object's global object.

I guess doing it this way is okay. We don't need a generic solution yet. But once we do, you might have to update service workers again.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed.

@annevk
Copy link
Member

annevk commented Feb 3, 2016

One other thought here, we should do this for removeEventListener too, no?

@jungkees
Copy link
Contributor Author

jungkees commented Feb 4, 2016

One other thought here, we should do this for removeEventListener too, no?

Agreed. Addressed. The point is the event listeners should be deterministically set in the very first run. Removing one in the later runs where there's no chance to add it back seems not reasonable.

I pushed the changes.

set, <a>throw</a> a <code>TypeError</code>. [[!SW]]

<p class="note no-backref">To optimize storing the event types allowed for the service worker and
to avoid non-deterministic additions of the event listeners, the user agent allows the invocation
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/additions of/changes to/
s/the user agent allows the invocation of the method/invocation of the method is allowed/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed.

@annevk
Copy link
Member

annevk commented Feb 4, 2016

The commit message should be updated as well to account for the changes to removeEventListener().

To optimize storing the event types allowed for the service worker and
to avoid non-deterministic changes to the event listeners, this change
makes addEventListener/removeEventListener throw when it is called
after the very first evaluation of the service worker script.
@annevk
Copy link
Member

annevk commented Feb 5, 2016

Thank you!

@annevk
Copy link
Member

annevk commented Feb 5, 2016

(Had to make a few changes for bikeshed. While there I also changed a couple of things to use HTTPS.)

@jungkees
Copy link
Contributor Author

Thanks!

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants