REGRESSION(249649): Unable to open local files in MiniBrowser on macOS
authorpvollan@apple.com <pvollan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 16 Sep 2019 19:15:51 +0000 (19:15 +0000)
committerpvollan@apple.com <pvollan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 16 Sep 2019 19:15:51 +0000 (19:15 +0000)
https://bugs.webkit.org/show_bug.cgi?id=201798

Reviewed by Brent Fulgham.

The commit <https://trac.webkit.org/changeset/249649> introduced a MiniBrowser regression on macOS where
MiniBrowser is not able to open local files. The change set r249649 fixed a problem where the WebContent
process PID was not ready to be used when creating a sandbox extension. This happened in the cases where
the WebContent process had not finished launching when the load started. The WebContent process is also
creating sandbox extensions for the Networking process for the files being loaded, and also needs to be
passing the PID of the Networking process when creating these. This patch is addressing this by getting
the PID of the Networking process when the WebContent process is initially getting the connection to the
Networking process. The PID is then stored in the NetworkProcessConnection class, from where it is passed
to the NetworkLoadParameters, and used when creating the sandbox extension for the Networking process.

* NetworkProcess/NetworkLoadParameters.h:
* NetworkProcess/NetworkResourceLoadParameters.cpp:
(WebKit::NetworkResourceLoadParameters::encode const):
* UIProcess/Network/NetworkProcessProxy.cpp:
(WebKit::NetworkProcessProxy::~NetworkProcessProxy):
(WebKit::NetworkProcessProxy::openNetworkProcessConnection):
(WebKit::NetworkProcessProxy::networkProcessCrashed):
(WebKit::NetworkProcessProxy::didFinishLaunching):
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::loadRequestWithNavigationShared):
* UIProcess/WebProcessProxy.messages.in:
* WebProcess/Network/NetworkProcessConnection.cpp:
(WebKit::NetworkProcessConnection::NetworkProcessConnection):
* WebProcess/Network/NetworkProcessConnection.h:
(WebKit::NetworkProcessConnection::create):
(WebKit::NetworkProcessConnection::networkProcessPID const):
* WebProcess/Network/WebLoaderStrategy.cpp:
(WebKit::WebLoaderStrategy::scheduleLoadFromNetworkProcess):
* WebProcess/WebProcess.cpp:
(WebKit::getNetworkProcessConnection):
(WebKit::WebProcess::ensureNetworkProcessConnection):

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

Source/WebKit/ChangeLog
Source/WebKit/NetworkProcess/NetworkLoadParameters.h
Source/WebKit/NetworkProcess/NetworkResourceLoadParameters.cpp
Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp
Source/WebKit/UIProcess/WebPageProxy.cpp
Source/WebKit/UIProcess/WebProcessProxy.messages.in
Source/WebKit/WebProcess/Network/NetworkProcessConnection.cpp
Source/WebKit/WebProcess/Network/NetworkProcessConnection.h
Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp
Source/WebKit/WebProcess/WebProcess.cpp

index 0b70e5b7d2ccd873c914829e90eba7d432ec9aff..86f729f37d0d033c1e69f1117f0618deb44adc27 100644 (file)
@@ -1,3 +1,42 @@
+2019-09-16  Per Arne Vollan  <pvollan@apple.com>
+
+        REGRESSION(249649): Unable to open local files in MiniBrowser on macOS
+        https://bugs.webkit.org/show_bug.cgi?id=201798
+
+        Reviewed by Brent Fulgham.
+
+        The commit <https://trac.webkit.org/changeset/249649> introduced a MiniBrowser regression on macOS where
+        MiniBrowser is not able to open local files. The change set r249649 fixed a problem where the WebContent
+        process PID was not ready to be used when creating a sandbox extension. This happened in the cases where
+        the WebContent process had not finished launching when the load started. The WebContent process is also
+        creating sandbox extensions for the Networking process for the files being loaded, and also needs to be
+        passing the PID of the Networking process when creating these. This patch is addressing this by getting
+        the PID of the Networking process when the WebContent process is initially getting the connection to the
+        Networking process. The PID is then stored in the NetworkProcessConnection class, from where it is passed
+        to the NetworkLoadParameters, and used when creating the sandbox extension for the Networking process.
+
+        * NetworkProcess/NetworkLoadParameters.h:
+        * NetworkProcess/NetworkResourceLoadParameters.cpp:
+        (WebKit::NetworkResourceLoadParameters::encode const):
+        * UIProcess/Network/NetworkProcessProxy.cpp:
+        (WebKit::NetworkProcessProxy::~NetworkProcessProxy):
+        (WebKit::NetworkProcessProxy::openNetworkProcessConnection):
+        (WebKit::NetworkProcessProxy::networkProcessCrashed):
+        (WebKit::NetworkProcessProxy::didFinishLaunching):
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::loadRequestWithNavigationShared):
+        * UIProcess/WebProcessProxy.messages.in:
+        * WebProcess/Network/NetworkProcessConnection.cpp:
+        (WebKit::NetworkProcessConnection::NetworkProcessConnection):
+        * WebProcess/Network/NetworkProcessConnection.h:
+        (WebKit::NetworkProcessConnection::create):
+        (WebKit::NetworkProcessConnection::networkProcessPID const):
+        * WebProcess/Network/WebLoaderStrategy.cpp:
+        (WebKit::WebLoaderStrategy::scheduleLoadFromNetworkProcess):
+        * WebProcess/WebProcess.cpp:
+        (WebKit::getNetworkProcessConnection):
+        (WebKit::WebProcess::ensureNetworkProcessConnection):
+
 2019-09-16  David Kilzer  <ddkilzer@apple.com>
 
         [WebAuthn] REGRESSION (r249059): Leak of WKMockNFTag objects and WKMockNFTag instance variables
index 92b81273251666243d851eb8a5abc32811385f29..f28924654c34ce2f4379993ef6609abc5dc7a117 100644 (file)
@@ -53,6 +53,7 @@ public:
     WebCore::FrameIdentifier webFrameID;
     RefPtr<WebCore::SecurityOrigin> topOrigin;
     WTF::ProcessID parentPID { 0 };
+    WTF::ProcessID networkProcessPID { 0 };
     WebCore::ResourceRequest request;
     WebCore::ContentSniffingPolicy contentSniffingPolicy { WebCore::ContentSniffingPolicy::SniffContent };
     WebCore::ContentEncodingSniffingPolicy contentEncodingSniffingPolicy { WebCore::ContentEncodingSniffingPolicy::Sniff };
index d5ffea336c26815c1ff0b9ec517f87b92f0043ac..50e223ebd66d49e63564d70fa20df20930a59100 100644 (file)
@@ -67,7 +67,11 @@ void NetworkResourceLoadParameters::encode(IPC::Encoder& encoder) const
 
     if (request.url().isLocalFile()) {
         SandboxExtension::Handle requestSandboxExtension;
+#if HAVE(SANDBOX_ISSUE_READ_EXTENSION_TO_PROCESS_BY_PID)
+        SandboxExtension::createHandleForReadByPid(request.url().fileSystemPath(), networkProcessPID, requestSandboxExtension);
+#else
         SandboxExtension::createHandle(request.url().fileSystemPath(), SandboxExtension::Type::ReadOnly, requestSandboxExtension);
+#endif
         encoder << requestSandboxExtension;
     }
 
index 851b58083df829bb9cf079c9576f127d435120d1..7fdee4865f0dd6223110dcee3b752cfee92c5d93 100644 (file)
@@ -104,7 +104,7 @@ NetworkProcessProxy::~NetworkProcessProxy()
         m_downloadProxyMap->invalidate();
 
     for (auto& request : m_connectionRequests.values())
-        request.reply({ });
+        request.reply({ }, 0);
 }
 
 void NetworkProcessProxy::getLaunchOptions(ProcessLauncher::LaunchOptions& launchOptions)
@@ -156,10 +156,10 @@ void NetworkProcessProxy::openNetworkProcessConnection(uint64_t connectionReques
         auto request = m_connectionRequests.take(connectionRequestIdentifier);
 
 #if USE(UNIX_DOMAIN_SOCKETS) || OS(WINDOWS)
-        request.reply(*connectionIdentifier);
+        request.reply(*connectionIdentifier, processIdentifier());
 #elif OS(DARWIN)
         MESSAGE_CHECK(MACH_PORT_VALID(connectionIdentifier->port()));
-        request.reply(IPC::Attachment { connectionIdentifier->port(), MACH_MSG_TYPE_MOVE_SEND });
+        request.reply(IPC::Attachment { connectionIdentifier->port(), MACH_MSG_TYPE_MOVE_SEND }, processIdentifier());
 #else
         notImplemented();
 #endif
@@ -248,7 +248,7 @@ void NetworkProcessProxy::networkProcessCrashed()
         if (request.webProcess)
             pendingRequests.uncheckedAppend(std::make_pair(makeRefPtr(request.webProcess.get()), WTFMove(request.reply)));
         else
-            request.reply({ });
+            request.reply({ }, 0);
     }
     m_connectionRequests.clear();
 
@@ -394,7 +394,7 @@ void NetworkProcessProxy::didFinishLaunching(ProcessLauncher* launcher, IPC::Con
         if (connectionRequest.webProcess)
             getNetworkProcessConnection(*connectionRequest.webProcess, WTFMove(connectionRequest.reply));
         else
-            connectionRequest.reply({ });
+            connectionRequest.reply({ }, 0);
     }
 
 #if PLATFORM(COCOA)
index 691cf0ff3a94f4b650383736a6a97a3393dcc101..8c47172f6d466297c06ffe494f0b5dd326cc6771 100644 (file)
@@ -1175,7 +1175,7 @@ void WebPageProxy::loadRequestWithNavigationShared(Ref<WebProcessProxy>&& proces
     addPlatformLoadParameters(loadParameters);
 
 #if HAVE(SANDBOX_ISSUE_READ_EXTENSION_TO_PROCESS_BY_PID)
-    if (processIdentifier() || !url.isLocalFile())
+    if (process->processIdentifier() || !url.isLocalFile())
         process->send(Messages::WebPage::LoadRequest(loadParameters), webPageID);
     else {
         String sandboxExtensionPath;
index d6d063985614c5242ad225f1852f5f8ce408b944..4fb84c6e490b0d7bc81f902ad990a28e7bebfdae 100644 (file)
@@ -36,7 +36,7 @@ messages -> WebProcessProxy LegacyReceiver {
     GetPlugins(bool refresh) -> (Vector<WebCore::PluginInfo> plugins, Vector<WebCore::PluginInfo> applicationPlugins, struct Optional<Vector<WebCore::SupportedPluginIdentifier>> supportedPluginIdentifiers) Synchronous
     GetPluginProcessConnection(uint64_t pluginProcessToken) -> (IPC::Attachment connectionHandle, bool supportsAsynchronousInitialization) Synchronous
 #endif
-    GetNetworkProcessConnection() -> (IPC::Attachment connectionHandle) Synchronous
+    GetNetworkProcessConnection() -> (IPC::Attachment connectionHandle, int32_t pid) Synchronous
     ProcessReadyToSuspend()
     DidCancelProcessSuspension()
 
index e18f0ae78f2e6257fd82469953fe324563ea9d6c..53e7e13a15b45e55f8cba4af66f973dcd9f171d4 100644 (file)
@@ -68,8 +68,9 @@
 namespace WebKit {
 using namespace WebCore;
 
-NetworkProcessConnection::NetworkProcessConnection(IPC::Connection::Identifier connectionIdentifier)
+NetworkProcessConnection::NetworkProcessConnection(IPC::Connection::Identifier connectionIdentifier, WTF::ProcessID pid)
     : m_connection(IPC::Connection::createClientConnection(connectionIdentifier, *this))
+    , m_networkProcessPID(pid)
 {
     m_connection->open();
 }
index e8849fec17ba0682ebbda4d3df8971b6d1e194be..62cf8f9d6f65beb023373d035a35a8d59e9f725d 100644 (file)
@@ -58,9 +58,9 @@ typedef uint64_t ResourceLoadIdentifier;
 
 class NetworkProcessConnection : public RefCounted<NetworkProcessConnection>, IPC::Connection::Client {
 public:
-    static Ref<NetworkProcessConnection> create(IPC::Connection::Identifier connectionIdentifier)
+    static Ref<NetworkProcessConnection> create(IPC::Connection::Identifier connectionIdentifier, WTF::ProcessID pid)
     {
-        return adoptRef(*new NetworkProcessConnection(connectionIdentifier));
+        return adoptRef(*new NetworkProcessConnection(connectionIdentifier, pid));
     }
     ~NetworkProcessConnection();
     
@@ -80,8 +80,10 @@ public:
     WebSWClientConnection& serviceWorkerConnectionForSession(PAL::SessionID);
 #endif
 
+    WTF::ProcessID networkProcessPID() const { return m_networkProcessPID; }
+
 private:
-    NetworkProcessConnection(IPC::Connection::Identifier);
+    NetworkProcessConnection(IPC::Connection::Identifier, WTF::ProcessID);
 
     // IPC::Connection::Client
     void didReceiveMessage(IPC::Connection&, IPC::Decoder&) override;
@@ -103,6 +105,7 @@ private:
 
     // The connection from the web process to the network process.
     Ref<IPC::Connection> m_connection;
+    WTF::ProcessID m_networkProcessPID { 0 };
 
 #if ENABLE(INDEXED_DATABASE)
     HashMap<uint64_t, RefPtr<WebIDBConnectionToServer>> m_webIDBConnectionsBySession;
index 55b37c293c15d9fa92bd754f93f39dfe23472058..a7b69c663396cb5353b361ebd0ab6f4e37e4cea5 100644 (file)
@@ -285,6 +285,7 @@ void WebLoaderStrategy::scheduleLoadFromNetworkProcess(ResourceLoader& resourceL
     loadParameters.webPageID = trackingParameters.pageID;
     loadParameters.webFrameID = trackingParameters.frameID;
     loadParameters.parentPID = presentingApplicationPID();
+    loadParameters.networkProcessPID = WebProcess::singleton().ensureNetworkProcessConnection().networkProcessPID();
     loadParameters.request = request;
     loadParameters.contentSniffingPolicy = contentSniffingPolicy;
     loadParameters.contentEncodingSniffingPolicy = contentEncodingSniffingPolicy;
index a0f019b72f5581e3969a0540ba627b581bfe03e2..58748dcc3144c464b7aeb88daf1ce9b8cc8df8d3 100644 (file)
@@ -1162,10 +1162,11 @@ void WebProcess::setInjectedBundleParameters(const IPC::DataReference& value)
     injectedBundle->setBundleParameters(value);
 }
 
-static IPC::Connection::Identifier getNetworkProcessConnection(IPC::Connection& connection)
+static std::pair<IPC::Connection::Identifier, WTF::ProcessID> getNetworkProcessConnection(IPC::Connection& connection)
 {
     IPC::Attachment encodedConnectionIdentifier;
-    if (!connection.sendSync(Messages::WebProcessProxy::GetNetworkProcessConnection(), Messages::WebProcessProxy::GetNetworkProcessConnection::Reply(encodedConnectionIdentifier), 0)) {
+    WTF::ProcessID pid;
+    if (!connection.sendSync(Messages::WebProcessProxy::GetNetworkProcessConnection(), Messages::WebProcessProxy::GetNetworkProcessConnection::Reply(encodedConnectionIdentifier, pid), 0)) {
 #if PLATFORM(GTK) || PLATFORM(WPE)
         // GTK+ and WPE ports don't exit on send sync message failure.
         // In this particular case, the network process can be terminated by the UI process while the
@@ -1179,14 +1180,14 @@ static IPC::Connection::Identifier getNetworkProcessConnection(IPC::Connection&
     }
 
 #if USE(UNIX_DOMAIN_SOCKETS)
-    return encodedConnectionIdentifier.releaseFileDescriptor();
+    return std::make_pair(encodedConnectionIdentifier.releaseFileDescriptor(), pid);
 #elif OS(DARWIN)
-    return encodedConnectionIdentifier.port();
+    return std::make_pair(encodedConnectionIdentifier.port(), pid);
 #elif OS(WINDOWS)
-    return encodedConnectionIdentifier.handle();
+    return std::make_pair(encodedConnectionIdentifier.handle(), pid);
 #else
     ASSERT_NOT_REACHED();
-    return IPC::Connection::Identifier();
+    return std::make_pair(IPC::Connection::Identifier(), pid);
 #endif
 }
 
@@ -1197,17 +1198,23 @@ NetworkProcessConnection& WebProcess::ensureNetworkProcessConnection()
 
     // If we've lost our connection to the network process (e.g. it crashed) try to re-establish it.
     if (!m_networkProcessConnection) {
-        IPC::Connection::Identifier connectionIdentifier = getNetworkProcessConnection(*parentProcessConnection());
+        auto connectionIdentifierAndPID = getNetworkProcessConnection(*parentProcessConnection());
+        
+        IPC::Connection::Identifier connectionIdentifier = connectionIdentifierAndPID.first;
+        WTF::ProcessID pid = connectionIdentifierAndPID.second;
 
         // Retry once if the IPC to get the connectionIdentifier succeeded but the connectionIdentifier we received
         // is invalid. This may indicate that the network process has crashed.
-        if (!IPC::Connection::identifierIsValid(connectionIdentifier))
-            connectionIdentifier = getNetworkProcessConnection(*parentProcessConnection());
+        if (!IPC::Connection::identifierIsValid(connectionIdentifier)) {
+            connectionIdentifierAndPID = getNetworkProcessConnection(*parentProcessConnection());
+            connectionIdentifier = connectionIdentifierAndPID.first;
+            pid = connectionIdentifierAndPID.second;
+        }
 
         if (!IPC::Connection::identifierIsValid(connectionIdentifier))
             CRASH();
 
-        m_networkProcessConnection = NetworkProcessConnection::create(connectionIdentifier);
+        m_networkProcessConnection = NetworkProcessConnection::create(connectionIdentifier, pid);
     }
     
     return *m_networkProcessConnection;