Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: cde2625ebe14befd13b4a825f77b68645b5ddb12
https://github.com/WebKit/WebKit/commit/cde2625ebe14befd13b4a825f77b68645b5ddb12
Author: Rupin Mittal <[email protected]>
Date: 2025-07-08 (Tue, 08 Jul 2025)
Changed paths:
M LayoutTests/ipc/serialized-type-info.html
M Source/WTF/wtf/Markable.h
M Source/WebCore/animation/WebAnimationTypes.h
M Source/WebCore/platform/network/ResourceRequestBase.cpp
M Source/WebCore/platform/network/ResourceRequestBase.h
M Source/WebCore/platform/network/cocoa/ResourceRequestCocoa.mm
M Source/WebKit/NetworkProcess/NetworkProcess.cpp
M Source/WebKit/NetworkProcess/NetworkProcess.h
M Source/WebKit/NetworkProcess/NetworkProcess.messages.in
M Source/WebKit/NetworkProcess/NetworkProcessCreationParameters.h
M
Source/WebKit/NetworkProcess/NetworkProcessCreationParameters.serialization.in
M Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
M Source/WebKit/Platform/IPC/ArgumentCoders.h
M Source/WebKit/Shared/API/APIURLRequest.cpp
M Source/WebKit/Shared/WebProcessCreationParameters.h
M Source/WebKit/Shared/WebProcessCreationParameters.serialization.in
M Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp
M Source/WebKit/UIProcess/Network/NetworkProcessProxy.h
M Source/WebKit/WebProcess/WebProcess.cpp
M Tools/DumpRenderTree/mac/DumpRenderTree.mm
M Tools/TestWebKitAPI/Tests/WebKitCocoa/Navigation.mm
M Tools/WebKitTestRunner/TestController.cpp
Log Message:
-----------
blackrock.com: REGRESSION(283494@main): Timeouts observed with TLS upgrade
https://bugs.webkit.org/show_bug.cgi?id=293286
rdar://151910984
Reviewed by Matthew Finkel.
An HTTP request may be optimistically upgraded to an HTTPS request
when possible. The HTTPS request will be a redirect request from
the original HTTP request. In this event, the original HTTP request
will get a timeout interval of 3 seconds. This timeout currently
gets carried over to the redirect request. In cases where the
redirect is not completed within the 3 seconds, the redirect request
will timeout. This results in the site hanging and a terrible
customer experience.
We don't need to have the short timeout on the redirect request.
To fix this, we ensure that in cases where the request is scheme
upgraded, the redirect does not get the 3 second timeout. Instead,
it's timeout will be set at the platform's default value (this is
usually a large value--for example INT_MAX on iOS).
This change was causing a layout test to fail
(http/tests/xmlhttprequest/on-network-timeout-error-during-preflight.html):
The issue was two things:
1. Layout tests were implicitly relying on a different part of the
code specifying a timeout value of 60 seconds. But with this change,
the redirect request case will no longer implicitly rely on that,
but rather explicity use ResourceRequestBase::defaultTimeoutInterval(),
which is 0 for non-iOS platforms. From testing, it seems like this is
interpreted as INT_MAX (or at least some high non-zero value) by
lower level frameworks. So there is essentially no timeout. And this
test is checking for a timeout. So we change TestController to explicity
use 60 seconds as the default timeout for layout tests. We do the same
for DumpRenderTree for WebKit1.
2. Even once we fixed this, the default value wasn't truly being
updated to 60 seconds. The issue was that the WebKit C API to set
the default value propagated the new value to all Web Processes
but not to Network Processes. In this test, the value is accessed
from the Network Process. Since the 60 seconds value wasn't set for
Network Processes, it returned 0 seconds, which gets interpreted as
INT_MAX (or some other higher non-zero value), and thus the test
failed. We fix this by propagating the new default value from the
API to Network Processes as well.
So we now explicitly set the default value to 60 seconds for each
layout test. And the WKURLRequestSetDefaultTimeoutInterval API
will propagate the value to Network Processes as well as Web Processes.
* LayoutTests/ipc/serialized-type-info.html:
* Source/WTF/wtf/Markable.h:
(WTF::DoubleMarkableTraits::isEmptyValue):
(WTF::DoubleMarkableTraits::emptyValue):
* Source/WebCore/animation/WebAnimationTypes.h:
(WebCore::WebAnimationsMarkableDoubleTraits::isEmptyValue): Deleted.
(WebCore::WebAnimationsMarkableDoubleTraits::emptyValue): Deleted.
* Source/WebCore/platform/network/ResourceRequestBase.cpp:
(WebCore::ResourceRequestBase::resetTimeoutInterval):
This will reset the timeout to the default value.
* Source/WebCore/platform/network/ResourceRequestBase.h:
* Source/WebCore/platform/network/cocoa/ResourceRequestCocoa.mm:
(WebCore::ResourceRequest::doUpdatePlatformRequest):
This function updates the NSRequest with the current information
stored in the updated WebCore::ResourceRequest.
Before the update happens, in the case of the redirect, the NSRequest
will contain the timeout from the original request (the short
time interval). Currently, if the updated WebCore::ResourceRequest's
timeout interval is 0, we don't change the NSRequest's timeout, so
it remains as the short timeout.
We update this so that if the ResourceRequest's timeout is
0, we'll use the ResourceRequest's default timeout (generally a large
value, like INT_MAX on iOS).
* Source/WebKit/NetworkProcess/NetworkProcess.cpp:
(WebKit::NetworkProcess::initializeNetworkProcess):
(WebKit::NetworkProcess::setDefaultRequestTimeoutInterval):
* Source/WebKit/NetworkProcess/NetworkProcess.h:
* Source/WebKit/NetworkProcess/NetworkProcess.messages.in:
* Source/WebKit/NetworkProcess/NetworkProcessCreationParameters.h:
The default timeout for Network Processes can be set here.
*
Source/WebKit/NetworkProcess/NetworkProcessCreationParameters.serialization.in:
* Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:
(WebKit::NetworkResourceLoader::willSendRedirectedRequestInternal):
If the original request was scheme upgraded, set the timeout on the
redirect request to the default value. If it's set it to 0, then when this
request is converted to an NSRequest, the default timeout will be used.
* Source/WebKit/Platform/IPC/ArgumentCoders.h:
* Source/WebKit/Shared/API/APIURLRequest.cpp:
(API::URLRequest::setDefaultTimeoutInterval):
When the default timeout is changed, this value gets propagated
to all the Web Processes. We make a change here to make sure the
value is propagated to all Network Processes as well (otherwise
the two are inconsistent).
* Source/WebKit/Shared/WebProcessCreationParameters.h:
* Source/WebKit/Shared/WebProcessCreationParameters.serialization.in:
* Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:
(WebKit::NetworkProcessProxy::sendCreationParametersToNewProcess):
(WebKit::NetworkProcessProxy::setDefaultRequestTimeoutInterval):
We set the timeout for new Network Processes in exactly the same
way we do for WebProcesses.
* Source/WebKit/UIProcess/Network/NetworkProcessProxy.h:
* Source/WebKit/WebProcess/WebProcess.cpp:
(WebKit::WebProcess::initializeWebProcess):
* Tools/DumpRenderTree/mac/DumpRenderTree.mm:
(runTest):
To make layout tests pass (as described above) we now explicitly
set the default value to 60 seconds for each layout test.
Tools/TestWebKitAPI/Tests/WebKitCocoa/Navigation.mm:
(TEST(WKNavigation,
DISABLED_PreferredHTTPSPolicyAutomaticHTTPFallbackRedirectNo3SecondTimeout)):
New API test to test this behavior. This functionality depends on
a change made to a lower level system framework (libnetcore). That
change currently isn't on EWS, so this test is disabled for now.
We'll enable it once EWS is upgraded.
* Tools/WebKitTestRunner/TestController.cpp:
(WTR::TestController::resetStateToConsistentValues):
To make layout tests pass (as described above) we now explicitly
set the default value to 60 seconds for each layout test.
Canonical link: https://commits.webkit.org/297128@main
To unsubscribe from these emails, change your notification settings at
https://github.com/WebKit/WebKit/settings/notifications
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
