Process swapping on navigation needs to handle server redirects.
authorbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 2 Apr 2018 20:05:48 +0000 (20:05 +0000)
committerbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 2 Apr 2018 20:05:48 +0000 (20:05 +0000)
<rdar://problem/38690465> and https://bugs.webkit.org/show_bug.cgi?id=184142

Reviewed by Alex Christensen.

Source/WebKit:

The same rules we apply to process swapping for basic navigations need to apply
to server redirects as well.

There's three interesting cases we need to support that are covered by new API tests:
1 - The initial load in a WKWebView redirects cross-origin.
2 - A WKWebView is showing content from a.com, we start a load to b.com, and that redirects to c.com
3 - A WKWebView is showing content from a.com, we start a load to a.com, that that redirects to b.com.

Supporting all 3 of these brought their own little challenges.

By teaching Navigation objects more about redirects I was able to support all 3 cases.

* UIProcess/API/APINavigation.cpp:
(API::Navigation::Navigation):
(API::Navigation::setCurrentRequest):
(API::Navigation::appendRedirectionURL):
(API::Navigation::loggingString const):
(API::Navigation::loggingURL const): Deleted.
* UIProcess/API/APINavigation.h:
(API::Navigation::originalRequest const):
(API::Navigation::currentRequest const):
(API::Navigation::currentRequestProcessIdentifier const):
(API::Navigation::setCurrentRequestIsRedirect):
(API::Navigation::currentRequestIsRedirect const):
(API::Navigation::request const): Deleted.

* UIProcess/API/Cocoa/WKNavigation.mm:
(-[WKNavigation _request]):

* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::receivedPolicyDecision):
(WebKit::WebPageProxy::continueNavigationInNewProcess): If this continued navigation is currently in a server
  redirect, save off a lambda to synthesize a "did receive server redirect" callback once the new WebProcess is running.
(WebKit::WebPageProxy::didCreateMainFrame):
(WebKit::WebPageProxy::didStartProvisionalLoadForFrame): Possibly ignore this notification if it is really a
  cross-origin redirect that is just starting back up in a new WebProcess.
(WebKit::WebPageProxy::didReceiveServerRedirectForProvisionalLoadForFrame):
(WebKit::WebPageProxy::didCommitLoadForFrame):
(WebKit::WebPageProxy::decidePolicyForNavigationAction):
(WebKit::WebPageProxy::resetStateAfterProcessExited): Do not clear pageLoadState if the process is exitting for
  a navigation swap, as we will need to pick up where we left off when the load continues in a new WebProcess.
* UIProcess/WebPageProxy.h:
* UIProcess/WebPageProxy.messages.in:

* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::processForNavigation): If a process has never committed any provisional load, it can always
  be used to continue a navigation.
* UIProcess/WebProcessPool.h:

* UIProcess/WebProcessProxy.h:
(WebKit::WebProcessProxy::didCommitProvisionalLoad):
(WebKit::WebProcessProxy::hasCommittedAnyProvisionalLoads const):

* WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
(WebKit::WebFrameLoaderClient::dispatchDidReceiveServerRedirectForProvisionalLoad):
(WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):

Tools:

* TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
(-[PSONNavigationDelegate webView:didFinishNavigation:]):
(-[PSONNavigationDelegate webView:decidePolicyForNavigationAction:decisionHandler:]):
(-[PSONNavigationDelegate webView:didReceiveServerRedirectForProvisionalNavigation:]):
(-[PSONScheme addRedirectFromURLString:toURLString:]):
(-[PSONScheme webView:startURLSchemeTask:]):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@230174 268f45cc-cd09-0410-ab3c-d52691b4dbfc

13 files changed:
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/API/APINavigation.cpp
Source/WebKit/UIProcess/API/APINavigation.h
Source/WebKit/UIProcess/API/Cocoa/WKNavigation.mm
Source/WebKit/UIProcess/WebPageProxy.cpp
Source/WebKit/UIProcess/WebPageProxy.h
Source/WebKit/UIProcess/WebPageProxy.messages.in
Source/WebKit/UIProcess/WebProcessPool.cpp
Source/WebKit/UIProcess/WebProcessPool.h
Source/WebKit/UIProcess/WebProcessProxy.h
Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm

index caa613a..8625682 100644 (file)
@@ -1,3 +1,67 @@
+2018-04-02  Brady Eidson  <beidson@apple.com>
+
+        Process swapping on navigation needs to handle server redirects.
+        <rdar://problem/38690465> and https://bugs.webkit.org/show_bug.cgi?id=184142
+
+        Reviewed by Alex Christensen.
+
+        The same rules we apply to process swapping for basic navigations need to apply
+        to server redirects as well.
+
+        There's three interesting cases we need to support that are covered by new API tests:
+        1 - The initial load in a WKWebView redirects cross-origin.
+        2 - A WKWebView is showing content from a.com, we start a load to b.com, and that redirects to c.com
+        3 - A WKWebView is showing content from a.com, we start a load to a.com, that that redirects to b.com.
+
+        Supporting all 3 of these brought their own little challenges.
+
+        By teaching Navigation objects more about redirects I was able to support all 3 cases.
+
+        * UIProcess/API/APINavigation.cpp:
+        (API::Navigation::Navigation):
+        (API::Navigation::setCurrentRequest):
+        (API::Navigation::appendRedirectionURL):
+        (API::Navigation::loggingString const):
+        (API::Navigation::loggingURL const): Deleted.
+        * UIProcess/API/APINavigation.h:
+        (API::Navigation::originalRequest const):
+        (API::Navigation::currentRequest const):
+        (API::Navigation::currentRequestProcessIdentifier const):
+        (API::Navigation::setCurrentRequestIsRedirect):
+        (API::Navigation::currentRequestIsRedirect const):
+        (API::Navigation::request const): Deleted.
+
+        * UIProcess/API/Cocoa/WKNavigation.mm:
+        (-[WKNavigation _request]):
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::receivedPolicyDecision):
+        (WebKit::WebPageProxy::continueNavigationInNewProcess): If this continued navigation is currently in a server
+          redirect, save off a lambda to synthesize a "did receive server redirect" callback once the new WebProcess is running.
+        (WebKit::WebPageProxy::didCreateMainFrame):
+        (WebKit::WebPageProxy::didStartProvisionalLoadForFrame): Possibly ignore this notification if it is really a
+          cross-origin redirect that is just starting back up in a new WebProcess.
+        (WebKit::WebPageProxy::didReceiveServerRedirectForProvisionalLoadForFrame):
+        (WebKit::WebPageProxy::didCommitLoadForFrame):
+        (WebKit::WebPageProxy::decidePolicyForNavigationAction):
+        (WebKit::WebPageProxy::resetStateAfterProcessExited): Do not clear pageLoadState if the process is exitting for
+          a navigation swap, as we will need to pick up where we left off when the load continues in a new WebProcess.
+        * UIProcess/WebPageProxy.h:
+        * UIProcess/WebPageProxy.messages.in:
+
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::processForNavigation): If a process has never committed any provisional load, it can always
+          be used to continue a navigation.
+        * UIProcess/WebProcessPool.h:
+
+        * UIProcess/WebProcessProxy.h:
+        (WebKit::WebProcessProxy::didCommitProvisionalLoad):
+        (WebKit::WebProcessProxy::hasCommittedAnyProvisionalLoads const):
+
+        * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
+        (WebKit::WebFrameLoaderClient::dispatchDidReceiveServerRedirectForProvisionalLoad):
+        (WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):
+
 2018-04-02  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         [Extra zoom mode] Zoom level is sometimes excessive when zooming to focused form controls
index bea333e..b95753c 100644 (file)
@@ -41,13 +41,16 @@ Navigation::Navigation(WebNavigationState& state)
 
 Navigation::Navigation(WebNavigationState& state, WebCore::ResourceRequest&& request)
     : m_navigationID(state.generateNavigationID())
-    , m_request(WTFMove(request))
+    , m_originalRequest(WTFMove(request))
+    , m_currentRequest(m_originalRequest)
 {
-    m_redirectChain.append(m_request.url());
+    m_redirectChain.append(m_originalRequest.url());
 }
 
 Navigation::Navigation(WebNavigationState& state, WebBackForwardListItem& item, FrameLoadType backForwardFrameLoadType)
     : m_navigationID(state.generateNavigationID())
+    , m_originalRequest(item.url())
+    , m_currentRequest(m_originalRequest)
     , m_backForwardListItem(&item)
     , m_backForwardFrameLoadType(backForwardFrameLoadType)
 {
@@ -57,16 +60,22 @@ Navigation::~Navigation()
 {
 }
 
-void Navigation::appendRedirectionURL(const WebCore::URL& url)
+void Navigation::setCurrentRequest(ResourceRequest&& request, ProcessIdentifier processIdentifier)
+{
+    m_currentRequest = WTFMove(request);
+    m_currentRequestProcessIdentifier = processIdentifier;
+}
+
+void Navigation::appendRedirectionURL(const URL& url)
 {
     if (m_redirectChain.isEmpty() || m_redirectChain.last() != url)
         m_redirectChain.append(url);
 }
 
 #if !LOG_DISABLED
-WTF::String Navigation::loggingURL() const
+WTF::String Navigation::loggingString() const
 {
-    return m_backForwardListItem ? m_backForwardListItem->url() : m_request.url().string();
+    return makeString("Most recent URL: ", m_currentRequest.url().string(), " Back/forward list item URL: '", m_backForwardListItem ? m_backForwardListItem->url() : String { }, String::format("' (%p)", m_backForwardListItem.get()));
 }
 #endif
 
index 07c48ee..947fb3c 100644 (file)
@@ -26,6 +26,7 @@
 #pragma once
 
 #include "APIObject.h"
+#include <WebCore/Process.h>
 #include <WebCore/ResourceRequest.h>
 #include <wtf/Ref.h>
 
@@ -62,7 +63,14 @@ public:
 
     uint64_t navigationID() const { return m_navigationID; }
 
-    const WebCore::ResourceRequest& request() const { return m_request; }
+    const WebCore::ResourceRequest& originalRequest() const { return m_originalRequest; }
+    void setCurrentRequest(WebCore::ResourceRequest&&, WebCore::ProcessIdentifier);
+    const WebCore::ResourceRequest& currentRequest() const { return m_currentRequest; }
+    std::optional<WebCore::ProcessIdentifier> currentRequestProcessIdentifier() const { return m_currentRequestProcessIdentifier; }
+
+    void setCurrentRequestIsRedirect(bool isRedirect) { m_isRedirect = isRedirect; }
+    bool currentRequestIsRedirect() const { return m_isRedirect; }
+
     WebKit::WebBackForwardListItem* backForwardListItem() { return m_backForwardListItem.get(); }
     std::optional<WebCore::FrameLoadType> backForwardFrameLoadType() const { return m_backForwardFrameLoadType; }
 
@@ -82,7 +90,7 @@ public:
     const std::optional<std::pair<uint64_t, uint64_t>>& opener() const { return m_opener; }
 
 #if !LOG_DISABLED
-    WTF::String loggingURL() const;
+    WTF::String loggingString() const;
 #endif
 
 private:
@@ -91,10 +99,13 @@ private:
     Navigation(WebKit::WebNavigationState&, WebKit::WebBackForwardListItem&, WebCore::FrameLoadType);
 
     uint64_t m_navigationID;
-    WebCore::ResourceRequest m_request;
+    WebCore::ResourceRequest m_originalRequest;
+    WebCore::ResourceRequest m_currentRequest;
+    std::optional<WebCore::ProcessIdentifier> m_currentRequestProcessIdentifier;
     Vector<WebCore::URL> m_redirectChain;
     bool m_wasUserInitiated { true };
     bool m_shouldForceDownload { false };
+    bool m_isRedirect { false };
 
     RefPtr<WebKit::WebBackForwardListItem> m_backForwardListItem;
     std::optional<WebCore::FrameLoadType> m_backForwardFrameLoadType;
index ed66632..42fb858 100644 (file)
@@ -43,7 +43,7 @@
 
 - (NSURLRequest *)_request
 {
-    return _navigation->request().nsURLRequest(WebCore::DoNotUpdateHTTPBody);
+    return _navigation->originalRequest().nsURLRequest(WebCore::DoNotUpdateHTTPBody);
 }
 
 #pragma mark WKObject protocol implementation
index 1bc537b..c14781d 100644 (file)
@@ -2352,8 +2352,9 @@ void WebPageProxy::receivedPolicyDecision(PolicyAction action, WebFrameProxy& fr
 
         if (action == PolicyAction::Use && navigation) {
             auto proposedProcess = process().processPool().processForNavigation(*this, *navigation, action);
+
             if (proposedProcess.ptr() != &process()) {
-                LOG(Loading, "Switching to new process for navigation %" PRIu64 " to url '%s' (WebBackForwardListItem %p)", navigation->navigationID(), navigation->loggingURL().utf8().data(), navigation->backForwardListItem());
+                LOG(Loading, "Switching from process %i to new process for navigation %" PRIu64 " '%s'", processIdentifier(), navigation->navigationID(), navigation->loggingString().utf8().data());
 
                 RunLoop::main().dispatch([this, protectedThis = makeRef(*this), navigation = makeRef(*navigation), proposedProcess = WTFMove(proposedProcess)]() mutable {
                     continueNavigationInNewProcess(navigation.get(), WTFMove(proposedProcess));
@@ -2367,8 +2368,9 @@ void WebPageProxy::receivedPolicyDecision(PolicyAction action, WebFrameProxy& fr
 
 void WebPageProxy::continueNavigationInNewProcess(API::Navigation& navigation, Ref<WebProcessProxy>&& process)
 {
-    LOG(Loading, "Continuing navigation %" PRIu64 " to URL %s in a new web process", navigation.navigationID(), navigation.loggingURL().utf8().data());
+    LOG(Loading, "Continuing navigation %" PRIu64 " '%s' in a new web process", navigation.navigationID(), navigation.loggingString().utf8().data());
 
+    ASSERT(m_process.ptr() != process.ptr());
     attachToProcessForNavigation(WTFMove(process));
 
     if (auto* item = navigation.backForwardListItem()) {
@@ -2384,7 +2386,20 @@ void WebPageProxy::continueNavigationInNewProcess(API::Navigation& navigation, R
     }
 
     // FIXME: Work out timing of responding with the last policy delegate, etc
-    loadRequestWithNavigation(navigation, ResourceRequest { navigation.request() }, WebCore::ShouldOpenExternalURLsPolicy::ShouldAllowExternalSchemes, nullptr, NavigationPolicyCheck::Bypass);
+    ASSERT(!navigation.currentRequest().isEmpty());
+    loadRequestWithNavigation(navigation, ResourceRequest { navigation.currentRequest() }, WebCore::ShouldOpenExternalURLsPolicy::ShouldAllowExternalSchemes, nullptr, NavigationPolicyCheck::Bypass);
+
+    // Normally, notification of a server redirect comes from the WebContent process.
+    // If we are process swapping in response to a server redirect then that notification will not come from the new WebContent process.
+    // In this case we have the UIProcess synthesize the redirect notification at the appropriate time.
+    if (navigation.currentRequestIsRedirect()) {
+        ASSERT(!m_mainFrame);
+        m_mainFrameCreationHandler = [this, protectedThis = makeRef(*this), navigation = makeRef(navigation), request =  navigation.currentRequest()]() mutable {
+            ASSERT(m_mainFrame);
+            m_mainFrame->frameLoadState().didStartProvisionalLoad(request.url());
+            didReceiveServerRedirectForProvisionalLoadForFrame(m_mainFrame->frameID(), navigation->navigationID(), WTFMove(request), { });
+        };
+    }
 }
 
 void WebPageProxy::setUserAgent(String&& userAgent)
@@ -3193,6 +3208,11 @@ void WebPageProxy::didCreateMainFrame(uint64_t frameID)
 
     // Add the frame to the process wide map.
     m_process->frameCreated(frameID, m_mainFrame.get());
+
+    if (m_mainFrameCreationHandler) {
+        m_mainFrameCreationHandler();
+        m_mainFrameCreationHandler = nullptr;
+    }
 }
 
 void WebPageProxy::didCreateSubframe(uint64_t frameID)
@@ -3269,10 +3289,6 @@ void WebPageProxy::didStartProvisionalLoadForFrame(uint64_t frameID, uint64_t na
 {
     PageClientProtector protector(m_pageClient);
 
-    auto transaction = m_pageLoadState.transaction();
-
-    m_pageLoadState.clearPendingAPIRequestURL(transaction);
-
     WebFrameProxy* frame = m_process->webFrame(frameID);
     MESSAGE_CHECK(frame);
     MESSAGE_CHECK_URL(url);
@@ -3282,6 +3298,20 @@ void WebPageProxy::didStartProvisionalLoadForFrame(uint64_t frameID, uint64_t na
     if (frame->isMainFrame() && navigationID)
         navigation = &navigationState().navigation(navigationID);
 
+    // If this seemingly new load is actually continuing a server redirect for a previous navigation in a new process,
+    // then we ignore this notification.
+    if (navigation && navigation->currentRequestIsRedirect()) {
+        auto navigationProcessIdentifier = navigation->currentRequestProcessIdentifier();
+        if (navigationProcessIdentifier && *navigationProcessIdentifier != m_process->coreProcessIdentifier())
+            return;
+    }
+
+    LOG(Loading, "WebPageProxy::didStartProvisionalLoadForFrame to frameID %" PRIu64 ", navigationID %" PRIu64 ", url %s", frameID, navigationID, url.string().utf8().data());
+
+    auto transaction = m_pageLoadState.transaction();
+
+    m_pageLoadState.clearPendingAPIRequestURL(transaction);
+
     if (frame->isMainFrame()) {
         if (m_pageLoadStart)
             reportPageLoadResult(ResourceError { ResourceError::Type::Cancellation });
@@ -3302,27 +3332,29 @@ void WebPageProxy::didStartProvisionalLoadForFrame(uint64_t frameID, uint64_t na
         m_loaderClient->didStartProvisionalLoadForFrame(*this, *frame, navigation.get(), m_process->transformHandlesToObjects(userData.object()).get());
 }
 
-void WebPageProxy::didReceiveServerRedirectForProvisionalLoadForFrame(uint64_t frameID, uint64_t navigationID, WebCore::URL&& url, const UserData& userData)
+void WebPageProxy::didReceiveServerRedirectForProvisionalLoadForFrame(uint64_t frameID, uint64_t navigationID, ResourceRequest&& request, const UserData& userData)
 {
+    LOG(Loading, "WebPageProxy::didReceiveServerRedirectForProvisionalLoadForFrame to frameID %" PRIu64 ", navigationID %" PRIu64 ", url %s", frameID, navigationID, request.url().string().utf8().data());
+
     PageClientProtector protector(m_pageClient);
 
     WebFrameProxy* frame = m_process->webFrame(frameID);
     MESSAGE_CHECK(frame);
-    MESSAGE_CHECK_URL(url);
+    MESSAGE_CHECK_URL(request.url());
 
     // FIXME: We should message check that navigationID is not zero here, but it's currently zero for some navigations through the page cache.
     RefPtr<API::Navigation> navigation;
     if (navigationID) {
         navigation = &navigationState().navigation(navigationID);
-        navigation->appendRedirectionURL(url);
+        navigation->appendRedirectionURL(request.url());
     }
 
     auto transaction = m_pageLoadState.transaction();
 
     if (frame->isMainFrame())
-        m_pageLoadState.didReceiveServerRedirectForProvisionalLoad(transaction, url);
+        m_pageLoadState.didReceiveServerRedirectForProvisionalLoad(transaction, request.url());
 
-    frame->didReceiveServerRedirectForProvisionalLoad(url);
+    frame->didReceiveServerRedirectForProvisionalLoad(request.url());
 
     m_pageLoadState.commitChanges();
     if (m_navigationClient) {
@@ -3443,6 +3475,8 @@ void WebPageProxy::didCommitLoadForFrame(uint64_t frameID, uint64_t navigationID
     if (frame->isMainFrame() && navigationID)
         navigation = &navigationState().navigation(navigationID);
 
+    m_process->didCommitProvisionalLoad();
+
 #if PLATFORM(IOS)
     if (frame->isMainFrame()) {
         m_hasReceivedLayerTreeTransactionAfterDidCommitLoad = false;
@@ -3791,7 +3825,7 @@ void WebPageProxy::frameDidBecomeFrameSet(uint64_t frameID, bool value)
 
 void WebPageProxy::decidePolicyForNavigationAction(uint64_t frameID, const SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&& navigationActionData, const FrameInfoData& originatingFrameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, ResourceRequest&& request, uint64_t listenerID, const UserData& userData)
 {
-    LOG(Loading, "WebPageProxy::didStartProvisionalLoadForFrame - Target url %s", originalRequest.url().string().utf8().data());
+    LOG(Loading, "WebPageProxy::decidePolicyForNavigationAction - Original URL %s, current target URL %s", originalRequest.url().string().utf8().data(), request.url().string().utf8().data());
 
     PageClientProtector protector(m_pageClient);
 
@@ -3806,15 +3840,19 @@ void WebPageProxy::decidePolicyForNavigationAction(uint64_t frameID, const Secur
     MESSAGE_CHECK_URL(request.url());
     MESSAGE_CHECK_URL(originalRequest.url());
     
-    uint64_t newNavigationID { 0 };
-    Ref<WebFramePolicyListenerProxy> listener = frame->setUpPolicyListenerProxy(listenerID, PolicyListenerType::NavigationAction);
     Ref<API::Navigation> navigation = navigationID ? makeRef(m_navigationState->navigation(navigationID)) : m_navigationState->createLoadRequestNavigation(ResourceRequest(request));
 
-    newNavigationID = navigation->navigationID();
+    ASSERT(navigation->originalRequest().url() == originalRequest.url());
+
+    uint64_t newNavigationID = navigation->navigationID();
     navigation->setWasUserInitiated(!!navigationActionData.userGestureTokenIdentifier);
     navigation->setShouldForceDownload(!navigationActionData.downloadAttribute.isNull());
+    navigation->setCurrentRequest(ResourceRequest(request), m_process->coreProcessIdentifier());
+    navigation->setCurrentRequestIsRedirect(navigationActionData.isRedirect);
     navigation->setIsCrossOriginWindowOpenNavigation(navigationActionData.isCrossOriginWindowOpenNavigation);
     navigation->setOpener(navigationActionData.opener);
+
+    auto listener = makeRef(frame->setUpPolicyListenerProxy(listenerID, PolicyListenerType::NavigationAction));
     listener->setNavigation(WTFMove(navigation));
 
 #if ENABLE(CONTENT_FILTERING)
@@ -5803,9 +5841,12 @@ void WebPageProxy::resetStateAfterProcessExited(ProcessTerminationReason termina
     m_touchEventQueue.clear();
 #endif
 
-    PageLoadState::Transaction transaction = m_pageLoadState.transaction();
-    m_pageLoadState.reset(transaction);
+    if (terminationReason != ProcessTerminationReason::NavigationSwap) {
+        PageLoadState::Transaction transaction = m_pageLoadState.transaction();
+        m_pageLoadState.reset(transaction);
+    }
 
+    // FIXME: <rdar://problem/38676604> In case of process swaps, the old process should gracefully suspend instead of terminating.
     m_process->processTerminated();
 }
 
index a6ae92b..cc0c0f9 100644 (file)
@@ -1339,7 +1339,7 @@ private:
     void didCreateSubframe(uint64_t frameID);
 
     void didStartProvisionalLoadForFrame(uint64_t frameID, uint64_t navigationID, WebCore::URL&&, WebCore::URL&& unreachableURL, const UserData&);
-    void didReceiveServerRedirectForProvisionalLoadForFrame(uint64_t frameID, uint64_t navigationID, WebCore::URL&&, const UserData&);
+    void didReceiveServerRedirectForProvisionalLoadForFrame(uint64_t frameID, uint64_t navigationID, WebCore::ResourceRequest&&, const UserData&);
     void willPerformClientRedirectForFrame(uint64_t frameID, const String& url, double delay);
     void didCancelClientRedirectForFrame(uint64_t frameID);
     void didChangeProvisionalURLForFrame(uint64_t frameID, uint64_t navigationID, WebCore::URL&&);
@@ -1777,6 +1777,8 @@ private:
     Ref<WebsiteDataStore> m_websiteDataStore;
 
     RefPtr<WebFrameProxy> m_mainFrame;
+    Function<void()> m_mainFrameCreationHandler;
+
     RefPtr<WebFrameProxy> m_focusedFrame;
     RefPtr<WebFrameProxy> m_frameSetLargestFrame;
 
index 0c39083..a92cd37 100644 (file)
@@ -116,7 +116,7 @@ messages -> WebPageProxy {
 
     # Frame load messages
     DidStartProvisionalLoadForFrame(uint64_t frameID, uint64_t navigationID, WebCore::URL url, WebCore::URL unreachableURL, WebKit::UserData userData)
-    DidReceiveServerRedirectForProvisionalLoadForFrame(uint64_t frameID, uint64_t navigationID, WebCore::URL url, WebKit::UserData userData)
+    DidReceiveServerRedirectForProvisionalLoadForFrame(uint64_t frameID, uint64_t navigationID, WebCore::ResourceRequest request, WebKit::UserData userData)
     WillPerformClientRedirectForFrame(uint64_t frameID, String url, double delay)
     DidCancelClientRedirectForFrame(uint64_t frameID)
     DidChangeProvisionalURLForFrame(uint64_t frameID, uint64_t navigationID, WebCore::URL url)
index c8bed00..40168ad 100644 (file)
@@ -1985,6 +1985,9 @@ Ref<WebProcessProxy> WebProcessPool::processForNavigation(WebPageProxy& page, co
     if (!m_configuration->processSwapsOnNavigation())
         return page.process();
 
+    if (!page.process().hasCommittedAnyProvisionalLoads())
+        return page.process();
+
     // FIXME: We should support process swap when a window has an opener.
     if (navigation.opener())
         return page.process();
@@ -1994,9 +1997,9 @@ Ref<WebProcessProxy> WebProcessPool::processForNavigation(WebPageProxy& page, co
         return createNewWebProcess(page.websiteDataStore());
     }
 
-    auto targetURL = navigation.request().url();
+    auto targetURL = navigation.currentRequest().url();
     auto url = URL { ParsedURLString, page.pageLoadState().url() };
-    if (!url.isValid() || url.isBlankURL() || protocolHostAndPortAreEqual(url, targetURL))
+    if (!url.isValid() || url.isEmpty() || url.isBlankURL() || protocolHostAndPortAreEqual(url, targetURL))
         return page.process();
 
     action = PolicyAction::Suspend;
index 85838aa..f824f85 100644 (file)
@@ -78,6 +78,7 @@ class DownloadClient;
 class HTTPCookieStore;
 class InjectedBundleClient;
 class LegacyContextHistoryClient;
+class Navigation;
 class PageConfiguration;
 }
 
index 3bad2ee..80ab8a6 100644 (file)
@@ -205,6 +205,9 @@ public:
 
     void checkProcessLocalPortForActivity(const WebCore::MessagePortIdentifier&, CompletionHandler<void(WebCore::MessagePortChannelProvider::HasActivity)>&&);
 
+    void didCommitProvisionalLoad() { m_hasCommittedAnyProvisionalLoads = true; }
+    bool hasCommittedAnyProvisionalLoads() const { return m_hasCommittedAnyProvisionalLoads; }
+
 protected:
     static uint64_t generatePageID();
     WebProcessProxy(WebProcessPool&, WebsiteDataStore&);
@@ -329,6 +332,8 @@ private:
     HashSet<WebCore::MessagePortIdentifier> m_processEntangledPorts;
     HashMap<uint64_t, Function<void()>> m_messageBatchDeliveryCompletionHandlers;
     HashMap<uint64_t, CompletionHandler<void(WebCore::MessagePortChannelProvider::HasActivity)>> m_localPortActivityCompletionHandlers;
+
+    bool m_hasCommittedAnyProvisionalLoads { false };
 };
 
 } // namespace WebKit
index cc16e01..fcbdd75 100644 (file)
@@ -35,6 +35,7 @@
 #include "InjectedBundleBackForwardListItem.h"
 #include "InjectedBundleDOMWindowExtension.h"
 #include "InjectedBundleNavigationAction.h"
+#include "Logging.h"
 #include "NavigationActionData.h"
 #include "NetworkConnectionToWebProcessMessages.h"
 #include "NetworkProcessConnection.h"
@@ -306,11 +307,13 @@ void WebFrameLoaderClient::dispatchDidReceiveServerRedirectForProvisionalLoad()
     WebDocumentLoader& documentLoader = static_cast<WebDocumentLoader&>(*m_frame->coreFrame()->loader().provisionalDocumentLoader());
     RefPtr<API::Object> userData;
 
+    LOG(Loading, "WebProcess %i - dispatchDidReceiveServerRedirectForProvisionalLoad to request url %s", getpid(), documentLoader.request().url().string().utf8().data());
+
     // Notify the bundle client.
     webPage->injectedBundleLoaderClient().didReceiveServerRedirectForProvisionalLoadForFrame(*webPage, *m_frame, userData);
 
     // Notify the UIProcess.
-    webPage->send(Messages::WebPageProxy::DidReceiveServerRedirectForProvisionalLoadForFrame(m_frame->frameID(), documentLoader.navigationID(), documentLoader.url(), UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get())));
+    webPage->send(Messages::WebPageProxy::DidReceiveServerRedirectForProvisionalLoadForFrame(m_frame->frameID(), documentLoader.navigationID(), documentLoader.request(), UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get())));
 }
 
 void WebFrameLoaderClient::dispatchDidChangeProvisionalURL()
@@ -818,6 +821,8 @@ void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const Navigat
         return;
     }
 
+    LOG(Loading, "WebProcess %i - dispatchDecidePolicyForNavigationAction to request url %s", getpid(), request.url().string().utf8().data());
+
     m_isDecidingNavigationPolicyDecision = true;
     if (m_frame->isMainFrame())
         webPage->didStartNavigationPolicyCheck();
index 0b0ab09..cc09ee4 100644 (file)
@@ -1,3 +1,17 @@
+2018-04-02  Brady Eidson  <beidson@apple.com>
+
+        Process swapping on navigation needs to handle server redirects.
+        <rdar://problem/38690465> and https://bugs.webkit.org/show_bug.cgi?id=184142
+
+        Reviewed by Alex Christensen.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+        (-[PSONNavigationDelegate webView:didFinishNavigation:]):
+        (-[PSONNavigationDelegate webView:decidePolicyForNavigationAction:decisionHandler:]):
+        (-[PSONNavigationDelegate webView:didReceiveServerRedirectForProvisionalNavigation:]):
+        (-[PSONScheme addRedirectFromURLString:toURLString:]):
+        (-[PSONScheme webView:startURLSchemeTask:]):
+
 2018-04-02  Thibault Saunier  <tsaunier@igalia.com>
 
         webkitpy: Use current environment value for GST_DEBUG(_FILE) and DOT_DIR env vars
index 1734c79..71a8ac3 100644 (file)
@@ -28,6 +28,7 @@
 #import "PlatformUtilities.h"
 #import "Test.h"
 #import <WebKit/WKNavigationDelegate.h>
+#import <WebKit/WKNavigationPrivate.h>
 #import <WebKit/WKPreferencesPrivate.h>
 #import <WebKit/WKProcessPoolPrivate.h>
 #import <WebKit/WKUIDelegatePrivate.h>
@@ -44,6 +45,7 @@
 #import <WebKit/_WKWebsitePolicies.h>
 #import <wtf/Deque.h>
 #import <wtf/HashMap.h>
+#import <wtf/HashSet.h>
 #import <wtf/RetainPtr.h>
 #import <wtf/Vector.h>
 #import <wtf/text/StringHash.h>
@@ -57,6 +59,8 @@ static int numberOfDecidePolicyCalls;
 
 static RetainPtr<NSMutableArray> receivedMessages = adoptNS([@[] mutableCopy]);
 static bool receivedMessage;
+static bool serverRedirected;
+static HashSet<pid_t> seenPIDs;
 @interface PSONMessageHandler : NSObject <WKScriptMessageHandler>
 @end
 
@@ -75,15 +79,23 @@ static bool receivedMessage;
 
 - (void)webView:(WKWebView *)webView didFinishNavigation:(WKNavigation *)navigation
 {
+    seenPIDs.add([webView _webProcessIdentifier]);
     done = true;
 }
 
 - (void)webView:(WKWebView *)webView decidePolicyForNavigationAction:(WKNavigationAction *)navigationAction decisionHandler:(void (^)(WKNavigationActionPolicy))decisionHandler
 {
     ++numberOfDecidePolicyCalls;
+    seenPIDs.add([webView _webProcessIdentifier]);
     decisionHandler(WKNavigationActionPolicyAllow);
 }
 
+- (void)webView:(WKWebView *)webView didReceiveServerRedirectForProvisionalNavigation:(WKNavigation *)navigation
+{
+    seenPIDs.add([webView _webProcessIdentifier]);
+    serverRedirected = true;
+}
+
 @end
 
 static RetainPtr<WKWebView> createdWebView;
@@ -117,8 +129,10 @@ static RetainPtr<WKWebView> createdWebView;
 
 @interface PSONScheme : NSObject <WKURLSchemeHandler> {
     const char* _bytes;
+    HashMap<String, String> _redirects;
 }
 - (instancetype)initWithBytes:(const char*)bytes;
+- (void)addRedirectFromURLString:(NSString *)sourceURLString toURLString:(NSString *)destinationURLString;
 @end
 
 @implementation PSONScheme
@@ -130,9 +144,25 @@ static RetainPtr<WKWebView> createdWebView;
     return self;
 }
 
+- (void)addRedirectFromURLString:(NSString *)sourceURLString toURLString:(NSString *)destinationURLString
+{
+    _redirects.set(sourceURLString, destinationURLString);
+}
+
 - (void)webView:(WKWebView *)webView startURLSchemeTask:(id <WKURLSchemeTask>)task
 {
-    RetainPtr<NSURLResponse> response = adoptNS([[NSURLResponse alloc] initWithURL:task.request.URL MIMEType:@"text/html" expectedContentLength:1 textEncodingName:nil]);
+    NSURL *finalURL = task.request.URL;
+    auto target = _redirects.get(task.request.URL.absoluteString);
+    if (!target.isEmpty()) {
+        auto redirectResponse = adoptNS([[NSURLResponse alloc] initWithURL:task.request.URL MIMEType:nil expectedContentLength:0 textEncodingName:nil]);
+
+        finalURL = [NSURL URLWithString:(NSString *)target];
+        auto request = adoptNS([[NSURLRequest alloc] initWithURL:finalURL]);
+
+        [(id<WKURLSchemeTaskPrivate>)task _didPerformRedirection:redirectResponse.get() newRequest:request.get()];
+    }
+
+    RetainPtr<NSURLResponse> response = adoptNS([[NSURLResponse alloc] initWithURL:finalURL MIMEType:@"text/html" expectedContentLength:1 textEncodingName:nil]);
     [task didReceiveResponse:response.get()];
 
     if (_bytes) {
@@ -444,4 +474,137 @@ TEST(ProcessSwap, SameOriginWindowOpenNoOpener)
 
 #endif // PLATFORM(MAC)
 
+TEST(ProcessSwap, ServerRedirectFromNewWebView)
+{
+    auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
+    processPoolConfiguration.get().processSwapsOnNavigation = YES;
+    auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
+
+    auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    [webViewConfiguration setProcessPool:processPool.get()];
+    RetainPtr<PSONScheme> handler = adoptNS([[PSONScheme alloc] init]);
+    [handler addRedirectFromURLString:@"pson://host/main1.html" toURLString:@"psonredirected://host/main1.html"];
+    [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"pson"];
+
+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
+    auto delegate = adoptNS([[PSONNavigationDelegate alloc] init]);
+    [webView setNavigationDelegate:delegate.get()];
+
+    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://host/main1.html"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&serverRedirected);
+    serverRedirected = false;
+
+    seenPIDs.add([webView _webProcessIdentifier]);
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    seenPIDs.add([webView _webProcessIdentifier]);
+
+    EXPECT_FALSE(serverRedirected);
+    EXPECT_EQ(2, numberOfDecidePolicyCalls);
+    EXPECT_EQ(1u, seenPIDs.size());
+}
+
+TEST(ProcessSwap, ServerRedirect)
+{
+    auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
+    processPoolConfiguration.get().processSwapsOnNavigation = YES;
+    auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
+
+    auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    [webViewConfiguration setProcessPool:processPool.get()];
+    RetainPtr<PSONScheme> handler1 = adoptNS([[PSONScheme alloc] init]);
+    [handler1 addRedirectFromURLString:@"pson://host/main1.html" toURLString:@"psonredirected://host/main1.html"];
+    [webViewConfiguration setURLSchemeHandler:handler1.get() forURLScheme:@"pson"];
+    RetainPtr<PSONScheme> handler2 = adoptNS([[PSONScheme alloc] init]);
+    [webViewConfiguration setURLSchemeHandler:handler2.get() forURLScheme:@"originalload"];
+
+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
+    auto delegate = adoptNS([[PSONNavigationDelegate alloc] init]);
+    [webView setNavigationDelegate:delegate.get()];
+
+    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"originalload://host/main1.html"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    auto pidAfterFirstLoad = [webView _webProcessIdentifier];
+
+    EXPECT_EQ(1, numberOfDecidePolicyCalls);
+    EXPECT_EQ(1u, seenPIDs.size());
+    EXPECT_TRUE(*seenPIDs.begin() == pidAfterFirstLoad);
+
+    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://host/main1.html"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&serverRedirected);
+    serverRedirected = false;
+
+    seenPIDs.add([webView _webProcessIdentifier]);
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    seenPIDs.add([webView _webProcessIdentifier]);
+
+    EXPECT_FALSE(serverRedirected);
+    EXPECT_EQ(3, numberOfDecidePolicyCalls);
+    EXPECT_EQ(2u, seenPIDs.size());
+}
+
+TEST(ProcessSwap, ServerRedirect2)
+{
+    // This tests a load that *starts out* to the same origin as the previous load, but then redirects to a new origin.
+    auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
+    processPoolConfiguration.get().processSwapsOnNavigation = YES;
+    auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
+
+    auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    [webViewConfiguration setProcessPool:processPool.get()];
+    RetainPtr<PSONScheme> handler1 = adoptNS([[PSONScheme alloc] init]);
+    [handler1 addRedirectFromURLString:@"pson://host/main2.html" toURLString:@"psonredirected://host/main1.html"];
+    [webViewConfiguration setURLSchemeHandler:handler1.get() forURLScheme:@"pson"];
+    RetainPtr<PSONScheme> handler2 = adoptNS([[PSONScheme alloc] init]);
+    [webViewConfiguration setURLSchemeHandler:handler2.get() forURLScheme:@"psonredirected"];
+
+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
+    auto delegate = adoptNS([[PSONNavigationDelegate alloc] init]);
+    [webView setNavigationDelegate:delegate.get()];
+
+    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://host/main1.html"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    auto pidAfterFirstLoad = [webView _webProcessIdentifier];
+
+    EXPECT_FALSE(serverRedirected);
+    EXPECT_EQ(1, numberOfDecidePolicyCalls);
+    EXPECT_EQ(1u, seenPIDs.size());
+    EXPECT_TRUE(*seenPIDs.begin() == pidAfterFirstLoad);
+
+    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://host/main2.html"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&serverRedirected);
+    serverRedirected = false;
+
+    seenPIDs.add([webView _webProcessIdentifier]);
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    seenPIDs.add([webView _webProcessIdentifier]);
+
+    EXPECT_FALSE(serverRedirected);
+    EXPECT_EQ(3, numberOfDecidePolicyCalls);
+    EXPECT_EQ(2u, seenPIDs.size());
+}
+
+
 #endif // WK_API_ENABLED