Ensure AddAllowedFirstPartyForCookies message is processed before Sch…

…eduleResourceLoad

https://bugs.webkit.org/show_bug.cgi?id=247582
rdar://102056030

Reviewed by Chris Dumez.

The API test ProcessSwap.NavigateCrossSiteBeforePageLoadEnd was so sensitive to the reordering of messages
that it started failing, so I made it respond asynchronously to give the network process time to respond
to these new roundtrip messages.  Otherwise, no change in behavior.  It's taking something that's async
and adding an async step.

* Source/WebKit/NetworkProcess/NetworkProcess.cpp:
(WebKit::NetworkProcess::initializeNetworkProcess):
(WebKit::NetworkProcess::addAllowedFirstPartyForCookies):
* Source/WebKit/NetworkProcess/NetworkProcess.h:
* Source/WebKit/NetworkProcess/NetworkProcess.messages.in:
* Source/WebKit/NetworkProcess/NetworkSession.cpp:
(WebKit::NetworkSession::ensureSWServer):
* Source/WebKit/NetworkProcess/SharedWorker/WebSharedWorkerServer.cpp:
(WebKit::WebSharedWorkerServer::createContextConnection):
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::loadAlternateHTML):
(WebKit::WebPageProxy::receivedNavigationPolicyDecision):
(WebKit::WebPageProxy::triggerBrowsingContextGroupSwitchForNavigation):
* Source/WebKit/UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::processForNavigation):

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

LayoutTests/platform/gtk/http/tests/navigation/keyboard-events-during-provisional-navigation-expected.txt

@@ -0,0 +1,48 @@
+CONSOLE MESSAGE: Provisional navigation started.
+CONSOLE MESSAGE: No trusted events should be logged and the input element should have the value "".
+CONSOLE MESSAGE: Dispatching untrusted keypress event.
+CONSOLE MESSAGE: keypressevent dispatched (isTrusted: false).
+CONSOLE MESSAGE: Pressing tab.
+CONSOLE MESSAGE: Active element after pressing tab: [object HTMLInputElement].
+CONSOLE MESSAGE: Pressing "a".
+CONSOLE MESSAGE: Setting marked text to "b".
+CONSOLE MESSAGE: Inserting text "c".
+CONSOLE MESSAGE: Pasting text "d".
+CONSOLE MESSAGE: Input element value after text input events: "".
+CONSOLE MESSAGE: Pressing "z" with access key modifiers should navigate to resources/keyboard-events-after-navigation.html.
+CONSOLE MESSAGE: keydownevent dispatched (isTrusted: true).
+CONSOLE MESSAGE: keyupevent dispatched (isTrusted: true).
+CONSOLE MESSAGE: Finished navigating to resources/keyboard-events-after-navigation.html.
+CONSOLE MESSAGE: Trusted events should be logged and the input element should have the value "acd".
+CONSOLE MESSAGE: Dispatching untrusted keypress event.
+CONSOLE MESSAGE: keypressevent dispatched (isTrusted: false).
+CONSOLE MESSAGE: Pressing tab.
+CONSOLE MESSAGE: keydownevent dispatched (isTrusted: true).
+CONSOLE MESSAGE: keyupevent dispatched (isTrusted: true).
+CONSOLE MESSAGE: Active element after pressing tab: [object HTMLInputElement].
+CONSOLE MESSAGE: Pressing "a".
+CONSOLE MESSAGE: keydownevent dispatched (isTrusted: true).
+CONSOLE MESSAGE: keypressevent dispatched (isTrusted: true).
+CONSOLE MESSAGE: textInputevent dispatched (isTrusted: true).
+CONSOLE MESSAGE: beforeinputevent dispatched (isTrusted: true).
+CONSOLE MESSAGE: inputevent dispatched (isTrusted: true).
+CONSOLE MESSAGE: keyupevent dispatched (isTrusted: true).
+CONSOLE MESSAGE: Setting marked text to "b".
+CONSOLE MESSAGE: compositionstartevent dispatched (isTrusted: true).
+CONSOLE MESSAGE: compositionupdateevent dispatched (isTrusted: true).
+CONSOLE MESSAGE: beforeinputevent dispatched (isTrusted: true).
+CONSOLE MESSAGE: inputevent dispatched (isTrusted: true).
+CONSOLE MESSAGE: Inserting text "c".
+CONSOLE MESSAGE: beforeinputevent dispatched (isTrusted: true).
+CONSOLE MESSAGE: inputevent dispatched (isTrusted: true).
+CONSOLE MESSAGE: textInputevent dispatched (isTrusted: true).
+CONSOLE MESSAGE: beforeinputevent dispatched (isTrusted: true).
+CONSOLE MESSAGE: inputevent dispatched (isTrusted: true).
+CONSOLE MESSAGE: compositionendevent dispatched (isTrusted: true).
+CONSOLE MESSAGE: Pasting text "d".
+CONSOLE MESSAGE: pasteevent dispatched (isTrusted: true).
+CONSOLE MESSAGE: textInputevent dispatched (isTrusted: true).
+CONSOLE MESSAGE: beforeinputevent dispatched (isTrusted: true).
+CONSOLE MESSAGE: inputevent dispatched (isTrusted: true).
+CONSOLE MESSAGE: Input element value after text input events: "acd".
+

Source/WebKit/NetworkProcess/NetworkProcess.cpp

@@ -328,7 +328,7 @@ void NetworkProcess::initializeNetworkProcess(NetworkProcessCreationParameters&&
     m_ftpEnabled = parameters.ftpEnabled;
 
     for (auto [processIdentifier, domain] : parameters.allowedFirstPartiesForCookies)
-        addAllowedFirstPartyForCookies(processIdentifier, WTFMove(domain));
+        addAllowedFirstPartyForCookies(processIdentifier, WTFMove(domain), [] { });
 
     for (auto& supplement : m_supplements.values())
         supplement->initialize(parameters);
@@ -386,11 +386,12 @@ void NetworkProcess::createNetworkConnectionToWebProcess(ProcessIdentifier ident
         session->storageManager().startReceivingMessageFromConnection(connection.connection());
 }
 
-void NetworkProcess::addAllowedFirstPartyForCookies(WebCore::ProcessIdentifier processIdentifier, WebCore::RegistrableDomain&& firstPartyForCookies)
+void NetworkProcess::addAllowedFirstPartyForCookies(WebCore::ProcessIdentifier processIdentifier, WebCore::RegistrableDomain&& firstPartyForCookies, CompletionHandler<void()>&& completionHandler)
 {
     m_allowedFirstPartiesForCookies.ensure(processIdentifier, [] {
         return HashSet<RegistrableDomain> { };
     }).iterator->value.add(WTFMove(firstPartyForCookies));
+    completionHandler();
 }
 
 bool NetworkProcess::allowsFirstPartyForCookies(WebCore::ProcessIdentifier processIdentifier, const URL& firstParty)

Source/WebKit/NetworkProcess/NetworkProcess.h

@@ -400,7 +400,7 @@ class NetworkProcess : public AuxiliaryProcess, private DownloadManager::Client,
     void deleteWebsiteDataForOrigins(PAL::SessionID, OptionSet<WebsiteDataType>, const Vector<WebCore::SecurityOriginData>& origins, const Vector<String>& cookieHostNames, const Vector<String>& HSTSCacheHostnames, const Vector<RegistrableDomain>&, CompletionHandler<void()>&&);
 
     bool allowsFirstPartyForCookies(WebCore::ProcessIdentifier, const URL&);
-    void addAllowedFirstPartyForCookies(WebCore::ProcessIdentifier, WebCore::RegistrableDomain&&);
+    void addAllowedFirstPartyForCookies(WebCore::ProcessIdentifier, WebCore::RegistrableDomain&&, CompletionHandler<void()>&&);
 
 private:
     void platformInitializeNetworkProcess(const NetworkProcessCreationParameters&);

Source/WebKit/NetworkProcess/NetworkProcess.messages.in

@@ -25,7 +25,7 @@ messages -> NetworkProcess LegacyReceiver {
 
     CreateNetworkConnectionToWebProcess(WebCore::ProcessIdentifier processIdentifier, PAL::SessionID sessionID) -> (std::optional<IPC::Connection::Handle> connectionHandle, enum:uint8_t WebCore::HTTPCookieAcceptPolicy cookieAcceptPolicy)
 
-    AddAllowedFirstPartyForCookies(WebCore::ProcessIdentifier processIdentifier, WebCore::RegistrableDomain firstPartyForCookies)
+    AddAllowedFirstPartyForCookies(WebCore::ProcessIdentifier processIdentifier, WebCore::RegistrableDomain firstPartyForCookies) -> ()
 
 #if USE(SOUP)
     SetIgnoreTLSErrors(PAL::SessionID sessionID, bool ignoreTLSErrors)

Source/WebKit/NetworkProcess/NetworkSession.cpp

@@ -662,7 +662,7 @@ SWServer& NetworkSession::ensureSWServer()
                 completionHandler();
             }, 0);
         }, WTFMove(appBoundDomainsCallback), [this](auto webProcessIdentifier, auto&& firstPartyForCookies) {
-            m_networkProcess->addAllowedFirstPartyForCookies(webProcessIdentifier, WTFMove(firstPartyForCookies));
+            m_networkProcess->addAllowedFirstPartyForCookies(webProcessIdentifier, WTFMove(firstPartyForCookies), [] { });
         });
     }
     return *m_swServer;

Source/WebKit/NetworkProcess/SharedWorker/WebSharedWorkerServer.cpp

@@ -142,7 +142,7 @@ void WebSharedWorkerServer::createContextConnection(const WebCore::RegistrableDo
         RELEASE_LOG(SharedWorker, "WebSharedWorkerServer::createContextConnection should now have created a connection");
 
         // FIXME: Add a check that the process this firstPartyForCookies came from was allowed to use it as a firstPartyForCookies.
-        m_session.networkProcess().addAllowedFirstPartyForCookies(remoteProcessIdentifier, WebCore::RegistrableDomain(firstPartyForCookies));
+        m_session.networkProcess().addAllowedFirstPartyForCookies(remoteProcessIdentifier, WebCore::RegistrableDomain(firstPartyForCookies), [] { });
 
         ASSERT(m_pendingContextConnectionDomains.contains(registrableDomain));
         m_pendingContextConnectionDomains.remove(registrableDomain);

Source/WebKit/UIProcess/WebPageProxy.cpp

@@ -1707,8 +1707,6 @@ void WebPageProxy::loadAlternateHTML(const IPC::DataReference& htmlData, const S
     if (m_mainFrame)
         m_mainFrame->setUnreachableURL(unreachableURL);
 
-    websiteDataStore().networkProcess().send(Messages::NetworkProcess::AddAllowedFirstPartyForCookies(m_process->coreProcessIdentifier(), RegistrableDomain(baseURL)), 0);
-
     LoadParameters loadParameters;
     loadParameters.navigationID = 0;
     loadParameters.data = htmlData;
@@ -1720,11 +1718,13 @@ void WebPageProxy::loadAlternateHTML(const IPC::DataReference& htmlData, const S
     loadParameters.userData = UserData(process().transformObjectsToHandles(userData).get());
     addPlatformLoadParameters(process(), loadParameters);
 
-    m_process->markProcessAsRecentlyUsed();
-    m_process->assumeReadAccessToBaseURL(*this, baseURL.string());
-    m_process->assumeReadAccessToBaseURL(*this, unreachableURL.string());
-    send(Messages::WebPage::LoadAlternateHTML(loadParameters));
-    m_process->startResponsivenessTimer();
+    websiteDataStore().networkProcess().sendWithAsyncReply(Messages::NetworkProcess::AddAllowedFirstPartyForCookies(m_process->coreProcessIdentifier(), RegistrableDomain(baseURL)), [this, protectedThis = Ref { *this }, process = m_process, loadParameters = WTFMove(loadParameters), baseURL, unreachableURL] {
+        process->markProcessAsRecentlyUsed();
+        process->assumeReadAccessToBaseURL(*this, baseURL.string());
+        process->assumeReadAccessToBaseURL(*this, unreachableURL.string());
+        send(Messages::WebPage::LoadAlternateHTML(loadParameters));
+        process->startResponsivenessTimer();
+    });
 }
 
 void WebPageProxy::loadWebArchiveData(API::Data* webArchiveData, API::Object* userData)
@@ -3560,9 +3560,6 @@ void WebPageProxy::receivedNavigationPolicyDecision(PolicyAction policyAction, A
         navigation->websitePolicies()->setContentBlockersEnabled(false);
     }
 
-    if (policyAction == PolicyAction::Use && navigation && frame.isMainFrame())
-        websiteDataStore->networkProcess().send(Messages::NetworkProcess::AddAllowedFirstPartyForCookies(process().coreProcessIdentifier(), RegistrableDomain(navigation->currentRequest().url())), 0);
-
 #if ENABLE(DEVICE_ORIENTATION)
     if (navigation && (!navigation->websitePolicies() || navigation->websitePolicies()->deviceOrientationAndMotionAccessState() == WebCore::DeviceOrientationOrMotionPermissionState::Prompt)) {
         auto deviceOrientationPermission = websiteDataStore->deviceOrientationAndMotionAccessController().cachedDeviceOrientationPermission(SecurityOriginData::fromURL(navigation->currentRequest().url()));
@@ -6023,13 +6020,14 @@ void WebPageProxy::triggerBrowsingContextGroupSwitchForNavigation(uint64_t navig
     else
         processForNavigation = m_process->processPool().processForRegistrableDomain(websiteDataStore(), responseDomain, m_process->lockdownMode());
 
-    websiteDataStore().networkProcess().send(Messages::NetworkProcess::AddAllowedFirstPartyForCookies(processForNavigation->coreProcessIdentifier(), RegistrableDomain(navigation->currentRequest().url())), 0);
-
-    // Tell committed process to stop loading since we're going to do the provisional load in a provisional page now.
-    if (!m_provisionalPage)
-        send(Messages::WebPage::StopLoadingDueToProcessSwap());
-    continueNavigationInNewProcess(*navigation, *m_mainFrame, nullptr, processForNavigation.releaseNonNull(), ProcessSwapRequestedByClient::No, ShouldTreatAsContinuingLoad::YesAfterProvisionalLoadStarted, existingNetworkResourceLoadIdentifierToResume);
-    completionHandler(true);
+    auto processIdentifier = processForNavigation->coreProcessIdentifier();
+    websiteDataStore().networkProcess().sendWithAsyncReply(Messages::NetworkProcess::AddAllowedFirstPartyForCookies(processIdentifier, RegistrableDomain(navigation->currentRequest().url())), [this, protectedThis = Ref { *this }, completionHandler = WTFMove(completionHandler), processForNavigation = WTFMove(processForNavigation), existingNetworkResourceLoadIdentifierToResume, navigation] () mutable {
+        // Tell committed process to stop loading since we're going to do the provisional load in a provisional page now.
+        if (!m_provisionalPage)
+            send(Messages::WebPage::StopLoadingDueToProcessSwap());
+        continueNavigationInNewProcess(*navigation, *m_mainFrame, nullptr, processForNavigation.releaseNonNull(), ProcessSwapRequestedByClient::No, ShouldTreatAsContinuingLoad::YesAfterProvisionalLoadStarted, existingNetworkResourceLoadIdentifierToResume);
+        completionHandler(true);
+    });
 }
 
 void WebPageProxy::unableToImplementPolicy(FrameIdentifier frameID, const ResourceError& error, const UserData& userData)

Source/WebKit/UIProcess/WebProcessPool.cpp

@@ -1827,8 +1827,6 @@ void WebProcessPool::processForNavigation(WebPageProxy& page, const API::Navigat
             configuration().setClientWouldBenefitFromAutomaticProcessPrewarming(true);
         }
 
-        page->websiteDataStore().networkProcess().send(Messages::NetworkProcess::AddAllowedFirstPartyForCookies(process->coreProcessIdentifier(), RegistrableDomain(navigation->currentRequest().url())), 0);
-
         if (m_configuration->alwaysKeepAndReuseSwappedProcesses() && process.ptr() != sourceProcess.ptr()) {
             static std::once_flag onceFlag;
             std::call_once(onceFlag, [] {
@@ -1840,7 +1838,10 @@ void WebProcessPool::processForNavigation(WebPageProxy& page, const API::Navigat
             LOG(ProcessSwapping, "(ProcessSwapping) Navigating from %s to %s, keeping around old process. Now holding on to old processes for %u origins.", sourceURL.string().utf8().data(), navigation->currentRequest().url().string().utf8().data(), m_swappedProcessesPerRegistrableDomain.size());
         }
 
-        completionHandler(WTFMove(process), suspendedPage, reason);
+        auto processIdentifier = process->coreProcessIdentifier();
+        page->websiteDataStore().networkProcess().sendWithAsyncReply(Messages::NetworkProcess::AddAllowedFirstPartyForCookies(processIdentifier, RegistrableDomain(navigation->currentRequest().url())), [completionHandler = WTFMove(completionHandler), process = WTFMove(process), suspendedPage = WTFMove(suspendedPage), reason] () mutable {
+            completionHandler(WTFMove(process), suspendedPage, reason);
+        });
     });
 }
 

Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm

@@ -3217,6 +3217,7 @@ static unsigned waitUntilClientWidthIs(WKWebView *webView, unsigned expectedClie
     [webViewConfiguration setProcessPool:processPool.get()];
     auto handler = adoptNS([[PSONScheme alloc] init]);
     [handler addMappingFromURLString:@"pson://www.webkit.org/main.html" toData:navigateBeforePageLoadEndBytes];
+    [handler setShouldRespondAsynchronously:YES];
     [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
 
     auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);