New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WebProcessProxy should keep WebsiteDataStore alive #3331
WebProcessProxy should keep WebsiteDataStore alive #3331
Conversation
c9f2c52
to
4d443fb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with suggested change.
if (!dataStore) | ||
return reply({ }); | ||
|
||
RELEASE_ASSERT(dataStore); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks very dangerous. If this happened, we'd be converting a WebContent process crash into a UIProcess crash, which is not a good trade off IMO. I'd recommend a debug ASSERT() instead and a RELEASE_LOG_FAULT() so we know it's happening.
4d443fb
to
d38fc18
Compare
d38fc18
to
7677d62
Compare
https://bugs.webkit.org/show_bug.cgi?id=243960 <rdar://98313174> Reviewed by Chris Dumez. This effectively reverts bug 238892 because it caused rdar://98313174 Tasks such as the one scheduled in WorkerSWClientConnection::unregisterServiceWorkerClient need a connection to the network process, and if the network process has crashed then they need a WebsiteDataStore to restart that network process, so a WebProcessProxy needs to keep a WebsiteDataStore alive. * Source/WebKit/UIProcess/WebProcessProxy.cpp: (WebKit::m_webPermissionController): (WebKit::WebProcessProxy::setWebsiteDataStore): (WebKit::WebProcessProxy::isDummyProcessProxy const): (WebKit::WebProcessProxy::addExistingWebPage): (WebKit::WebProcessProxy::getNetworkProcessConnection): (WebKit::WebProcessProxy::sessionID const): (WebKit::WebProcessProxy::websiteDataStore const): Deleted. * Source/WebKit/UIProcess/WebProcessProxy.h: (WebKit::WebProcessProxy::websiteDataStore const): Canonical link: https://commits.webkit.org/253449@main
7677d62
to
2cd7eec
Compare
Committed 253449@main (2cd7eec): https://commits.webkit.org/253449@main Reviewed commits have been landed. Closing PR #3331 and removing active labels. |
2cd7eec