Only allows instrument.set() in active service worker

[romandev]

Like other service worker family features, we should only allow
instrument.set() in active service worker.

This fixes #223 issue.

---

Preview | Diff

[marcoscaceres]

The proposed text seems fine, but does this also apply to things like IndexedDB? Is there a pointer to an issue describing why we are adding this restriction? Just wondering what the rationale is.

[ianbjacobs]

@romandev, we discussed this briefly at the editor call today and would like to request additional explanation of why you believe this change address issue #223. Thanks!

[romandev]

Thank you for reviews and I'm sorry for delayed reply.
I was on long vacations last week.

This pull request is initiated from #223. The problem would not have happened before (bfe952b).
Because there was a spec text that it should wait for waiting/installing worker to become its active worker. We guessed the spec text was removed mistakenly. So, I'd like to restore the spec text from the large change.

One solution is navigator.serviceWorker.ready but it's not perfect.
For examples, there are the following issues:

• What happens when calling register()/set() in redaundant service worker?
• What happens in case of multiple registrations?

I think this patch can resolve them.

[romandev]

The proposed text seems fine, but does this also apply to things like IndexedDB?

If my understanding is correct, IndexedDB storage is not associated with a service worker.
We might compare it with CacheStorage, BackgroundSync, and Push rather than IndexedDB.

The following link will be helpful to us:

• Wait for an active worker on .register (consistency with push API) WICG/background-sync#101

[marcoscaceres]

If my understanding is correct, IndexedDB storage is not associated with a service worker.

Ah, got it... it's "exposed" in a service worker, but not associated with it.

Right - I'm my mind the instruments were not bound to the service worker... that's again the discussion around storage and credentials.

[rsolomakhin]

@romandev: Shouldn't all the methods (.get(), .set(), .delete(), .has()) work only in an active service worker?

[romandev]

@rsolomakhin PTAL

I think other methods(get(), delete() and so on) no needs the check because instruments storage always empty before calling set().

get() will resolve undefined.
delete() will resolve false.
keys() will resolve empty array.
has() will resolve false.
clear() is unmeaningful because the storage is already empty.

[romandev]

FYI, when you wanna merge this or other PRs, according to #156, it's better to use a squash commit. (No merge commits)

I think merge commits make it difficult for us to track our commit history. :)