#128 Open
Repository
chromiumembedded
Branch
master

Bitbucket cannot automatically merge this request.

The commits that make up this pull request have been removed.

Bitbucket cannot automatically merge this request due to conflicts.

Review the conflicts on the Overview tab. You can then either decline the request or merge it manually on your local system using the following commands:

git checkout master
git remote add Santosh_Collabora/cef https://bitbucket.org/Santosh_Collabora/cef.git
git fetch Santosh_Collabora/cef
git merge --no-ff -m 'Merged in Santosh_Collabora/cef/Santosh/cef_mus_support (pull request #128)' remotes/Santosh_Collabora/cef/Santosh/cef_mus_support
Author
  1. santosh mahto
Reviewers
Description
  • Move X11 dependency under compile time flag.

Comments (20)

  1. santosh mahto author

    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.

  2. Marshall Greenblatt

    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.

  3. santosh mahto author

    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

  4. santosh mahto author

    @Marshall Greenblatt : 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.

  5. santoshmahto

    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.

    @Marshall Greenblatt : I would appreciate if you could review it and take it forward.

  6. Heinrich Fink

    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.

    1. 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.
    2. 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.
    3. 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).
    4. 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?):

      1. the generated cef_config.h doesn’t seem to get copied, breaking builds of e.g. cefsimple inside the generated distribution.
      2. 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!

  7. Marshall Greenblatt

    Please see some minor style issues commented inline.

    Testing instructions from issue #2296:

    1. Build CEF with the following GN_DEFINES: use_ozone=true use_intel_minigbm=true (varies for gpu type)
    2. Run as: $ ./out/Release/cefsimple --use-views --ozone-platform=wayland

    @Santosh Mahto 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)?

    @Heinrich Fink Are all of your build issues resolved?

    1. Heinrich Fink

      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
      #include <glib.h>
               ^~~~~~~~
      1 error generated.
      
      1. Heinrich Fink

        After working around the glib issue with the commit I linked above, I am running into this linker error:

        builduser@d47d47260efa ~/c/c/c/src> ninja -j14 -C out/Release_GN_x64/ cef cefsimple                           │···················
        ninja: Entering directory `out/Release_GN_x64/'                                                               │···················
        [1/1] Regenerating ninja files                                                                                │···················
        [439/440] LINK ./ceftests                                                                                     │···················
        FAILED: ceftests                                                                                              │···················
        python "../../build/toolchain/gcc_link_wrapper.py" --output="./ceftests" -- ../../third_party/llvm-build/Relea│···················
        se+Asserts/bin/clang++ -fPIC -Wl,-z,noexecstack -Wl,-z,relro -Wl,-z,now -Wl,-z,defs -Wl,--as-needed -fuse-ld=l│···················
        ld -Wl,--icf=all -Wl,--color-diagnostics -m64 -Werror -Wl,-O2 -Wl,--gc-sections -rdynamic -nostdlib++ -pie -Wl│···················
        ,--disable-new-dtags -Wl,-rpath=\$ORIGIN -o "./ceftests" -Wl,--start-group @"./ceftests.rsp" ./libcef.so -Wl,-│···················
        -end-group   -ldl -lpthread -lrt -lglib-2.0 -latomic                                                          │···················
        ld.lld: error: undefined symbol: XSetErrorHandler                                                             │···················
        >>> referenced by run_all_unittests.cc                                                                        │···················
        >>>               obj/cef/ceftests/run_all_unittests.o:(main)                                                 │···················
                                                                                                                      │···················
        ld.lld: error: undefined symbol: XSetIOErrorHandler                                                           │···················
        >>> referenced by run_all_unittests.cc                                                                        │···················
        >>>               obj/cef/ceftests/run_all_unittests.o:(main)  
        

        Looks like tests/ceftests/run_all_unittests.cc uses XSetErrorHandler in an unconditional way