## 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

Reply via email to