www/qt5-webengine: update to 5.12.1

[kai]

Summary

Import the update of Kai Knoblich <[e-mail]>

Test Plan

• F it, we'll do it live!

• Builds fine on 11.2-, 12.0-RELEASE, 13.0-CURRENT@r344874 amd64 + i386 for ALSA/SNDIO/PULSE
• Playback of multimedia content should be smoother (thanks to rakuco)

[tcberner]

After that:
https://github.com/freebsd/freebsd-ports-kde/commit/e435c7415cd7fb7fe61f6678e30e2c4764220b55

[kai]

Update www/qt5-webengine to 5.12.1. Many kudos to rakuco and tcberner!

[kai]

Add missing debug files in pkg-plist.

[tcberner]

As long as @rakuco has nothing to add, ship it!

[rakuco]

In D18757#420567, @kai wrote:

Add missing debug files in pkg-plist.

That's not supposed to work, and I think the corresponding commit to the qtwebengine-5.12.1 branch is wrong as well. See https://github.com/freebsd/freebsd-ports-kde/commit/081752e6ca70e2fb3f66452928048e5b776f890a for a longer explanation.

[kai]

Prefixed *.debug entries with @comment in pkg-plist as suggested by @tcberner.

I forgot to mention that I also did some minor clean-up of some patches to reduce the overall churn.

[rakuco]

Thanks a lot for working on this. I've left some inline comments in some of the patches, but there isn't a lot that I think needs changing \o/

www/qt5-webengine/files/patch-config.tests-enable-on-FreeBSD

1

I think it makes sense to keep this patch (which has an explanation and groups the related changes together) rather than splitting it up into two separate ones.

www/qt5-webengine/files/patch-configure.pri

9

Is this patch really necessary? We shouldn't be detecting glibc in the first place (and we seem to disable the check in the configure.prf patch), and this change is actually replacing ldd with a completely unrelated program.

www/qt5-webengine/files/patch-mkspecs_features_functions.prf

1

I think it makes sense to keep the patch explanation at the top.

www/qt5-webengine/files/patch-src_3rdparty_chromium_chrome_app_generated__resources.grd

17

is_is_posix should be is_posix here.

www/qt5-webengine/files/patch-src_3rdparty_chromium_content_browser_webui_web__ui__data__source__impl.h

2

These web_ui patches (I think there are 4 or 5) that replace base::StringPiece usage with std::string seem wrong to me. StringPiece is a core part of Chromium (and it's used everywhere, so it should be working on FreeBSD), and removing those patches passed in my build-test on HEAD.

www/qt5-webengine/files/patch-src_3rdparty_chromium_net_proxy__resolution_proxy__config__service__linux.cc

18

Why do we need to disable KDE support here?

www/qt5-webengine/files/patch-src_3rdparty_chromium_third__party_skia_src_images_SkJpegEncoder.cpp

2

Do you know why this patch (as well as patch-src_3rdparty_chromium_third__party_skia_src_ports_SkFontHost__FreeType.cpp and patch-src_3rdparty_chromium_ui_gfx_codec_jpeg__codec.cc) is needed?

www/qt5-webengine/files/patch-src_3rdparty_chromium_third__party_yasm_BUILD.gn

1

Do we need this? We're unbundling yasm.

www/qt5-webengine/files/patch-src_core_core__module.pro

1

Can you restore the patch explanation here?

[kai]

Addressing the most of @rakuco's suggestions:

• Restore patch-config.test-enable-on-FreeBSD (and remove both patch-config.tests_{khr_khr,snappy_snappy}.pro)
• Fixed typo in patch-src_3rdparty_chromium_chrome_app_generated_resources.grd

Restore explanations/boiler plates:

• patch-mkspecs_features_functions.prf
• patch-src_code_core__module.pro

Remove superflous patches:

• Remove superflous patch-configure.pri (it was a leftover from the phase where I tried to get around the glibc check)
• patch-src_3rdparty_chromium_third__party_skia_src_images_SkJpegEncoder.cpp
• patch-src_3rdparty_chromium_thirdparty_skia_src_ports_SkFontHostFreeType.cpp
• patch-src_3rdparty_chromium_third__party_yasm_BUILD.gn
• patch-src_3rdparty_chromium_ui_gfx_codec_jpeg__codec.cc

[kai]

www/qt5-webengine/files/patch-src_3rdparty_chromium_content_browser_webui_web__ui__data__source__impl.h

2

Hrm, I removed all four patch-src_3rdparty_chromium_content_browser_webui_web__ui* patches and then the build failed with

../../../../qtwebengine-everywhere-src-5.12.1/src/3rdparty/chromium/content/browser/webui/web_ui_impl.h:56:65: error: non-virtual member function marked 'override' hides virtual member function
                               const MessageCallback& callback) override;
                                                                ^
../../../../qtwebengine-everywhere-src-5.12.1/src/3rdparty/chromium/content/public/browser/web_ui.h:77:16: note: hidden overloaded virtual function 'content::WebUI::RegisterMessageCallback' declared here: type mismatch at 1st parameter ('const std::string &' (aka 'const basic_string<char, char_traits<char>, allocator<char> > &') vs 'base::StringPiece' (aka 'BasicStringPiece<basic_string<char, char_traits<char>, allocator<char> > >'))
  virtual void RegisterMessageCallback(const std::string& message,

www/qt5-webengine/files/patch-src_3rdparty_chromium_net_proxy__resolution_proxy__config__service__linux.cc

18

I tried to build webengine without the both hunks that exclude the KDE support but it failed with:

../../../../qtwebengine-everywhere-src-5.12.1/src/3rdparty/chromium/base/message_loop/message_loop_current.h:190:22: note: copy assignment operator of 'MessageLoopCurrent' is implicitly deleted because field 'current_' is of const-qualified type 'base::MessageLoop *const'
  MessageLoop* const current_;
                     ^
../../../../qtwebengine-everywhere-src-5.12.1/src/3rdparty/chromium/net/proxy_resolution/proxy_config_service_linux.cc:589:19: error: use of undeclared identifier 'inotify_init'
    inotify_fd_ = inotify_init();
                  ^
../../../../qtwebengine-everywhere-src-5.12.1/src/3rdparty/chromium/net/proxy_resolution/proxy_config_service_linux.cc:633:27: error: use of undeclared identifier 'IN_MODIFY'
                          IN_MODIFY | IN_MOVED_TO) < 0) {
                          ^
../../../../qtwebengine-everywhere-src-5.12.1/src/3rdparty/chromium/net/proxy_resolution/proxy_config_service_linux.cc:633:39: error: use of undeclared identifier 'IN_MOVED_TO'
                          IN_MODIFY | IN_MOVED_TO) < 0) {
                                      ^
../../../../qtwebengine-everywhere-src-5.12.1/src/3rdparty/chromium/net/proxy_resolution/proxy_config_service_linux.cc:911:28: error: use of undeclared identifier 'inotify_event'
    char event_buf[(sizeof(inotify_event) + NAME_MAX + 1) * 4];
                           ^
../../../../qtwebengine-everywhere-src-5.12.1/src/3rdparty/chromium/net/proxy_resolution/proxy_config_service_linux.cc:919:9: error: unknown type name 'inotify_event'
        inotify_event* event = reinterpret_cast<inotify_event*>(event_ptr);
        ^
../../../../qtwebengine-everywhere-src-5.12.1/src/3rdparty/chromium/net/proxy_resolution/proxy_config_service_linux.cc:919:49: error: unknown type name 'inotify_event'
        inotify_event* event = reinterpret_cast<inotify_event*>(event_ptr);
                                                ^
../../../../qtwebengine-everywhere-src-5.12.1/src/3rdparty/chromium/net/proxy_resolution/proxy_config_service_linux.cc:921:37: error: use of undeclared identifier 'inotify_event'
        CHECK_LE(event_ptr + sizeof(inotify_event), event_buf + r);

[kai]

Really add back files/patch-config.tests-enable-on-FreeBSD

[rakuco]

www/qt5-webengine/files/patch-src_3rdparty_chromium_content_browser_webui_web__ui__data__source__impl.h

2

Sorry, I should've been more specific. These are the files I removed locally to get the port to build:

• patch-src_3rdparty_chromium_content_browser_webui_web__ui__data__source__impl.cc
• patch-src_3rdparty_chromium_content_browser_webui_web__ui__data__source__impl.h
• patch-src_3rdparty_chromium_content_browser_webui_web__ui__impl.cc
• patch-src_3rdparty_chromium_content_browser_webui_web__ui__impl.h
• patch-src_3rdparty_chromium_content_public_browser_web__ui.h
• patch-src_3rdparty_chromium_content_public_browser_web__ui__data__source.h

[kai]

Remove further superfluous patches pointed out by @rakuco.

Many thanks for your all your hints, @rakuco, and that you took the time to look at it all!

Actually now only the thing with patch-src_3rdparty_chromium_net_proxyresolution_proxyconfigservicelinux.cc seems to be open? Otherwise nothing more would stand in the way to bring it finally into the ports tree... \o/

[kai]

www/qt5-webengine/files/patch-src_3rdparty_chromium_content_browser_webui_web__ui__data__source__impl.h

2

Maybe it makes sense to remove those patches from the www/chromium port where I took them, as well?

[rakuco]

Thank you very much for this patch. I think we're good to go, and the KDE inotify thing can be tracked separately. I haven't test-built the latest version of this patch in older releases like 11.2 though, it'd be good to do the following:

• File a bug for tracking the KDE-inotify bug.
• Test this on at least 11.2 if you haven't already (I can do it tonight if you prefer).
• Land and start worrying about the next update :-)

@tcberner had different results with the .debug files in the plist, but I think we can also follow this separately and not block the update on it.

www/qt5-webengine/files/patch-src_3rdparty_chromium_content_browser_webui_web__ui__data__source__impl.h

2

I think it would be a good idea, yes. It's not clear to me why they were needed in the first place.

[kai]

In D18757#422018, @rakuco wrote:

Thank you very much for this patch. I think we're good to go, and the KDE inotify thing can be tracked separately. I haven't test-built the latest version of this patch in older releases like 11.2 though, it'd be good to do the following:

• File a bug for tracking the KDE-inotify bug.

If I'm not mistaken the Chromium Bug-Tracker is the right platform for that bug?

• Test this on at least 11.2 if you haven't already (I can do it tonight if you prefer).

I'll do a final build for all releases (11.2-, 12.0-RELEASE and 13.0-CURRENT@r345355 amd64 + i386) with the ALSA or SNDIO or PULSE (and with/without DEBUG) options enabled. I'm optimistic, that all builds will succeed.

• Land and start worrying about the next update :-)

I did some preparations for qt5-webengine 5.12.2 and as far I can tell is that the same set of patches as here with 5.12.1 work seamless. I did also a quick runtime test that went without any issues. ;-)

Well, qt5-webengine 5.13.x is another story...

[rakuco]

In D18757#422029, @kai wrote:

• File a bug for tracking the KDE-inotify bug.

If I'm not mistaken the Chromium Bug-Tracker is the right platform for that bug?

I meant a bug in our very own bugs.freebsd.org -- from an upstream Chromium point of view everything's working fine on Linux, we just need to fix that on FreeBSD.

[kai]

In D18757#422034, @rakuco wrote:

If I'm not mistaken the Chromium Bug-Tracker is the right platform for that bug?

I meant a bug in our very own bugs.freebsd.org -- from an upstream Chromium point of view everything's working fine on Linux, we just need to fix that on FreeBSD.

Ah, k, the bug has been filed under PR236787 . The builders are running...it's only a matter of hours now.

[rakuco]

www/qt5-webengine/files/freebsd.pri

3

Just one thing I forgot to mention before: when landing, you can get rid of disable_nacl here, this option doesn't exist in Chromium anymore (and GN warns about it during the build).

[kai]

Thank you for the pointer. I removed that line from the freebsd.pri file. I discovered one small issue with the patch-src_3rdparty_chromium_third__party_yasm_BUILD.gn:

That file already exist with the old webengine and I did accidentially a "svn revert" instead of "svn rm" and created this version of diff then. So that file isn't not listed in this diff but patching will fail then. On my builders the patch-src_3rdparty_chromium_third__party_yasm_BUILD.gn is removed with "svn rm" and all builds were successful so far (except the builds with debug symbols on 13-CURRENT i386 due memory exhaustion during the linking).

If you like I'll update this diff to reflect those minor changes for the sake of completeness.

Edit: Typo fix.

[tcberner]

If you like I'll update this diff to reflect those minor changes for the sake of completeness.

I trust you tested it well enough :) so just go ahead.

[kai]

In D18757#422739, @tcberner wrote:

I trust you tested it well enough :) so just go ahead.

Alright, I'll unleash the behemoth in a few minutes...

www/qt5-webengine/files/freebsd.pri

7

The enable_hidpi variable seems also not to exist anymore but I think we can rid of that with the update to 5.12.2 .

[arrowd]

This can be closed, I guess.

[kai]

In D18757#426176, @arrowd wrote:

This can be closed, I guess.

Thanks for your info. Indeed, I was wondering why this review was still open after www/qt5-webengine has been committed in rP496989. It seems that Phabricator has some problems with the import of that commit as it still display "This commit is still importing. Changes will be visible once the import finishes."

[tcberner]

In D18757#427717, @kai wrote:

In D18757#426176, @arrowd wrote:

This can be closed, I guess.

Thanks for your info. Indeed, I was wondering why this review was still open after www/qt5-webengine has been committed in rP496989. It seems that Phabricator has some problems with the import of that commit as it still display "This commit is still importing. Changes will be visible once the import finishes."

Yeah, big diffs tend to never get auto closed... well sometimes after weeks :D