Modified: trunk/Source/WebKit/ChangeLog (247111 => 247112)
--- trunk/Source/WebKit/ChangeLog 2019-07-03 22:14:04 UTC (rev 247111)
+++ trunk/Source/WebKit/ChangeLog 2019-07-03 22:16:06 UTC (rev 247112)
@@ -1,3 +1,21 @@
+2019-07-03 Chris Dumez <[email protected]>
+
+ Fix a couple of thread safety issues in ResourceLoadStatisticsStore
+ https://bugs.webkit.org/show_bug.cgi?id=199463
+
+ Reviewed by Alex Christensen.
+
+ The ResourceLoadStatisticsStore object is constructed / used / destroyed on a background queue.
+ It is therefore not safe to use a WeakPtr to the ResourceLoadStatisticsStore on the main thread.
+
+ The safe pattern is to have the ResourceLoadStatisticsStore capture a Ref<> of its m_store before
+ dispatching to the main thread and use this store on the main thread instead of weakThis->m_store.
+ ResourceLoadStatisticsStore's m_store is constructed / used / destroyed on the main thread.
+
+ * NetworkProcess/Classifier/ResourceLoadStatisticsStore.cpp:
+ (WebKit::ResourceLoadStatisticsStore::removeDataRecords):
+ (WebKit::ResourceLoadStatisticsStore::processStatisticsAndDataRecords):
+
2019-07-03 Youenn Fablet <[email protected]>
Isolate CacheStorage::Engine path when hopping to a background thread
Modified: trunk/Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsStore.cpp (247111 => 247112)
--- trunk/Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsStore.cpp 2019-07-03 22:14:04 UTC (rev 247111)
+++ trunk/Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsStore.cpp 2019-07-03 22:16:06 UTC (rev 247112)
@@ -205,13 +205,8 @@
setDataRecordsBeingRemoved(true);
- RunLoop::main().dispatch([domainsToRemoveWebsiteDataFor = crossThreadCopy(domainsToRemoveWebsiteDataFor), completionHandler = WTFMove(completionHandler), weakThis = makeWeakPtr(*this), shouldNotifyPagesWhenDataRecordsWereScanned = m_parameters.shouldNotifyPagesWhenDataRecordsWereScanned, workQueue = m_workQueue.copyRef()] () mutable {
- if (!weakThis) {
- completionHandler();
- return;
- }
-
- weakThis->m_store.deleteWebsiteDataForRegistrableDomains(WebResourceLoadStatisticsStore::monitoredDataTypes(), WTFMove(domainsToRemoveWebsiteDataFor), shouldNotifyPagesWhenDataRecordsWereScanned, [completionHandler = WTFMove(completionHandler), weakThis = WTFMove(weakThis), workQueue = workQueue.copyRef()](const HashSet<RegistrableDomain>& domainsWithDeletedWebsiteData) mutable {
+ RunLoop::main().dispatch([store = makeRef(m_store), domainsToRemoveWebsiteDataFor = crossThreadCopy(domainsToRemoveWebsiteDataFor), completionHandler = WTFMove(completionHandler), weakThis = makeWeakPtr(*this), shouldNotifyPagesWhenDataRecordsWereScanned = m_parameters.shouldNotifyPagesWhenDataRecordsWereScanned, workQueue = m_workQueue.copyRef()] () mutable {
+ store->deleteWebsiteDataForRegistrableDomains(WebResourceLoadStatisticsStore::monitoredDataTypes(), WTFMove(domainsToRemoveWebsiteDataFor), shouldNotifyPagesWhenDataRecordsWereScanned, [completionHandler = WTFMove(completionHandler), weakThis = WTFMove(weakThis), workQueue = workQueue.copyRef()](const HashSet<RegistrableDomain>& domainsWithDeletedWebsiteData) mutable {
workQueue->dispatch([domainsWithDeletedWebsiteData = crossThreadCopy(domainsWithDeletedWebsiteData), completionHandler = WTFMove(completionHandler), weakThis = WTFMove(weakThis)] () mutable {
if (!weakThis) {
completionHandler();
@@ -247,12 +242,8 @@
if (!m_parameters.shouldNotifyPagesWhenDataRecordsWereScanned)
return;
- RunLoop::main().dispatch([this, weakThis = WTFMove(weakThis)] {
- ASSERT(RunLoop::isMain());
- if (!weakThis)
- return;
-
- m_store.notifyResourceLoadStatisticsProcessed();
+ RunLoop::main().dispatch([store = makeRef(m_store)] {
+ store->notifyResourceLoadStatisticsProcessed();
});
});
}
