Diff
Modified: trunk/Source/WebKit/ChangeLog (246451 => 246452)
--- trunk/Source/WebKit/ChangeLog 2019-06-14 23:14:14 UTC (rev 246451)
+++ trunk/Source/WebKit/ChangeLog 2019-06-15 01:07:16 UTC (rev 246452)
@@ -1,5 +1,44 @@
2019-06-14 Youenn Fablet <[email protected]>
+ WebProcessPool::clearWebProcessHasUploads cannot assume its given processIdentifier is valid
+ https://bugs.webkit.org/show_bug.cgi?id=198865
+ <rdar://problem/51618878>
+
+ Reviewed by Brady Eidson.
+
+ NetworkProcess currently instructs UIProcess whether a given WebProcess is doing upload.
+ There is no guarantee though that the WebProcessProxy is still there when the IPC is arriving at UIProcess.
+ Instead, let WebProcess handles its upload state and notify WebProcessPool about its state.
+ Make sure WebProcessProxy unregisters itself in case of crash.
+ In case of NetworkProcess crash, WebProcesses will see all their uploads fail
+ and will notify automatically UIProcess to update their state.
+
+ Since the processID given to WebProcessPool is coming from IPC, we cannot not trust it.
+ Add early return in case of not finding a WebProcessProxy for WebProcessPool clear/set methods.
+
+ * NetworkProcess/NetworkConnectionToWebProcess.cpp:
+ (WebKit::NetworkConnectionToWebProcess::NetworkConnectionToWebProcess):
+ * NetworkProcess/NetworkConnectionToWebProcess.h:
+ * NetworkProcess/NetworkConnectionToWebProcess.messages.in:
+ * NetworkProcess/NetworkResourceLoadMap.cpp:
+ (WebKit::NetworkResourceLoadMap::add):
+ (WebKit::NetworkResourceLoadMap::take):
+ * NetworkProcess/NetworkResourceLoadMap.h:
+ * UIProcess/WebProcessPool.cpp:
+ (WebKit::WebProcessPool::setWebProcessHasUploads):
+ (WebKit::WebProcessPool::clearWebProcessHasUploads):
+ * UIProcess/WebProcessProxy.cpp:
+ (WebKit::WebProcessProxy::~WebProcessProxy):
+ * WebProcess/Network/WebLoaderStrategy.cpp:
+ (WebKit::WebLoaderStrategy::scheduleLoadFromNetworkProcess):
+ (WebKit::WebLoaderStrategy::remove):
+ (WebKit::WebLoaderStrategy::tryLoadingSynchronouslyUsingURLSchemeHandler):
+ * WebProcess/Network/WebLoaderStrategy.h:
+ * WebProcess/WebProcess.cpp:
+ (WebKit::WebProcess::ensureNetworkProcessConnection):
+
+2019-06-14 Youenn Fablet <[email protected]>
+
WebResourceLoadStatisticsStore should not use its network session if invalidated
https://bugs.webkit.org/show_bug.cgi?id=198814
Modified: trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp (246451 => 246452)
--- trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp 2019-06-14 23:14:14 UTC (rev 246451)
+++ trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp 2019-06-15 01:07:16 UTC (rev 246452)
@@ -83,7 +83,6 @@
NetworkConnectionToWebProcess::NetworkConnectionToWebProcess(NetworkProcess& networkProcess, IPC::Connection::Identifier connectionIdentifier)
: m_connection(IPC::Connection::createServerConnection(connectionIdentifier, *this))
, m_networkProcess(networkProcess)
- , m_networkResourceLoaders(*this)
#if ENABLE(WEB_RTC)
, m_mdnsRegister(*this)
#endif
@@ -899,25 +898,6 @@
}
#endif
-void NetworkConnectionToWebProcess::setWebProcessIdentifier(ProcessIdentifier webProcessIdentifier)
-{
- m_webProcessIdentifier = webProcessIdentifier;
-}
-
-void NetworkConnectionToWebProcess::setConnectionHasUploads()
-{
- ASSERT(!m_connectionHasUploads);
- m_connectionHasUploads = true;
- m_networkProcess->parentProcessConnection()->send(Messages::WebProcessPool::SetWebProcessHasUploads(m_webProcessIdentifier), 0);
-}
-
-void NetworkConnectionToWebProcess::clearConnectionHasUploads()
-{
- ASSERT(m_connectionHasUploads);
- m_connectionHasUploads = false;
- m_networkProcess->parentProcessConnection()->send(Messages::WebProcessPool::ClearWebProcessHasUploads(m_webProcessIdentifier), 0);
-}
-
void NetworkConnectionToWebProcess::webPageWasAdded(PAL::SessionID sessionID, PageIdentifier pageID, WebCore::PageIdentifier oldPageID)
{
m_networkProcess->webPageWasAdded(m_connection.get(), sessionID, pageID, oldPageID);
Modified: trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.h (246451 => 246452)
--- trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.h 2019-06-14 23:14:14 UTC (rev 246451)
+++ trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.h 2019-06-15 01:07:16 UTC (rev 246452)
@@ -142,10 +142,6 @@
Vector<RefPtr<WebCore::BlobDataFileReference>> filesInBlob(const URL&);
Vector<RefPtr<WebCore::BlobDataFileReference>> resolveBlobReferences(const NetworkResourceLoadParameters&);
- void setWebProcessIdentifier(WebCore::ProcessIdentifier);
- void setConnectionHasUploads();
- void clearConnectionHasUploads();
-
void webPageWasAdded(PAL::SessionID, WebCore::PageIdentifier, WebCore::PageIdentifier oldPageID);
void webPageWasRemoved(PAL::SessionID, WebCore::PageIdentifier);
void webProcessSessionChanged(PAL::SessionID newSessionID, const Vector<WebCore::PageIdentifier>& pages);
@@ -318,9 +314,6 @@
#if ENABLE(APPLE_PAY_REMOTE_UI)
std::unique_ptr<WebPaymentCoordinatorProxy> m_paymentCoordinator;
#endif
-
- WebCore::ProcessIdentifier m_webProcessIdentifier;
- bool m_connectionHasUploads { false };
};
} // namespace WebKit
Modified: trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.messages.in (246451 => 246452)
--- trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.messages.in 2019-06-14 23:14:14 UTC (rev 246451)
+++ trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.messages.in 2019-06-15 01:07:16 UTC (rev 246452)
@@ -86,8 +86,6 @@
EstablishSWServerConnection(PAL::SessionID sessionID) -> (WebCore::SWServerConnectionIdentifier serverConnectionIdentifier) Synchronous
#endif
- SetWebProcessIdentifier(WebCore::ProcessIdentifier processIdentifier)
-
WebPageWasAdded(PAL::SessionID sessionID, WebCore::PageIdentifier pageID, WebCore::PageIdentifier oldPageID)
WebPageWasRemoved(PAL::SessionID sessionID, WebCore::PageIdentifier pageID)
WebProcessSessionChanged(PAL::SessionID newSessionID, Vector<WebCore::PageIdentifier> pages)
Modified: trunk/Source/WebKit/NetworkProcess/NetworkResourceLoadMap.cpp (246451 => 246452)
--- trunk/Source/WebKit/NetworkProcess/NetworkResourceLoadMap.cpp 2019-06-14 23:14:14 UTC (rev 246451)
+++ trunk/Source/WebKit/NetworkProcess/NetworkResourceLoadMap.cpp 2019-06-15 01:07:16 UTC (rev 246452)
@@ -26,22 +26,12 @@
#include "config.h"
#include "NetworkResourceLoadMap.h"
-#include "NetworkConnectionToWebProcess.h"
-
namespace WebKit {
NetworkResourceLoadMap::MapType::AddResult NetworkResourceLoadMap::add(ResourceLoadIdentifier identifier, Ref<NetworkResourceLoader>&& loader)
{
- auto result = m_loaders.add(identifier, WTFMove(loader));
- ASSERT(result.isNewEntry);
-
- if (result.iterator->value->originalRequest().hasUpload()) {
- if (m_loadersWithUploads.isEmpty())
- m_connectionToWebProcess.setConnectionHasUploads();
- m_loadersWithUploads.add(result.iterator->value.ptr());
- }
-
- return result;
+ ASSERT(!m_loaders.contains(identifier));
+ return m_loaders.add(identifier, WTFMove(loader));
}
bool NetworkResourceLoadMap::remove(ResourceLoadIdentifier identifier)
@@ -54,13 +44,6 @@
auto loader = m_loaders.take(identifier);
if (!loader)
return nullptr;
-
- if ((*loader)->originalRequest().hasUpload()) {
- m_loadersWithUploads.remove(loader->ptr());
- if (m_loadersWithUploads.isEmpty())
- m_connectionToWebProcess.clearConnectionHasUploads();
- }
-
return WTFMove(*loader);
}
Modified: trunk/Source/WebKit/NetworkProcess/NetworkResourceLoadMap.h (246451 => 246452)
--- trunk/Source/WebKit/NetworkProcess/NetworkResourceLoadMap.h 2019-06-14 23:14:14 UTC (rev 246451)
+++ trunk/Source/WebKit/NetworkProcess/NetworkResourceLoadMap.h 2019-06-15 01:07:16 UTC (rev 246452)
@@ -43,11 +43,6 @@
public:
typedef HashMap<ResourceLoadIdentifier, Ref<NetworkResourceLoader>> MapType;
- NetworkResourceLoadMap(NetworkConnectionToWebProcess& connection)
- : m_connectionToWebProcess(connection)
- {
- }
-
bool isEmpty() const { return m_loaders.isEmpty(); }
bool contains(ResourceLoadIdentifier identifier) const { return m_loaders.contains(identifier); }
MapType::iterator begin() { return m_loaders.begin(); }
@@ -59,9 +54,7 @@
RefPtr<NetworkResourceLoader> take(ResourceLoadIdentifier);
private:
- NetworkConnectionToWebProcess& m_connectionToWebProcess;
MapType m_loaders;
- HashSet<NetworkResourceLoader*> m_loadersWithUploads;
};
} // namespace WebKit
Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.cpp (246451 => 246452)
--- trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2019-06-14 23:14:14 UTC (rev 246451)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2019-06-15 01:07:16 UTC (rev 246452)
@@ -2531,9 +2531,13 @@
void WebProcessPool::setWebProcessHasUploads(ProcessIdentifier processID)
{
+ ASSERT(processID);
auto* process = WebProcessProxy::processForIdentifier(processID);
ASSERT(process);
+ if (!process)
+ return;
+
RELEASE_LOG(ProcessSuspension, "Web process pid %u now has uploads in progress", (unsigned)process->processIdentifier());
if (m_processesWithUploads.isEmpty()) {
@@ -2553,8 +2557,11 @@
void WebProcessPool::clearWebProcessHasUploads(ProcessIdentifier processID)
{
+ ASSERT(processID);
auto result = m_processesWithUploads.take(processID);
- ASSERT_UNUSED(result, result);
+ ASSERT(result);
+ if (!result)
+ return;
auto* process = WebProcessProxy::processForIdentifier(processID);
ASSERT_UNUSED(process, process);
Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp (246451 => 246452)
--- trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp 2019-06-14 23:14:14 UTC (rev 246451)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp 2019-06-15 01:07:16 UTC (rev 246452)
@@ -168,6 +168,9 @@
RELEASE_ASSERT(isMainThreadOrCheckDisabled());
ASSERT(m_pageURLRetainCountMap.isEmpty());
+ if (m_processPool)
+ m_processPool->clearWebProcessHasUploads(coreProcessIdentifier());
+
auto result = allProcesses().remove(coreProcessIdentifier());
ASSERT_UNUSED(result, result);
Modified: trunk/Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp (246451 => 246452)
--- trunk/Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp 2019-06-14 23:14:14 UTC (rev 246451)
+++ trunk/Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp 2019-06-15 01:07:16 UTC (rev 246452)
@@ -41,6 +41,7 @@
#include "WebPage.h"
#include "WebPageProxyMessages.h"
#include "WebProcess.h"
+#include "WebProcessPoolMessages.h"
#include "WebResourceLoader.h"
#include "WebServiceWorkerProvider.h"
#include "WebURLSchemeHandlerProxy.h"
@@ -357,7 +358,14 @@
return;
}
- m_webResourceLoaders.set(identifier, WebResourceLoader::create(resourceLoader, trackingParameters));
+ auto loader = WebResourceLoader::create(resourceLoader, trackingParameters);
+ if (resourceLoader.originalRequest().hasUpload()) {
+ if (m_loadersWithUploads.isEmpty())
+ WebProcess::singleton().parentProcessConnection()->send(Messages::WebProcessPool::SetWebProcessHasUploads(Process::identifier()), 0);
+ m_loadersWithUploads.add(loader.ptr());
+ }
+
+ m_webResourceLoaders.set(identifier, WTFMove(loader));
}
void WebLoaderStrategy::scheduleInternallyFailedLoad(WebCore::ResourceLoader& resourceLoader)
@@ -423,6 +431,9 @@
WebProcess::singleton().ensureNetworkProcessConnection().connection().send(Messages::NetworkConnectionToWebProcess::RemoveLoadIdentifier(identifier), 0);
+ if (m_loadersWithUploads.remove(loader.get()) && m_loadersWithUploads.isEmpty())
+ WebProcess::singleton().parentProcessConnection()->send(Messages::WebProcessPool::ClearWebProcessHasUploads { Process::identifier() }, 0);
+
// It's possible that this WebResourceLoader might be just about to message back to the NetworkProcess (e.g. ContinueWillSendRequest)
// but there's no point in doing so anymore.
loader->detachFromCoreLoader();
@@ -554,6 +565,10 @@
HangDetectionDisabler hangDetectionDisabler;
+ bool shouldNotifyOfUpload = request.hasUpload() && m_loadersWithUploads.isEmpty();
+ if (shouldNotifyOfUpload)
+ WebProcess::singleton().parentProcessConnection()->send(Messages::WebProcessPool::SetWebProcessHasUploads { Process::identifier() }, 0);
+
if (!WebProcess::singleton().ensureNetworkProcessConnection().connection().sendSync(Messages::NetworkConnectionToWebProcess::PerformSynchronousLoad(loadParameters), Messages::NetworkConnectionToWebProcess::PerformSynchronousLoad::Reply(error, response, data), 0)) {
RELEASE_LOG_ERROR_IF_ALLOWED(sessionID, "loadResourceSynchronously: failed sending synchronous network process message (pageID = %" PRIu64 ", frameID = %" PRIu64 ", resourceID = %lu)", pageID.toUInt64(), frameID, resourceLoadIdentifier);
if (auto* page = webPage ? webPage->corePage() : nullptr)
@@ -561,6 +576,9 @@
response = ResourceResponse();
error = internalError(request.url());
}
+
+ if (shouldNotifyOfUpload)
+ WebProcess::singleton().parentProcessConnection()->send(Messages::WebProcessPool::ClearWebProcessHasUploads { Process::identifier() }, 0);
}
void WebLoaderStrategy::pageLoadCompleted(PageIdentifier webPageID)
Modified: trunk/Source/WebKit/WebProcess/Network/WebLoaderStrategy.h (246451 => 246452)
--- trunk/Source/WebKit/WebProcess/Network/WebLoaderStrategy.h 2019-06-14 23:14:14 UTC (rev 246451)
+++ trunk/Source/WebKit/WebProcess/Network/WebLoaderStrategy.h 2019-06-15 01:07:16 UTC (rev 246452)
@@ -122,6 +122,7 @@
HashMap<unsigned long, PreconnectCompletionHandler> m_preconnectCompletionHandlers;
Vector<Function<void(bool)>> m_onlineStateChangeListeners;
bool m_isOnLine { true };
+ HashSet<WebResourceLoader*> m_loadersWithUploads;
};
} // namespace WebKit
Modified: trunk/Source/WebKit/WebProcess/WebProcess.cpp (246451 => 246452)
--- trunk/Source/WebKit/WebProcess/WebProcess.cpp 2019-06-14 23:14:14 UTC (rev 246451)
+++ trunk/Source/WebKit/WebProcess/WebProcess.cpp 2019-06-15 01:07:16 UTC (rev 246452)
@@ -1246,7 +1246,6 @@
CRASH();
m_networkProcessConnection = NetworkProcessConnection::create(connectionIdentifier);
- m_networkProcessConnection->connection().send(Messages::NetworkConnectionToWebProcess::SetWebProcessIdentifier(Process::identifier()), 0);
// To recover web storage, network process needs to know active webpages to prepare session storage.
// FIXME: https://bugs.webkit.org/show_bug.cgi?id=198051.
