Bug 119799 – [EFL][WK2] Add pasteboard client to WKContext

• Status: RESOLVED
• Resolution: WONTFIX
• Component: WebKit2

[Michal Pakula vel Rutka]

Add pasteboard client to WKContext for ports that want to handle copy and paste operations in UIProcess.

[Michal Pakula vel Rutka]

Created attachment 208727 [details]
patch

[WebKit Commit Bot]

Attachment 208727 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/CMakeLists.txt', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/GNUmakefile.list.am', u'Source/WebKit2/Target.pri', u'Source/WebKit2/UIProcess/API/C/WKContext.cpp', u'Source/WebKit2/UIProcess/API/C/WKContext.h', u'Source/WebKit2/UIProcess/WebContext.cpp', u'Source/WebKit2/UIProcess/WebContext.h', u'Source/WebKit2/WebKit2.xcodeproj/project.pbxproj']" exit_code: 1
Source/WebKit2/UIProcess/WebContext.h:45:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EFL EWS Bot]

Comment on attachment 208727 [details]
patch

Attachment 208727 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/1448926

[Michal Pakula vel Rutka]

Created attachment 208728 [details]
add missing files

[WebKit Commit Bot]

Attachment 208728 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/CMakeLists.txt', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/GNUmakefile.list.am', u'Source/WebKit2/Target.pri', u'Source/WebKit2/UIProcess/API/C/WKContext.cpp', u'Source/WebKit2/UIProcess/API/C/WKContext.h', u'Source/WebKit2/UIProcess/WebContext.cpp', u'Source/WebKit2/UIProcess/WebContext.h', u'Source/WebKit2/UIProcess/WebPasteboardClient.cpp', u'Source/WebKit2/UIProcess/WebPasteboardClient.h', u'Source/WebKit2/WebKit2.xcodeproj/project.pbxproj']" exit_code: 1
Source/WebKit2/UIProcess/WebPasteboardClient.h:40:  The parameter name "context" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit2/UIProcess/WebPasteboardClient.h:41:  The parameter name "context" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 2 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Michal Pakula vel Rutka]

Created attachment 208729 [details]
fixed style

[Build Bot]

Comment on attachment 208729 [details]
fixed style

Attachment 208729 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/1451874

[Sam Weinig]

I don't think this should go on the WebContext, it seems like it is better suited at the page level.

[Darin Adler]

(In reply to comment #8)
> I don't think this should go on the WebContext, it seems like it is better suited at the page level.

Currently many basic pasteboard operations are done in the platform layer where this no concept of page; I’m not sure exactly how the page level exists at the platform layer, although it’s possible there is a platform-level concept that can end up communicating up through the page.

iOS is an exception because pasteboard operations have to go through the frame delegate, and it’s an exception I am trying to fix.

Sam, could you clarify how you would want to see this work?

[Sam Weinig]

(In reply to comment #9)
> (In reply to comment #8)
> > I don't think this should go on the WebContext, it seems like it is better suited at the page level.
> 
> Currently many basic pasteboard operations are done in the platform layer where this no concept of page; I’m not sure exactly how the page level exists at the platform layer, although it’s possible there is a platform-level concept that can end up communicating up through the page.
> 
> iOS is an exception because pasteboard operations have to go through the frame delegate, and it’s an exception I am trying to fix.
> 
> Sam, could you clarify how you would want to see this work?

Oh right, we do pasteboard through PlatformStrategies.  I'm still not sure a client on WebContext makes sense.  Really, there should be no client in the UIProcess, and ports should just implement the necessary functions in WebContext, like the Mac port does.

[Michal Pakula vel Rutka]

(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > I don't think this should go on the WebContext, it seems like it is better suited at the page level.
> > 
> > Currently many basic pasteboard operations are done in the platform layer where this no concept of page; I’m not sure exactly how the page level exists at the platform layer, although it’s possible there is a platform-level concept that can end up communicating up through the page.
> > 
> > iOS is an exception because pasteboard operations have to go through the frame delegate, and it’s an exception I am trying to fix.
> > 
> > Sam, could you clarify how you would want to see this work?
> 
> Oh right, we do pasteboard through PlatformStrategies.  I'm still not sure a client on WebContext makes sense.  Really, there should be no client in the UIProcess, and ports should just implement the necessary functions in WebContext, like the Mac port does.

EFL framework's copy and paste API is located in Elementary (http://docs.enlightenment.org/auto/elementary/group__CopyPaste.html) which is a widget toolkit for creating applications containing UI widgets. We cannot include elementary into WebKit because elementary depends on WebKit via elm_web and elm_web2 widgets (a simple webviews). Second thing is that you can use Elementary copy and paste API, only in Elementary application (sample at the bottom https://trac.enlightenment.org/e/wiki/Elementary).
Because of the conditions above in EFL port pasteboard cannot be done in WebCore like i.e. in GTK+ or Qt ports or in WebProcess like Mac, but copy and paste has to be delegated to Elementary application and the client in UIProcess is needed for implementation in EFL port.

This patch is split from bug 102484 where now the rest of EFL pasteboard implementation is posted. In previous patch I made the WK client EFL-only, while here I made it common as it possibly may be used by Nix port.

[Michal Pakula vel Rutka]

Created attachment 232941 [details]
new patch

As WebKit Nix was removed from upstream, new version of a patch introduces pasteboard client used only by EFL port.

[Gyuyoung Kim]

Comment on attachment 232941 [details]
new patch

View in context: https://bugs.webkit.org/attachment.cgi?id=232941&action=review

> Source/WebKit2/UIProcess/API/C/efl/WKContextEfl.cpp:43
> +void WKContextSetPasteboardClient(WKContextRef contextRef, const WKContextPasteboardClientBase* wkClient)

When/where is this client called ? I mean EFL needs to register own callbacks by using this function.

> Source/WebKit2/UIProcess/API/C/efl/WKContextEfl.cpp:62
> +        }

Need a new line.

> Source/WebKit2/UIProcess/API/C/efl/WKContextEfl.cpp:70
> +

Unneeded line.

> Source/WebKit2/UIProcess/efl/WebContextEfl.cpp:149
> +    if (!pasteboardClient)

I'm not sure whether we need to check if pasteboardClient is null. I don't see any code which checks if std::unique_ptr argument is null.

[Michal Pakula vel Rutka]

Comment on attachment 232941 [details]
new patch

View in context: https://bugs.webkit.org/attachment.cgi?id=232941&action=review

>> Source/WebKit2/UIProcess/API/C/efl/WKContextEfl.cpp:43
>> +void WKContextSetPasteboardClient(WKContextRef contextRef, const WKContextPasteboardClientBase* wkClient)
> 
> When/where is this client called ? I mean EFL needs to register own callbacks by using this function.

The rest of implementation is in bug 120209. WKContextSetPasteboardClient is called when PasteboardClientEfl is created, which happen during constructing Ewk_Context. Callbacks are registered by ewk_context API.

>> Source/WebKit2/UIProcess/efl/WebContextEfl.cpp:149
>> +    if (!pasteboardClient)
> 
> I'm not sure whether we need to check if pasteboardClient is null. I don't see any code which checks if std::unique_ptr argument is null.

Similar code can be found here: https://trac.webkit.org/browser/trunk/Source/WebKit2/UIProcess/WebContext.cpp#L299. The difference is that setPasteboardClient which is called by WKContextSetPasteboardClient will be used internally and null cannot be passed (bug 120209). I will remove this check.

[Michal Pakula vel Rutka]

Created attachment 235218 [details]
fixes after review

[Gyuyoung Kim]

Comment on attachment 235218 [details]
fixes after review

Can we test this feature using unit test or layout test ?

[Michal Pakula vel Rutka]

(In reply to comment #16)
> (From update of attachment 235218 [details])
> Can we test this feature using unit test or layout test ?

Unit test are added for EWK API in bug 120209, layout test will start to pass after all patches will land.

[Gyuyoung Kim]

Michal, I wonder whether EFL port still needs this patch. If so, please rebase this patch against latest trunk.

[Gyuyoung Kim]

Comment on attachment 235218 [details]
fixes after review

r- because this patch needs to be updated against latest trunk.

[Michael Catanzaro]

Closing this bug because the EFL port has been removed from trunk.

If you feel this bug applies to a different upstream WebKit port and was closed in error, please either update the title and reopen the bug, or leave a comment to request this.