## 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
changing the application code from identity compare (`==`) to `.equals()`.
The change also adds synchronization on the registry inside the constructor and
`DataFormat::lookupMimeType(String)`.
Additionally, the constructor checks for `null` ids, throwing an
`IllegalArgumentException`. (Quite possibly, we might want to disallow `new
DataFormat((String[])null)` which is currently allowed.
## Compatibility Impact
The applications that relied on identity check ( `if(dataFormat ==
DataFormat.FILES)` ) must be changed to use the `equals()` method. This might
be a rare problem because most of the time the applications declare custom
format as a static constant and use it to access the clipboard / dragboard data.
## Alternatives
One alternative is to deprecate the multi-id constructor and only allow one
mime type per `DataFormat`, also removing the `java.file-list` id. We could
then still keep `Set<String> DataFormat.getIdentifiers()` which will always
return a set with one element.
## Notes
1. This PR presents an alternative solution to
https://github.com/openjdk/jfx/pull/2006 (please refer to some very good
comments there made by @mstr2 and @nlisker )
2. I think there is enough behavioral differences that a CSR is warranted.
---------
- [x] I confirm that I make this contribution in accordance with the [OpenJDK
Interim AI Policy](https://openjdk.org/legal/ai).
-------------
Commit messages:
- nulls
- data format
Changes: https://git.openjdk.org/jfx/pull/2197/files
Webrev: https://webrevs.openjdk.org/?repo=jfx&pr=2197&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8373452
Stats: 136 lines in 3 files changed: 63 ins; 31 del; 42 mod
Patch: https://git.openjdk.org/jfx/pull/2197.diff
Fetch: git fetch https://git.openjdk.org/jfx.git pull/2197/head:pull/2197
PR: https://git.openjdk.org/jfx/pull/2197