Diff
Modified: trunk/Source/WTF/ChangeLog (217514 => 217515)
--- trunk/Source/WTF/ChangeLog 2017-05-27 00:22:49 UTC (rev 217514)
+++ trunk/Source/WTF/ChangeLog 2017-05-27 00:38:37 UTC (rev 217515)
@@ -1,3 +1,15 @@
+2017-05-26 Brent Fulgham <[email protected]>
+
+ [WK2] Address thread safety issues with ResourceLoadStatistics
+ https://bugs.webkit.org/show_bug.cgi?id=172519
+ <rdar://problem/31707642>
+
+ Reviewed by Chris Dumez.
+
+ Add a new specialization for HashSet.
+
+ * wtf/CrossThreadCopier.h:
+
2017-05-26 Ryan Haddad <[email protected]>
Unreviewed, rolling out r217458.
Modified: trunk/Source/WTF/wtf/CrossThreadCopier.h (217514 => 217515)
--- trunk/Source/WTF/wtf/CrossThreadCopier.h 2017-05-27 00:22:49 UTC (rev 217514)
+++ trunk/Source/WTF/wtf/CrossThreadCopier.h 2017-05-27 00:38:37 UTC (rev 217515)
@@ -1,6 +1,6 @@
/*
* Copyright (C) 2009, 2010 Google Inc. All rights reserved.
- * Copyright (C) 2014, 2015, 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2014-2017 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are
@@ -33,6 +33,7 @@
#include <wtf/Assertions.h>
#include <wtf/Forward.h>
+#include <wtf/HashSet.h>
#include <wtf/RefPtr.h>
#include <wtf/Threading.h>
#include <wtf/text/WTFString.h>
@@ -126,7 +127,19 @@
return destination;
}
};
-
+
+// Default specialization for HashSets of CrossThreadCopyable classes
+template<typename T> struct CrossThreadCopierBase<false, false, HashSet<T> > {
+ typedef HashSet<T> Type;
+ static Type copy(const Type& source)
+ {
+ Type destination;
+ for (auto& object : source)
+ destination.add(CrossThreadCopier<T>::copy(object));
+ return destination;
+ }
+};
+
} // namespace WTF
using WTF::CrossThreadCopierBaseHelper;
Modified: trunk/Source/WebCore/ChangeLog (217514 => 217515)
--- trunk/Source/WebCore/ChangeLog 2017-05-27 00:22:49 UTC (rev 217514)
+++ trunk/Source/WebCore/ChangeLog 2017-05-27 00:38:37 UTC (rev 217515)
@@ -1,3 +1,67 @@
+2017-05-26 Brent Fulgham <[email protected]>
+
+ [WK2] Address thread safety issues with ResourceLoadStatistics
+ https://bugs.webkit.org/show_bug.cgi?id=172519
+ <rdar://problem/31707642>
+
+ Reviewed by Chris Dumez.
+
+ * loader/ResourceLoadObserver.cpp:
+ (WebCore::ResourceLoadObserver::setStatisticsQueue): Added.
+ (WebCore::ResourceLoadObserver::clearInMemoryStore): Only interact with the HashTable on the statistics queue.
+ (WebCore::ResourceLoadObserver::clearInMemoryAndPersistentStore): Ditto.
+ (WebCore::ResourceLoadObserver::logFrameNavigation): Ditto.
+ (WebCore::ResourceLoadObserver::logSubresourceLoading): Ditto.
+ (WebCore::ResourceLoadObserver::logWebSocketLoading): Ditto.
+ (WebCore::ResourceLoadObserver::logUserInteractionWithReducedTimeResolution): Ditto.
+ (WebCore::ResourceLoadObserver::logUserInteraction): Ditto.
+ (WebCore::ResourceLoadObserver::clearUserInteraction): Protect HashTable while reading.
+ (WebCore::ResourceLoadObserver::hasHadUserInteraction): Ditto.
+ (WebCore::ResourceLoadObserver::setPrevalentResource): Ditto.
+ (WebCore::ResourceLoadObserver::isPrevalentResource): Ditto.
+ (WebCore::ResourceLoadObserver::clearPrevalentResource): Ditto.
+ (WebCore::ResourceLoadObserver::setGrandfathered): Ditto.
+ (WebCore::ResourceLoadObserver::isGrandfathered): Ditto.
+ (WebCore::ResourceLoadObserver::setSubframeUnderTopFrameOrigin): Only interact with the HashTable on the statistics queue.
+ (WebCore::ResourceLoadObserver::setSubresourceUnderTopFrameOrigin): Ditto.
+ (WebCore::ResourceLoadObserver::setSubresourceUniqueRedirectTo): Ditto.
+ (WebCore::ResourceLoadObserver::fireDataModificationHandler): ASSERT this is only called from the main thread, since this is
+ only meant to be used as part of the testing harness.
+ (WebCore::ResourceLoadObserver::fireShouldPartitionCookiesHandler): Ditto.
+ (WebCore::ResourceLoadObserver::fireShouldPartitionCookiesHandler): Ditto.
+ * loader/ResourceLoadObserver.h:
+ * loader/ResourceLoadStatisticsStore.cpp:
+ (WebCore::ResourceLoadStatisticsStore::isPrevalentResource): Protect HashTable while using it.
+ (WebCore::ResourceLoadStatisticsStore::ensureResourceStatisticsForPrimaryDomain): Ditto.
+ (WebCore::ResourceLoadStatisticsStore::setResourceStatisticsForPrimaryDomain): Ditto.
+ (WebCore::ResourceLoadStatisticsStore::createEncoderFromData): ASSERT this isn't being done on the main thread, and
+ protect HashTable while using it.
+ (WebCore::ResourceLoadStatisticsStore::readDataFromDecoder): Ditto.
+ (WebCore::ResourceLoadStatisticsStore::clearInMemory): Ditto.
+ (WebCore::ResourceLoadStatisticsStore::clearInMemoryAndPersistent): Ditto.
+ (WebCore::ResourceLoadStatisticsStore::statisticsForOrigin): Protect HashTable while using it.
+ (WebCore::ResourceLoadStatisticsStore::takeStatistics): Ditto.
+ (WebCore::ResourceLoadStatisticsStore::mergeStatistics): Ditto.
+ (WebCore::ResourceLoadStatisticsStore::setNotificationCallback): Use WTF::Function.
+ (WebCore::ResourceLoadStatisticsStore::setShouldPartitionCookiesCallback): Ditto.
+ (WebCore::ResourceLoadStatisticsStore::setWritePersistentStoreCallback): Ditto.
+ (WebCore::ResourceLoadStatisticsStore::setGrandfatherExistingWebsiteDataCallback): Ditto.
+ (WebCore::ResourceLoadStatisticsStore::fireDataModificationHandler): ASSERT this is not called on the main thread,
+ but dispatch the registered handler on the main thread.
+ (WebCore::ResourceLoadStatisticsStore::fireShouldPartitionCookiesHandler): Ditto.
+ (WebCore::ResourceLoadStatisticsStore::processStatistics): ASSERT this isn't being done on the main thread, and
+ protect the HashTable while using it. Also switch to WTF::Function.
+ (WebCore::ResourceLoadStatisticsStore::hasHadRecentUserInteraction): Make const correct.
+ (WebCore::ResourceLoadStatisticsStore::topPrivatelyControlledDomainsToRemoveWebsiteDataFor): Protect HashTable while using it.
+ (WebCore::ResourceLoadStatisticsStore::updateStatisticsForRemovedDataRecords): Ditto.
+ (WebCore::ResourceLoadStatisticsStore::handleFreshStartWithEmptyOrNoStore): Ditto.
+ (WebCore::ResourceLoadStatisticsStore::shouldRemoveDataRecords): Make const correct. ASSERT this is not being called
+ on the main thread.
+ (WebCore::ResourceLoadStatisticsStore::dataRecordsBeingRemoved): ASSERT this is not being called on the main thread.
+ (WebCore::ResourceLoadStatisticsStore::dataRecordsWereRemoved): Ditto.
+ (WebCore::ResourceLoadStatisticsStore::statisticsLock): Added.
+ * loader/ResourceLoadStatisticsStore.h:
+
2017-05-26 Joseph Pecoraro <[email protected]>
JSContext Inspector: Improve the reliability of automatically pausing in auto-attach
Modified: trunk/Source/WebCore/loader/ResourceLoadObserver.cpp (217514 => 217515)
--- trunk/Source/WebCore/loader/ResourceLoadObserver.cpp 2017-05-27 00:22:49 UTC (rev 217514)
+++ trunk/Source/WebCore/loader/ResourceLoadObserver.cpp 2017-05-27 00:38:37 UTC (rev 217515)
@@ -42,8 +42,10 @@
#include "Settings.h"
#include "SharedBuffer.h"
#include "URL.h"
+#include <wtf/CrossThreadCopier.h>
#include <wtf/CurrentTime.h>
#include <wtf/NeverDestroyed.h>
+#include <wtf/WorkQueue.h>
#include <wtf/text/StringBuilder.h>
namespace WebCore {
@@ -59,19 +61,37 @@
void ResourceLoadObserver::setStatisticsStore(Ref<ResourceLoadStatisticsStore>&& store)
{
+ if (m_store && m_queue)
+ m_queue = nullptr;
m_store = WTFMove(store);
}
-
+
+void ResourceLoadObserver::setStatisticsQueue(Ref<WTF::WorkQueue>&& queue)
+{
+ ASSERT(!m_queue);
+ m_queue = WTFMove(queue);
+}
+
void ResourceLoadObserver::clearInMemoryStore()
{
- if (m_store)
+ if (!m_store)
+ return;
+
+ ASSERT(m_queue);
+ m_queue->dispatch([this] () {
m_store->clearInMemory();
+ });
}
void ResourceLoadObserver::clearInMemoryAndPersistentStore()
{
- if (m_store)
+ if (!m_store)
+ return;
+
+ ASSERT(m_queue);
+ m_queue->dispatch([this] () {
m_store->clearInMemoryAndPersistent();
+ });
}
void ResourceLoadObserver::clearInMemoryAndPersistentStore(std::chrono::system_clock::time_point modifiedSince)
@@ -126,62 +146,70 @@
if (targetPrimaryDomain == mainFramePrimaryDomain || targetPrimaryDomain == sourcePrimaryDomain)
return;
+
+ ASSERT(m_queue);
+ m_queue->dispatch([this, isMainFrame, isRedirect, sourcePrimaryDomain = sourcePrimaryDomain.isolatedCopy(), mainFramePrimaryDomain = mainFramePrimaryDomain.isolatedCopy(), targetURL = CrossThreadCopier<URL>::copy(targetURL), mainFrameURL = CrossThreadCopier<URL>::copy(mainFrameURL), targetPrimaryDomain = targetPrimaryDomain.isolatedCopy()] () {
+
+ auto targetOrigin = SecurityOrigin::create(targetURL);
+ bool shouldFireDataModificationHandler = false;
+
+ {
+ auto locker = holdLock(m_store->statisticsLock());
+ auto& targetStatistics = m_store->ensureResourceStatisticsForPrimaryDomain(targetPrimaryDomain);
- auto targetOrigin = SecurityOrigin::create(targetURL);
- auto targetStatistics = m_store->ensureResourceStatisticsForPrimaryDomain(targetPrimaryDomain);
+ // Always fire if we have previously removed data records for this domain
+ shouldFireDataModificationHandler = targetStatistics.dataRecordsRemoved > 0;
- // Always fire if we have previously removed data records for this domain
- bool shouldFireDataModificationHandler = targetStatistics.dataRecordsRemoved > 0;
+ if (isMainFrame)
+ targetStatistics.topFrameHasBeenNavigatedToBefore = true;
+ else {
+ targetStatistics.subframeHasBeenLoadedBefore = true;
- if (isMainFrame)
- targetStatistics.topFrameHasBeenNavigatedToBefore = true;
- else {
- targetStatistics.subframeHasBeenLoadedBefore = true;
-
- auto mainFrameOrigin = SecurityOrigin::create(mainFrameURL);
- auto subframeUnderTopFrameOriginsResult = targetStatistics.subframeUnderTopFrameOrigins.add(mainFramePrimaryDomain);
- if (subframeUnderTopFrameOriginsResult.isNewEntry)
- shouldFireDataModificationHandler = true;
- }
-
- if (isRedirect) {
- auto& redirectingOriginResourceStatistics = m_store->ensureResourceStatisticsForPrimaryDomain(sourcePrimaryDomain);
+ auto mainFrameOrigin = SecurityOrigin::create(mainFrameURL);
+ auto subframeUnderTopFrameOriginsResult = targetStatistics.subframeUnderTopFrameOrigins.add(mainFramePrimaryDomain);
+ if (subframeUnderTopFrameOriginsResult.isNewEntry)
+ shouldFireDataModificationHandler = true;
+ }
- if (m_store->isPrevalentResource(targetPrimaryDomain))
- redirectingOriginResourceStatistics.redirectedToOtherPrevalentResourceOrigins.add(targetPrimaryDomain);
-
- if (isMainFrame) {
- ++targetStatistics.topFrameHasBeenRedirectedTo;
- ++redirectingOriginResourceStatistics.topFrameHasBeenRedirectedFrom;
- } else {
- ++targetStatistics.subframeHasBeenRedirectedTo;
- ++redirectingOriginResourceStatistics.subframeHasBeenRedirectedFrom;
- redirectingOriginResourceStatistics.subframeUniqueRedirectsTo.add(targetPrimaryDomain);
+ if (isRedirect) {
+ auto& redirectingOriginResourceStatistics = m_store->ensureResourceStatisticsForPrimaryDomain(sourcePrimaryDomain);
- ++targetStatistics.subframeSubResourceCount;
- }
- } else {
- if (sourcePrimaryDomain.isNull() || sourcePrimaryDomain.isEmpty() || sourcePrimaryDomain == "nullOrigin") {
- if (isMainFrame)
- ++targetStatistics.topFrameInitialLoadCount;
- else
+ if (m_store->isPrevalentResource(targetPrimaryDomain))
+ redirectingOriginResourceStatistics.redirectedToOtherPrevalentResourceOrigins.add(targetPrimaryDomain);
+
+ if (isMainFrame) {
+ ++targetStatistics.topFrameHasBeenRedirectedTo;
+ ++redirectingOriginResourceStatistics.topFrameHasBeenRedirectedFrom;
+ } else {
+ ++targetStatistics.subframeHasBeenRedirectedTo;
+ ++redirectingOriginResourceStatistics.subframeHasBeenRedirectedFrom;
+ redirectingOriginResourceStatistics.subframeUniqueRedirectsTo.add(targetPrimaryDomain);
+
++targetStatistics.subframeSubResourceCount;
+ }
} else {
- auto& sourceOriginResourceStatistics = m_store->ensureResourceStatisticsForPrimaryDomain(sourcePrimaryDomain);
+ if (sourcePrimaryDomain.isNull() || sourcePrimaryDomain.isEmpty() || sourcePrimaryDomain == "nullOrigin") {
+ if (isMainFrame)
+ ++targetStatistics.topFrameInitialLoadCount;
+ else
+ ++targetStatistics.subframeSubResourceCount;
+ } else {
+ auto& sourceOriginResourceStatistics = m_store->ensureResourceStatisticsForPrimaryDomain(sourcePrimaryDomain);
- if (isMainFrame) {
- ++sourceOriginResourceStatistics.topFrameHasBeenNavigatedFrom;
- ++targetStatistics.topFrameHasBeenNavigatedTo;
- } else {
- ++sourceOriginResourceStatistics.subframeHasBeenNavigatedFrom;
- ++targetStatistics.subframeHasBeenNavigatedTo;
+ if (isMainFrame) {
+ ++sourceOriginResourceStatistics.topFrameHasBeenNavigatedFrom;
+ ++targetStatistics.topFrameHasBeenNavigatedTo;
+ } else {
+ ++sourceOriginResourceStatistics.subframeHasBeenNavigatedFrom;
+ ++targetStatistics.subframeHasBeenNavigatedTo;
+ }
}
}
- }
-
- m_store->setResourceStatisticsForPrimaryDomain(targetPrimaryDomain, WTFMove(targetStatistics));
- if (shouldFireDataModificationHandler)
- m_store->fireDataModificationHandler();
+ } // Release lock
+
+ if (shouldFireDataModificationHandler)
+ m_store->fireDataModificationHandler();
+ });
}
void ResourceLoadObserver::logSubresourceLoading(const Frame* frame, const ResourceRequest& newRequest, const ResourceResponse& redirectResponse)
@@ -211,48 +239,57 @@
if (targetPrimaryDomain == mainFramePrimaryDomain || (isRedirect && targetPrimaryDomain == sourcePrimaryDomain))
return;
+
+ ASSERT(m_queue);
+ m_queue->dispatch([this, isRedirect, sourcePrimaryDomain = sourcePrimaryDomain.isolatedCopy(), mainFramePrimaryDomain = mainFramePrimaryDomain.isolatedCopy(), targetPrimaryDomain = targetPrimaryDomain.isolatedCopy(), mainFrameURL = mainFrameURL.isolatedCopy()] () {
+
+ bool shouldFireDataModificationHandler = false;
+
+ {
+ auto locker = holdLock(m_store->statisticsLock());
+ auto& targetStatistics = m_store->ensureResourceStatisticsForPrimaryDomain(targetPrimaryDomain);
- auto& targetStatistics = m_store->ensureResourceStatisticsForPrimaryDomain(targetPrimaryDomain);
+ // Always fire if we have previously removed data records for this domain
+ shouldFireDataModificationHandler = targetStatistics.dataRecordsRemoved > 0;
- // Always fire if we have previously removed data records for this domain
- bool shouldFireDataModificationHandler = targetStatistics.dataRecordsRemoved > 0;
+ auto mainFrameOrigin = SecurityOrigin::create(mainFrameURL);
+ auto subresourceUnderTopFrameOriginsResult = targetStatistics.subresourceUnderTopFrameOrigins.add(mainFramePrimaryDomain);
+ if (subresourceUnderTopFrameOriginsResult.isNewEntry)
+ shouldFireDataModificationHandler = true;
- auto mainFrameOrigin = SecurityOrigin::create(mainFrameURL);
- auto subresourceUnderTopFrameOriginsResult = targetStatistics.subresourceUnderTopFrameOrigins.add(mainFramePrimaryDomain);
- if (subresourceUnderTopFrameOriginsResult.isNewEntry)
- shouldFireDataModificationHandler = true;
+ if (isRedirect) {
+ auto& redirectingOriginStatistics = m_store->ensureResourceStatisticsForPrimaryDomain(sourcePrimaryDomain);
+
+ // We just inserted to the store, so we need to reget 'targetStatistics'
+ auto& updatedTargetStatistics = m_store->ensureResourceStatisticsForPrimaryDomain(targetPrimaryDomain);
- if (isRedirect) {
- auto& redirectingOriginStatistics = m_store->ensureResourceStatisticsForPrimaryDomain(sourcePrimaryDomain);
-
- // We just inserted to the store, so we need to reget 'targetStatistics'
- auto& updatedTargetStatistics = m_store->ensureResourceStatisticsForPrimaryDomain(targetPrimaryDomain);
+ if (m_store->isPrevalentResource(targetPrimaryDomain))
+ redirectingOriginStatistics.redirectedToOtherPrevalentResourceOrigins.add(targetPrimaryDomain);
+
+ ++redirectingOriginStatistics.subresourceHasBeenRedirectedFrom;
+ ++updatedTargetStatistics.subresourceHasBeenRedirectedTo;
- if (m_store->isPrevalentResource(targetPrimaryDomain))
- redirectingOriginStatistics.redirectedToOtherPrevalentResourceOrigins.add(targetPrimaryDomain);
-
- ++redirectingOriginStatistics.subresourceHasBeenRedirectedFrom;
- ++updatedTargetStatistics.subresourceHasBeenRedirectedTo;
+ auto subresourceUniqueRedirectsToResult = redirectingOriginStatistics.subresourceUniqueRedirectsTo.add(targetPrimaryDomain);
+ if (subresourceUniqueRedirectsToResult.isNewEntry)
+ shouldFireDataModificationHandler = true;
- auto subresourceUniqueRedirectsToResult = redirectingOriginStatistics.subresourceUniqueRedirectsTo.add(targetPrimaryDomain);
- if (subresourceUniqueRedirectsToResult.isNewEntry)
- shouldFireDataModificationHandler = true;
+ ++updatedTargetStatistics.subresourceHasBeenSubresourceCount;
- ++updatedTargetStatistics.subresourceHasBeenSubresourceCount;
+ auto totalVisited = std::max(m_originsVisitedMap.size(), 1U);
+
+ updatedTargetStatistics.subresourceHasBeenSubresourceCountDividedByTotalNumberOfOriginsVisited = static_cast<double>(updatedTargetStatistics.subresourceHasBeenSubresourceCount) / totalVisited;
+ } else {
+ ++targetStatistics.subresourceHasBeenSubresourceCount;
- auto totalVisited = std::max(m_originsVisitedMap.size(), 1U);
+ auto totalVisited = std::max(m_originsVisitedMap.size(), 1U);
+
+ targetStatistics.subresourceHasBeenSubresourceCountDividedByTotalNumberOfOriginsVisited = static_cast<double>(targetStatistics.subresourceHasBeenSubresourceCount) / totalVisited;
+ }
+ } // Release lock
- updatedTargetStatistics.subresourceHasBeenSubresourceCountDividedByTotalNumberOfOriginsVisited = static_cast<double>(updatedTargetStatistics.subresourceHasBeenSubresourceCount) / totalVisited;
- } else {
- ++targetStatistics.subresourceHasBeenSubresourceCount;
-
- auto totalVisited = std::max(m_originsVisitedMap.size(), 1U);
-
- targetStatistics.subresourceHasBeenSubresourceCountDividedByTotalNumberOfOriginsVisited = static_cast<double>(targetStatistics.subresourceHasBeenSubresourceCount) / totalVisited;
- }
-
- if (shouldFireDataModificationHandler)
- m_store->fireDataModificationHandler();
+ if (shouldFireDataModificationHandler)
+ m_store->fireDataModificationHandler();
+ });
}
void ResourceLoadObserver::logWebSocketLoading(const Frame* frame, const URL& targetURL)
@@ -281,24 +318,33 @@
if (targetPrimaryDomain == mainFramePrimaryDomain)
return;
- auto& targetStatistics = m_store->ensureResourceStatisticsForPrimaryDomain(targetPrimaryDomain);
+ ASSERT(m_queue);
+ m_queue->dispatch([this, targetPrimaryDomain = targetPrimaryDomain.isolatedCopy(), mainFramePrimaryDomain = mainFramePrimaryDomain.isolatedCopy(), mainFrameURL = mainFrameURL.isolatedCopy()] () {
+
+ bool shouldFireDataModificationHandler = false;
+
+ {
+ auto locker = holdLock(m_store->statisticsLock());
+ auto& targetStatistics = m_store->ensureResourceStatisticsForPrimaryDomain(targetPrimaryDomain);
- // Always fire if we have previously removed data records for this domain
- bool shouldFireDataModificationHandler = targetStatistics.dataRecordsRemoved > 0;
-
- auto mainFrameOrigin = SecurityOrigin::create(mainFrameURL);
- auto subresourceUnderTopFrameOriginsResult = targetStatistics.subresourceUnderTopFrameOrigins.add(mainFramePrimaryDomain);
- if (subresourceUnderTopFrameOriginsResult.isNewEntry)
- shouldFireDataModificationHandler = true;
+ // Always fire if we have previously removed data records for this domain
+ shouldFireDataModificationHandler = targetStatistics.dataRecordsRemoved > 0;
+
+ auto mainFrameOrigin = SecurityOrigin::create(mainFrameURL);
+ auto subresourceUnderTopFrameOriginsResult = targetStatistics.subresourceUnderTopFrameOrigins.add(mainFramePrimaryDomain);
+ if (subresourceUnderTopFrameOriginsResult.isNewEntry)
+ shouldFireDataModificationHandler = true;
- ++targetStatistics.subresourceHasBeenSubresourceCount;
-
- auto totalVisited = std::max(m_originsVisitedMap.size(), 1U);
-
- targetStatistics.subresourceHasBeenSubresourceCountDividedByTotalNumberOfOriginsVisited = static_cast<double>(targetStatistics.subresourceHasBeenSubresourceCount) / totalVisited;
-
- if (shouldFireDataModificationHandler)
- m_store->fireDataModificationHandler();
+ ++targetStatistics.subresourceHasBeenSubresourceCount;
+
+ auto totalVisited = std::max(m_originsVisitedMap.size(), 1U);
+
+ targetStatistics.subresourceHasBeenSubresourceCountDividedByTotalNumberOfOriginsVisited = static_cast<double>(targetStatistics.subresourceHasBeenSubresourceCount) / totalVisited;
+ } // Release lock
+
+ if (shouldFireDataModificationHandler)
+ m_store->fireDataModificationHandler();
+ });
}
static double reduceTimeResolution(double seconds)
@@ -317,17 +363,23 @@
if (url.isBlankURL() || url.isEmpty())
return;
- auto primaryDomainStr = primaryDomain(url);
+ auto primaryDomainString = primaryDomain(url);
+
+ ASSERT(m_queue);
+ m_queue->dispatch([this, primaryDomainString = primaryDomainString.isolatedCopy()] () {
+ {
+ auto locker = holdLock(m_store->statisticsLock());
+ auto& statistics = m_store->ensureResourceStatisticsForPrimaryDomain(primaryDomainString);
+ double newTimestamp = reduceTimeResolution(WTF::currentTime());
+ if (newTimestamp == statistics.mostRecentUserInteraction)
+ return;
- auto& statistics = m_store->ensureResourceStatisticsForPrimaryDomain(primaryDomainStr);
- double newTimestamp = reduceTimeResolution(WTF::currentTime());
- if (newTimestamp == statistics.mostRecentUserInteraction)
- return;
-
- statistics.hadUserInteraction = true;
- statistics.mostRecentUserInteraction = newTimestamp;
-
- m_store->fireDataModificationHandler();
+ statistics.hadUserInteraction = true;
+ statistics.mostRecentUserInteraction = newTimestamp;
+ }
+
+ m_store->fireDataModificationHandler();
+ });
}
void ResourceLoadObserver::logUserInteraction(const URL& url)
@@ -335,13 +387,19 @@
if (url.isBlankURL() || url.isEmpty())
return;
- auto primaryDomainStr = primaryDomain(url);
+ auto primaryDomainString = primaryDomain(url);
- auto& statistics = m_store->ensureResourceStatisticsForPrimaryDomain(primaryDomainStr);
- statistics.hadUserInteraction = true;
- statistics.mostRecentUserInteraction = WTF::currentTime();
-
- m_store->fireShouldPartitionCookiesHandler({primaryDomainStr}, { }, false);
+ ASSERT(m_queue);
+ m_queue->dispatch([this, primaryDomainString = primaryDomainString.isolatedCopy()] () {
+ {
+ auto locker = holdLock(m_store->statisticsLock());
+ auto& statistics = m_store->ensureResourceStatisticsForPrimaryDomain(primaryDomainString);
+ statistics.hadUserInteraction = true;
+ statistics.mostRecentUserInteraction = WTF::currentTime();
+ }
+
+ m_store->fireShouldPartitionCookiesHandler({ primaryDomainString }, { }, false);
+ });
}
void ResourceLoadObserver::clearUserInteraction(const URL& url)
@@ -349,6 +407,7 @@
if (url.isBlankURL() || url.isEmpty())
return;
+ auto locker = holdLock(m_store->statisticsLock());
auto& statistics = m_store->ensureResourceStatisticsForPrimaryDomain(primaryDomain(url));
statistics.hadUserInteraction = false;
@@ -360,6 +419,7 @@
if (url.isBlankURL() || url.isEmpty())
return false;
+ auto locker = holdLock(m_store->statisticsLock());
auto& statistics = m_store->ensureResourceStatisticsForPrimaryDomain(primaryDomain(url));
return m_store->hasHadRecentUserInteraction(statistics);
@@ -370,6 +430,7 @@
if (url.isBlankURL() || url.isEmpty())
return;
+ auto locker = holdLock(m_store->statisticsLock());
auto& statistics = m_store->ensureResourceStatisticsForPrimaryDomain(primaryDomain(url));
statistics.isPrevalentResource = true;
@@ -380,6 +441,7 @@
if (url.isBlankURL() || url.isEmpty())
return false;
+ auto locker = holdLock(m_store->statisticsLock());
auto& statistics = m_store->ensureResourceStatisticsForPrimaryDomain(primaryDomain(url));
return statistics.isPrevalentResource;
@@ -390,6 +452,7 @@
if (url.isBlankURL() || url.isEmpty())
return;
+ auto locker = holdLock(m_store->statisticsLock());
auto& statistics = m_store->ensureResourceStatisticsForPrimaryDomain(primaryDomain(url));
statistics.isPrevalentResource = false;
@@ -400,6 +463,7 @@
if (url.isBlankURL() || url.isEmpty())
return;
+ auto locker = holdLock(m_store->statisticsLock());
auto& statistics = m_store->ensureResourceStatisticsForPrimaryDomain(primaryDomain(url));
statistics.grandfathered = value;
@@ -410,6 +474,7 @@
if (url.isBlankURL() || url.isEmpty())
return false;
+ auto locker = holdLock(m_store->statisticsLock());
auto& statistics = m_store->ensureResourceStatisticsForPrimaryDomain(primaryDomain(url));
return statistics.grandfathered;
@@ -420,8 +485,15 @@
if (subframe.isBlankURL() || subframe.isEmpty() || topFrame.isBlankURL() || topFrame.isEmpty())
return;
- auto& statistics = m_store->ensureResourceStatisticsForPrimaryDomain(primaryDomain(subframe));
- statistics.subframeUnderTopFrameOrigins.add(primaryDomain(topFrame));
+ auto primaryTopFrameDomainString = primaryDomain(topFrame);
+ auto primarySubFrameDomainString = primaryDomain(subframe);
+
+ ASSERT(m_queue);
+ m_queue->dispatch([this, primaryTopFrameDomainString = primaryTopFrameDomainString.isolatedCopy(), primarySubFrameDomainString = primarySubFrameDomainString.isolatedCopy()] () {
+ auto locker = holdLock(m_store->statisticsLock());
+ auto& statistics = m_store->ensureResourceStatisticsForPrimaryDomain(primarySubFrameDomainString);
+ statistics.subframeUnderTopFrameOrigins.add(primaryTopFrameDomainString);
+ });
}
void ResourceLoadObserver::setSubresourceUnderTopFrameOrigin(const URL& subresource, const URL& topFrame)
@@ -429,8 +501,15 @@
if (subresource.isBlankURL() || subresource.isEmpty() || topFrame.isBlankURL() || topFrame.isEmpty())
return;
- auto& statistics = m_store->ensureResourceStatisticsForPrimaryDomain(primaryDomain(subresource));
- statistics.subresourceUnderTopFrameOrigins.add(primaryDomain(topFrame));
+ auto primaryTopFrameDomainString = primaryDomain(topFrame);
+ auto primarySubresourceDomainString = primaryDomain(subresource);
+
+ ASSERT(m_queue);
+ m_queue->dispatch([this, primaryTopFrameDomainString = primaryTopFrameDomainString.isolatedCopy(), primarySubresourceDomainString = primarySubresourceDomainString.isolatedCopy()] () {
+ auto locker = holdLock(m_store->statisticsLock());
+ auto& statistics = m_store->ensureResourceStatisticsForPrimaryDomain(primarySubresourceDomainString);
+ statistics.subresourceUnderTopFrameOrigins.add(primaryTopFrameDomainString);
+ });
}
void ResourceLoadObserver::setSubresourceUniqueRedirectTo(const URL& subresource, const URL& hostNameRedirectedTo)
@@ -438,8 +517,15 @@
if (subresource.isBlankURL() || subresource.isEmpty() || hostNameRedirectedTo.isBlankURL() || hostNameRedirectedTo.isEmpty())
return;
- auto& statistics = m_store->ensureResourceStatisticsForPrimaryDomain(primaryDomain(subresource));
- statistics.subresourceUniqueRedirectsTo.add(primaryDomain(hostNameRedirectedTo));
+ auto primarySubresourceDomainString = primaryDomain(subresource);
+ auto primaryRedirectDomainString = primaryDomain(hostNameRedirectedTo);
+
+ ASSERT(m_queue);
+ m_queue->dispatch([this, primaryRedirectDomainString = primaryRedirectDomainString.isolatedCopy(), primarySubresourceDomainString = primarySubresourceDomainString.isolatedCopy()] () {
+ auto locker = holdLock(m_store->statisticsLock());
+ auto& statistics = m_store->ensureResourceStatisticsForPrimaryDomain(primarySubresourceDomainString);
+ statistics.subresourceUniqueRedirectsTo.add(primaryRedirectDomainString);
+ });
}
void ResourceLoadObserver::setTimeToLiveUserInteraction(double seconds)
@@ -470,17 +556,29 @@
void ResourceLoadObserver::fireDataModificationHandler()
{
- m_store->fireDataModificationHandler();
+ // Helper function used by testing system. Should only be called from the main thread.
+ ASSERT(isMainThread());
+ m_queue->dispatch([this] () {
+ m_store->fireDataModificationHandler();
+ });
}
void ResourceLoadObserver::fireShouldPartitionCookiesHandler()
{
- m_store->fireShouldPartitionCookiesHandler();
+ // Helper function used by testing system. Should only be called from the main thread.
+ ASSERT(isMainThread());
+ m_queue->dispatch([this] () {
+ m_store->fireShouldPartitionCookiesHandler();
+ });
}
void ResourceLoadObserver::fireShouldPartitionCookiesHandler(const Vector<String>& domainsToRemove, const Vector<String>& domainsToAdd, bool clearFirst)
{
- m_store->fireShouldPartitionCookiesHandler(domainsToRemove, domainsToAdd, clearFirst);
+ // Helper function used by testing system. Should only be called from the main thread.
+ ASSERT(isMainThread());
+ m_queue->dispatch([this, domainsToRemove = CrossThreadCopier<Vector<String>>::copy(domainsToRemove), domainsToAdd = CrossThreadCopier<Vector<String>>::copy(domainsToAdd), clearFirst] () {
+ m_store->fireShouldPartitionCookiesHandler(domainsToRemove, domainsToAdd, clearFirst);
+ });
}
String ResourceLoadObserver::primaryDomain(const URL& url)
@@ -511,7 +609,10 @@
String ResourceLoadObserver::statisticsForOrigin(const String& origin)
{
- return m_store ? m_store->statisticsForOrigin(origin) : emptyString();
+ if (!m_store)
+ return emptyString();
+
+ return m_store->statisticsForOrigin(origin);
}
}
Modified: trunk/Source/WebCore/loader/ResourceLoadObserver.h (217514 => 217515)
--- trunk/Source/WebCore/loader/ResourceLoadObserver.h 2017-05-27 00:22:49 UTC (rev 217514)
+++ trunk/Source/WebCore/loader/ResourceLoadObserver.h 2017-05-27 00:38:37 UTC (rev 217515)
@@ -29,6 +29,11 @@
#include <wtf/HashMap.h>
#include <wtf/text/WTFString.h>
+namespace WTF {
+class Lock;
+class WorkQueue;
+}
+
namespace WebCore {
class Document;
@@ -75,6 +80,7 @@
WEBCORE_EXPORT void fireShouldPartitionCookiesHandler(const Vector<String>& domainsToRemove, const Vector<String>& domainsToAdd, bool clearFirst);
WEBCORE_EXPORT void setStatisticsStore(Ref<ResourceLoadStatisticsStore>&&);
+ WEBCORE_EXPORT void setStatisticsQueue(Ref<WTF::WorkQueue>&&);
WEBCORE_EXPORT void clearInMemoryStore();
WEBCORE_EXPORT void clearInMemoryAndPersistentStore();
WEBCORE_EXPORT void clearInMemoryAndPersistentStore(std::chrono::system_clock::time_point modifiedSince);
@@ -87,6 +93,7 @@
static String primaryDomain(const String& host);
RefPtr<ResourceLoadStatisticsStore> m_store;
+ RefPtr<WTF::WorkQueue> m_queue;
HashMap<String, size_t> m_originsVisitedMap;
};
Modified: trunk/Source/WebCore/loader/ResourceLoadStatisticsStore.cpp (217514 => 217515)
--- trunk/Source/WebCore/loader/ResourceLoadStatisticsStore.cpp 2017-05-27 00:22:49 UTC (rev 217514)
+++ trunk/Source/WebCore/loader/ResourceLoadStatisticsStore.cpp 2017-05-27 00:38:37 UTC (rev 217515)
@@ -33,8 +33,11 @@
#include "ResourceLoadStatistics.h"
#include "SharedBuffer.h"
#include "URL.h"
+#include <wtf/CrossThreadCopier.h>
#include <wtf/CurrentTime.h>
+#include <wtf/MainThread.h>
#include <wtf/NeverDestroyed.h>
+#include <wtf/RunLoop.h>
namespace WebCore {
@@ -53,6 +56,7 @@
bool ResourceLoadStatisticsStore::isPrevalentResource(const String& primaryDomain) const
{
+ auto locker = holdLock(m_statisticsLock);
auto mapEntry = m_resourceStatisticsMap.find(primaryDomain);
if (mapEntry == m_resourceStatisticsMap.end())
return false;
@@ -62,6 +66,7 @@
ResourceLoadStatistics& ResourceLoadStatisticsStore::ensureResourceStatisticsForPrimaryDomain(const String& primaryDomain)
{
+ ASSERT(m_statisticsLock.isLocked());
auto addResult = m_resourceStatisticsMap.ensure(primaryDomain, [&primaryDomain] {
return ResourceLoadStatistics(primaryDomain);
});
@@ -71,6 +76,8 @@
void ResourceLoadStatisticsStore::setResourceStatisticsForPrimaryDomain(const String& primaryDomain, ResourceLoadStatistics&& statistics)
{
+ ASSERT(!isMainThread());
+ auto locker = holdLock(m_statisticsLock);
m_resourceStatisticsMap.set(primaryDomain, WTFMove(statistics));
}
@@ -78,10 +85,13 @@
std::unique_ptr<KeyedEncoder> ResourceLoadStatisticsStore::createEncoderFromData()
{
+ ASSERT(!isMainThread());
auto encoder = KeyedEncoder::encoder();
+ auto locker = holdLock(m_statisticsLock);
encoder->encodeUInt32("version", statisticsModelVersion);
encoder->encodeDouble("endOfGrandfatheringTimestamp", m_endOfGrandfatheringTimestamp);
+
encoder->encodeObjects("browsingStatistics", m_resourceStatisticsMap.begin(), m_resourceStatisticsMap.end(), [](KeyedEncoder& encoderInner, const StatisticsValue& origin) {
origin.value.encode(encoderInner);
});
@@ -91,6 +101,8 @@
void ResourceLoadStatisticsStore::readDataFromDecoder(KeyedDecoder& decoder)
{
+ ASSERT(!isMainThread());
+ ASSERT(m_statisticsLock.isLocked());
if (m_resourceStatisticsMap.size())
return;
@@ -117,6 +129,9 @@
Vector<String> prevalentResourceDomainsWithoutUserInteraction;
prevalentResourceDomainsWithoutUserInteraction.reserveInitialCapacity(loadedStatistics.size());
+
+ {
+ auto locker = holdLock(m_statisticsLock);
for (auto& statistics : loadedStatistics) {
if (statistics.isPrevalentResource && !statistics.hadUserInteraction) {
prevalentResourceDomainsWithoutUserInteraction.uncheckedAppend(statistics.highLevelDomain);
@@ -124,18 +139,25 @@
}
m_resourceStatisticsMap.set(statistics.highLevelDomain, statistics);
}
-
+ }
+
fireShouldPartitionCookiesHandler({ }, prevalentResourceDomainsWithoutUserInteraction, true);
}
void ResourceLoadStatisticsStore::clearInMemory()
{
+ ASSERT(!isMainThread());
+ {
+ auto locker = holdLock(m_statisticsLock);
m_resourceStatisticsMap.clear();
+ }
+
fireShouldPartitionCookiesHandler({ }, { }, true);
}
void ResourceLoadStatisticsStore::clearInMemoryAndPersistent()
{
+ ASSERT(!isMainThread());
clearInMemory();
if (m_writePersistentStoreHandler)
m_writePersistentStoreHandler();
@@ -145,6 +167,7 @@
String ResourceLoadStatisticsStore::statisticsForOrigin(const String& origin)
{
+ auto locker = holdLock(m_statisticsLock);
auto iter = m_resourceStatisticsMap.find(origin);
if (iter == m_resourceStatisticsMap.end())
return emptyString();
@@ -155,17 +178,22 @@
Vector<ResourceLoadStatistics> ResourceLoadStatisticsStore::takeStatistics()
{
Vector<ResourceLoadStatistics> statistics;
+
+ {
+ auto locker = holdLock(m_statisticsLock);
statistics.reserveInitialCapacity(m_resourceStatisticsMap.size());
for (auto& statistic : m_resourceStatisticsMap.values())
statistics.uncheckedAppend(WTFMove(statistic));
m_resourceStatisticsMap.clear();
-
+ }
+
return statistics;
}
void ResourceLoadStatisticsStore::mergeStatistics(const Vector<ResourceLoadStatistics>& statistics)
{
+ auto locker = holdLock(m_statisticsLock);
for (auto& statistic : statistics) {
auto result = m_resourceStatisticsMap.ensure(statistic.highLevelDomain, [&statistic] {
return ResourceLoadStatistics(statistic.highLevelDomain);
@@ -175,22 +203,22 @@
}
}
-void ResourceLoadStatisticsStore::setNotificationCallback(std::function<void()> handler)
+void ResourceLoadStatisticsStore::setNotificationCallback(WTF::Function<void()>&& handler)
{
m_dataAddedHandler = WTFMove(handler);
}
-void ResourceLoadStatisticsStore::setShouldPartitionCookiesCallback(std::function<void(const Vector<String>& domainsToRemove, const Vector<String>& domainsToAdd, bool clearFirst)>&& handler)
+void ResourceLoadStatisticsStore::setShouldPartitionCookiesCallback(WTF::Function<void(const Vector<String>& domainsToRemove, const Vector<String>& domainsToAdd, bool clearFirst)>&& handler)
{
m_shouldPartitionCookiesForDomainsHandler = WTFMove(handler);
}
-void ResourceLoadStatisticsStore::setWritePersistentStoreCallback(std::function<void()>&& handler)
+void ResourceLoadStatisticsStore::setWritePersistentStoreCallback(WTF::Function<void()>&& handler)
{
m_writePersistentStoreHandler = WTFMove(handler);
}
-void ResourceLoadStatisticsStore::setGrandfatherExistingWebsiteDataCallback(std::function<void()>&& handler)
+void ResourceLoadStatisticsStore::setGrandfatherExistingWebsiteDataCallback(WTF::Function<void()>&& handler)
{
m_grandfatherExistingWebsiteDataHandler = WTFMove(handler);
}
@@ -197,8 +225,11 @@
void ResourceLoadStatisticsStore::fireDataModificationHandler()
{
- if (m_dataAddedHandler)
- m_dataAddedHandler();
+ ASSERT(!isMainThread());
+ RunLoop::main().dispatch([this, protectedThis = makeRef(*this)] () {
+ if (m_dataAddedHandler)
+ m_dataAddedHandler();
+ });
}
static inline bool shouldPartitionCookies(const ResourceLoadStatistics& statistic)
@@ -209,9 +240,11 @@
void ResourceLoadStatisticsStore::fireShouldPartitionCookiesHandler()
{
+ ASSERT(!isMainThread());
Vector<String> domainsToRemove;
Vector<String> domainsToAdd;
+ auto locker = holdLock(m_statisticsLock);
for (auto& resourceStatistic : m_resourceStatisticsMap.values()) {
bool shouldPartition = shouldPartitionCookies(resourceStatistic);
if (resourceStatistic.isMarkedForCookiePartitioning && !shouldPartition) {
@@ -226,18 +259,24 @@
if (domainsToRemove.isEmpty() && domainsToAdd.isEmpty())
return;
- if (m_shouldPartitionCookiesForDomainsHandler)
- m_shouldPartitionCookiesForDomainsHandler(domainsToRemove, domainsToAdd, false);
+ RunLoop::main().dispatch([this, protectedThis = makeRef(*this), domainsToRemove = CrossThreadCopier<Vector<String>>::copy(domainsToRemove), domainsToAdd = CrossThreadCopier<Vector<String>>::copy(domainsToAdd)] () {
+ if (m_shouldPartitionCookiesForDomainsHandler)
+ m_shouldPartitionCookiesForDomainsHandler(domainsToRemove, domainsToAdd, false);
+ });
}
void ResourceLoadStatisticsStore::fireShouldPartitionCookiesHandler(const Vector<String>& domainsToRemove, const Vector<String>& domainsToAdd, bool clearFirst)
{
+ ASSERT(!isMainThread());
if (domainsToRemove.isEmpty() && domainsToAdd.isEmpty())
return;
- if (m_shouldPartitionCookiesForDomainsHandler)
- m_shouldPartitionCookiesForDomainsHandler(domainsToRemove, domainsToAdd, clearFirst);
-
+ RunLoop::main().dispatch([this, clearFirst, protectedThis = makeRef(*this), domainsToRemove = CrossThreadCopier<Vector<String>>::copy(domainsToRemove), domainsToAdd = CrossThreadCopier<Vector<String>>::copy(domainsToAdd)] () {
+ if (m_shouldPartitionCookiesForDomainsHandler)
+ m_shouldPartitionCookiesForDomainsHandler(domainsToRemove, domainsToAdd, clearFirst);
+ });
+
+ auto locker = holdLock(m_statisticsLock);
if (clearFirst) {
for (auto& resourceStatistic : m_resourceStatisticsMap.values())
resourceStatistic.isMarkedForCookiePartitioning = false;
@@ -274,13 +313,15 @@
grandfatheringTime = seconds;
}
-void ResourceLoadStatisticsStore::processStatistics(std::function<void(ResourceLoadStatistics&)>&& processFunction)
+void ResourceLoadStatisticsStore::processStatistics(WTF::Function<void(ResourceLoadStatistics&)>&& processFunction)
{
+ ASSERT(!isMainThread());
+ auto locker = holdLock(m_statisticsLock);
for (auto& resourceStatistic : m_resourceStatisticsMap.values())
processFunction(resourceStatistic);
}
-bool ResourceLoadStatisticsStore::hasHadRecentUserInteraction(ResourceLoadStatistics& resourceStatistic)
+bool ResourceLoadStatisticsStore::hasHadRecentUserInteraction(ResourceLoadStatistics& resourceStatistic) const
{
if (!resourceStatistic.hadUserInteraction)
return false;
@@ -307,6 +348,7 @@
m_endOfGrandfatheringTimestamp = 0;
Vector<String> prevalentResources;
+ auto locker = holdLock(m_statisticsLock);
for (auto& statistic : m_resourceStatisticsMap.values()) {
if (statistic.isPrevalentResource
&& !hasHadRecentUserInteraction(statistic)
@@ -322,6 +364,7 @@
void ResourceLoadStatisticsStore::updateStatisticsForRemovedDataRecords(const Vector<String>& prevalentResourceDomains)
{
+ auto locker = holdLock(m_statisticsLock);
for (auto& prevalentResourceDomain : prevalentResourceDomains) {
ResourceLoadStatistics& statistic = ensureResourceStatisticsForPrimaryDomain(prevalentResourceDomain);
++statistic.dataRecordsRemoved;
@@ -330,6 +373,7 @@
void ResourceLoadStatisticsStore::handleFreshStartWithEmptyOrNoStore(HashSet<String>&& topPrivatelyControlledDomainsToGrandfather)
{
+ auto locker = holdLock(m_statisticsLock);
for (auto& topPrivatelyControlledDomain : topPrivatelyControlledDomainsToGrandfather) {
ResourceLoadStatistics& statistic = ensureResourceStatisticsForPrimaryDomain(topPrivatelyControlledDomain);
statistic.grandfathered = true;
@@ -337,8 +381,9 @@
m_endOfGrandfatheringTimestamp = std::floor(currentTime()) + grandfatheringTime;
}
-bool ResourceLoadStatisticsStore::shouldRemoveDataRecords()
+bool ResourceLoadStatisticsStore::shouldRemoveDataRecords() const
{
+ ASSERT(!isMainThread());
if (m_dataRecordsRemovalPending)
return false;
@@ -350,6 +395,7 @@
void ResourceLoadStatisticsStore::dataRecordsBeingRemoved()
{
+ ASSERT(!isMainThread());
m_lastTimeDataRecordsWereRemoved = currentTime();
m_dataRecordsRemovalPending = true;
}
@@ -356,7 +402,13 @@
void ResourceLoadStatisticsStore::dataRecordsWereRemoved()
{
+ ASSERT(!isMainThread());
m_dataRecordsRemovalPending = false;
}
-
+
+WTF::RecursiveLockAdapter<Lock>& ResourceLoadStatisticsStore::statisticsLock()
+{
+ return m_statisticsLock;
}
+
+}
Modified: trunk/Source/WebCore/loader/ResourceLoadStatisticsStore.h (217514 => 217515)
--- trunk/Source/WebCore/loader/ResourceLoadStatisticsStore.h 2017-05-27 00:22:49 UTC (rev 217514)
+++ trunk/Source/WebCore/loader/ResourceLoadStatisticsStore.h 2017-05-27 00:38:37 UTC (rev 217515)
@@ -26,7 +26,9 @@
#pragma once
#include "ResourceLoadStatistics.h"
+#include <wtf/Function.h>
#include <wtf/HashSet.h>
+#include <wtf/RecursiveLockAdapter.h>
namespace WebCore {
@@ -58,10 +60,10 @@
WEBCORE_EXPORT void mergeStatistics(const Vector<ResourceLoadStatistics>&);
WEBCORE_EXPORT Vector<ResourceLoadStatistics> takeStatistics();
- WEBCORE_EXPORT void setNotificationCallback(std::function<void()>);
- WEBCORE_EXPORT void setShouldPartitionCookiesCallback(std::function<void(const Vector<String>& domainsToRemove, const Vector<String>& domainsToAdd, bool clearFirst)>&&);
- WEBCORE_EXPORT void setWritePersistentStoreCallback(std::function<void()>&&);
- WEBCORE_EXPORT void setGrandfatherExistingWebsiteDataCallback(std::function<void()>&&);
+ WEBCORE_EXPORT void setNotificationCallback(WTF::Function<void()>&&);
+ WEBCORE_EXPORT void setShouldPartitionCookiesCallback(WTF::Function<void(const Vector<String>& domainsToRemove, const Vector<String>& domainsToAdd, bool clearFirst)>&&);
+ WEBCORE_EXPORT void setWritePersistentStoreCallback(WTF::Function<void()>&&);
+ WEBCORE_EXPORT void setGrandfatherExistingWebsiteDataCallback(WTF::Function<void()>&&);
void fireDataModificationHandler();
void setTimeToLiveUserInteraction(double seconds);
@@ -71,25 +73,29 @@
WEBCORE_EXPORT void fireShouldPartitionCookiesHandler();
void fireShouldPartitionCookiesHandler(const Vector<String>& domainsToRemove, const Vector<String>& domainsToAdd, bool clearFirst);
- WEBCORE_EXPORT void processStatistics(std::function<void(ResourceLoadStatistics&)>&&);
+ WEBCORE_EXPORT void processStatistics(WTF::Function<void(ResourceLoadStatistics&)>&&);
- WEBCORE_EXPORT bool hasHadRecentUserInteraction(ResourceLoadStatistics&);
+ WEBCORE_EXPORT bool hasHadRecentUserInteraction(ResourceLoadStatistics&) const;
WEBCORE_EXPORT Vector<String> topPrivatelyControlledDomainsToRemoveWebsiteDataFor();
WEBCORE_EXPORT void updateStatisticsForRemovedDataRecords(const Vector<String>& prevalentResourceDomains);
WEBCORE_EXPORT void handleFreshStartWithEmptyOrNoStore(HashSet<String>&& topPrivatelyControlledDomainsToGrandfather);
- WEBCORE_EXPORT bool shouldRemoveDataRecords();
+ WEBCORE_EXPORT bool shouldRemoveDataRecords() const;
WEBCORE_EXPORT void dataRecordsBeingRemoved();
WEBCORE_EXPORT void dataRecordsWereRemoved();
+
+ WEBCORE_EXPORT WTF::RecursiveLockAdapter<Lock>& statisticsLock();
private:
ResourceLoadStatisticsStore() = default;
HashMap<String, ResourceLoadStatistics> m_resourceStatisticsMap;
- std::function<void()> m_dataAddedHandler;
- std::function<void(const Vector<String>&, const Vector<String>&, bool clearFirst)> m_shouldPartitionCookiesForDomainsHandler;
- std::function<void()> m_writePersistentStoreHandler;
- std::function<void()> m_grandfatherExistingWebsiteDataHandler;
+ mutable WTF::RecursiveLockAdapter<Lock> m_statisticsLock;
+ WTF::Function<void()> m_dataAddedHandler;
+ WTF::Function<void(const Vector<String>&, const Vector<String>&, bool clearFirst)> m_shouldPartitionCookiesForDomainsHandler;
+ WTF::Function<void()> m_writePersistentStoreHandler;
+ WTF::Function<void()> m_grandfatherExistingWebsiteDataHandler;
+
double m_endOfGrandfatheringTimestamp { 0 };
double m_lastTimeDataRecordsWereRemoved { 0 };
bool m_dataRecordsRemovalPending { false };
Modified: trunk/Source/WebKit/mac/ChangeLog (217514 => 217515)
--- trunk/Source/WebKit/mac/ChangeLog 2017-05-27 00:22:49 UTC (rev 217514)
+++ trunk/Source/WebKit/mac/ChangeLog 2017-05-27 00:38:37 UTC (rev 217515)
@@ -1,3 +1,16 @@
+2017-05-26 Brent Fulgham <[email protected]>
+
+ [WK2] Address thread safety issues with ResourceLoadStatistics
+ https://bugs.webkit.org/show_bug.cgi?id=172519
+ <rdar://problem/31707642>
+
+ Reviewed by Chris Dumez.
+
+ Create a new WorkQueue for the ResourceLoadStatistics store to use for processing data.
+
+ * WebView/WebView.mm:
+ (WebKitInitializeApplicationStatisticsStoragePathIfNecessary): Pass WorkQueue to the observer.
+
2017-05-25 Myles C. Maxfield <[email protected]>
[WK1] iframes in layer-backed NSViews are not cleared between successive draws
Modified: trunk/Source/WebKit/mac/WebView/WebView.mm (217514 => 217515)
--- trunk/Source/WebKit/mac/WebView/WebView.mm 2017-05-27 00:22:49 UTC (rev 217514)
+++ trunk/Source/WebKit/mac/WebView/WebView.mm 2017-05-27 00:38:37 UTC (rev 217515)
@@ -226,6 +226,7 @@
#import <wtf/RunLoop.h>
#import <wtf/SetForScope.h>
#import <wtf/StdLibExtras.h>
+#import <wtf/WorkQueue.h>
#import <wtf/spi/darwin/dyldSPI.h>
#if !PLATFORM(IOS)
@@ -761,6 +762,7 @@
static NSMutableSet *schemesWithRepresentationsSet;
static WebCore::ResourceLoadStatisticsStore* resourceLoadStatisticsStore;
+static WTF::WorkQueue* statisticsQueue;
#if !PLATFORM(IOS)
NSString *_WebCanGoBackKey = @"canGoBack";
@@ -1167,6 +1169,9 @@
resourceLoadStatisticsStore = &WebCore::ResourceLoadStatisticsStore::create().leakRef();
ResourceLoadObserver::sharedObserver().setStatisticsStore(*resourceLoadStatisticsStore);
+ statisticsQueue = &WTF::WorkQueue::create("WebView ResourceLoadStatisticsStore Process Data Queue").leakRef();
+ ResourceLoadObserver::sharedObserver().setStatisticsQueue(*statisticsQueue);
+
initialized = YES;
}
Modified: trunk/Source/WebKit2/ChangeLog (217514 => 217515)
--- trunk/Source/WebKit2/ChangeLog 2017-05-27 00:22:49 UTC (rev 217514)
+++ trunk/Source/WebKit2/ChangeLog 2017-05-27 00:38:37 UTC (rev 217515)
@@ -1,3 +1,32 @@
+2017-05-26 Brent Fulgham <[email protected]>
+
+ [WK2] Address thread safety issues with ResourceLoadStatistics
+ https://bugs.webkit.org/show_bug.cgi?id=172519
+ <rdar://problem/31707642>
+
+ Reviewed by Chris Dumez.
+
+ Address some thread safety issues with the ResourceLoadStatistics architecture.
+
+ * UIProcess/WebResourceLoadStatisticsStore.cpp:
+ (WebKit::WebResourceLoadStatisticsStore::removeDataRecords): Assert that this is never called on the main thread. Also
+ ensure that coreStore is only accessed on the statistics queue, not the main thread.
+ (WebKit::WebResourceLoadStatisticsStore::processStatisticsAndDataRecords): Dispatch coreStore-accessing code
+ on the statistics queue.
+ (WebKit::WebResourceLoadStatisticsStore::resourceLoadStatisticsUpdated): Assert we do not hit this method
+ on the main thread.
+ (WebKit::WebResourceLoadStatisticsStore::registerSharedResourceLoadObserver): Assert that this is being called on the
+ main thread. Also ensure that coreStore is only accessed on the statistics queue, not the main thread.
+ (WebKit::WebResourceLoadStatisticsStore::grandfatherExistingWebsiteData): Dispatch coreStore-accessing code
+ on the statistics queue.
+ (WebKit::WebResourceLoadStatisticsStore::readDataFromDiskIfNeeded): Lock data before operating on it.
+ (WebKit::WebResourceLoadStatisticsStore::writeStoreToDisk): Assert we do not hit this method on the main thread.
+ (WebKit::WebResourceLoadStatisticsStore::writeEncoderToDisk): Ditto.
+ * UIProcess/WebResourceLoadStatisticsStore.h:
+ * WebProcess/WebProcess.cpp: Add a queue for the local WebProcess ResourceLoadStatisticsStore to use while processing data.
+ (WebKit::m_statisticsQueue): Added.
+ * WebProcess/WebProcess.h:
+
2017-05-26 Joseph Pecoraro <[email protected]>
[Cocoa] Simplify some WebViewImpl pasteboard code
Modified: trunk/Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.cpp (217514 => 217515)
--- trunk/Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.cpp 2017-05-27 00:22:49 UTC (rev 217514)
+++ trunk/Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.cpp 2017-05-27 00:38:37 UTC (rev 217515)
@@ -36,6 +36,7 @@
#include <WebCore/KeyedCoding.h>
#include <WebCore/ResourceLoadObserver.h>
#include <WebCore/ResourceLoadStatistics.h>
+#include <wtf/CrossThreadCopier.h>
#include <wtf/MainThread.h>
#include <wtf/MathExtras.h>
#include <wtf/RunLoop.h>
@@ -102,6 +103,8 @@
void WebResourceLoadStatisticsStore::removeDataRecords()
{
+ ASSERT(!isMainThread());
+
if (!coreStore().shouldRemoveDataRecords())
return;
@@ -115,10 +118,13 @@
initializeDataTypesToRemove();
// Switch to the main thread to get the default website data store
- RunLoop::main().dispatch([prevalentResourceDomains = WTFMove(prevalentResourceDomains), this] () mutable {
+ RunLoop::main().dispatch([prevalentResourceDomains = CrossThreadCopier<Vector<String>>::copy(prevalentResourceDomains), this, protectedThis = makeRef(*this)] () mutable {
WebProcessProxy::deleteWebsiteDataForTopPrivatelyControlledDomainsInAllPersistentDataStores(dataTypesToRemove, WTFMove(prevalentResourceDomains), notifyPages, [this](Vector<String> domainsWithDeletedWebsiteData) mutable {
- this->coreStore().updateStatisticsForRemovedDataRecords(domainsWithDeletedWebsiteData);
- this->coreStore().dataRecordsWereRemoved();
+ // But always touch the ResourceLoadStatistics store on the worker queue.
+ m_statisticsQueue->dispatch([this, protectedThis = makeRef(*this), topDomains = CrossThreadCopier<Vector<String>>::copy(domainsWithDeletedWebsiteData)] () mutable {
+ this->coreStore().updateStatisticsForRemovedDataRecords(topDomains);
+ this->coreStore().dataRecordsWereRemoved();
+ });
});
});
}
@@ -125,14 +131,17 @@
void WebResourceLoadStatisticsStore::processStatisticsAndDataRecords()
{
- if (shouldClassifyResourcesBeforeDataRecordsRemoval) {
- coreStore().processStatistics([this] (ResourceLoadStatistics& resourceStatistic) {
- classifyResource(resourceStatistic);
- });
- }
- removeDataRecords();
+ m_statisticsQueue->dispatch([this, protectedThis = makeRef(*this)] () {
+ auto locker = holdLock(coreStore().statisticsLock());
+ if (shouldClassifyResourcesBeforeDataRecordsRemoval) {
+ coreStore().processStatistics([this] (ResourceLoadStatistics& resourceStatistic) {
+ classifyResource(resourceStatistic);
+ });
+ }
+ removeDataRecords();
- writeStoreToDisk();
+ writeStoreToDisk();
+ });
}
void WebResourceLoadStatisticsStore::resourceLoadStatisticsUpdated(const Vector<WebCore::ResourceLoadStatistics>& origins)
@@ -161,7 +170,10 @@
void WebResourceLoadStatisticsStore::registerSharedResourceLoadObserver()
{
+ ASSERT(isMainThread());
+
ResourceLoadObserver::sharedObserver().setStatisticsStore(m_resourceLoadStatisticsStore.copyRef());
+ ResourceLoadObserver::sharedObserver().setStatisticsQueue(m_statisticsQueue.copyRef());
m_resourceLoadStatisticsStore->setNotificationCallback([this] {
if (m_resourceLoadStatisticsStore->isEmpty())
return;
@@ -168,7 +180,9 @@
processStatisticsAndDataRecords();
});
m_resourceLoadStatisticsStore->setWritePersistentStoreCallback([this]() {
- writeStoreToDisk();
+ m_statisticsQueue->dispatch([this, protectedThis = makeRef(*this)] {
+ writeStoreToDisk();
+ });
});
m_resourceLoadStatisticsStore->setGrandfatherExistingWebsiteDataCallback([this]() {
grandfatherExistingWebsiteData();
@@ -178,8 +192,10 @@
#endif
}
-void WebResourceLoadStatisticsStore::registerSharedResourceLoadObserver(std::function<void(const Vector<String>& domainsToRemove, const Vector<String>& domainsToAdd, bool clearFirst)>&& shouldPartitionCookiesForDomainsHandler)
+void WebResourceLoadStatisticsStore::registerSharedResourceLoadObserver(WTF::Function<void(const Vector<String>& domainsToRemove, const Vector<String>& domainsToAdd, bool clearFirst)>&& shouldPartitionCookiesForDomainsHandler)
{
+ ASSERT(isMainThread());
+
registerSharedResourceLoadObserver();
m_resourceLoadStatisticsStore->setShouldPartitionCookiesCallback([shouldPartitionCookiesForDomainsHandler = WTFMove(shouldPartitionCookiesForDomainsHandler)] (const Vector<String>& domainsToRemove, const Vector<String>& domainsToAdd, bool clearFirst) {
shouldPartitionCookiesForDomainsHandler(domainsToRemove, domainsToAdd, clearFirst);
@@ -192,9 +208,12 @@
initializeDataTypesToRemove();
// Switch to the main thread to get the default website data store
- RunLoop::main().dispatch([this] () mutable {
+ RunLoop::main().dispatch([this, protectedThis = makeRef(*this)] () mutable {
WebProcessProxy::topPrivatelyControlledDomainsWithWebiteData(dataTypesToRemove, notifyPages, [this](HashSet<String>&& topPrivatelyControlledDomainsWithWebsiteData) mutable {
- this->coreStore().handleFreshStartWithEmptyOrNoStore(WTFMove(topPrivatelyControlledDomainsWithWebsiteData));
+ // But always touch the ResourceLoadStatistics store on the worker queue
+ m_statisticsQueue->dispatch([this, protectedThis = makeRef(*this), topDomains = CrossThreadCopier<HashSet<String>>::copy(topPrivatelyControlledDomainsWithWebsiteData)] () mutable {
+ this->coreStore().handleFreshStartWithEmptyOrNoStore(WTFMove(topDomains));
+ });
});
});
}
@@ -205,14 +224,14 @@
return;
m_statisticsQueue->dispatch([this, protectedThis = makeRef(*this)] {
- coreStore().clearInMemory();
-
auto decoder = createDecoderFromDisk("full_browsing_session");
if (!decoder) {
grandfatherExistingWebsiteData();
return;
}
-
+
+ auto locker = holdLock(coreStore().statisticsLock());
+ coreStore().clearInMemory();
coreStore().readDataFromDecoder(*decoder);
if (coreStore().isEmpty())
@@ -251,6 +270,8 @@
void WebResourceLoadStatisticsStore::writeStoreToDisk()
{
+ ASSERT(!isMainThread());
+
auto encoder = coreStore().createEncoderFromData();
writeEncoderToDisk(*encoder.get(), "full_browsing_session");
}
@@ -257,6 +278,8 @@
void WebResourceLoadStatisticsStore::writeEncoderToDisk(KeyedEncoder& encoder, const String& label) const
{
+ ASSERT(!isMainThread());
+
RefPtr<SharedBuffer> rawData = encoder.finishEncoding();
if (!rawData)
return;
Modified: trunk/Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.h (217514 => 217515)
--- trunk/Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.h 2017-05-27 00:22:49 UTC (rev 217514)
+++ trunk/Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.h 2017-05-27 00:38:37 UTC (rev 217515)
@@ -63,7 +63,7 @@
void setResourceLoadStatisticsEnabled(bool);
bool resourceLoadStatisticsEnabled() const;
void registerSharedResourceLoadObserver();
- void registerSharedResourceLoadObserver(std::function<void(const Vector<String>& domainsToRemove, const Vector<String>& domainsToAdd, bool clearFirst)>&& shouldPartitionCookiesForDomainsHandler);
+ void registerSharedResourceLoadObserver(WTF::Function<void(const Vector<String>& domainsToRemove, const Vector<String>& domainsToAdd, bool clearFirst)>&& shouldPartitionCookiesForDomainsHandler);
void resourceLoadStatisticsUpdated(const Vector<WebCore::ResourceLoadStatistics>& origins);
Modified: trunk/Source/WebKit2/WebProcess/WebProcess.cpp (217514 => 217515)
--- trunk/Source/WebKit2/WebProcess/WebProcess.cpp 2017-05-27 00:22:49 UTC (rev 217514)
+++ trunk/Source/WebKit2/WebProcess/WebProcess.cpp 2017-05-27 00:38:37 UTC (rev 217515)
@@ -172,6 +172,7 @@
, m_webSQLiteDatabaseTracker(*this)
#endif
, m_resourceLoadStatisticsStore(WebCore::ResourceLoadStatisticsStore::create())
+ , m_statisticsQueue(WorkQueue::create("ResourceLoadStatisticsStore Process Data Queue"))
{
// Initialize our platform strategies.
WebPlatformStrategies::initialize();
@@ -198,6 +199,7 @@
m_plugInAutoStartOriginHashes.add(SessionID::defaultSessionID(), HashMap<unsigned, double>());
ResourceLoadObserver::sharedObserver().setStatisticsStore(m_resourceLoadStatisticsStore.copyRef());
+ ResourceLoadObserver::sharedObserver().setStatisticsQueue(m_statisticsQueue.copyRef());
m_resourceLoadStatisticsStore->setNotificationCallback([this] {
if (m_statisticsChangedTimer.isActive())
return;
Modified: trunk/Source/WebKit2/WebProcess/WebProcess.h (217514 => 217515)
--- trunk/Source/WebKit2/WebProcess/WebProcess.h 2017-05-27 00:22:49 UTC (rev 217514)
+++ trunk/Source/WebKit2/WebProcess/WebProcess.h 2017-05-27 00:38:37 UTC (rev 217515)
@@ -417,6 +417,7 @@
#endif
Ref<WebCore::ResourceLoadStatisticsStore> m_resourceLoadStatisticsStore;
+ Ref<WTF::WorkQueue> m_statisticsQueue;
unsigned m_pagesMarkingLayersAsVolatile { 0 };
bool m_suppressMemoryPressureHandler { false };
