Skip to content
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

Adopt smart pointers in WebProcessProxy #19674

Merged

Conversation

cdumez
Copy link
Contributor

@cdumez cdumez commented Oct 28, 2023

6c1701a

Adopt smart pointers in WebProcessProxy
https://bugs.webkit.org/show_bug.cgi?id=263820

Reviewed by Ryosuke Niwa.

* Source/WebKit/Shared/WebBackForwardListItem.h:
* Source/WebKit/UIProcess/API/C/WKPage.cpp:
(WKPageCopyRelatedPages):
* Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp:
(WebKit::WebAutomationSession::getBrowsingContexts):
* Source/WebKit/UIProcess/HighPerformanceGraphicsUsageSampler.cpp:
(WebKit::HighPerformanceGraphicsUsageSampler::timerFired):
* Source/WebKit/UIProcess/PerActivityStateCPUUsageSampler.cpp:
(WebKit::PerActivityStateCPUUsageSampler::pageForLogging const):
* Source/WebKit/UIProcess/ProvisionalPageProxy.h:
* Source/WebKit/UIProcess/RemotePageProxy.cpp:
(WebKit::RemotePageProxy::protectedPage const):
* Source/WebKit/UIProcess/RemotePageProxy.h:
* Source/WebKit/UIProcess/WebBackForwardCache.h:
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::~WebPageProxy):
(WebKit::WebPageProxy::nonEphemeralWebPageProxy):
(WebKit::WebPageProxy::protectedVisitedLinkStore):
* Source/WebKit/UIProcess/WebPageProxy.h:
* Source/WebKit/UIProcess/WebProcessCache.h:
* Source/WebKit/UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::anyProcessPoolNeedsUIBackgroundAssertion):
(WebKit::WebProcessPool::forEachProcessForSession):
(WebKit::WebProcessPool::checkedWebProcessCache):
(WebKit::WebProcessPool::checkedBackForwardCache):
* Source/WebKit/UIProcess/WebProcessPool.h:
* Source/WebKit/UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::allProcesses):
(WebKit::WebProcessProxy::processForIdentifier):
(WebKit::WebProcessProxy::globalPages):
(WebKit::WebProcessProxy::pages const):
(WebKit::WebProcessProxy::forWebPagesWithOrigin):
(WebKit::WebProcessProxy::allowedFirstPartiesForCookies):
(WebKit::WebProcessProxy::create):
(WebKit::WebProcessProxy::createForRemoteWorkers):
(WebKit::m_routingArbitrator):
(WebKit::WebProcessProxy::~WebProcessProxy):
(WebKit::WebProcessProxy::updateRegistrationWithDataStore):
(WebKit::WebProcessProxy::addProvisionalPageProxy):
(WebKit::WebProcessProxy::addRemotePageProxy):
(WebKit::WebProcessProxy::getLaunchOptions):
(WebKit::WebProcessProxy::shouldSendPendingMessage):
(WebKit::WebProcessProxy::processWillShutDown):
(WebKit::WebProcessProxy::startDisplayLink):
(WebKit::WebProcessProxy::stopDisplayLink):
(WebKit::WebProcessProxy::setDisplayLinkPreferredFramesPerSecond):
(WebKit::WebProcessProxy::setDisplayLinkForDisplayWantsFullSpeedUpdates):
(WebKit::WebProcessProxy::shutDown):
(WebKit::WebProcessProxy::webPage):
(WebKit::WebProcessProxy::audioCapturingWebPage):
(WebKit::WebProcessProxy::webPageWithActiveXRSession):
(WebKit::WebProcessProxy::notifyPageStatisticsAndDataRecordsProcessed):
(WebKit::WebProcessProxy::notifyWebsiteDataScanForRegistrableDomainsFinished):
(WebKit::WebProcessProxy::notifyWebsiteDataDeletionForRegistrableDomainsFinished):
(WebKit::WebProcessProxy::createWebPage):
(WebKit::WebProcessProxy::addExistingWebPage):
(WebKit::WebProcessProxy::removeWebPage):
(WebKit::WebProcessProxy::updateBackForwardItem):
(WebKit::WebProcessProxy::getNetworkProcessConnection):
(WebKit::WebProcessProxy::createGPUProcessConnection):
(WebKit::WebProcessProxy::gpuProcessDidFinishLaunching):
(WebKit::WebProcessProxy::gpuProcessExited):
(WebKit::WebProcessProxy::didReceiveMessage):
(WebKit::WebProcessProxy::didReceiveSyncMessage):
(WebKit::WebProcessProxy::processDidTerminateOrFailedToLaunch):
(WebKit::WebProcessProxy::didBecomeUnresponsive):
(WebKit::WebProcessProxy::didBecomeResponsive):
(WebKit::WebProcessProxy::willChangeIsResponsive):
(WebKit::WebProcessProxy::didChangeIsResponsive):
(WebKit::WebProcessProxy::setIgnoreInvalidMessageForTesting):
(WebKit::WebProcessProxy::didFinishLaunching):
(WebKit::WebProcessProxy::didDestroyFrame):
(WebKit::WebProcessProxy::recordUserGestureAuthorizationToken):
(WebKit::WebProcessProxy::postMessageToRemote):
(WebKit::WebProcessProxy::closeRemoteFrame):
(WebKit::WebProcessProxy::renderTreeAsText):
(WebKit::WebProcessProxy::maybeShutDown):
(WebKit::WebProcessProxy::canTerminateAuxiliaryProcess):
(WebKit::WebProcessProxy::windowServerConnectionStateChanged):
(WebKit::WebProcessProxy::updateAudibleMediaAssertions):
(WebKit::WebProcessProxy::updateMediaStreamingActivity):
(WebKit::WebProcessProxy::isResponsive):
(WebKit::WebProcessProxy::isResponsiveWithLazyStop):
(WebKit::WebProcessProxy::logDiagnosticMessageForResourceLimitTermination):
(WebKit::WebProcessProxy::didExceedCPULimit):
(WebKit::WebProcessProxy::updateBlobRegistryPartitioningState const):
(WebKit::WebProcessProxy::didCollectPrewarmInformation):
(WebKit::WebProcessProxy::didStartProvisionalLoadForMainFrame):
(WebKit::WebProcessProxy::reportProcessDisassociatedWithPageIfNecessary):
(WebKit::WebProcessProxy::createSpeechRecognitionServer):
(WebKit::WebProcessProxy::muteCaptureInPagesExcept):
(WebKit::WebProcessProxy::disableRemoteWorkers):
(WebKit::WebProcessProxy::enableRemoteWorkers):
(WebKit::WebProcessProxy::protectedWebsiteDataStore const):
(WebKit::WebProcessProxy::getNotifications):
(WebKit::WebProcessProxy::setAppBadge):
(WebKit::WebProcessProxy::setClientBadge):
(WebKit::WebProcessProxy::logger):
* Source/WebKit/UIProcess/WebProcessProxy.h:
* Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:
(WebKit::WebsiteDataStore::isBlobRegistryPartitioningEnabled const):

Canonical link: https://commits.webkit.org/269895@main

668ea96

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  wincairo
βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug   πŸ§ͺ wpe-wk2
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac βœ… πŸ›  gtk
βœ… πŸ§ͺ ios-wk2-wpt βœ… πŸ§ͺ gtk-wk2
βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk2 ❌ πŸ§ͺ api-gtk
βœ… πŸ›  tv βœ… πŸ§ͺ mac-AS-debug-wk2
βœ… πŸ›  tv-sim
βœ… πŸ›  watch
βœ… πŸ›  πŸ§ͺ unsafe-merge βœ… πŸ›  watch-sim

@cdumez cdumez self-assigned this Oct 28, 2023
@cdumez cdumez added the WebKit2 Bugs relating to the WebKit2 API layer label Oct 28, 2023
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Oct 28, 2023
@cdumez cdumez removed the merging-blocked Applied to prevent a change from being merged label Oct 28, 2023
@cdumez cdumez force-pushed the 263820_WebProcessProxy_smart_pointers branch from e594fc3 to fb2f134 Compare October 28, 2023 04:26
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Oct 28, 2023
@cdumez cdumez removed the merging-blocked Applied to prevent a change from being merged label Oct 28, 2023
@cdumez cdumez force-pushed the 263820_WebProcessProxy_smart_pointers branch from fb2f134 to 5659316 Compare October 28, 2023 04:42
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Oct 28, 2023
@cdumez cdumez removed the merging-blocked Applied to prevent a change from being merged label Oct 28, 2023
@cdumez cdumez force-pushed the 263820_WebProcessProxy_smart_pointers branch from 5659316 to 166f321 Compare October 28, 2023 05:00
if (page)
result.append(std::make_pair(page->process().coreProcessIdentifier(), RegistrableDomain(URL(page->currentURL()))));
}
for (auto& page : globalPages())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See CheckedRef?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

auto& is already a Ref& here. globalPages() returns a Vector<Ref<WebPageProxy>>

if (page)
page->disconnectFramesFromPage();
}
for (auto& page : pages())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use CheckedRef/Ref?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pages() returns a Vector<Ref<WebPageProxy>>. I guess I can use Ref&& instead of auto&.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ref& or Ref&& doesn't build. I'll just use Ref then. It's a bit more ref counting churn than auto& though.

}

RefPtr<WebPageProxy> WebProcessProxy::webPage(WebPageProxyIdentifier pageID)
{
return globalPageMap().get(pageID).get();
return globalPageMap().get(pageID);
}

RefPtr<WebPageProxy> WebProcessProxy::audioCapturingWebPage()
{
for (auto& page : globalPages()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use CheckedRef/Ref?

if (page)
page->postMessageToInjectedBundle("WebsiteDataScanForRegistrableDomainsFinished"_s, nullptr);
}
for (auto& page : globalPages())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use CheckedRef/Ref?

if (page)
page->postMessageToInjectedBundle("WebsiteDataScanForRegistrableDomainsFinished"_s, nullptr);
}
for (auto& page : globalPages())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

if (page)
page->postMessageToInjectedBundle("WebsiteDataDeletionForRegistrableDomainsFinished"_s, nullptr);
}
for (auto& page : globalPages())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

if (page)
page->gpuProcessDidFinishLaunching();
}
for (auto& page : pages())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

if (page)
page->gpuProcessExited(reason);
}
for (auto& page : pages())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

if (page)
page->dispatchProcessDidTerminate(reason);
}
for (auto& page : pages)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

if (page)
page->processDidBecomeUnresponsive();
}
for (auto& page : pages())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

if (page)
page->processDidBecomeResponsive();
}
for (auto& page : pages())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

if (page)
page->willChangeProcessIsResponsive();
}
for (auto& page : pages())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

if (page)
page->didChangeProcessIsResponsive();
}
for (auto& page : pages())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

@@ -1299,8 +1277,8 @@ void WebProcessProxy::didFinishLaunching(ProcessLauncher* launcher, IPC::Connect
}
#endif // USE(EXTENSIONKIT_ASSERTIONS)
#if PLATFORM(MAC)
for (const auto& page : pages()) {
if (page && page->preferences().backgroundWebContentRunningBoardThrottlingEnabled())
for (auto& page : pages()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

if (page)
page->activityStateDidChange(ActivityState::IsVisuallyIdle);
}
for (auto& page : pages())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

if (page)
page->processWillBecomeSuspended();
}
for (auto& page : pages())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

if (page)
page->processWillBecomeForeground();
}
for (auto& page : pages())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

@@ -2152,8 +2120,8 @@ PAL::SessionID WebProcessProxy::sessionID() const
void WebProcessProxy::createSpeechRecognitionServer(SpeechRecognitionServerIdentifier identifier)
{
RefPtr<WebPageProxy> targetPage;
for (auto& page : pages()) {
if (page && page->webPageID() == identifier) {
for (auto&& page : pages()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why &&? But also use Ref/CheckedRef instead?

@cdumez cdumez marked this pull request as ready for review October 28, 2023 21:00
@cdumez cdumez force-pushed the 263820_WebProcessProxy_smart_pointers branch from 166f321 to 8429de7 Compare October 28, 2023 21:01
@cdumez cdumez requested a review from a team as a code owner October 28, 2023 21:01
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Oct 28, 2023
@cdumez cdumez removed the merging-blocked Applied to prevent a change from being merged label Oct 28, 2023
@cdumez cdumez force-pushed the 263820_WebProcessProxy_smart_pointers branch from 8429de7 to 668ea96 Compare October 28, 2023 21:34
@cdumez cdumez added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Oct 28, 2023
https://bugs.webkit.org/show_bug.cgi?id=263820

Reviewed by Ryosuke Niwa.

* Source/WebKit/Shared/WebBackForwardListItem.h:
* Source/WebKit/UIProcess/API/C/WKPage.cpp:
(WKPageCopyRelatedPages):
* Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp:
(WebKit::WebAutomationSession::getBrowsingContexts):
* Source/WebKit/UIProcess/HighPerformanceGraphicsUsageSampler.cpp:
(WebKit::HighPerformanceGraphicsUsageSampler::timerFired):
* Source/WebKit/UIProcess/PerActivityStateCPUUsageSampler.cpp:
(WebKit::PerActivityStateCPUUsageSampler::pageForLogging const):
* Source/WebKit/UIProcess/ProvisionalPageProxy.h:
* Source/WebKit/UIProcess/RemotePageProxy.cpp:
(WebKit::RemotePageProxy::protectedPage const):
* Source/WebKit/UIProcess/RemotePageProxy.h:
* Source/WebKit/UIProcess/WebBackForwardCache.h:
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::~WebPageProxy):
(WebKit::WebPageProxy::nonEphemeralWebPageProxy):
(WebKit::WebPageProxy::protectedVisitedLinkStore):
* Source/WebKit/UIProcess/WebPageProxy.h:
* Source/WebKit/UIProcess/WebProcessCache.h:
* Source/WebKit/UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::anyProcessPoolNeedsUIBackgroundAssertion):
(WebKit::WebProcessPool::forEachProcessForSession):
(WebKit::WebProcessPool::checkedWebProcessCache):
(WebKit::WebProcessPool::checkedBackForwardCache):
* Source/WebKit/UIProcess/WebProcessPool.h:
* Source/WebKit/UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::allProcesses):
(WebKit::WebProcessProxy::processForIdentifier):
(WebKit::WebProcessProxy::globalPages):
(WebKit::WebProcessProxy::pages const):
(WebKit::WebProcessProxy::forWebPagesWithOrigin):
(WebKit::WebProcessProxy::allowedFirstPartiesForCookies):
(WebKit::WebProcessProxy::create):
(WebKit::WebProcessProxy::createForRemoteWorkers):
(WebKit::m_routingArbitrator):
(WebKit::WebProcessProxy::~WebProcessProxy):
(WebKit::WebProcessProxy::updateRegistrationWithDataStore):
(WebKit::WebProcessProxy::addProvisionalPageProxy):
(WebKit::WebProcessProxy::addRemotePageProxy):
(WebKit::WebProcessProxy::getLaunchOptions):
(WebKit::WebProcessProxy::shouldSendPendingMessage):
(WebKit::WebProcessProxy::processWillShutDown):
(WebKit::WebProcessProxy::startDisplayLink):
(WebKit::WebProcessProxy::stopDisplayLink):
(WebKit::WebProcessProxy::setDisplayLinkPreferredFramesPerSecond):
(WebKit::WebProcessProxy::setDisplayLinkForDisplayWantsFullSpeedUpdates):
(WebKit::WebProcessProxy::shutDown):
(WebKit::WebProcessProxy::webPage):
(WebKit::WebProcessProxy::audioCapturingWebPage):
(WebKit::WebProcessProxy::webPageWithActiveXRSession):
(WebKit::WebProcessProxy::notifyPageStatisticsAndDataRecordsProcessed):
(WebKit::WebProcessProxy::notifyWebsiteDataScanForRegistrableDomainsFinished):
(WebKit::WebProcessProxy::notifyWebsiteDataDeletionForRegistrableDomainsFinished):
(WebKit::WebProcessProxy::createWebPage):
(WebKit::WebProcessProxy::addExistingWebPage):
(WebKit::WebProcessProxy::removeWebPage):
(WebKit::WebProcessProxy::updateBackForwardItem):
(WebKit::WebProcessProxy::getNetworkProcessConnection):
(WebKit::WebProcessProxy::createGPUProcessConnection):
(WebKit::WebProcessProxy::gpuProcessDidFinishLaunching):
(WebKit::WebProcessProxy::gpuProcessExited):
(WebKit::WebProcessProxy::didReceiveMessage):
(WebKit::WebProcessProxy::didReceiveSyncMessage):
(WebKit::WebProcessProxy::processDidTerminateOrFailedToLaunch):
(WebKit::WebProcessProxy::didBecomeUnresponsive):
(WebKit::WebProcessProxy::didBecomeResponsive):
(WebKit::WebProcessProxy::willChangeIsResponsive):
(WebKit::WebProcessProxy::didChangeIsResponsive):
(WebKit::WebProcessProxy::setIgnoreInvalidMessageForTesting):
(WebKit::WebProcessProxy::didFinishLaunching):
(WebKit::WebProcessProxy::didDestroyFrame):
(WebKit::WebProcessProxy::recordUserGestureAuthorizationToken):
(WebKit::WebProcessProxy::postMessageToRemote):
(WebKit::WebProcessProxy::closeRemoteFrame):
(WebKit::WebProcessProxy::renderTreeAsText):
(WebKit::WebProcessProxy::maybeShutDown):
(WebKit::WebProcessProxy::canTerminateAuxiliaryProcess):
(WebKit::WebProcessProxy::windowServerConnectionStateChanged):
(WebKit::WebProcessProxy::updateAudibleMediaAssertions):
(WebKit::WebProcessProxy::updateMediaStreamingActivity):
(WebKit::WebProcessProxy::isResponsive):
(WebKit::WebProcessProxy::isResponsiveWithLazyStop):
(WebKit::WebProcessProxy::logDiagnosticMessageForResourceLimitTermination):
(WebKit::WebProcessProxy::didExceedCPULimit):
(WebKit::WebProcessProxy::updateBlobRegistryPartitioningState const):
(WebKit::WebProcessProxy::didCollectPrewarmInformation):
(WebKit::WebProcessProxy::didStartProvisionalLoadForMainFrame):
(WebKit::WebProcessProxy::reportProcessDisassociatedWithPageIfNecessary):
(WebKit::WebProcessProxy::createSpeechRecognitionServer):
(WebKit::WebProcessProxy::muteCaptureInPagesExcept):
(WebKit::WebProcessProxy::disableRemoteWorkers):
(WebKit::WebProcessProxy::enableRemoteWorkers):
(WebKit::WebProcessProxy::protectedWebsiteDataStore const):
(WebKit::WebProcessProxy::getNotifications):
(WebKit::WebProcessProxy::setAppBadge):
(WebKit::WebProcessProxy::setClientBadge):
(WebKit::WebProcessProxy::logger):
* Source/WebKit/UIProcess/WebProcessProxy.h:
* Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:
(WebKit::WebsiteDataStore::isBlobRegistryPartitioningEnabled const):

Canonical link: https://commits.webkit.org/269895@main
@webkit-commit-queue webkit-commit-queue force-pushed the 263820_WebProcessProxy_smart_pointers branch from 668ea96 to 6c1701a Compare October 28, 2023 23:39
@webkit-commit-queue
Copy link
Collaborator

Committed 269895@main (6c1701a): https://commits.webkit.org/269895@main

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

@webkit-commit-queue webkit-commit-queue merged commit 6c1701a into WebKit:main Oct 28, 2023
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Oct 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebKit2 Bugs relating to the WebKit2 API layer
Projects
None yet
5 participants