RFR: 8233955: VM crashes if more than one file are added to ClipboardContent via drag and drop [v2]

Lukasz Kostyra lkostyra at openjdk.org
Fri May 12 15:05:01 UTC 2023


> Crashes started happening due to macOS DnD API change from macOS 10.14 onwards. 10.14 incrodues some [DnD constrains](https://developer.apple.com/documentation/macos-release-notes/appkit-release-notes-for-macos-10_14#Drag-and-Drop) which our DnD code happened to trigger on some occasions.
> 
> Our code used deprecated `dragImage` API which since 10.14 had new requirement - each NSPasteboard item had to have a corresponding drag image. Not meeting this constraint raised an exception, which crashed the app. Since there was no possibility to add more drag images to `dragImage` API (it only took one NSImage as parameter) the code had to be rewritten - as such I upgraded it to new DnD API utilizing NSDraggingSession and related protocols.
> 
> One side-effect of new DnD API is that it now modifies NSPasteboard for us - previous behavior was more "separated" from user code perspective (write items to pasteboard -> initiate a drag via `dragImage`). Manually updating NSPasteboard before calling `beginDraggingSessionWithItems` raised another exception related to NSPasteboard already having DnD-ed elements inside it. Some system tests, however, relied on that behavior and writing to NSPasteboard (`MacPasteboardShim.java` used in some tests creates a `Clipboard.DND` for test purposes). Since this path is in tests I assumed this behavior should stay and tried to make it as close to working as possible. Tests (including those using `MacPasteboardShim`) pass after my changes.
> 
> Additionally, added a new manual test based on `DndTest.java` test which creates two temporary files and allows for testing faulty behavior.

Lukasz Kostyra 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 10 additional commits since the last revision:

 - GlassViewDelegate: Return to deprecated *PboardType symbols
   
   New NSPasteboardType* symbols were introduced starting 10.13, and we're
   targetting 10.12. This deprecation should probably be resolved once we target
   macOS 10.13+.
   
   Old NS*PboardType symbols do not have an equivalent of NSPasteboardTypeFileURL,
   so this branch was removed.
 - GlassPasteboard: Correct macOS version information in comments
   
   New DnD API was introduced starting macOS 10.7, not 10.14
 - GlassView: Remove deprecated draggingSourceOperationMask
 - DndTest: Update instructions
   
   On Mac to enable moving you must press Cmd, not Shift.
 - Restore logic responsible for Cmd key support
 - Merge branch 'master' into dnd_multiple_items-JDK-8233955
 - Add DnD Multiple File manual test
 - Fix preview position; cleanup code
 - Add image previews and finish DnD code
   
   Fixes issues with tests caused by first commit.
   
   Removes old code used as placeholder.
 - Migrate DnD native code to DraggingSession API
   
   Previous implementation used dragImage call which is deprecated since macOS 10.14. Additionally,
   10.14 introduced a restriction not allowing for more than one drag item in the Pasteboard. This
   change fixes crashes caused by old API use when DnD-ing more than one item.

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

Changes:
  - all: https://git.openjdk.org/jfx/pull/1089/files
  - new: https://git.openjdk.org/jfx/pull/1089/files/939a0d2f..fa42b1b1

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx&pr=1089&range=01
 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=1089&range=00-01

  Stats: 10564 lines in 133 files changed: 8844 ins; 1440 del; 280 mod
  Patch: https://git.openjdk.org/jfx/pull/1089.diff
  Fetch: git fetch https://git.openjdk.org/jfx.git pull/1089/head:pull/1089

PR: https://git.openjdk.org/jfx/pull/1089


More information about the openjfx-dev mailing list