WebKit.ApplicationManifestBasic API test is failing when enabling PSON
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Nov 2018 16:58:17 +0000 (16:58 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Nov 2018 16:58:17 +0000 (16:58 +0000)
https://bugs.webkit.org/show_bug.cgi?id=191602

Reviewed by Alex Christensen.

Source/WebKit:

Add support for process swapping for a [WKWebView loadHTML:] load by storing
the necessary data on the API::Navigation and doing a loadData() instead of
a loadRequest() after process swapping when this data is present on the
navigation.

* UIProcess/API/APINavigation.cpp:
(API::Navigation::Navigation):
* UIProcess/API/APINavigation.h:
(API::Navigation::create):
(API::Navigation::substituteData const):
* UIProcess/WebNavigationState.cpp:
(WebKit::WebNavigationState::createLoadDataNavigation):
* UIProcess/WebNavigationState.h:
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::loadData):
(WebKit::WebPageProxy::continueNavigationInNewProcess):

Tools:

Add API test coverage.

* TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:

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

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/API/APINavigation.cpp
Source/WebKit/UIProcess/API/APINavigation.h
Source/WebKit/UIProcess/WebNavigationState.cpp
Source/WebKit/UIProcess/WebNavigationState.h
Source/WebKit/UIProcess/WebPageProxy.cpp
Source/WebKit/UIProcess/WebPageProxy.h
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm

index 6f9bed9..ed9fb08 100644 (file)
@@ -1,3 +1,27 @@
+2018-11-14  Chris Dumez  <cdumez@apple.com>
+
+        WebKit.ApplicationManifestBasic API test is failing when enabling PSON
+        https://bugs.webkit.org/show_bug.cgi?id=191602
+
+        Reviewed by Alex Christensen.
+
+        Add support for process swapping for a [WKWebView loadHTML:] load by storing
+        the necessary data on the API::Navigation and doing a loadData() instead of
+        a loadRequest() after process swapping when this data is present on the
+        navigation.
+
+        * UIProcess/API/APINavigation.cpp:
+        (API::Navigation::Navigation):
+        * UIProcess/API/APINavigation.h:
+        (API::Navigation::create):
+        (API::Navigation::substituteData const):
+        * UIProcess/WebNavigationState.cpp:
+        (WebKit::WebNavigationState::createLoadDataNavigation):
+        * UIProcess/WebNavigationState.h:
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::loadData):
+        (WebKit::WebPageProxy::continueNavigationInNewProcess):
+
 2018-11-14  Antti Koivisto  <antti@apple.com>
 
         Align Mac WK2 layer flush throttling with iOS
index 68b87d8..f5fa3a5 100644 (file)
@@ -59,6 +59,13 @@ Navigation::Navigation(WebNavigationState& state, WebBackForwardListItem& target
 {
 }
 
+Navigation::Navigation(WebKit::WebNavigationState& state, std::unique_ptr<SubstituteData>&& substituteData)
+    : Navigation(state)
+{
+    ASSERT(substituteData);
+    m_substituteData = WTFMove(substituteData);
+}
+
 Navigation::~Navigation()
 {
 }
index ba1e393..6a87cc3 100644 (file)
@@ -26,6 +26,7 @@
 #pragma once
 
 #include "APIObject.h"
+#include "DataReference.h"
 #include "WebBackForwardListItem.h"
 #include <WebCore/Process.h>
 #include <WebCore/ResourceRequest.h>
@@ -42,6 +43,24 @@ class WebNavigationState;
 
 namespace API {
 
+struct SubstituteData {
+    WTF_MAKE_FAST_ALLOCATED;
+public:
+    SubstituteData(Vector<uint8_t>&& content, const WTF::String& MIMEType, const WTF::String& encoding, const WTF::String& baseURL, API::Object* userData)
+        : content(WTFMove(content))
+        , MIMEType(MIMEType)
+        , encoding(encoding)
+        , baseURL(baseURL)
+        , userData(userData)
+    { }
+
+    Vector<uint8_t> content;
+    WTF::String MIMEType;
+    WTF::String encoding;
+    WTF::String baseURL;
+    RefPtr<API::Object> userData;
+};
+
 class Navigation : public ObjectImpl<Object::Type::Navigation> {
     WTF_MAKE_NONCOPYABLE(Navigation);
 public:
@@ -60,6 +79,11 @@ public:
         return adoptRef(*new Navigation(state, WTFMove(request), fromItem));
     }
 
+    static Ref<Navigation> create(WebKit::WebNavigationState& state, std::unique_ptr<SubstituteData>&& substituteData)
+    {
+        return adoptRef(*new Navigation(state, WTFMove(substituteData)));
+    }
+
     virtual ~Navigation();
 
     uint64_t navigationID() const { return m_navigationID; }
@@ -114,10 +138,13 @@ public:
     const char* loggingString() const;
 #endif
 
+    const std::unique_ptr<SubstituteData>& substituteData() const { return m_substituteData; }
+
 private:
     explicit Navigation(WebKit::WebNavigationState&);
     Navigation(WebKit::WebNavigationState&, WebCore::ResourceRequest&&, WebKit::WebBackForwardListItem* fromItem);
     Navigation(WebKit::WebNavigationState&, WebKit::WebBackForwardListItem& targetItem, WebKit::WebBackForwardListItem* fromItem, WebCore::FrameLoadType);
+    Navigation(WebKit::WebNavigationState&, std::unique_ptr<SubstituteData>&&);
 
     uint64_t m_navigationID;
     WebCore::ResourceRequest m_originalRequest;
@@ -139,6 +166,7 @@ private:
     WebCore::LockHistory m_lockHistory;
     WebCore::LockBackForwardList m_lockBackForwardList;
     WTF::String m_clientRedirectSourceForHistory;
+    std::unique_ptr<SubstituteData> m_substituteData;
 };
 
 } // namespace API
index 5e3f241..79661a4 100644 (file)
@@ -68,9 +68,9 @@ Ref<API::Navigation> WebNavigationState::createReloadNavigation()
     return navigation;
 }
 
-Ref<API::Navigation> WebNavigationState::createLoadDataNavigation()
+Ref<API::Navigation> WebNavigationState::createLoadDataNavigation(std::unique_ptr<API::SubstituteData>&& substituteData)
 {
-    auto navigation = API::Navigation::create(*this);
+    auto navigation = API::Navigation::create(*this, WTFMove(substituteData));
 
     m_navigations.set(navigation->navigationID(), navigation.ptr());
 
index 0036d22..d958eb4 100644 (file)
@@ -30,6 +30,7 @@
 
 namespace API {
 class Navigation;
+struct SubstituteData;
 }
 
 namespace WebCore {
@@ -51,7 +52,7 @@ public:
     Ref<API::Navigation> createBackForwardNavigation(WebBackForwardListItem& targetItem, WebBackForwardListItem* currentItem, WebCore::FrameLoadType);
     Ref<API::Navigation> createLoadRequestNavigation(WebCore::ResourceRequest&&, WebBackForwardListItem* currentItem);
     Ref<API::Navigation> createReloadNavigation();
-    Ref<API::Navigation> createLoadDataNavigation();
+    Ref<API::Navigation> createLoadDataNavigation(std::unique_ptr<API::SubstituteData>&&);
 
     API::Navigation* navigation(uint64_t navigationID);
     RefPtr<API::Navigation> takeNavigation(uint64_t navigationID);
index f6fda75..5f7e0fb 100644 (file)
@@ -1087,7 +1087,14 @@ RefPtr<API::Navigation> WebPageProxy::loadData(const IPC::DataReference& data, c
     if (m_isClosed)
         return nullptr;
 
-    auto navigation = m_navigationState->createLoadDataNavigation();
+    auto navigation = m_navigationState->createLoadDataNavigation(std::make_unique<API::SubstituteData>(data.vector(), MIMEType, encoding, baseURL, userData));
+    loadDataWithNavigation(navigation, data, MIMEType, encoding, baseURL, userData);
+    return WTFMove(navigation);
+}
+
+void WebPageProxy::loadDataWithNavigation(API::Navigation& navigation, const IPC::DataReference& data, const String& MIMEType, const String& encoding, const String& baseURL, API::Object* userData)
+{
+    ASSERT(!m_isClosed);
 
     auto transaction = m_pageLoadState.transaction();
 
@@ -1097,7 +1104,7 @@ RefPtr<API::Navigation> WebPageProxy::loadData(const IPC::DataReference& data, c
         reattachToWebProcess();
 
     LoadParameters loadParameters;
-    loadParameters.navigationID = navigation->navigationID();
+    loadParameters.navigationID = navigation.navigationID();
     loadParameters.data = data;
     loadParameters.MIMEType = MIMEType;
     loadParameters.encodingName = encoding;
@@ -1108,8 +1115,6 @@ RefPtr<API::Navigation> WebPageProxy::loadData(const IPC::DataReference& data, c
     m_process->assumeReadAccessToBaseURL(baseURL);
     m_process->send(Messages::WebPage::LoadData(loadParameters), m_pageID);
     m_process->responsivenessTimer().start();
-
-    return WTFMove(navigation);
 }
 
 void WebPageProxy::loadAlternateHTML(const IPC::DataReference& htmlData, const String& encoding, const WebCore::URL& baseURL, const WebCore::URL& unreachableURL, API::Object* userData, bool forSafeBrowsing)
@@ -2616,7 +2621,10 @@ void WebPageProxy::continueNavigationInNewProcess(API::Navigation& navigation, R
 
     // FIXME: Work out timing of responding with the last policy delegate, etc
     ASSERT(!navigation.currentRequest().isEmpty());
-    loadRequestWithNavigation(navigation, ResourceRequest { navigation.currentRequest() }, WebCore::ShouldOpenExternalURLsPolicy::ShouldAllowExternalSchemes, nullptr, ShouldTreatAsContinuingLoad::Yes);
+    if (auto& substituteData = navigation.substituteData())
+        loadDataWithNavigation(navigation, { substituteData->content.data(), substituteData->content.size() }, substituteData->MIMEType, substituteData->encoding, substituteData->baseURL, substituteData->userData.get());
+    else
+        loadRequestWithNavigation(navigation, ResourceRequest { navigation.currentRequest() }, WebCore::ShouldOpenExternalURLsPolicy::ShouldAllowExternalSchemes, nullptr, ShouldTreatAsContinuingLoad::Yes);
 
     ASSERT(!m_mainFrame);
     m_mainFrameCreationHandler = [this, protectedThis = makeRef(*this), navigation = makeRef(navigation), request =  navigation.currentRequest(), mainFrameURL, isServerRedirect = navigation.currentRequestIsRedirect()]() mutable {
index f10d615..3f84408 100644 (file)
@@ -1539,6 +1539,7 @@ private:
     RefPtr<API::Navigation> reattachToWebProcessForReload();
     RefPtr<API::Navigation> reattachToWebProcessWithItem(WebBackForwardListItem&);
 
+    void loadDataWithNavigation(API::Navigation&, const IPC::DataReference&, const String& MIMEType, const String& encoding, const String& baseURL, API::Object* userData = nullptr);
     void loadRequestWithNavigation(API::Navigation&, WebCore::ResourceRequest&&, WebCore::ShouldOpenExternalURLsPolicy, API::Object* userData, WebCore::ShouldTreatAsContinuingLoad);
 
     void requestNotificationPermission(uint64_t notificationID, const String& originString);
index a8c2c1a..ba21985 100644 (file)
@@ -1,3 +1,14 @@
+2018-11-14  Chris Dumez  <cdumez@apple.com>
+
+        WebKit.ApplicationManifestBasic API test is failing when enabling PSON
+        https://bugs.webkit.org/show_bug.cgi?id=191602
+
+        Reviewed by Alex Christensen.
+
+        Add API test coverage.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
 2018-11-14  Jonathan Bedard  <jbedard@apple.com>
 
         webkitpy: Refactor port code for devices
index fbe8acc..8cb1303 100644 (file)
@@ -2647,6 +2647,46 @@ TEST(ProcessSwap, NavigateBackAndForth)
     EXPECT_WK_STREQ(@"pson://www.webkit.org/main.html", [backForwardList.backItem.URL absoluteString]);
 }
 
+TEST(ProcessSwap, SwapOnLoadHTMLString)
+{
+    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()];
+    auto handler = adoptNS([[PSONScheme alloc] init]);
+    [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://www.webkit.org/main.html"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    auto pid1 = [webView _webProcessIdentifier];
+
+    NSString *htmlString = @"<html><body>substituteData</body></html>";
+    [webView loadHTMLString:htmlString baseURL:[NSURL URLWithString:@"http://example.com"]];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    auto pid2 = [webView _webProcessIdentifier];
+    EXPECT_NE(pid1, pid2);
+
+    [webView evaluateJavaScript:@"document.body.innerText" completionHandler:^(id innerText, NSError *error) {
+        EXPECT_WK_STREQ(@"substituteData", (NSString *)innerText);
+        done = true;
+    }];
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+}
+
 #if PLATFORM(MAC)
 
 TEST(ProcessSwap, GoBackToSuspendedPageWithMainFrameIDThatIsNotOne)