[WK2] Address thread safety issues with ResourceLoadStatistics
authorbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 27 May 2017 00:38:37 +0000 (00:38 +0000)
committerbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 27 May 2017 00:38:37 +0000 (00:38 +0000)
https://bugs.webkit.org/show_bug.cgi?id=172519
<rdar://problem/31707642>

Reviewed by Chris Dumez.

Source/WebCore:

* 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:

Source/WebKit/mac:

Create a new WorkQueue for the ResourceLoadStatistics store to use for processing data.

* WebView/WebView.mm:
(WebKitInitializeApplicationStatisticsStoragePathIfNecessary): Pass WorkQueue to the observer.

Source/WebKit2:

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:

Source/WTF:

Add a new specialization for HashSet.

* wtf/CrossThreadCopier.h:

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

14 files changed:
Source/WTF/ChangeLog
Source/WTF/wtf/CrossThreadCopier.h
Source/WebCore/ChangeLog
Source/WebCore/loader/ResourceLoadObserver.cpp
Source/WebCore/loader/ResourceLoadObserver.h
Source/WebCore/loader/ResourceLoadStatisticsStore.cpp
Source/WebCore/loader/ResourceLoadStatisticsStore.h
Source/WebKit/mac/ChangeLog
Source/WebKit/mac/WebView/WebView.mm
Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.cpp
Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.h
Source/WebKit2/WebProcess/WebProcess.cpp
Source/WebKit2/WebProcess/WebProcess.h

index deaf30e..8bf6391 100644 (file)
@@ -1,3 +1,15 @@
+2017-05-26  Brent Fulgham  <bfulgham@apple.com>
+
+        [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  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r217458.
index f8e742f..96b795e 100644 (file)
@@ -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 @@ template<typename T> struct CrossThreadCopierBase<false, false, Vector<T>> {
         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;
index 482ab8f..9c1f316 100644 (file)
@@ -1,3 +1,67 @@
+2017-05-26  Brent Fulgham  <bfulgham@apple.com>
+
+        [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  <pecoraro@apple.com>
 
         JSContext Inspector: Improve the reliability of automatically pausing in auto-attach
index 7dd4923..6c07fac 100644 (file)
 #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 @@ ResourceLoadObserver& ResourceLoadObserver::sharedObserver()
 
 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 @@ void ResourceLoadObserver::logFrameNavigation(const Frame& frame, const Frame& t
     
     if (targetPrimaryDomain == mainFramePrimaryDomain || targetPrimaryDomain == sourcePrimaryDomain)
         return;
-
-    auto targetOrigin = SecurityOrigin::create(targetURL);
-    auto targetStatistics = m_store->ensureResourceStatisticsForPrimaryDomain(targetPrimaryDomain);
-
-    // 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;
-
-        auto mainFrameOrigin = SecurityOrigin::create(mainFrameURL);
-        auto subframeUnderTopFrameOriginsResult = targetStatistics.subframeUnderTopFrameOrigins.add(mainFramePrimaryDomain);
-        if (subframeUnderTopFrameOriginsResult.isNewEntry)
-            shouldFireDataModificationHandler = true;
-    }
     
-    if (isRedirect) {
-        auto& redirectingOriginResourceStatistics = m_store->ensureResourceStatisticsForPrimaryDomain(sourcePrimaryDomain);
+    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()] () {
         
-        if (m_store->isPrevalentResource(targetPrimaryDomain))
-            redirectingOriginResourceStatistics.redirectedToOtherPrevalentResourceOrigins.add(targetPrimaryDomain);
+        auto targetOrigin = SecurityOrigin::create(targetURL);
+        bool shouldFireDataModificationHandler = false;
         
-        if (isMainFrame) {
-            ++targetStatistics.topFrameHasBeenRedirectedTo;
-            ++redirectingOriginResourceStatistics.topFrameHasBeenRedirectedFrom;
-        } else {
-            ++targetStatistics.subframeHasBeenRedirectedTo;
-            ++redirectingOriginResourceStatistics.subframeHasBeenRedirectedFrom;
-            redirectingOriginResourceStatistics.subframeUniqueRedirectsTo.add(targetPrimaryDomain);
-            
-            ++targetStatistics.subframeSubResourceCount;
+        {
+        auto locker = holdLock(m_store->statisticsLock());
+        auto& targetStatistics = m_store->ensureResourceStatisticsForPrimaryDomain(targetPrimaryDomain);
+
+        // Always fire if we have previously removed data records for this domain
+        shouldFireDataModificationHandler = targetStatistics.dataRecordsRemoved > 0;
+
+        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;
         }
-    } else {
-        if (sourcePrimaryDomain.isNull() || sourcePrimaryDomain.isEmpty() || sourcePrimaryDomain == "nullOrigin") {
-            if (isMainFrame)
-                ++targetStatistics.topFrameInitialLoadCount;
-            else
+        
+        if (isRedirect) {
+            auto& redirectingOriginResourceStatistics = m_store->ensureResourceStatisticsForPrimaryDomain(sourcePrimaryDomain);
+            
+            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 (isMainFrame) {
-                ++sourceOriginResourceStatistics.topFrameHasBeenNavigatedFrom;
-                ++targetStatistics.topFrameHasBeenNavigatedTo;
+            if (sourcePrimaryDomain.isNull() || sourcePrimaryDomain.isEmpty() || sourcePrimaryDomain == "nullOrigin") {
+                if (isMainFrame)
+                    ++targetStatistics.topFrameInitialLoadCount;
+                else
+                    ++targetStatistics.subframeSubResourceCount;
             } else {
-                ++sourceOriginResourceStatistics.subframeHasBeenNavigatedFrom;
-                ++targetStatistics.subframeHasBeenNavigatedTo;
+                auto& sourceOriginResourceStatistics = m_store->ensureResourceStatisticsForPrimaryDomain(sourcePrimaryDomain);
+
+                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 @@ void ResourceLoadObserver::logSubresourceLoading(const Frame* frame, const Resou
     
     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
-    bool shouldFireDataModificationHandler = targetStatistics.dataRecordsRemoved > 0;
+        // 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;
+        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);
+            
+            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
         
-        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 @@ void ResourceLoadObserver::logWebSocketLoading(const Frame* frame, const URL& ta
     if (targetPrimaryDomain == mainFramePrimaryDomain)
         return;
 
-    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;
+    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);
 
-    ++targetStatistics.subresourceHasBeenSubresourceCount;
-    
-    auto totalVisited = std::max(m_originsVisitedMap.size(), 1U);
-    
-    targetStatistics.subresourceHasBeenSubresourceCountDividedByTotalNumberOfOriginsVisited = static_cast<double>(targetStatistics.subresourceHasBeenSubresourceCount) / totalVisited;
+        // 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;
 
-    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 @@ void ResourceLoadObserver::logUserInteractionWithReducedTimeResolution(const Doc
     if (url.isBlankURL() || url.isEmpty())
         return;
 
-    auto primaryDomainStr = primaryDomain(url);
-
-    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();
+    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;
+
+        statistics.hadUserInteraction = true;
+        statistics.mostRecentUserInteraction = newTimestamp;
+        }
+        
+        m_store->fireDataModificationHandler();
+    });
 }
 
 void ResourceLoadObserver::logUserInteraction(const URL& url)
@@ -335,13 +387,19 @@ void ResourceLoadObserver::logUserInteraction(const URL& url)
     if (url.isBlankURL() || url.isEmpty())
         return;
 
-    auto primaryDomainStr = primaryDomain(url);
-
-    auto& statistics = m_store->ensureResourceStatisticsForPrimaryDomain(primaryDomainStr);
-    statistics.hadUserInteraction = true;
-    statistics.mostRecentUserInteraction = WTF::currentTime();
+    auto primaryDomainString = primaryDomain(url);
 
-    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 @@ void ResourceLoadObserver::clearUserInteraction(const URL& url)
     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 @@ bool ResourceLoadObserver::hasHadUserInteraction(const URL& url)
     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 @@ void ResourceLoadObserver::setPrevalentResource(const URL& url)
     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 @@ bool ResourceLoadObserver::isPrevalentResource(const URL& url)
     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 @@ void ResourceLoadObserver::clearPrevalentResource(const URL& url)
     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 @@ void ResourceLoadObserver::setGrandfathered(const URL& url, bool value)
     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 @@ bool ResourceLoadObserver::isGrandfathered(const URL& url)
     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 @@ void ResourceLoadObserver::setSubframeUnderTopFrameOrigin(const URL& subframe, c
     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 @@ void ResourceLoadObserver::setSubresourceUnderTopFrameOrigin(const URL& subresou
     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 @@ void ResourceLoadObserver::setSubresourceUniqueRedirectTo(const URL& subresource
     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::setGrandfatheringTime(double seconds)
     
 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::primaryDomain(const String& host)
 
 String ResourceLoadObserver::statisticsForOrigin(const String& origin)
 {
-    return m_store ? m_store->statisticsForOrigin(origin) : emptyString();
+    if (!m_store)
+        return emptyString();
+    
+    return m_store->statisticsForOrigin(origin);
 }
 
 }
index 799353a..312ec5a 100644 (file)
 #include <wtf/HashMap.h>
 #include <wtf/text/WTFString.h>
 
+namespace WTF {
+class Lock;
+class WorkQueue;
+}
+
 namespace WebCore {
 
 class Document;
@@ -75,6 +80,7 @@ public:
     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 @@ private:
     static String primaryDomain(const String& host);
 
     RefPtr<ResourceLoadStatisticsStore> m_store;
+    RefPtr<WTF::WorkQueue> m_queue;
     HashMap<String, size_t> m_originsVisitedMap;
 };
     
index 47721f2..310d712 100644 (file)
 #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 @@ Ref<ResourceLoadStatisticsStore> ResourceLoadStatisticsStore::create()
     
 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 @@ bool ResourceLoadStatisticsStore::isPrevalentResource(const String& primaryDomai
     
 ResourceLoadStatistics& ResourceLoadStatisticsStore::ensureResourceStatisticsForPrimaryDomain(const String& primaryDomain)
 {
+    ASSERT(m_statisticsLock.isLocked());
     auto addResult = m_resourceStatisticsMap.ensure(primaryDomain, [&primaryDomain] {
         return ResourceLoadStatistics(primaryDomain);
     });
@@ -71,6 +76,8 @@ ResourceLoadStatistics& ResourceLoadStatisticsStore::ensureResourceStatisticsFor
 
 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 @@ typedef HashMap<String, ResourceLoadStatistics>::KeyValuePairType StatisticsValu
 
 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 @@ std::unique_ptr<KeyedEncoder> ResourceLoadStatisticsStore::createEncoderFromData
 
 void ResourceLoadStatisticsStore::readDataFromDecoder(KeyedDecoder& decoder)
 {
+    ASSERT(!isMainThread());
+    ASSERT(m_statisticsLock.isLocked());
     if (m_resourceStatisticsMap.size())
         return;
 
@@ -117,6 +129,9 @@ void ResourceLoadStatisticsStore::readDataFromDecoder(KeyedDecoder& decoder)
 
     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 @@ void ResourceLoadStatisticsStore::readDataFromDecoder(KeyedDecoder& decoder)
         }
         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 @@ void ResourceLoadStatisticsStore::clearInMemoryAndPersistent()
 
 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 @@ String ResourceLoadStatisticsStore::statisticsForOrigin(const String& origin)
 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,30 +203,33 @@ void ResourceLoadStatisticsStore::mergeStatistics(const Vector<ResourceLoadStati
     }
 }
 
-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);
 }
 
 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 @@ static inline bool shouldPartitionCookies(const ResourceLoadStatistics& statisti
 
 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 @@ void ResourceLoadStatisticsStore::fireShouldPartitionCookiesHandler()
     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 @@ void ResourceLoadStatisticsStore::setGrandfatheringTime(double seconds)
         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 @@ Vector<String> ResourceLoadStatisticsStore::topPrivatelyControlledDomainsToRemov
         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 @@ Vector<String> ResourceLoadStatisticsStore::topPrivatelyControlledDomainsToRemov
 
 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::updateStatisticsForRemovedDataRecords(const Ve
 
 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 @@ void ResourceLoadStatisticsStore::handleFreshStartWithEmptyOrNoStore(HashSet<Str
     m_endOfGrandfatheringTimestamp = std::floor(currentTime()) + grandfatheringTime;
 }
 
-bool ResourceLoadStatisticsStore::shouldRemoveDataRecords()
+bool ResourceLoadStatisticsStore::shouldRemoveDataRecords() const
 {
+    ASSERT(!isMainThread());
     if (m_dataRecordsRemovalPending)
         return false;
 
@@ -350,13 +395,20 @@ bool ResourceLoadStatisticsStore::shouldRemoveDataRecords()
 
 void ResourceLoadStatisticsStore::dataRecordsBeingRemoved()
 {
+    ASSERT(!isMainThread());
     m_lastTimeDataRecordsWereRemoved = currentTime();
     m_dataRecordsRemovalPending = true;
 }
 
 void ResourceLoadStatisticsStore::dataRecordsWereRemoved()
 {
+    ASSERT(!isMainThread());
     m_dataRecordsRemovalPending = false;
 }
-
+    
+WTF::RecursiveLockAdapter<Lock>& ResourceLoadStatisticsStore::statisticsLock()
+{
+    return m_statisticsLock;
+}
+    
 }
index e1af557..1ef4ef2 100644 (file)
@@ -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 @@ public:
     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,24 +73,28 @@ public:
     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 };
index 7966901..19a7dee 100644 (file)
@@ -1,3 +1,16 @@
+2017-05-26  Brent Fulgham  <bfulgham@apple.com>
+
+        [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  <mmaxfield@apple.com>
 
         [WK1] iframes in layer-backed NSViews are not cleared between successive draws
index 3266f77..9f535f4 100644 (file)
 #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 @@ enum { WebViewVersion = 4 };
 
 static NSMutableSet *schemesWithRepresentationsSet;
 static WebCore::ResourceLoadStatisticsStore* resourceLoadStatisticsStore;
+static WTF::WorkQueue* statisticsQueue;
 
 #if !PLATFORM(IOS)
 NSString *_WebCanGoBackKey =            @"canGoBack";
@@ -1167,6 +1169,9 @@ static void WebKitInitializeApplicationStatisticsStoragePathIfNecessary()
     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;
 }
 
index 6e961cc..25b7f27 100644 (file)
@@ -1,3 +1,32 @@
+2017-05-26  Brent Fulgham  <bfulgham@apple.com>
+
+        [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  <pecoraro@apple.com>
 
         [Cocoa] Simplify some WebViewImpl pasteboard code
index c23a8f6..e97aafb 100644 (file)
@@ -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 @@ static inline void initializeDataTypesToRemove()
     
 void WebResourceLoadStatisticsStore::removeDataRecords()
 {
+    ASSERT(!isMainThread());
+    
     if (!coreStore().shouldRemoveDataRecords())
         return;
 
@@ -115,24 +118,30 @@ void WebResourceLoadStatisticsStore::removeDataRecords()
         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();
+            });
         });
     });
 }
 
 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,14 +170,19 @@ bool WebResourceLoadStatisticsStore::resourceLoadStatisticsEnabled() const
 
 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;
         processStatisticsAndDataRecords();
     });
     m_resourceLoadStatisticsStore->setWritePersistentStoreCallback([this]() {
-        writeStoreToDisk();
+        m_statisticsQueue->dispatch([this, protectedThis = makeRef(*this)] {
+            writeStoreToDisk();
+        });
     });
     m_resourceLoadStatisticsStore->setGrandfatherExistingWebsiteDataCallback([this]() {
         grandfatherExistingWebsiteData();
@@ -178,8 +192,10 @@ void WebResourceLoadStatisticsStore::registerSharedResourceLoadObserver()
 #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 @@ void WebResourceLoadStatisticsStore::grandfatherExistingWebsiteData()
         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 @@ void WebResourceLoadStatisticsStore::readDataFromDiskIfNeeded()
         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,12 +270,16 @@ String WebResourceLoadStatisticsStore::persistentStoragePath(const String& label
 
 void WebResourceLoadStatisticsStore::writeStoreToDisk()
 {
+    ASSERT(!isMainThread());
+    
     auto encoder = coreStore().createEncoderFromData();
     writeEncoderToDisk(*encoder.get(), "full_browsing_session");
 }
 
 void WebResourceLoadStatisticsStore::writeEncoderToDisk(KeyedEncoder& encoder, const String& label) const
 {
+    ASSERT(!isMainThread());
+    
     RefPtr<SharedBuffer> rawData = encoder.finishEncoding();
     if (!rawData)
         return;
index 74f11ba..7185f68 100644 (file)
@@ -63,7 +63,7 @@ public:
     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);
 
index 122cf2e..f56ef60 100644 (file)
@@ -172,6 +172,7 @@ WebProcess::WebProcess()
     , 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 @@ WebProcess::WebProcess()
     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;
index 59c206b..6ed1998 100644 (file)
@@ -417,6 +417,7 @@ private:
 #endif
 
     Ref<WebCore::ResourceLoadStatisticsStore> m_resourceLoadStatisticsStore;
+    Ref<WTF::WorkQueue> m_statisticsQueue;
 
     unsigned m_pagesMarkingLayersAsVolatile { 0 };
     bool m_suppressMemoryPressureHandler { false };