Bug 173770

Summary: [WPE] Add env var WPE_USE_HEADLESS_VIEW_BACKEND
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: WPE WebKitAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, buildbot, clopez, mcatanzaro, zan
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 178655    
Bug Blocks: 173772, 179817    
Attachments:
Description Flags
Patch
zan: review-
New patch mcatanzaro: review+

Description Carlos Garcia Campos 2017-06-23 09:19:20 PDT
To automatically create the views with the headless view backend. Right now the headless view backend is only used by WTR using WKViewCreateWithViewBackend. However, we want to use the headless backend to run the unit tests too. We still don't have glib API to create a WebKitWebView with a custom WPE view backend, and even in that case, using a different webkit_web_view_new function in the tests would require adding ifdefs in every single case since GTK+ doesn't have such AI, of course. Also, we would need to find a place to share the headless view backend implementation. Using an environment variable we can simply add a headless driver to webkitpy, used by default, that ensures the WPE_USE_HEADLESS_VIEW_BACKEND variable is present in the environment. Then, we can move the headless view backend to WKWPE and use it in the View constructor when no backend is given and the variable WPE_USE_HEADLESS_VIEW_BACKEND is present and enabled. This will make unit tests work automatically once we add run-wpe-tests scripts, and also allows to run the layout tests in X11 without having to unset environment variables.
Comment 1 Carlos Garcia Campos 2017-06-23 09:24:14 PDT
Created attachment 313722 [details]
Patch
Comment 2 Build Bot 2017-06-23 09:26:11 PDT
Attachment 313722 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/API/wpe/HeadlessViewBackend.h:28:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 14 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Carlos Garcia Campos 2017-06-23 09:33:56 PDT
It failed to build in EWS, it works here, maybe it needs a clean build
Comment 4 Zan Dobersek 2017-06-26 02:46:51 PDT
Comment on attachment 313722 [details]
Patch

We can't add a WPEBackend-mesa dependency to libWPEWebKit. I didn't account for that.

Same approach should be taken as with WKTR, creating a custom headless view backend.
Comment 5 Carlos Garcia Campos 2017-06-26 02:50:24 PDT
(In reply to Zan Dobersek from comment #4)
> Comment on attachment 313722 [details]
> Patch
> 
> We can't add a WPEBackend-mesa dependency to libWPEWebKit. I didn't account
> for that.

Ah :-( Could you elaborate? what's the problem of making libWPEWebKit link to WPEBackend-mesa?

> Same approach should be taken as with WKTR, creating a custom headless view
> backend.

What do you mean? Using a different headless view backend that doesn't need WPEBackend-mesa?
Comment 6 Zan Dobersek 2017-06-26 02:59:48 PDT
(In reply to Carlos Garcia Campos from comment #5)
> (In reply to Zan Dobersek from comment #4)
> > Comment on attachment 313722 [details]
> > Patch
> > 
> > We can't add a WPEBackend-mesa dependency to libWPEWebKit. I didn't account
> > for that.
> 
> Ah :-( Could you elaborate? what's the problem of making libWPEWebKit link
> to WPEBackend-mesa?
> 

Because other platforms can't use that, so it can't be depended on universally.

> > Same approach should be taken as with WKTR, creating a custom headless view
> > backend.
> 
> What do you mean? Using a different headless view backend that doesn't need
> WPEBackend-mesa?

We still use WPEBackend-mesa as a reference backend implementation, and depend on it for testing. There's no generic exportable interface, but it might make sense to look into creating one. But for now we have to leverage WPEBackend-mesa, just not as a hard-coded dependency of the main libWPEWebKit.so library.

What could be used for unit tests is basically a pass-through wpe_mesa_view_backend_exportable_dma_buf_client implementation. The export_dma_buf callback would only call wpe_mesa_view_backend_exportable_dma_buf_dispatch_frame_complete() and wpe_mesa_view_backend_exportable_dma_buf_dispatch_release_buffer(), and not bother with any other operation like we do in the WKTR HeadlessViewBackend.
Comment 7 Carlos Garcia Campos 2017-06-26 03:05:12 PDT
(In reply to Zan Dobersek from comment #6)
> (In reply to Carlos Garcia Campos from comment #5)
> > (In reply to Zan Dobersek from comment #4)
> > > Comment on attachment 313722 [details]
> > > Patch
> > > 
> > > We can't add a WPEBackend-mesa dependency to libWPEWebKit. I didn't account
> > > for that.
> > 
> > Ah :-( Could you elaborate? what's the problem of making libWPEWebKit link
> > to WPEBackend-mesa?
> > 
> 
> Because other platforms can't use that, so it can't be depended on
> universally.

I see.

> > > Same approach should be taken as with WKTR, creating a custom headless view
> > > backend.
> > 
> > What do you mean? Using a different headless view backend that doesn't need
> > WPEBackend-mesa?
> 
> We still use WPEBackend-mesa as a reference backend implementation, and
> depend on it for testing. There's no generic exportable interface, but it
> might make sense to look into creating one. But for now we have to leverage
> WPEBackend-mesa, just not as a hard-coded dependency of the main
> libWPEWebKit.so library.

Understood.

> What could be used for unit tests is basically a pass-through
> wpe_mesa_view_backend_exportable_dma_buf_client implementation. The
> export_dma_buf callback would only call
> wpe_mesa_view_backend_exportable_dma_buf_dispatch_frame_complete() and
> wpe_mesa_view_backend_exportable_dma_buf_dispatch_release_buffer(), and not
> bother with any other operation like we do in the WKTR HeadlessViewBackend.

I'm not familiar enough with WPE yet to understand this, thought :-P
Comment 8 Carlos Garcia Campos 2017-10-03 00:32:45 PDT
Oops, sorry, yusuke and Caio I added you to the wrong bug :-P
Comment 9 Carlos Garcia Campos 2017-11-17 03:22:23 PST
Created attachment 327162 [details]
New patch

This won't apply because it depends on the patch adding the new API.
Comment 10 Michael Catanzaro 2017-11-17 08:51:20 PST
Comment on attachment 327162 [details]
New patch

Looks good to me, but maybe you should wait for Zan as this obviously does not implement his suggestion regarding the wpe_mesa_view_backend_exportable_dma_buf_client... thing.
Comment 11 Carlos Garcia Campos 2017-11-20 01:48:04 PST
Committed r225045: <https://trac.webkit.org/changeset/225045>