Work in progress towards implementing the async Clipboard API.
<rdar://problem/56038126>
Created attachment 380333 [details] IDL files and API stubs.
I’m working on upstreaming the existing web platform tests as well.
Created attachment 380334 [details] Fix CMake builds
Created attachment 380339 [details] Adjust WK1 test expectations.
Comment on attachment 380339 [details] Adjust WK1 test expectations. View in context: https://bugs.webkit.org/attachment.cgi?id=380339&action=review You can fix the GC issue in a separate patch but please be sure to file a bug before landing this so that we won't forget to fix it. > Source/WebCore/ChangeLog:9 > + Adds IDL support for the async clipboard API (with the exception of delayed item generation interfaces, which Nit: Adds IDL. It's not like IDL is a feature to support.x > Source/WebCore/ChangeLog:11 > + Please add a link to the spec. > Source/WebCore/ChangeLog:18 > + * Modules/clipboard/Clipboard.cpp: Can we use Modules/async-clipboard/ instead? It's a bit confusing as is since we already have the support for sync clipboard API support. > Source/WebCore/Modules/clipboard/Clipboard.h:58 > + void refEventTarget() final { ref(); } > + void derefEventTarget() final { deref(); } Why do we need these?? > Source/WebCore/Modules/clipboard/Clipboard.idl:32 > + EnabledBySetting=AsyncClipboardAPI, > +] interface Clipboard : EventTarget { Probably need GenerateIsReachable=ReachableFromDOMWindow or something. Otherwise, the wrapper of this object can be collected. > Source/WebCore/Modules/clipboard/ClipboardItem.idl:40 > + ImplementationLacksVTable > +] interface ClipboardItem { We need to start thinking about GC model of this object. You'd probably need to make this item's Clipboard an opaque root if it's associated with one. Alternatively, add GenerateIsReachable=ReachableFromDOMWindow and make it return DOMWindow* when it's associated with Clipboard. > Source/WebCore/Modules/clipboard/NavigatorClipboard.h:43 > + static Clipboard* clipboard(Navigator&); > + Clipboard* clipboard(); Return RefPtr instead? > LayoutTests/platform/ios-wk1/TestExpectations:19 > +# Async Clipboard API is not supported in WebKit1. > +editing/clipboard [ Skip ] > + Can we add the support in a subsequent patch? It's not great to have discrepancy between WK1 and WK2 if we can avoid it.
Comment on attachment 380339 [details] Adjust WK1 test expectations. View in context: https://bugs.webkit.org/attachment.cgi?id=380339&action=review >> Source/WebCore/ChangeLog:9 >> + Adds IDL support for the async clipboard API (with the exception of delayed item generation interfaces, which > > Nit: Adds IDL. It's not like IDL is a feature to support.x Fixed. >> Source/WebCore/ChangeLog:11 >> + > > Please add a link to the spec. Added. >> Source/WebCore/ChangeLog:18 >> + * Modules/clipboard/Clipboard.cpp: > > Can we use Modules/async-clipboard/ instead? > It's a bit confusing as is since we already have the support for sync clipboard API support. Sure! Renamed to async-clipboard. I also renamed the tests in the editing directory from clipboard to async-clipboard to match. >> Source/WebCore/Modules/clipboard/Clipboard.h:58 >> + void derefEventTarget() final { deref(); } > > Why do we need these?? Clipboard is an EventTarget, and these are pure virtual methods on EventTarget. It looks like all other event target subclasses override refEventTarget() and derefEventTarget(), as well. >> Source/WebCore/Modules/clipboard/Clipboard.idl:32 >> +] interface Clipboard : EventTarget { > > Probably need GenerateIsReachable=ReachableFromDOMWindow or something. > Otherwise, the wrapper of this object can be collected. Good catch! I added `GenerateIsReachable=ReachableFromNavigator` here to ensure that the wrapper object is kept alive by the Navigator, and then added a new layout test to verify that the wrapper is kept alive between GC passes. >> Source/WebCore/Modules/clipboard/ClipboardItem.idl:40 >> +] interface ClipboardItem { > > We need to start thinking about GC model of this object. > You'd probably need to make this item's Clipboard an opaque root if it's associated with one. > Alternatively, add GenerateIsReachable=ReachableFromDOMWindow > and make it return DOMWindow* when it's associated with Clipboard. Good call. As we chatted about in person, I’ll make this reachable from navigator, with the intention of returning the Clipboard’s navigator if the item is associated with a Clipboard (and otherwise, return null). Since all APIs are stubs at the moment and there’s no way to get items associated with a Clipboard yet, this will just return null for now, until the subsequent patch that adds support for Clipboard.read(). >> Source/WebCore/Modules/clipboard/NavigatorClipboard.h:43 >> + Clipboard* clipboard(); > > Return RefPtr instead? Done! >> LayoutTests/platform/ios-wk1/TestExpectations:19 >> + > > Can we add the support in a subsequent patch? It's not great to have discrepancy between WK1 and WK2 if we can avoid it. Okay — I filed https://bugs.webkit.org/show_bug.cgi?id=202654 to track this effort, and referenced the bug in the test expectations.
Created attachment 380374 [details] Incorporate review feedback + rebase on trunk
Created attachment 380376 [details] Fix WPE build
Created attachment 380378 [details] Try to fix GTK/WPE build again
Created attachment 380386 [details] Try to fix GTK/WPE build (3)
Created attachment 380401 [details] Rebase on trunk again
Comment on attachment 380401 [details] Rebase on trunk again Clearing flags on attachment: 380401 Committed r250824: <https://trac.webkit.org/changeset/250824>
All reviewed patches have been landed. Closing bug.