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.
Created attachment 402101 [details] Patch
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
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.
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?
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"?
(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 :-(
(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.
(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.
Committed r263204: <https://trac.webkit.org/changeset/263204>
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....
(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.