This PR has two commits, one for moving X11 dependency and another for actual MUS support implementation.
Please see comments inline. Also run the tools/fix_style.sh script to fix style issues.
Thanks for review, I will update those.
I have addressed comments in this patch. Could you looks again.
Regarding manifest, I have used the older manifest file with name changed and kept under libcef/common/service_manifests.
Hi Marshall, Would you like to look it again.
Hi Marshall, The patch has been quite cleaned up, it would be great if you could review again
@magreenblatt : Apologies if you are very busy, but I wanted this private PR to get reviewed , It would be great if you could look again anytime soon.
Thank you for your patience with the review process. This PR is looking pretty good. Have you tested it with a normal Linux build to make sure nothing has broken there?
Once the merge conflicts and comments are addressed I think we can merge this to master. I'm working on a Chromium update currently so there might be one more round of merge conflict resolution before that point.
Thanks for showing interest to merge it,
I have addressed all your comments , rebased with latest cef to avoid conflicts, and also did some more cleanup, like removing extra manifests.
For all the patchset I have been testing with linux build on x11 to avoid any regression,
I think it would be good to have final look from again
@magreenblatt : I revisited the patch and updated it as per latest CEF, since the manifest stuffs are already applied with chromium rebase, now this patch requires lesser changes. With this changes Ozone build (with use_ozone) and X11 (use_x11/default) builds are seperated by compile time flag. As of now ozone build works but running CEF for different ozone will work once external window mode is merged in chromium which is being seperately developed by igalia folks (here)and now is in alpha testing.
Locally I tested this patch with ozone-wayland-dev branch and I can launch CEF for wayland and x11 platform seperately and I verified that it doesn't break any default CEF X11.
@magreenblatt : Would you like revisit the patch.
I have updated this long pending patch, which was blocked due to chromium official repository content layer doesn’t supported ozone without Mus, Now Mus has been deprecated
and content module supports ozone build for different platform, which allows to directly build CEF with this patch for different ozone platform.
@magreenblatt : I would appreciate if you could review it and take it forward.
@magreenblatt : I have applied review comments, Would you like to revisit again
@magreenblatt : updated the code which now generate cef_config.h exposing the CEF build configuration. This cef_config.h could be used to expose build related configuration to application. Let me know if anything need to be taken care.
Overall looks good. Please add /include/cef_config.h to the .gitignore file. Also see comments inline.
We have a use-case where an X11-less version of CEF would be really useful. So I did some testing of this branch (rebased on master) in the last couple of days, with some help from Santosh, and ran into a few minor build issues, which I think we should address.
tests/shared/browser/main_message_loop_external_pump_linux.cc has a dependency on glib.h. Since the ozone-path doesn’t pull in gtk-2, this test would fail to compile. I was able to solve this by simply adding glib-2 as a dependency explicitly.
Running cefsimple in debug mode reached an assertion in window_view.cc, which I believe shouldn’t be necessary? Simply commenting that assertion out works with for ozone-enabled builds.
The newly pulled in ui_controls_factory_ozone.cc caused linker errors. I had to pull in a utility class from mus to fix this. Do we want that, or just remove those test files all together? (see inline comment as well).
Finally, there are a few issues with makeDistrib.py for ozone-enabled builds. For those I don’t have a good fix due to my lack of knowledge of CEF (any suggestions?):
the generated cef_config.h doesn’t seem to get copied, breaking builds of e.g. cefsimple inside the generated distribution.
apparently the generated distribution bundle still attempts to build cefclient as well, which of course fails now since excluded from the main tree when not using X11.
Finally, thanks for putting all this work into this, being able to use CEF w/o X11 is really useful. I would love to see this being merged back (and maybe released?), happy to help where I can!
@magreenblatt : I rebased path with latest and resolved build issue observed in latest. Would you like to look at it again
Please see some minor style issues commented inline.
Build CEF with the following GN_DEFINES: use_ozone=true use_intel_minigbm=true (varies for gpu type)
Run as: $ ./out/Release/cefsimple --use-views --ozone-platform=wayland
@santoshmahto1906 Can you attach a screenshot to issue #2296 so we can see how cefsimple looks when running in this mode? Is views required, or can OSR also be used with this build (provided the client implements the proper opengl handling)?
@hfink_daqri Are all of your build issues resolved?
Still seeing this error (as mentioned above):
../../cef/tests/shared/browser/main_message_loop_external_pump_linux.cc:11:10: fatal error: 'glib.h' file not found
1 error generated.
After working around the glib issue with the commit I linked above, I am running into this linker error:
Looks like tests/ceftests/run_all_unittests.cc uses XSetErrorHandler in an unconditional way
I have updated the PR with latest commit which address the issue faced by hfink, build and tested locally, looks fine. I am attaching screenshot to issue #2296.
@hfink_daqri : I have fixed those build issue in ceftests in latest commit to this PR. Thanks for reporting.
@magreenblatt : View mode is required for ozone build to work and OSR mode will not help here since as of now some bits of OSR files depends on X11 types(cursor, font related, see render_widget_host_view_osr_linux.cc) which is put under flag for ozone build.
thanks, santosh. I can confirm that I can now build cef without the aforementioned errors. I went one step further, creating a package via executing ./make_distrib.sh --ninja-build --x64-build (it's in chromium/src/cef/tools, also see these instructions). While creating the package succeeds (after executing ninja libEGL.so in the respective build-dirs before, some how the packaging step needs those libs), there are still issues building the sources / tests that are accompanying the binary package itself. If I cd into the distribution package, execute cmake -H. -Bbuild -G Ninja -DCMAKE_BUILD_TYPE=Release, then ninja -C build, I get errors during the attempt of compiling cefclient. I believe, for ozone-enabled packages, cefclient shouldn’t be built at all, should it? Can we exclude this in the ozone-enabled package? Here is the output of the build error: pastebin-link.
You bring up a good point here. We probably shouldn’t include cefclient in this distribution.
@magreenblatt : The PR is up to date handling all current issues. Is it possible for you to look again in this?
Please see comments inline (some minor style issues).
@Marshall Greenblatt : I updated the patch to fix distribution issue, Now binary distribution for ozone build is created wth --ozone options
which excludes cefclient. So with ozone binary dist cefclient won’t be available for build.
Could you have a look again and let me know if you have any comments.
Thanks, this looks good to me (see one minor style issue inline).
@hfink_daqri How does it look to you?
Found a minor issue in the distribution packager (see this comment), otherwise looks good to me.
@hfink_daqri Thanks for pointing out that, I was building all platform so libminigbm.so was always available to copy thus missed the issue. I added new commit with modification as suggested by you.
Thank you, Santosh
I’m testing this PR now and will merge if everything looks OK.
@magreenblatt : Is there any update on testing this patch ?