Right now, coordination units are fairly generic property bags partitioned into namespaces. This can make it harder to understand the assumptions around certain properties and what type of base::Value they hold.
It also means that code like https://chromium-review.googlesource.com/c/578482/, which has to traverse all the frame coordination units, has to explicitly typecheck that the kURL property is, in fact, a string--and later that string has to be turned back into a GURL.
If we could just associate a GURL directly, that would at least save the cost of conversion and make the type of the property clear. There's a tradeoff here between tying things too closely to //content, but in practice, we already have an implicit coupling (see the coordination unit types here: https://cs.chromium.org/chromium/src/services/resource_coordinator/public/interfaces/coordination_unit.mojom?rcl=c81ebd0ecc1d3fe0318cf844cfa9d1d546d64eaf&l=14)
I have a suggestion (which I brought up in a GRC meeting today and it sounds like people are onboard with): Changing the GRC properties from generic base::Value to int64. Right now the only need for strings is for the URL, and as the use-case for the URL is for UKM, we can just use UkmSourceId instead (like what's already done in nasko's link). It also avoids the security/performance implications of the generic types, and constrains the GRC properties more towards the intended performance metrics use-cases.
erikchen: wdyt? I'm happy to make this change after your CL lands; it shouldn't affect your UKM logging, I believe.
Also, I should mention: there's several other components to this bug:
- breaking coordination units out into actual subtypes rather than namespacing them with the coordination unit type enum
- providing typed setters (though this is less important to me if we do limit properties to integer types) and have concrete subtypes
Comment 1 by nasko@chromium.org
, Jul 26 2017