Open Bug 1450070 Opened 6 years ago Updated 11 months ago

Application panel: add button to force Update

Categories

(DevTools :: Application Panel, enhancement, P3)

enhancement

Tracking

(Not tracked)

People

(Reporter: jdescottes, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(4 files)

Follow up to Bug 1445197.

The Application panel will show a button to unregister a service worker. We should also have a button to force an update of a service worker. See mockups at https://projects.invisionapp.com/share/ZQGIRLJ2KBU#/screens/287275492_Artboard
I am trying to add an "Update" button in the application panel, that should trigger the update of a service worker registration (ie https://developer.mozilla.org/en-US/docs/Web/API/ServiceWorkerRegistration/update).

I tried using the same approach as in about:serviceworkers here, but the update button available there triggers a "soft update" (https://searchfox.org/mozilla-central/source/dom/interfaces/base/nsIServiceWorkerManager.idl#141) which is not followed by a new installation of the service worker. Is there another way to trigger a "real" update on a service worker registration? Or should we add new APIs to ServiceWorkerManager or ServiceWorkerRegistrationInfo?
Flags: needinfo?(bkelly)
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
I think we probably need to expose a skipWaiting() on the nsIServiceWorkerInfo.  That would allow devtools to trigger an update and then set the skipWaiting() on the installing worker.  This would then fully activate.  Once the activation completes you could refresh the tab.

Currently skipWaiting() is only available within the ServiceWorkerGlobalScope on the sw thread.
Flags: needinfo?(bkelly)
Mass move to the new application panel component.
Component: Developer Tools → Developer Tools: Application Panel
Depends on: 1459581, 1450067
I will pick this back up when the blocking bugs have landed, but the current state is that we have:
- new actor method that "simulates" an update by doing a propagateSoftUpdate/attachDebugger/detachDebugger
- new button to trigger the update
- end to end test
Based on your suggestions, I tried to expose skipWaiting in the patch attached here. 

I don't think this is exactly what I need here, because the current issue is that after calling `propagateSoftUpdate`, installation is not triggered. 

I don't necessarily want to force skipWaiting as part of the update anyway, we should rather surface it as a dedicated control, for instance a "skip waiting" button or link displayed only when the current service worker is waiting.

I need an API that would do the exact same as calling "update()" on a service worker registration. Right now doing propagateSoftUpdate then attachDebugger then detachDebugger seems to "emulate" that (except it tries to start the worker even if there was no update). But that is really hacky. Should I try to implement a "propagateUpdate" that would perform the update of the service worker registration?
Flags: needinfo?(bkelly)
You should not be calling propagateSoftUpdate, IMO.  We probably want you to be able to trigger an update similar to what navigations trigger, which is ServiceWorkerRegistrationInfo::MaybeScheduleUpdate().  Of course, that uses a 1 second delay that may not be what you want.

Honestly I think it would be easier to implement all this after the e10s work is complete.  I'm worry you are going to implement something that needs to be re-done shortly.  For example, all the "propagate" stuff is going to go away.

Also, what process are you accessing the nsIServiceWorker* interface is it the parent process?  These interfaces are going to go away in the content processes soon.
Flags: needinfo?(bkelly)
Thanks for the feedback. 

> We probably want you to be able to trigger an update similar to what navigations trigger, 
> which is ServiceWorkerRegistrationInfo::MaybeScheduleUpdate().  Of course, that uses a 1 
> second delay that may not be what you want.

A one second delay is not a big issue for now, we could run with that.

> Honestly I think it would be easier to implement all this after the e10s work is complete.
> I'm worry you are going to implement something that needs to be re-done shortly.  

Sure, let's block this on Bug 1231208 then. I will re-upload cleaned up versions of the UI and test patches, but will leave the actor patch for later.

> For example, all the "propagate" stuff is going to go away.

We already rely on propagate* to unregister service workers for now: 
https://searchfox.org/mozilla-central/rev/da499aac682d0bbda5829327b60a865cbc491611/devtools/server/actors/worker.js#387

That's one spot that needs to be updated when the refactor lands.

> Also, what process are you accessing the nsIServiceWorker* interface is it the parent process?
> These interfaces are going to go away in the content processes soon.

Normally from the parent process, but to start service workers (with attach/detachDebugger), we also broadcast a message to all processes: https://searchfox.org/mozilla-central/rev/da499aac682d0bbda5829327b60a865cbc491611/devtools/server/actors/worker.js#359-367

That will also need to be updated.
Product: Firefox → DevTools
Priority: P3 → P2
unassigning since we are not working on this at the moment.
Assignee: jdescottes → nobody
Status: ASSIGNED → NEW
Priority: P2 → P3

(Switched blocking to parent intercept being enabled)

Depends on: 1588154
No longer depends on: ServiceWorkers-e10s
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: