Diff
Modified: branches/safari-609.2.9.0-branch/Source/WebKit/ChangeLog (260954 => 260955)
--- branches/safari-609.2.9.0-branch/Source/WebKit/ChangeLog 2020-04-30 17:52:08 UTC (rev 260954)
+++ branches/safari-609.2.9.0-branch/Source/WebKit/ChangeLog 2020-04-30 17:56:42 UTC (rev 260955)
@@ -1,3 +1,43 @@
+2020-04-30 Russell Epstein <repst...@apple.com>
+
+ Apply patch. rdar://problem/62623152
+
+ 2020-04-30 Alex Christensen <achristen...@webkit.org>
+
+ Revert most of r260370 and all of r260795
+ <rdar://problem/62623152>
+
+ r260370 not only fixed client certificate authentication when using non-persistent WKWebsiteDataStores,
+ but it also removed a strong reference to the WebsiteDataStores, which changed their lifetime and caused their destructor
+ to be called more often in SafariViewController, which caused hangs because of a deadlock in the destruction code I tried to
+ fix in r260795, which introduced suspension hangs. This keeps only the part of r260370 that is necessary to fix client certificate authentication
+ (as verified by the Challenge.ClientCertificateNonPersistentDataStore unit test from r260370 that continues to pass) but reverts the part of that
+ change that changed the lifetime of the WebsiteDataStore, restoring behavior to how the safari-609-branch was without hangs in SafariViewController
+ or in suspension, but with client certificate authentication working in non-persistent WKWebsiteDataStores.
+
+ This patch applies to the safari-609.2.9.0-branch.
+
+ * NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp:
+ (WebKit::WebResourceLoadStatisticsStore::WebResourceLoadStatisticsStore):
+ (WebKit::WebResourceLoadStatisticsStore::didDestroyNetworkSession):
+ (WebKit::WebResourceLoadStatisticsStore::flushAndDestroyPersistentStore):
+ (WebKit::WebResourceLoadStatisticsStore::applicationWillTerminate):
+ (WebKit::sharedStatisticsQueue): Deleted.
+ * NetworkProcess/Classifier/WebResourceLoadStatisticsStore.h:
+ * NetworkProcess/NetworkProcess.cpp:
+ (WebKit::NetworkProcess::didClose):
+ * NetworkProcess/NetworkSession.cpp:
+ (WebKit::NetworkSession::~NetworkSession):
+ (WebKit::NetworkSession::destroyResourceLoadStatistics):
+ (WebKit::NetworkSession::setResourceLoadStatisticsEnabled):
+ (WebKit::NetworkSession::recreateResourceLoadStatisticStore):
+ * NetworkProcess/NetworkSession.h:
+ * UIProcess/Network/NetworkProcessProxy.cpp:
+ (WebKit::NetworkProcessProxy::NetworkProcessProxy):
+ (WebKit::NetworkProcessProxy::addSession):
+ (WebKit::NetworkProcessProxy::removeSession):
+ * UIProcess/Network/NetworkProcessProxy.h:
+
2020-04-27 Russell Epstein <repst...@apple.com>
Apply patch. rdar://problem/62377357
Modified: branches/safari-609.2.9.0-branch/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp (260954 => 260955)
--- branches/safari-609.2.9.0-branch/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp 2020-04-30 17:52:08 UTC (rev 260954)
+++ branches/safari-609.2.9.0-branch/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp 2020-04-30 17:56:42 UTC (rev 260955)
@@ -150,15 +150,9 @@
completionHandler();
}
-static Ref<WorkQueue> sharedStatisticsQueue()
-{
- static NeverDestroyed<Ref<WorkQueue>> queue(WorkQueue::create("WebResourceLoadStatisticsStore Process Data Queue", WorkQueue::Type::Serial, WorkQueue::QOS::Utility));
- return queue.get().copyRef();
-}
-
WebResourceLoadStatisticsStore::WebResourceLoadStatisticsStore(NetworkSession& networkSession, const String& resourceLoadStatisticsDirectory, ShouldIncludeLocalhost shouldIncludeLocalhost)
: m_networkSession(makeWeakPtr(networkSession))
- , m_statisticsQueue(sharedStatisticsQueue())
+ , m_statisticsQueue(WorkQueue::create("WebResourceLoadStatisticsStore Process Data Queue", WorkQueue::Type::Serial, WorkQueue::QOS::Utility))
, m_dailyTasksTimer(RunLoop::main(), this, &WebResourceLoadStatisticsStore::performDailyTasks)
{
RELEASE_ASSERT(RunLoop::isMain());
@@ -188,12 +182,12 @@
RELEASE_ASSERT(!m_persistentStorage);
}
-void WebResourceLoadStatisticsStore::didDestroyNetworkSession(CompletionHandler<void()>&& completionHandler)
+void WebResourceLoadStatisticsStore::didDestroyNetworkSession()
{
ASSERT(RunLoop::isMain());
m_networkSession = nullptr;
- flushAndDestroyPersistentStore(WTFMove(completionHandler));
+ flushAndDestroyPersistentStore();
}
inline void WebResourceLoadStatisticsStore::postTask(WTF::Function<void()>&& task)
@@ -210,17 +204,21 @@
RunLoop::main().dispatch(WTFMove(reply));
}
-void WebResourceLoadStatisticsStore::flushAndDestroyPersistentStore(CompletionHandler<void()>&& completionHandler)
+void WebResourceLoadStatisticsStore::flushAndDestroyPersistentStore()
{
RELEASE_ASSERT(RunLoop::isMain());
- // Make sure we destroy the persistent store on the background queue and stay alive until it
- // is destroyed because it has a C++ reference to us.
- m_statisticsQueue->dispatch([this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)] () mutable {
+ // Make sure we destroy the persistent store on the background queue and wait for it to die
+ // synchronously since it has a C++ reference to us. Blocking nature of this task allows us
+ // to not maintain a WebResourceLoadStatisticsStore reference for the duration of dispatch,
+ // avoiding double-deletion issues when this is invoked from the destructor.
+ BinarySemaphore semaphore;
+ m_statisticsQueue->dispatch([&semaphore, this] {
m_persistentStorage = nullptr;
m_statisticsStore = nullptr;
- RunLoop::main().dispatch(WTFMove(completionHandler));
+ semaphore.signal();
});
+ semaphore.wait();
}
void WebResourceLoadStatisticsStore::populateMemoryStoreFromDisk(CompletionHandler<void()>&& completionHandler)
@@ -522,6 +520,12 @@
completionHandler();
}
+void WebResourceLoadStatisticsStore::applicationWillTerminate()
+{
+ ASSERT(RunLoop::isMain());
+ flushAndDestroyPersistentStore();
+}
+
void WebResourceLoadStatisticsStore::performDailyTasks()
{
ASSERT(RunLoop::isMain());
Modified: branches/safari-609.2.9.0-branch/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.h (260954 => 260955)
--- branches/safari-609.2.9.0-branch/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.h 2020-04-30 17:52:08 UTC (rev 260954)
+++ branches/safari-609.2.9.0-branch/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.h 2020-04-30 17:56:42 UTC (rev 260955)
@@ -194,7 +194,7 @@
}
};
- void didDestroyNetworkSession(CompletionHandler<void()>&&);
+ void didDestroyNetworkSession();
static const OptionSet<WebsiteDataType>& monitoredDataTypes();
@@ -208,6 +208,8 @@
void grantStorageAccess(const SubFrameDomain&, const TopFrameDomain&, WebCore::FrameIdentifier, WebCore::PageIdentifier, StorageAccessPromptWasShown, CompletionHandler<void(StorageAccessWasGranted, StorageAccessPromptWasShown)>&&);
+ void applicationWillTerminate();
+
void logFrameNavigation(const NavigatedToDomain&, const TopFrameDomain&, const NavigatedFromDomain&, bool isRedirect, bool isMainFrame, Seconds delayAfterMainFrameDocumentLoad, bool wasPotentiallyInitiatedByUser);
void logUserInteraction(const TopFrameDomain&, CompletionHandler<void()>&&);
void logCrossSiteLoadWithLinkDecoration(const NavigatedFromDomain&, const NavigatedToDomain&, CompletionHandler<void()>&&);
@@ -298,7 +300,7 @@
StorageAccessStatus storageAccessStatus(const String& subFramePrimaryDomain, const String& topFramePrimaryDomain);
- void flushAndDestroyPersistentStore(CompletionHandler<void()>&&);
+ void flushAndDestroyPersistentStore();
WeakPtr<NetworkSession> m_networkSession;
Ref<WTF::WorkQueue> m_statisticsQueue;
Modified: branches/safari-609.2.9.0-branch/Source/WebKit/NetworkProcess/NetworkProcess.cpp (260954 => 260955)
--- branches/safari-609.2.9.0-branch/Source/WebKit/NetworkProcess/NetworkProcess.cpp 2020-04-30 17:52:08 UTC (rev 260954)
+++ branches/safari-609.2.9.0-branch/Source/WebKit/NetworkProcess/NetworkProcess.cpp 2020-04-30 17:56:42 UTC (rev 260955)
@@ -258,18 +258,10 @@
{
ASSERT(RunLoop::isMain());
- auto callbackAggregator = CallbackAggregator::create([this] {
- ASSERT(RunLoop::isMain());
+ // Make sure we flush all cookies to disk before exiting.
+ platformSyncAllCookies([this] {
stopRunLoop();
});
-
- // Make sure we flush all cookies and resource load statistics to disk before exiting.
-#if ENABLE(RESOURCE_LOAD_STATISTICS)
- forEachNetworkSession([&] (auto& networkSession) {
- networkSession.destroyResourceLoadStatistics([callbackAggregator = callbackAggregator.copyRef()] { });
- });
-#endif
- platformSyncAllCookies([callbackAggregator = callbackAggregator.copyRef()] { });
}
void NetworkProcess::didCreateDownload()
Modified: branches/safari-609.2.9.0-branch/Source/WebKit/NetworkProcess/NetworkSession.cpp (260954 => 260955)
--- branches/safari-609.2.9.0-branch/Source/WebKit/NetworkProcess/NetworkSession.cpp 2020-04-30 17:52:08 UTC (rev 260954)
+++ branches/safari-609.2.9.0-branch/Source/WebKit/NetworkProcess/NetworkSession.cpp 2020-04-30 17:56:42 UTC (rev 260955)
@@ -122,17 +122,17 @@
NetworkSession::~NetworkSession()
{
#if ENABLE(RESOURCE_LOAD_STATISTICS)
- destroyResourceLoadStatistics([] { });
+ destroyResourceLoadStatistics();
#endif
}
#if ENABLE(RESOURCE_LOAD_STATISTICS)
-void NetworkSession::destroyResourceLoadStatistics(CompletionHandler<void()>&& completionHandler)
+void NetworkSession::destroyResourceLoadStatistics()
{
if (!m_resourceLoadStatistics)
- return completionHandler();
+ return;
- m_resourceLoadStatistics->didDestroyNetworkSession(WTFMove(completionHandler));
+ m_resourceLoadStatistics->didDestroyNetworkSession();
m_resourceLoadStatistics = nullptr;
}
#endif
@@ -157,7 +157,7 @@
if (auto* storageSession = networkStorageSession())
storageSession->setResourceLoadStatisticsEnabled(enable);
if (!enable) {
- destroyResourceLoadStatistics([] { });
+ destroyResourceLoadStatistics();
return;
}
@@ -181,13 +181,10 @@
void NetworkSession::recreateResourceLoadStatisticStore(CompletionHandler<void()>&& completionHandler)
{
- destroyResourceLoadStatistics([this, weakThis = makeWeakPtr(*this), completionHandler = WTFMove(completionHandler)] () mutable {
- if (!weakThis)
- return completionHandler();
- m_resourceLoadStatistics = WebResourceLoadStatisticsStore::create(*this, m_resourceLoadStatisticsDirectory, m_shouldIncludeLocalhostInResourceLoadStatistics);
- m_resourceLoadStatistics->populateMemoryStoreFromDisk(WTFMove(completionHandler));
- forwardResourceLoadStatisticsSettings();
- });
+ destroyResourceLoadStatistics();
+ m_resourceLoadStatistics = WebResourceLoadStatisticsStore::create(*this, m_resourceLoadStatisticsDirectory, m_shouldIncludeLocalhostInResourceLoadStatistics);
+ m_resourceLoadStatistics->populateMemoryStoreFromDisk(WTFMove(completionHandler));
+ forwardResourceLoadStatisticsSettings();
}
void NetworkSession::forwardResourceLoadStatisticsSettings()
Modified: branches/safari-609.2.9.0-branch/Source/WebKit/NetworkProcess/NetworkSession.h (260954 => 260955)
--- branches/safari-609.2.9.0-branch/Source/WebKit/NetworkProcess/NetworkSession.h 2020-04-30 17:52:08 UTC (rev 260954)
+++ branches/safari-609.2.9.0-branch/Source/WebKit/NetworkProcess/NetworkSession.h 2020-04-30 17:56:42 UTC (rev 260955)
@@ -100,7 +100,6 @@
void setShouldDowngradeReferrerForTesting(bool);
bool shouldDowngradeReferrer() const;
void setThirdPartyCookieBlockingMode(WebCore::ThirdPartyCookieBlockingMode);
- void destroyResourceLoadStatistics(CompletionHandler<void()>&&);
#endif
void storeAdClickAttribution(WebCore::AdClickAttribution&&);
void handleAdClickAttributionConversion(WebCore::AdClickAttribution::Conversion&&, const URL& requestURL, const WebCore::ResourceRequest& redirectRequest);
@@ -139,6 +138,7 @@
NetworkSession(NetworkProcess&, const NetworkSessionCreationParameters&);
#if ENABLE(RESOURCE_LOAD_STATISTICS)
+ void destroyResourceLoadStatistics();
void forwardResourceLoadStatisticsSettings();
#endif
Modified: branches/safari-609.2.9.0-branch/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp (260954 => 260955)
--- branches/safari-609.2.9.0-branch/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp 2020-04-30 17:52:08 UTC (rev 260954)
+++ branches/safari-609.2.9.0-branch/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp 2020-04-30 17:56:42 UTC (rev 260955)
@@ -91,6 +91,9 @@
, m_throttler(*this, processPool.shouldTakeUIBackgroundAssertion())
{
connect();
+
+ if (auto* websiteDataStore = m_processPool.websiteDataStore())
+ m_websiteDataStores.set(websiteDataStore->sessionID(), makeRef(*websiteDataStore));
}
NetworkProcessProxy::~NetworkProcessProxy()
@@ -1167,6 +1170,7 @@
#if ENABLE(INDEXED_DATABASE)
createSymLinkForFileUpgrade(store->resolvedIndexedDatabaseDirectory());
#endif
+ m_websiteDataStores.set(sessionID, WTFMove(store));
}
}
@@ -1174,6 +1178,8 @@
{
if (canSendMessage())
send(Messages::NetworkProcess::DestroySession { sessionID }, 0);
+ if (!sessionID.isEphemeral())
+ m_websiteDataStores.remove(sessionID);
}
WebsiteDataStore* NetworkProcessProxy::websiteDataStoreFromSessionID(PAL::SessionID sessionID)
Modified: branches/safari-609.2.9.0-branch/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h (260954 => 260955)
--- branches/safari-609.2.9.0-branch/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h 2020-04-30 17:52:08 UTC (rev 260954)
+++ branches/safari-609.2.9.0-branch/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h 2020-04-30 17:56:42 UTC (rev 260955)
@@ -295,6 +295,8 @@
HashSet<WebUserContentControllerProxy*> m_webUserContentControllerProxies;
#endif
+ HashMap<PAL::SessionID, RefPtr<WebsiteDataStore>> m_websiteDataStores;
+
std::unique_ptr<ProcessAssertion> m_uploadAssertion;
};
