[Webkit-unassigned] [Bug 186345] [WPE] Add a MiniBrowser and use it to run WebDriver tests

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=186345

Michael Catanzaro <mcatanzaro at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mcatanzaro at igalia.com

--- Comment #4 from Michael Catanzaro <mcatanzaro at igalia.com> ---
Comment on attachment 342043
  --> https://bugs.webkit.org/attachment.cgi?id=342043
Patch

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

This seems like a great idea. I've reviewed the CMake bits and the scripts, not the code.

> Source/cmake/FindWaylandProtocols.cmake:48
> +macro(_WP_GET_PKGCONFIG_VAR _outvar _varname)
> +    execute_process(
> +        COMMAND ${PKG_CONFIG_EXECUTABLE} --variable=${_varname} wayland-protocols
> +        OUTPUT_VARIABLE _result
> +        RESULT_VARIABLE _null
> +    )
> +
> +    if (_null)
> +    else ()
> +        string(REGEX REPLACE "[\r\n]" " " _result "${_result}")
> +        string(REGEX REPLACE " +$" ""  _result "${_result}")
> +        separate_arguments(_result)
> +        set(${_outvar} ${_result} CACHE INTERNAL "")
> +    endif ()
> +endmacro(_WP_GET_PKGCONFIG_VAR)

Did you write this yourself? Use pkg_get_variable() instead. See /usr/share/cmake/Modules/FindPkgConfig.cmake.

> Source/cmake/FindWaylandProtocols.cmake:52
> +  _wp_get_pkgconfig_var(WAYLAND_PROTOCOLS_DATADIR "pkgdatadir")
> +  find_program(WAYLAND_SCANNER NAMES wayland-scanner)

I think you might need to call mark_as_advanced() to hide WAYLAND_PROTOCOLS_DATADIR and WAYLAND_SCANNER_NAMES from the list of build options... not sure. Run 'cmake -LH' in your build directory and scan the list of variables. If they show up, then it's needed.

> Source/cmake/OptionsWPE.cmake:115
> +  find_package(EGL)
> +  find_package(OpenGLES2)

I don't understand, why are these not required? Surely it's not going to link successfully without them? I would expect some code to handle the case where they're missing if not marked as REQUIRED.

> Tools/MiniBrowser/wpe/CMakeLists.txt:7
> +    ${DERIVED_SOURCES_MINIBROWSER_DIR}/xdg-shell-unstable-v6-protocol.c

Hm, xdg-shell is stable now, do you know how hard it would be to switch to the stable version of the protocol? Probably not much? This will start crashing once compositors drop support for v6.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180606/1585a867/attachment-0001.html>