REGRESSION: WKWebView navigation fails when navigating from about:blank
authorpvollan@apple.com <pvollan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Nov 2019 01:59:52 +0000 (01:59 +0000)
committerpvollan@apple.com <pvollan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Nov 2019 01:59:52 +0000 (01:59 +0000)
https://bugs.webkit.org/show_bug.cgi?id=203852
Source/WebKit:

<rdar://problem/56973112>

Reviewed by Brent Fulgham.

Previously, WebPageProxy::loadFile would unconditionally create a sandbox extension for the resource directory URL. Currently,
this method is calling WebPageProxy::maybeInitializeSandboxExtension to create a sandbox extension if needed. The sandbox
extension for the resource URL is not created if the WebContent process already has assumed access to the resource URL, which
is the case when loading the same file for the second time. The sandbox extension still needs to be issued in this case, since
the WebContent process is revoking its extension when the load is done. This patch restore the original behaviour by adding a
flag to WebPageProxy::maybeInitializeSandboxExtension to indicate whether the method should check if the process already has
assumed access.

API test: WKWebView.LoadRelativeFileURL

* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::maybeInitializeSandboxExtensionHandle):
(WebKit::WebPageProxy::loadRequestWithNavigationShared):
(WebKit::WebPageProxy::loadFile):
* UIProcess/WebPageProxy.h:
* UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::shouldSendPendingMessage):
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::loadRequestWaitingForProcessLaunch):
(WebKit::WebPage::loadRequestWaitingForPID): Deleted.
* WebProcess/WebPage/WebPage.h:
* WebProcess/WebPage/WebPage.messages.in:

Tools:

Reviewed by Brent Fulgham.

* TestWebKitAPI/Tests/WebKitCocoa/LoadFileURL.mm:
(TEST):

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

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/WebPageProxy.cpp
Source/WebKit/UIProcess/WebPageProxy.h
Source/WebKit/UIProcess/WebProcessProxy.cpp
Source/WebKit/WebProcess/WebPage/WebPage.cpp
Source/WebKit/WebProcess/WebPage/WebPage.h
Source/WebKit/WebProcess/WebPage/WebPage.messages.in
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/LoadFileURL.mm

index cb9c763..41df77a 100644 (file)
@@ -1,3 +1,34 @@
+2019-11-13  Per Arne Vollan  <pvollan@apple.com>
+
+        REGRESSION: WKWebView navigation fails when navigating from about:blank
+        https://bugs.webkit.org/show_bug.cgi?id=203852
+        <rdar://problem/56973112>
+
+        Reviewed by Brent Fulgham.
+
+        Previously, WebPageProxy::loadFile would unconditionally create a sandbox extension for the resource directory URL. Currently,
+        this method is calling WebPageProxy::maybeInitializeSandboxExtension to create a sandbox extension if needed. The sandbox
+        extension for the resource URL is not created if the WebContent process already has assumed access to the resource URL, which
+        is the case when loading the same file for the second time. The sandbox extension still needs to be issued in this case, since
+        the WebContent process is revoking its extension when the load is done. This patch restore the original behaviour by adding a
+        flag to WebPageProxy::maybeInitializeSandboxExtension to indicate whether the method should check if the process already has
+        assumed access.
+
+        API test: WKWebView.LoadRelativeFileURL
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::maybeInitializeSandboxExtensionHandle):
+        (WebKit::WebPageProxy::loadRequestWithNavigationShared):
+        (WebKit::WebPageProxy::loadFile):
+        * UIProcess/WebPageProxy.h:
+        * UIProcess/WebProcessProxy.cpp:
+        (WebKit::WebProcessProxy::shouldSendPendingMessage):
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::loadRequestWaitingForProcessLaunch):
+        (WebKit::WebPage::loadRequestWaitingForPID): Deleted.
+        * WebProcess/WebPage/WebPage.h:
+        * WebProcess/WebPage/WebPage.messages.in:
+
 2019-11-13  Myles C. Maxfield  <mmaxfield@apple.com>
 
         [Mac] Fix build
index fd9d354..a3e2066 100644 (file)
@@ -1082,7 +1082,7 @@ bool WebPageProxy::tryClose()
     return false;
 }
 
-void WebPageProxy::maybeInitializeSandboxExtensionHandle(WebProcessProxy& process, const URL& url, const URL& resourceDirectoryURL, SandboxExtension::Handle& sandboxExtensionHandle)
+void WebPageProxy::maybeInitializeSandboxExtensionHandle(WebProcessProxy& process, const URL& url, const URL& resourceDirectoryURL, SandboxExtension::Handle& sandboxExtensionHandle, bool checkAssumedReadAccessToResourceURL)
 {
     if (!url.isLocalFile())
         return;
@@ -1095,7 +1095,7 @@ void WebPageProxy::maybeInitializeSandboxExtensionHandle(WebProcessProxy& proces
 #endif
 
     if (!resourceDirectoryURL.isEmpty()) {
-        if (process.hasAssumedReadAccessToURL(resourceDirectoryURL))
+        if (checkAssumedReadAccessToResourceURL && process.hasAssumedReadAccessToURL(resourceDirectoryURL))
             return;
 
 #if HAVE(SANDBOX_ISSUE_READ_EXTENSION_TO_PROCESS_BY_AUDIT_TOKEN)
@@ -1226,7 +1226,7 @@ void WebPageProxy::loadRequestWithNavigationShared(Ref<WebProcessProxy>&& proces
     if (!process->isLaunching() || !url.isLocalFile())
         process->send(Messages::WebPage::LoadRequest(loadParameters), webPageID);
     else
-        process->send(Messages::WebPage::LoadRequestWaitingForPID(loadParameters, m_pageLoadState.resourceDirectoryURL(), m_identifier), webPageID);
+        process->send(Messages::WebPage::LoadRequestWaitingForProcessLaunch(loadParameters, m_pageLoadState.resourceDirectoryURL(), m_identifier, true), webPageID);
 #else
     process->send(Messages::WebPage::LoadRequest(loadParameters), webPageID);
 #endif
@@ -1273,12 +1273,13 @@ RefPtr<API::Navigation> WebPageProxy::loadFile(const String& fileURLString, cons
     loadParameters.request = fileURL;
     loadParameters.shouldOpenExternalURLsPolicy = ShouldOpenExternalURLsPolicy::ShouldNotAllow;
     loadParameters.userData = UserData(process().transformObjectsToHandles(userData).get());
-    maybeInitializeSandboxExtensionHandle(m_process, fileURL, resourceDirectoryURL, loadParameters.sandboxExtensionHandle);
+    const bool checkAssumedReadAccessToResourceURL = false;
+    maybeInitializeSandboxExtensionHandle(m_process, fileURL, resourceDirectoryURL, loadParameters.sandboxExtensionHandle, checkAssumedReadAccessToResourceURL);
     addPlatformLoadParameters(loadParameters);
 
 #if HAVE(SANDBOX_ISSUE_READ_EXTENSION_TO_PROCESS_BY_AUDIT_TOKEN)
     if (m_process->isLaunching())
-        m_process->send(Messages::WebPage::LoadRequestWaitingForPID(loadParameters, resourceDirectoryURL, m_identifier), m_webPageID);
+        m_process->send(Messages::WebPage::LoadRequestWaitingForProcessLaunch(loadParameters, resourceDirectoryURL, m_identifier, checkAssumedReadAccessToResourceURL), m_webPageID);
     else
         m_process->send(Messages::WebPage::LoadRequest(loadParameters), m_webPageID);
 #else
index c6c36f3..8488b76 100644 (file)
@@ -1609,7 +1609,7 @@ public:
     void setMockCaptureDevicesEnabledOverride(Optional<bool>);
 #endif
 
-    void maybeInitializeSandboxExtensionHandle(WebProcessProxy&, const URL&, const URL& resourceDirectoryURL, SandboxExtension::Handle&);
+    void maybeInitializeSandboxExtensionHandle(WebProcessProxy&, const URL&, const URL& resourceDirectoryURL, SandboxExtension::Handle&, bool checkAssumedReadAccessToResourceURL = true);
 
 #if ENABLE(WEB_AUTHN)
     void setMockWebAuthenticationConfiguration(WebCore::MockWebAuthenticationConfiguration&&);
index d2920cb..5b21313 100644 (file)
@@ -314,16 +314,17 @@ void WebProcessProxy::platformGetLaunchOptions(ProcessLauncher::LaunchOptions& l
 bool WebProcessProxy::shouldSendPendingMessage(const PendingMessage& message)
 {
 #if HAVE(SANDBOX_ISSUE_READ_EXTENSION_TO_PROCESS_BY_AUDIT_TOKEN)
-    if (message.encoder->messageName() == "LoadRequestWaitingForPID") {
+    if (message.encoder->messageName() == "LoadRequestWaitingForProcessLaunch") {
         auto buffer = message.encoder->buffer();
         auto bufferSize = message.encoder->bufferSize();
         std::unique_ptr<IPC::Decoder> decoder = makeUnique<IPC::Decoder>(buffer, bufferSize, nullptr, Vector<IPC::Attachment> { });
         LoadParameters loadParameters;
         URL resourceDirectoryURL;
         WebPageProxyIdentifier pageID;
-        if (decoder->decode(loadParameters) && decoder->decode(resourceDirectoryURL) && decoder->decode(pageID)) {
+        bool checkAssumedReadAccessToResourceURL;
+        if (decoder->decode(loadParameters) && decoder->decode(resourceDirectoryURL) && decoder->decode(pageID) && decoder->decode(checkAssumedReadAccessToResourceURL)) {
             if (auto* page = WebProcessProxy::webPage(pageID)) {
-                page->maybeInitializeSandboxExtensionHandle(static_cast<WebProcessProxy&>(*this), loadParameters.request.url(), resourceDirectoryURL, loadParameters.sandboxExtensionHandle);
+                page->maybeInitializeSandboxExtensionHandle(static_cast<WebProcessProxy&>(*this), loadParameters.request.url(), resourceDirectoryURL, loadParameters.sandboxExtensionHandle, checkAssumedReadAccessToResourceURL);
                 send(Messages::WebPage::LoadRequest(loadParameters), decoder->destinationID());
             }
         } else
index 0c58804..a556faf 100644 (file)
@@ -1555,8 +1555,8 @@ void WebPage::loadRequest(LoadParameters&& loadParameters)
     ASSERT(!m_pendingWebsitePolicies);
 }
 
-// LoadRequestWaitingForPID should never be sent to the WebProcess. It must always be converted to a LoadRequest message.
-NO_RETURN void WebPage::loadRequestWaitingForPID(LoadParameters&&, URL&&, WebPageProxyIdentifier)
+// LoadRequestWaitingForProcessLaunch should never be sent to the WebProcess. It must always be converted to a LoadRequest message.
+NO_RETURN void WebPage::loadRequestWaitingForProcessLaunch(LoadParameters&&, URL&&, WebPageProxyIdentifier, bool)
 {
     RELEASE_ASSERT_NOT_REACHED();
 }
index 62308dc..55830e8 100644 (file)
@@ -1335,7 +1335,7 @@ private:
     void tryClose();
     void platformDidReceiveLoadParameters(const LoadParameters&);
     void loadRequest(LoadParameters&&);
-    NO_RETURN void loadRequestWaitingForPID(LoadParameters&&, URL&&, WebPageProxyIdentifier);
+    NO_RETURN void loadRequestWaitingForProcessLaunch(LoadParameters&&, URL&&, WebPageProxyIdentifier, bool);
     void loadData(LoadParameters&&);
     void loadAlternateHTML(LoadParameters&&);
     void navigateToPDFLinkWithSimulatedClick(const String& url, WebCore::IntPoint documentPoint, WebCore::IntPoint screenPoint);
index da1c755..262daef 100644 (file)
@@ -165,7 +165,7 @@ GenerateSyntheticEditingCommand(enum:uint8_t WebKit::SyntheticEditingCommandType
     LoadURLInFrame(URL url, String referrer, WebCore::FrameIdentifier frameID)
     LoadDataInFrame(IPC::DataReference data, String MIMEType, String encodingName, URL baseURL, WebCore::FrameIdentifier frameID)
     LoadRequest(struct WebKit::LoadParameters loadParameters)
-    LoadRequestWaitingForPID(struct WebKit::LoadParameters loadParameters, URL resourceDirectoryURL, WebKit::WebPageProxyIdentifier pageID)
+    LoadRequestWaitingForProcessLaunch(struct WebKit::LoadParameters loadParameters, URL resourceDirectoryURL, WebKit::WebPageProxyIdentifier pageID, bool checkAssumedReadAccessToResourceURL)
     LoadData(struct WebKit::LoadParameters loadParameters)
     LoadAlternateHTML(struct WebKit::LoadParameters loadParameters)
 
index 9401a25..2a90d02 100644 (file)
@@ -1,3 +1,13 @@
+2019-11-13  Per Arne Vollan  <pvollan@apple.com>
+
+        REGRESSION: WKWebView navigation fails when navigating from about:blank
+        https://bugs.webkit.org/show_bug.cgi?id=203852
+
+        Reviewed by Brent Fulgham.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/LoadFileURL.mm:
+        (TEST):
+
 2019-11-13  Fujii Hironori  <Hironori.Fujii@sony.com>
 
         [Win][DumpRenderTree][WebKitTestRunner] eventSender.keyDown should support function keys
index 095fc4c..caff5be 100644 (file)
 #import "PlatformUtilities.h"
 #import "Test.h"
 #import "TestNavigationDelegate.h"
+#import <WebKit/WKProcessPoolPrivate.h>
 #import <WebKit/WebKit.h>
 #import <WebKit/WebViewPrivate.h>
+#import <WebKit/_WKProcessPoolConfiguration.h>
 #import <wtf/RetainPtr.h>
 
 TEST(WKWebView, LoadFileWithLoadRequest)
@@ -92,3 +94,29 @@ TEST(WKWebView, LoadRelativeFileURL)
     [delegate waitForDidFinishNavigation];
     EXPECT_WK_STREQ([webView _resourceDirectoryURL].path, fileURL.URLByDeletingLastPathComponent.path);
 }
+
+TEST(WKWebView, RepeatLoadFileURL)
+{
+    auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
+    processPoolConfiguration.get().processSwapsOnNavigation = NO;
+    auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
+
+    auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    [webViewConfiguration setProcessPool:processPool.get()];
+
+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
+
+    auto delegate = adoptNS([[TestNavigationDelegate alloc] init]);
+    [webView setNavigationDelegate:delegate.get()];
+    
+    NSString *path = [NSBundle.mainBundle pathForResource:@"simple" ofType:@"html" inDirectory:@"TestWebKitAPI.resources"];
+    NSURL *baseURL = [NSURL fileURLWithPath:path.stringByDeletingLastPathComponent];
+    NSURL *fileURL = [NSURL fileURLWithPath:path.lastPathComponent relativeToURL:baseURL];
+    EXPECT_NOT_NULL([webView loadFileURL:fileURL allowingReadAccessToURL:fileURL.URLByDeletingLastPathComponent]);
+    [delegate waitForDidFinishNavigation];
+    NSURL *aboutBlankURL = [NSURL URLWithString:@"about:blank"];
+    [webView loadRequest:[NSURLRequest requestWithURL:aboutBlankURL]];
+    [delegate waitForDidFinishNavigation];
+    EXPECT_NOT_NULL([webView loadFileURL:fileURL allowingReadAccessToURL:fileURL.URLByDeletingLastPathComponent]);
+    [delegate waitForDidFinishNavigation];
+}