RFR: JDK-8346465 : Add a check in setData() to restrict the update of Built-In ICC_Profiles [v5]
Harshitha Onkar
honkar at openjdk.org
Sat Mar 1 00:00:06 UTC 2025
On Fri, 28 Feb 2025 23:44:40 GMT, Phil Race <prr at openjdk.org> wrote:
>>> Yes, there are. Does any other way create a built-in profile? No, it doesn't as far as I can see.
>>
>> Yes, currently ProfileDeferralInfo is the only way to create built-in profile.
>>
>>> Is this flexibility needed? I'd say, it's not needed… unless there's a very high chance there'll soon be introduced a new build-in ICC profile which is created in another way but ICC_Profile(ProfileDeferralInfo) constructor.
>>
>> As of now we don't need the flexibility but not sure if that is something that we may require or need to consider in future. @prrace Your suggestion ?
>
> So in fact Harshitha had prototyped the constructor mechanism - but it was passing a parameter so wasn't as small a diff as the most recent alternate, because I think it rippled into changing the sub-class constructors as well, which made it less desirable.
> Another particular concern was that it was equating ProfileDeferralInfo to meaning isBuiltIn == true, which is the case today, but it is not a fundamental truth.
> And the proposal here is more direct and you don't have to go follow the bread crumbs to see exactly which profiles have this set to true. So that is why the PR is as it is.
> Neither is "wrong" - they are just different choices based on different perceptions.
>
> But if we can do this with changes to *just this class*, then may be .. and since I see the sub-class constructors (for RGB and Gray) call the same signature super-class constructors, then I think this should work, and it will be OK.
> I tried it on a local repo, and came up with the same changes as suggested and it least builds ..
> So we can do it that way too, but there should be a comment somewhere that ProfileDeferralInfo is only for built-in profiles - and that in fact all built-in profiles should use it.
Thanks for the update on this discussion Phil. I'll update the code to set built-in flag in the constructor and add a comment stating - ProfileDeferralInfo to be used only for built-in profiles.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23606#discussion_r1976131267
More information about the client-libs-dev
mailing list