[webkit-reviews] review granted: [Bug 177943] [GTK][WPE] Add API to configure and enable resource load statistics : [Attachment 396270] Patch

[bugzilla-daemon at webkit.org]

Michael Catanzaro <mcatanzaro at gnome.org> has granted Carlos Garcia Campos
<cgarcia at igalia.com>'s request for review:
Bug 177943: [GTK][WPE] Add API to configure and enable resource load statistics
https://bugs.webkit.org/show_bug.cgi?id=177943

Attachment 396270: Patch

https://bugs.webkit.org/attachment.cgi?id=396270&action=review




--- Comment #8 from Michael Catanzaro <mcatanzaro at gnome.org> ---
Comment on attachment 396270
  --> https://bugs.webkit.org/attachment.cgi?id=396270
Patch

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

> Source/WebKit/UIProcess/API/glib/WebKitWebsiteDataManager.cpp:735
> + * webkit_website_data_manager_set_resource_load_statistics_enabled:

I'm OK with this, but it's important to understand that the
WebsiteDataStore-level API is a bit misnamed. It controls all of ITP, not just
resource load statistics. E.g. this will probably cause third-party cookies to
be blocked regardless of the WebKitCookieAcceptPolicy(?). It will certainly
cause Referer headers to be stripped. Etc. A less-misleading name might be:
webkit_website_data_manager_set_intelligent_tracking_prevention_enabled(). But
then it would feel more at home on the WebKitWebContext, rather than
WebKitWebsiteDataManager. But that would be the wrong place, because internally
it really is a property of the WebsiteDataStore, not the WebProcessPool.

Conclusion: *shrug*. Maybe we could keep the current name but document that it
has additional effects in addition to enabling statistics tracking? I don't
know, just something to think about.

I would investigate the impact on cookie policy regardless. We might want to
use a g_warning() if the WebKitCookiePolicy is inconsistent with ITP?