245576 2022-09-23 07:58:00 -0700 Do not try to preconnect on link click when link preconnect setting is disabled 2022-09-30 02:22:18 -0700 1 1 1 Unclassified WebKit WebCore Misc. WebKit Nightly Build PC Linux RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=244026 InRadar P2 Normal --- 1 mcatanzaro cgarcia achristensen bugs-noreply cdumez cgarcia mcatanzaro webkit-bug-importer webkit oldest_to_newest 1900639 0 mcatanzaro 2022-09-23 07:58:00 -0700 Splitting this from bug #244026. We currently have a network process crash when calling NetworkConnectionToWebProcess::preconnectTo. We should look closer to decide what to do here. Although fixing WTFReleaseLogStackTrace would avoid the crash, I think we should go further and ensure that ResourceError::internalError does not get called. Note this only happens when ENABLE_SERVER_PRECONNECT is disabled, so the crash is specific to libsoup 2 builds only. Probably we should drop the request in NetworkConnectionToWebProcess::preconnectTo with some different error, but another option would be to find everywhere that calls it and guard it behind ENABLE_SERVER_PRECONNECT. Backtrace: /usr/lib/aarch64-linux-gnu/webkit2gtk-4.0/WebKitNetworkProcess (gdb) bt #0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50 #1 0x0000ffff7cfdfaa0 in __GI_abort () at abort.c:79 #2 0x0000ffff7fa8ac50 in WTFCrashWithInfo(int, char const*, char const*, int) () at WTF/Headers/wtf/Assertions.h:741 #3 0x0000ffff80a2d5a8 in captureStackTrace () at ../Source/WTF/wtf/StackTrace.cpp:79 #4 0x0000ffff80a08ea0 in WTFReleaseLogStackTrace () at ../Source/WTF/wtf/Assertions.cpp:592 #5 0x0000ffff83c06550 in internalError () at ../Source/WebCore/platform/network/ResourceErrorBase.cpp:97 #6 0x0000ffff820e8d1c in preconnectTo () at ../Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:735 #7 0x0000ffff81fc62f4 in callMemberFunctionImpl<WebKit::NetworkConnectionToWebProcess, void (WebKit::NetworkConnectionToWebProcess::*)(std::optional<WTF::ObjectIdentifier<WebCore::ResourceLoader> >, WebKit::NetworkResourceLoadParameters&&), std::tuple<std::optional<WTF::ObjectIdentifier<WebCore::ResourceLoader> >, WebKit::NetworkResourceLoadParameters>, 0, 1> () at ../Source/WebKit/Platform/IPC/HandleMessage.h:125 #8 callMemberFunction<WebKit::NetworkConnectionToWebProcess, void (WebKit::NetworkConnectionToWebProcess::*)(std::optional<WTF::ObjectIdentifier<WebCore::ResourceLoader> >, WebKit::NetworkResourceLoadParameters&&), std::tuple<std::optional<WTF::ObjectIdentifier<WebCore::ResourceLoader> >, WebKit::NetworkResourceLoadParameters>, std::integer_sequence<unsigned long, 0, 1> > () at ../Source/WebKit/Platform/IPC/HandleMessage.h:131 #9 handleMessage<Messages::NetworkConnectionToWebProcess::PreconnectTo, WebKit::NetworkConnectionToWebProcess, void (WebKit::NetworkConnectionToWebProcess::*)(std::optional<WTF::ObjectIdentifier<WebCore::ResourceLoader> >, WebKit::NetworkResourceLoadParameters&&)> () at ../Source/WebKit/Platform/IPC/HandleMessage.h:196 #10 didReceiveNetworkConnectionToWebProcessMessage () at DerivedSources/WebKit/NetworkConnectionToWebProcessMessageReceiver.cpp:479 #11 0x0000ffff822543d0 in dispatchMessage () at ../Source/WebKit/Platform/IPC/Connection.cpp:1134 #12 0x0000ffff82254768 in dispatchOneIncomingMessage () at ../Source/WebKit/Platform/IPC/Connection.cpp:1203 #13 0x0000ffff80a2bf40 in operator() () at ../Source/WTF/wtf/Function.h:82 #14 performWork () at ../Source/WTF/wtf/RunLoop.cpp:133 #15 0x0000ffff80a85190 in operator() () at ../Source/WTF/wtf/glib/RunLoopGLib.cpp:80 #16 __invoke () at ../Source/WTF/wtf/glib/RunLoopGLib.cpp:79 #17 0x0000ffff80a84524 in operator() () at ../Source/WTF/wtf/glib/RunLoopGLib.cpp:53 #18 __invoke () at ../Source/WTF/wtf/glib/RunLoopGLib.cpp:45 #19 0x0000ffff7d551ab4 in g_main_context_dispatch () from /usr/lib/aarch64-linux-gnu/libglib-2.0.so.0 #20 0x0000ffff7d551e5c in ?? () from /usr/lib/aarch64-linux-gnu/libglib-2.0.so.0 #21 0x0000ffff7d5521b0 in g_main_loop_run () from /usr/lib/aarch64-linux-gnu/libglib-2.0.so.0 #22 0x0000ffff80a84b20 in run () at ../Source/WTF/wtf/glib/RunLoopGLib.cpp:108 #23 0x0000ffff822280d8 in run () at ../Source/WebKit/Shared/AuxiliaryProcessMain.h:70 #24 AuxiliaryProcessMain<WebKit::NetworkProcessMainSoup> () at ../Source/WebKit/Shared/AuxiliaryProcessMain.h:96 #25 0x0000ffff7cfdfe18 in __libc_start_main (main=0x400878 <__wrap_main>, argc=3, argv=0xfffff1c90058, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=<optimized out>) at ../csu/libc-start.c:308 #26 0x0000000000400874 in _start () Please see bug #244026 for more context. 1901055 1 462600 webkit 2022-09-25 14:58:38 -0700 Created attachment 462600 Add ENABLE(SERVER_PRECONNECT) guards Not sure if those are all instances that need to be guarded, but at least those two seem to make the browser appear usable again. 1901455 2 462600 cgarcia 2022-09-27 04:22:13 -0700 Comment on attachment 462600 Add ENABLE(SERVER_PRECONNECT) guards View in context: https://bugs.webkit.org/attachment.cgi?id=462600&action=review > Source/WebCore/html/HTMLAnchorElement.cpp:542 > +#if !ENABLE(SERVER_PRECONNECT) > + return; > +#endif It would be better to add the guard around the block below. But I wonder if it would be better to check linkPreconnectEnabled in settings instead. Alex? Chris? > Source/WebCore/loader/LinkLoader.cpp:214 > +#if !ENABLE(SERVER_PRECONNECT) > + return; > +#endif Are you sure you need this one? If server preconnect is disabled then params.relAttribute.isLinkPreconnect should be false below, so it will return early in any case. 1901536 3 webkit 2022-09-27 09:00:03 -0700 (In reply to Carlos Garcia Campos from comment #2) > Are you sure you need this one? No. Consider my patch just a step in debugging rather than a fix proposal. I know nothing about WebKit's codebase and my setup takes a few hours to build a working WK package, so I simply went with testing a minimal change that seemed likely to avoid the crash, and reported my findings here :) 1901602 4 462600 achristensen 2022-09-27 13:39:05 -0700 Comment on attachment 462600 Add ENABLE(SERVER_PRECONNECT) guards View in context: https://bugs.webkit.org/attachment.cgi?id=462600&action=review >> Source/WebCore/html/HTMLAnchorElement.cpp:542 >> +#endif > > It would be better to add the guard around the block below. But I wonder if it would be better to check linkPreconnectEnabled in settings instead. Alex? Chris? Yeah, this isn't great. It adds a bunch of unreachable code when SERVER_PRECONNECT is disabled. 1901744 5 cgarcia 2022-09-28 05:10:39 -0700 Pull request: https://github.com/WebKit/WebKit/pull/4790 1902295 6 ews-feeder 2022-09-30 02:21:25 -0700 Committed 255024@main (49f6854f09b5): <https://commits.webkit.org/255024@main> Reviewed commits have been landed. Closing PR #4790 and removing active labels. 1902296 7 webkit-bug-importer 2022-09-30 02:22:18 -0700 <rdar://problem/100603273> 462600 2022-09-25 14:58:38 -0700 2022-09-25 14:58:38 -0700 Add ENABLE(SERVER_PRECONNECT) guards 0009-patch2.patch text/plain 1679 webkit RnJvbTogU2ViYXN0aWFuIEtyenlzemtvd2lhayA8c2ViYXN0aWFuLmtyenlzemtvd2lha0BwdXJp LnNtPgpEYXRlOiBTYXQsIDI0IFNlcCAyMDIyIDA2OjE3OjEyICswMjAwClN1YmplY3Q6IHBhdGNo MgoKLS0tCiBTb3VyY2UvV2ViQ29yZS9odG1sL0hUTUxBbmNob3JFbGVtZW50LmNwcCB8IDQgKysr KwogU291cmNlL1dlYkNvcmUvbG9hZGVyL0xpbmtMb2FkZXIuY3BwICAgICAgfCAzICsrKwogMiBm aWxlcyBjaGFuZ2VkLCA3IGluc2VydGlvbnMoKykKCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29y ZS9odG1sL0hUTUxBbmNob3JFbGVtZW50LmNwcCBiL1NvdXJjZS9XZWJDb3JlL2h0bWwvSFRNTEFu Y2hvckVsZW1lbnQuY3BwCmluZGV4IGMzMGRjNmYuLjdmOGYxZjUgMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJDb3JlL2h0bWwvSFRNTEFuY2hvckVsZW1lbnQuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3Jl L2h0bWwvSFRNTEFuY2hvckVsZW1lbnQuY3BwCkBAIC01MzcsNiArNTM3LDEwIEBAIHZvaWQgSFRN TEFuY2hvckVsZW1lbnQ6OmhhbmRsZUNsaWNrKEV2ZW50JiBldmVudCkKIAogICAgIHNlbmRQaW5n cyhjb21wbGV0ZWRVUkwpOwogCisjaWYgIUVOQUJMRShTRVJWRVJfUFJFQ09OTkVDVCkKKyAgICBy ZXR1cm47CisjZW5kaWYKKwogICAgIC8vIFByZWNvbm5lY3QgdG8gdGhlIGxpbmsncyB0YXJnZXQg Zm9yIGltcHJvdmVkIHBhZ2UgbG9hZCB0aW1lLgogICAgIGlmIChjb21wbGV0ZWRVUkwucHJvdG9j b2xJc0luSFRUUEZhbWlseSgpICYmICgoZnJhbWUtPmlzTWFpbkZyYW1lKCkgJiYgaXNTZWxmVGFy Z2V0RnJhbWVOYW1lKGVmZmVjdGl2ZVRhcmdldCkpIHx8IGlzQmxhbmtUYXJnZXRGcmFtZU5hbWUo ZWZmZWN0aXZlVGFyZ2V0KSkpIHsKICAgICAgICAgYXV0byBzdG9yYWdlQ3JlZGVudGlhbHNQb2xp Y3kgPSBmcmFtZS0+cGFnZSgpICYmIGZyYW1lLT5wYWdlKCktPmNhblVzZUNyZWRlbnRpYWxTdG9y YWdlKCkgPyBTdG9yZWRDcmVkZW50aWFsc1BvbGljeTo6VXNlIDogU3RvcmVkQ3JlZGVudGlhbHNQ b2xpY3k6OkRvTm90VXNlOwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvbG9hZGVyL0xpbmtM b2FkZXIuY3BwIGIvU291cmNlL1dlYkNvcmUvbG9hZGVyL0xpbmtMb2FkZXIuY3BwCmluZGV4IDlm MTdjMTIuLmI5ZWIwMzAgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2xvYWRlci9MaW5rTG9h ZGVyLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9sb2FkZXIvTGlua0xvYWRlci5jcHAKQEAgLTIw OSw2ICsyMDksOSBAQCBib29sIExpbmtMb2FkZXI6OmlzU3VwcG9ydGVkVHlwZShDYWNoZWRSZXNv dXJjZTo6VHlwZSByZXNvdXJjZVR5cGUsIGNvbnN0IFN0cmluZwogCiB2b2lkIExpbmtMb2FkZXI6 OnByZWNvbm5lY3RJZk5lZWRlZChjb25zdCBMaW5rTG9hZFBhcmFtZXRlcnMmIHBhcmFtcywgRG9j dW1lbnQmIGRvY3VtZW50KQogeworI2lmICFFTkFCTEUoU0VSVkVSX1BSRUNPTk5FQ1QpCisgICAg cmV0dXJuOworI2VuZGlmCiAgICAgY29uc3QgVVJMIGhyZWYgPSBwYXJhbXMuaHJlZjsKICAgICBp ZiAoIXBhcmFtcy5yZWxBdHRyaWJ1dGUuaXNMaW5rUHJlY29ubmVjdCB8fCAhaHJlZi5pc1ZhbGlk KCkgfHwgIXBhcmFtcy5ocmVmLnByb3RvY29sSXNJbkhUVFBGYW1pbHkoKSB8fCAhZG9jdW1lbnQu ZnJhbWUoKSkKICAgICAgICAgcmV0dXJuOwo=