Bug 213290 – [GTK][WPE] Add API to configure and handle service worker registrations to WebKitWebsiteDataManager

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Carlos Garcia Campos]

The default path is always used even for apps setting a base data directory. We should handle WebsiteDataType::ServiceWorkerRegistrations to configure the directory and allow to fetch and clear them.

[Carlos Garcia Campos]

Created attachment 402101 [details]
Patch

[EWS Watchlist]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Michael Catanzaro]

Comment on attachment 402101 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=402101&action=review

> Source/WebKit/ChangeLog:8
> +        The default path is always used even for apps setting a base data directory. We should handle

I suggest we rm -rf the default path each time WebKit is started, and change the default path to something new. Leaking website data outside the application's base data directory is really bad.

[Michael Catanzaro]

i.e. we could do the equivalent of:

$ rm -rf ~/.local/share/webkitgtk/serviceworkers

And change the new default path to: ~/.local/share/webkitgtk/ServiceWorkers.

That way, previously-leaked website data gets cleaned up on upgrade to 2.30. We have a one-time loss of previous service workers, but I guess that's OK?

[Michael Catanzaro]

Comment on attachment 402101 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=402101&action=review

> Source/WebKit/UIProcess/API/glib/WebKitWebsiteDataManager.cpp:222
> +        if (!priv->swRegistrationsDirectory)
> +            priv->swRegistrationsDirectory.reset(g_build_filename(priv->baseDataDirectory.get(), "serviceworkers", nullptr));

Maybe: "ServiceWorkers"?

[Carlos Garcia Campos]

(In reply to Michael Catanzaro from comment #4)
> i.e. we could do the equivalent of:
> 
> $ rm -rf ~/.local/share/webkitgtk/serviceworkers
> 
> And change the new default path to: ~/.local/share/webkitgtk/ServiceWorkers.
> 
> That way, previously-leaked website data gets cleaned up on upgrade to 2.30.
> We have a one-time loss of previous service workers, but I guess that's OK?

But this is the default location, applications not using base data directory would still use that one. I think it was a mistake to use shared locations, and we should probably change all default locations to depend on the app name, but unfortunately we can't remove the default dirs :-(

[Carlos Garcia Campos]

(In reply to Michael Catanzaro from comment #5)
> Comment on attachment 402101 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=402101&action=review
> 
> > Source/WebKit/UIProcess/API/glib/WebKitWebsiteDataManager.c:222
> > +        if (!priv->swRegistrationsDirectory)
> > +            priv->swRegistrationsDirectory.reset(g_build_filename(priv->baseDataDirectory.get(), "serviceworkers", nullptr));
> 
> Maybe: "ServiceWorkers"?

That's consistent with other default dir names that are lowercase like localstorage, but I don't really mind, this is not expected to be ever accessed by users.

[Michael Catanzaro]

(In reply to Carlos Garcia Campos from comment #7)
> That's consistent with other default dir names that are lowercase like
> localstorage, but I don't really mind, this is not expected to be ever
> accessed by users.

It doesn't matter either way. I guess matching the default location is best, though. If we were going to rename the default location and delete the old default -- note I used capitals for the proposed new default default location to distinguish it from the old default location -- then it would be needed for consistency.

(In reply to Carlos Garcia Campos from comment #6)
> I think it was a mistake to use shared locations,
> and we should probably change all default locations to depend on the app
> name, but unfortunately we can't remove the default dirs :-(

I agree it was a mistake. Let's fix this in the GTK 4 API? We can use GtkApplication ID if g_application_get_default() returns non-null, otherwise use g_get_prgname(). Applications that set base data and cache directory would be unaffected.

[Carlos Garcia Campos]

Committed r263204: <https://trac.webkit.org/changeset/263204>

[Michael Catanzaro]

OK, we missed a couple spots. It also has to be added to WebProcessProxy::platformGetLaunchOptions in WebProcessProxyGLib.cpp and bubblewrapSpawn in BubblewrapLauncher.cpp. We should probably add comments to warn about this....

[Carlos Garcia Campos]

(In reply to Michael Catanzaro from comment #10)
> OK, we missed a couple spots. It also has to be added to
> WebProcessProxy::platformGetLaunchOptions in WebProcessProxyGLib.cpp and
> bubblewrapSpawn in BubblewrapLauncher.cpp. We should probably add comments
> to warn about this....

Registrations are handled by the network process.