Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: 5ea58380f09490bd9a91cc1ab713e5c7a067e4d2
https://github.com/WebKit/WebKit/commit/5ea58380f09490bd9a91cc1ab713e5c7a067e4d2
Author: Chris Dumez <cdu...@apple.com>
Date: 2023-12-14 (Thu, 14 Dec 2023)
Changed paths:
R
LayoutTests/platform/gtk/http/tests/navigation/keyboard-events-during-provisional-navigation-expected.txt
M Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp
M Source/WebKit/UIProcess/Network/NetworkProcessProxy.h
M Source/WebKit/UIProcess/WebNavigationState.cpp
M Source/WebKit/UIProcess/WebProcessPool.cpp
M Source/WebKit/UIProcess/WebProcessPool.h
Log Message:
-----------
High CPU usage may occur due to infinite loop in
WebProcessPool::processForNavigation()
https://bugs.webkit.org/show_bug.cgi?id=266389
rdar://118280525
Reviewed by Alex Christensen.
>From the profile in the radar, we can see that we can end up in a state where:
1. WebProcessPool::processForNavigation() send the
AddAllowedFirstPartyForCookies
IPC to the network process
2. Once we received the IPC response from the network process, we check if the
destination WebProcess is still running.
3. We notice that the WebProcess is no longer running so we call
WebProcessPool::processForNavigation() -> Back to step 1.
We end up in an infinite loop (with an AddAllowedFirstPartyForCookies async IPC
in the middle) because we determine over and over that the destination process
for the navigation is no longer running, likely because processForNavigation()
keeps selecting the same WebProcess.
To make sure this can no longer happen, I made the following changes:
1. Add a cache to NetworkProcessProxy to avoid sending the
AddAllowedFirstPartyForCookies async IPC unnecessarily. It is common for
navigations to happen within the same process when the registrable domain
doesn't change. In this case, there is no point in sending the IPC again.
2. Make sure processForNavigationInternal() never returns a terminated process
by adding a couple of missing checks.
3. Move the AddAllowedFirstPartyForCookies handshake to a new function named
prepareProcessForNavigation(). This function calls itself recursively when
the destination process is no longer running, like we used to. However,
I now make sure to fall back to calling prepareProcessForNavigation() with
a fresh new WebProcess. Also, we attempt this fall back 3 times at most to
guarantee no infinite looping can occur.
* Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:
(WebKit::NetworkProcessProxy::addAllowedFirstPartyForCookies):
* Source/WebKit/UIProcess/Network/NetworkProcessProxy.h:
* Source/WebKit/UIProcess/WebNavigationState.cpp:
(WebKit::WebNavigationState::navigation):
Drop assertion that required the navigationID to be present in the map when
calling navigation(navigationID). I changed timing by avoiding the async IPC
handshake with the network process in some cases. This timing change caused
the ProcessSwap.ConcurrentHistoryNavigations API test to hit this assertion
in didStartProvisionalLoad(). The API test purposefully starts parallel /
competing navigations. We used to get lucky and the navigation would get
cancelled before receiving the response from the network process. Now the
cancellation happens after the navigation has started in the provisional
process.
* Source/WebKit/UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::processForNavigation):
(WebKit::WebProcessPool::prepareProcessForNavigation):
(WebKit::WebProcessPool::processForNavigationInternal):
* Source/WebKit/UIProcess/WebProcessPool.h:
Canonical link: https://commits.webkit.org/272059@main
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
