On Fri, 26 Jun 2026 14:57:23 GMT, Andy Goryachev <[email protected]> wrote:
>> ## Summary
>>
>> This PR changes the behavior of `DataFormat` by allowing multiple instances
>> that contain the same set of mime types (ids).
>>
>>
>>
>> ## Problem
>>
>> There seems to be several issues with DataFormat API and implementation
>> discovered during review of the `Clipboard`-related code:
>>
>> 1. `static DataFormat::lookupMimeType(String)` is not thread safe: while
>> iterating over previously registered entries in the `DATA_FORMAT_LIST`
>> another thread might create a new instance (DataFormat L227)
>>
>> 2. `public DataFormat(String...)` constructor might throw an
>> `IllegalArgumentException` if one of the given mime types is already
>> assigned to another `DataFormat`. The origin of this requirement is unclear,
>> but one possible issue I can see is if the application has two libraries
>> that both attempt to create a `DataFormat` for let's say `"text/css"`. Then,
>> depending on the timing or the exact code path, an exception will be thrown
>> for which the library(-ies) might not be prepared. The constructor is also
>> not thread safe.
>>
>> 3. To avoid a situation mentioned in bullet 2, a developer would typically
>> call `lookupMimeType()` to obtain the already registered instance, followed
>> by a constructor call if such an instance has not been found. An example of
>> such code can be seen in webkit/UIClientImpl:299 - but even then, despite
>> that two-step process being synchronized, the code might still fail if *some
>> other* library or the application attempts to create a new instance of
>> DataFormat, since the constructor itself is not synchronized.
>>
>> 4. `DataFormat(new String[] { null })` is allowed but makes no sense!
>>
>> 5. The current implementation uses the `WeakReferenceQueue` which
>> theoretically might, under certain conditions, allow the application to
>> create mismatched `DataFormat`s.
>>
>> Why do we need to have the registry of previously created instances?
>> Unclear. My theory is that the DataFormat allows to have multiple mime-types
>> (ids) - example being `DataFormat.FILES = new
>> DataFormat("application/x-java-file-list", "java.file-list");` - and the
>> registry was added to prevent creation of a `DataFormat` with just one id
>> for some reason.
>>
>> Also, I could not find the origin of the multi-id requirement, or the origin
>> of the `java.file-list` id itself (it is not a valid mime type).
>>
>>
>>
>> ## Solution
>>
>> The proposed solution is to relax the constraint on the constructor to allow
>> multiple instances with the same set of mime types (ids). This might
>> require chan...
>
> Andy Goryachev has updated the pull request incrementally with one additional
> commit since the last revision:
>
> review comments
modules/javafx.graphics/src/main/java/javafx/scene/input/DataFormat.java line
127:
> 125: if (ids == null) {
> 126: this.identifier = Set.of();
> 127: // don't care about registry
You can `return` here and avoid the big `else` branch.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/2197#discussion_r3482775009
