On Mon, 29 Jun 2026 18:53:36 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 with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains five additional
> commits since the last revision:
>
> - Merge branch 'master' into 8373452.dataformat.2
> - review comments
> - review comments
> - nulls
> - data format
re: disjoint
`Collections.disjoint()` returns true with two empty sets.
And you are right, it's rather unlikely for any DataFormat (except FILES) to
contain more than one element, so we are basically bikeshedding.
The main question is whether this PR solves the problem sufficiently, or should
we ban multi-id data formats altogether? I could not find an origin or any use
of `java.file-list` id, it's not even a valid mime type.
-------------
PR Comment: https://git.openjdk.org/jfx/pull/2197#issuecomment-4845055700
