Diff
Modified: branches/safari-609-branch/Source/WebKit/ChangeLog (261307 => 261308)
--- branches/safari-609-branch/Source/WebKit/ChangeLog 2020-05-07 17:58:01 UTC (rev 261307)
+++ branches/safari-609-branch/Source/WebKit/ChangeLog 2020-05-07 17:58:06 UTC (rev 261308)
@@ -1,5 +1,44 @@
2020-05-07 Russell Epstein <[email protected]>
+ Apply patch. rdar://problem/62977647
+
+ 2020-05-07 Alex Christensen <[email protected]>
+
+ Stop waiting for a BinarySemaphore on the main thread in the NetworkProcess
+ https://bugs.webkit.org/show_bug.cgi?id=211080
+ <rdar://problem/62377357>
+
+ Reviewed by Darin Adler and Chris Dumez.
+
+ There was an out-of-date comment suggesting we needed to use a semaphore, but we've since added these in the destructor:
+ RELEASE_ASSERT(!m_statisticsStore);
+ RELEASE_ASSERT(!m_persistentStorage);
+ This indicates that flushAndDestroyPersistentStore is called before the destructor, at which time it is safe to add a reference to keep it alive.
+ WebResourceLoadStatisticsStore is also marked as WTF::DestructionThread::Main so this should do everything we need.
+ We also flush these databases to disk before closing like we did cookies.
+
+ In order to keep tests working as they are, I needed to make recreateResourceLoadStatisticStore have a CompletionHandler and have all
+ WebResourceLoadStatisticsStores share the same queue to serialize background tasks between WebResourceLoadStatisticsStores with and without database stores
+ sequentially to avoid opening a SQLiteDatabase before the previous WebResourceLoadStatisticsStore had closed it.
+
+ * NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp:
+ (WebKit::sharedStatisticsQueue):
+ (WebKit::WebResourceLoadStatisticsStore::WebResourceLoadStatisticsStore):
+ (WebKit::WebResourceLoadStatisticsStore::didDestroyNetworkSession):
+ (WebKit::WebResourceLoadStatisticsStore::flushAndDestroyPersistentStore):
+ (WebKit::WebResourceLoadStatisticsStore::applicationWillTerminate): 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:
+
+2020-05-07 Russell Epstein <[email protected]>
+
Apply patch. rdar://problem/62272254
2020-05-07 Per Arne Vollan <[email protected]>
Modified: branches/safari-609-branch/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp (261307 => 261308)
--- branches/safari-609-branch/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp 2020-05-07 17:58:01 UTC (rev 261307)
+++ branches/safari-609-branch/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp 2020-05-07 17:58:06 UTC (rev 261308)
@@ -150,9 +150,15 @@
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(WorkQueue::create("WebResourceLoadStatisticsStore Process Data Queue", WorkQueue::Type::Serial, WorkQueue::QOS::Utility))
+ , m_statisticsQueue(sharedStatisticsQueue())
, m_dailyTasksTimer(RunLoop::main(), this, &WebResourceLoadStatisticsStore::performDailyTasks)
{
RELEASE_ASSERT(RunLoop::isMain());
@@ -182,12 +188,12 @@
RELEASE_ASSERT(!m_persistentStorage);
}
-void WebResourceLoadStatisticsStore::didDestroyNetworkSession()
+void WebResourceLoadStatisticsStore::didDestroyNetworkSession(CompletionHandler<void()>&& completionHandler)
{
ASSERT(RunLoop::isMain());
m_networkSession = nullptr;
- flushAndDestroyPersistentStore();
+ flushAndDestroyPersistentStore(WTFMove(completionHandler));
}
inline void WebResourceLoadStatisticsStore::postTask(WTF::Function<void()>&& task)
@@ -204,21 +210,17 @@
RunLoop::main().dispatch(WTFMove(reply));
}
-void WebResourceLoadStatisticsStore::flushAndDestroyPersistentStore()
+void WebResourceLoadStatisticsStore::flushAndDestroyPersistentStore(CompletionHandler<void()>&& completionHandler)
{
RELEASE_ASSERT(RunLoop::isMain());
- // 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] {
+ // 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 {
m_persistentStorage = nullptr;
m_statisticsStore = nullptr;
- semaphore.signal();
+ RunLoop::main().dispatch(WTFMove(completionHandler));
});
- semaphore.wait();
}
void WebResourceLoadStatisticsStore::populateMemoryStoreFromDisk(CompletionHandler<void()>&& completionHandler)
@@ -520,12 +522,6 @@
completionHandler();
}
-void WebResourceLoadStatisticsStore::applicationWillTerminate()
-{
- ASSERT(RunLoop::isMain());
- flushAndDestroyPersistentStore();
-}
-
void WebResourceLoadStatisticsStore::performDailyTasks()
{
ASSERT(RunLoop::isMain());
Modified: branches/safari-609-branch/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.h (261307 => 261308)
--- branches/safari-609-branch/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.h 2020-05-07 17:58:01 UTC (rev 261307)
+++ branches/safari-609-branch/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.h 2020-05-07 17:58:06 UTC (rev 261308)
@@ -194,7 +194,7 @@
}
};
- void didDestroyNetworkSession();
+ void didDestroyNetworkSession(CompletionHandler<void()>&&);
static const OptionSet<WebsiteDataType>& monitoredDataTypes();
@@ -208,8 +208,6 @@
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()>&&);
@@ -300,7 +298,7 @@
StorageAccessStatus storageAccessStatus(const String& subFramePrimaryDomain, const String& topFramePrimaryDomain);
- void flushAndDestroyPersistentStore();
+ void flushAndDestroyPersistentStore(CompletionHandler<void()>&&);
WeakPtr<NetworkSession> m_networkSession;
Ref<WTF::WorkQueue> m_statisticsQueue;
Modified: branches/safari-609-branch/Source/WebKit/NetworkProcess/NetworkProcess.cpp (261307 => 261308)
--- branches/safari-609-branch/Source/WebKit/NetworkProcess/NetworkProcess.cpp 2020-05-07 17:58:01 UTC (rev 261307)
+++ branches/safari-609-branch/Source/WebKit/NetworkProcess/NetworkProcess.cpp 2020-05-07 17:58:06 UTC (rev 261308)
@@ -258,10 +258,18 @@
{
ASSERT(RunLoop::isMain());
- // Make sure we flush all cookies to disk before exiting.
- platformSyncAllCookies([this] {
+ auto callbackAggregator = CallbackAggregator::create([this] {
+ ASSERT(RunLoop::isMain());
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-branch/Source/WebKit/NetworkProcess/NetworkSession.cpp (261307 => 261308)
--- branches/safari-609-branch/Source/WebKit/NetworkProcess/NetworkSession.cpp 2020-05-07 17:58:01 UTC (rev 261307)
+++ branches/safari-609-branch/Source/WebKit/NetworkProcess/NetworkSession.cpp 2020-05-07 17:58:06 UTC (rev 261308)
@@ -122,17 +122,17 @@
NetworkSession::~NetworkSession()
{
#if ENABLE(RESOURCE_LOAD_STATISTICS)
- destroyResourceLoadStatistics();
+ destroyResourceLoadStatistics([] { });
#endif
}
#if ENABLE(RESOURCE_LOAD_STATISTICS)
-void NetworkSession::destroyResourceLoadStatistics()
+void NetworkSession::destroyResourceLoadStatistics(CompletionHandler<void()>&& completionHandler)
{
if (!m_resourceLoadStatistics)
- return;
+ return completionHandler();
- m_resourceLoadStatistics->didDestroyNetworkSession();
+ m_resourceLoadStatistics->didDestroyNetworkSession(WTFMove(completionHandler));
m_resourceLoadStatistics = nullptr;
}
#endif
@@ -157,7 +157,7 @@
if (auto* storageSession = networkStorageSession())
storageSession->setResourceLoadStatisticsEnabled(enable);
if (!enable) {
- destroyResourceLoadStatistics();
+ destroyResourceLoadStatistics([] { });
return;
}
@@ -181,10 +181,13 @@
void NetworkSession::recreateResourceLoadStatisticStore(CompletionHandler<void()>&& completionHandler)
{
- destroyResourceLoadStatistics();
- m_resourceLoadStatistics = WebResourceLoadStatisticsStore::create(*this, m_resourceLoadStatisticsDirectory, m_shouldIncludeLocalhostInResourceLoadStatistics);
- m_resourceLoadStatistics->populateMemoryStoreFromDisk(WTFMove(completionHandler));
- forwardResourceLoadStatisticsSettings();
+ 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();
+ });
}
void NetworkSession::forwardResourceLoadStatisticsSettings()
Modified: branches/safari-609-branch/Source/WebKit/NetworkProcess/NetworkSession.h (261307 => 261308)
--- branches/safari-609-branch/Source/WebKit/NetworkProcess/NetworkSession.h 2020-05-07 17:58:01 UTC (rev 261307)
+++ branches/safari-609-branch/Source/WebKit/NetworkProcess/NetworkSession.h 2020-05-07 17:58:06 UTC (rev 261308)
@@ -100,6 +100,7 @@
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);
@@ -138,7 +139,6 @@
NetworkSession(NetworkProcess&, const NetworkSessionCreationParameters&);
#if ENABLE(RESOURCE_LOAD_STATISTICS)
- void destroyResourceLoadStatistics();
void forwardResourceLoadStatisticsSettings();
#endif
