Bug 245576 – Do not try to preconnect on link click when link preconnect setting is disabled

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Michael Catanzaro]

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.

[Sebastian Krzyszkowiak]

Created attachment 462600 [details]
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.

[Carlos Garcia Campos]

Comment on attachment 462600 [details]
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.

[Sebastian Krzyszkowiak]

(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 :)

[Alex Christensen]

Comment on attachment 462600 [details]
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.

[Carlos Garcia Campos]

Pull request: https://github.com/WebKit/WebKit/pull/4790

[EWS]

Committed 255024@main (49f6854f09b5): <https://commits.webkit.org/255024@main>

Reviewed commits have been landed. Closing PR #4790 and removing active labels.

[Radar WebKit Bug Importer]

<rdar://problem/100603273>