RFR: JDK-8346465 : Add a check in setData() to restrict the update of Built-In ICC_Profiles [v5]

Harshitha Onkar honkar at openjdk.org
Mon Feb 24 19:09:56 UTC 2025


On Mon, 24 Feb 2025 18:58:14 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

>> I don't get the reason why it's bad to modify the constructors.
>> 
>> At this time, all the built-in profiles are created with `ProfileDeferralInfo`.
>> 
>> If `ProfileDeferralInfo` is used to create only built-in profiles, then modifying `ICC_Profile(ProfileDeferralInfo pdi)` is enough:
>> 
>> 
>>     ICC_Profile(ProfileDeferralInfo pdi) {
>>         deferralInfo = pdi;
>>         isBuiltIn = true;
>>     }
>> 
>> 
>> And this is a *minimal* change.
>> 
>> When the `isBuiltIn` is final, it's much easier to find where the value gets set.
>
>> At this time, all the built-in profiles are created with `ProfileDeferralInfo`.
>> 
>> If `ProfileDeferralInfo` is used to create only built-in profiles, then modifying `ICC_Profile(ProfileDeferralInfo pdi)` is enough.
> 
> I've updated my branch with this change: `ICC_Profile(ProfileDeferralInfo pdi)` sets the `isBuiltIn` flag to `true`; `ICC_Profile(Profile p)` sets `isBuiltIn` to `false`.
> 
> The diff to ‘master’: https://github.com/openjdk/jdk/compare/2a5d1da3355a4df3109ec42646b5b0cf088b4c2a..a34f16860c2f7d393f4a1fb57f46fba1a68a8412
> 
> Only `ICC_Profile.java` is modified, which is *minimal*. Only several lines are added.
> 
> If there's another way to create a built-in profile in the future, this approach will need to updated to take into account the new way. We'll deal with it at that time in the future.
> 
> The diff to your branch: https://github.com/openjdk/jdk/compare/8da82c10b5be3b92655abef95fd5be0c8b6eaa00..a34f16860c2f7d393f4a1fb57f46fba1a68a8412

There are other way to create a profile - directly loading it form a file (serialization)
`ICC_Profile.getInstance(<path to sRGB.pf or any custom profiles>); `or using the byte array representation of the profile. So the main intention here was not to tie ProfileDeferralInfo with isBuiltIn.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/23606#discussion_r1968253349


More information about the client-libs-dev mailing list