[webkit-changes] [248372] trunk/Source/WebKit

Wed, 07 Aug 2019 10:24:33 -0700

Title: [248372] trunk/Source/WebKit
Revision
248372
Author
cdu...@apple.com
Date
2019-08-07 10:22:38 -0700 (Wed, 07 Aug 2019)

Log Message

Add more threading assertions to ITP code
https://bugs.webkit.org/show_bug.cgi?id=200505

Reviewed by Brent Fulgham.

Add more threading assertions to ITP code to help catch bugs and protect against future bad usage.

* NetworkProcess/Classifier/ResourceLoadStatisticsMemoryStore.cpp:
(WebKit::ResourceLoadStatisticsMemoryStore::isEmpty const):
(WebKit::ResourceLoadStatisticsMemoryStore::setPersistentStorage):
(WebKit::ResourceLoadStatisticsMemoryStore::incrementRecordsDeletedCountForDomains):
(WebKit::ResourceLoadStatisticsMemoryStore::classifyPrevalentResources):
(WebKit::ResourceLoadStatisticsMemoryStore::syncStorageIfNeeded):
(WebKit::ResourceLoadStatisticsMemoryStore::syncStorageImmediately):
(WebKit::ResourceLoadStatisticsMemoryStore::grandfatherDataForDomains):
(WebKit::ResourceLoadStatisticsMemoryStore::ensurePrevalentResourcesForDebugMode):
* NetworkProcess/Classifier/ResourceLoadStatisticsStore.cpp:
(WebKit::ResourceLoadStatisticsStore::statisticsEpirationTime const):
(WebKit::ResourceLoadStatisticsStore::mergeOperatingDates):
* NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp:
(WebKit::WebResourceLoadStatisticsStore::hasStorageAccessForFrame):
(WebKit::WebResourceLoadStatisticsStore::requestStorageAccess):
(WebKit::WebResourceLoadStatisticsStore::grantStorageAccess):
(WebKit::WebResourceLoadStatisticsStore::applicationWillTerminate):
(WebKit::WebResourceLoadStatisticsStore::logFrameNavigation):
(WebKit::WebResourceLoadStatisticsStore::logWebSocketLoading):
(WebKit::WebResourceLoadStatisticsStore::logSubresourceLoading):
(WebKit::WebResourceLoadStatisticsStore::logSubresourceRedirect):
(WebKit::WebResourceLoadStatisticsStore::hasHadUserInteraction):
(WebKit::WebResourceLoadStatisticsStore::removePrevalentDomains):
(WebKit::WebResourceLoadStatisticsStore::networkSession):
(WebKit::WebResourceLoadStatisticsStore::invalidateAndCancel):
(WebKit::WebResourceLoadStatisticsStore::sendDiagnosticMessageWithValue const):
(WebKit::WebResourceLoadStatisticsStore::notifyPageStatisticsTelemetryFinished const):

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (248371 => 248372)


--- trunk/Source/WebKit/ChangeLog	2019-08-07 17:19:08 UTC (rev 248371)
+++ trunk/Source/WebKit/ChangeLog	2019-08-07 17:22:38 UTC (rev 248372)
@@ -1,3 +1,40 @@
+2019-08-07  Chris Dumez  <cdu...@apple.com>
+
+        Add more threading assertions to ITP code
+        https://bugs.webkit.org/show_bug.cgi?id=200505
+
+        Reviewed by Brent Fulgham.
+
+        Add more threading assertions to ITP code to help catch bugs and protect against future bad usage.
+
+        * NetworkProcess/Classifier/ResourceLoadStatisticsMemoryStore.cpp:
+        (WebKit::ResourceLoadStatisticsMemoryStore::isEmpty const):
+        (WebKit::ResourceLoadStatisticsMemoryStore::setPersistentStorage):
+        (WebKit::ResourceLoadStatisticsMemoryStore::incrementRecordsDeletedCountForDomains):
+        (WebKit::ResourceLoadStatisticsMemoryStore::classifyPrevalentResources):
+        (WebKit::ResourceLoadStatisticsMemoryStore::syncStorageIfNeeded):
+        (WebKit::ResourceLoadStatisticsMemoryStore::syncStorageImmediately):
+        (WebKit::ResourceLoadStatisticsMemoryStore::grandfatherDataForDomains):
+        (WebKit::ResourceLoadStatisticsMemoryStore::ensurePrevalentResourcesForDebugMode):
+        * NetworkProcess/Classifier/ResourceLoadStatisticsStore.cpp:
+        (WebKit::ResourceLoadStatisticsStore::statisticsEpirationTime const):
+        (WebKit::ResourceLoadStatisticsStore::mergeOperatingDates):
+        * NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp:
+        (WebKit::WebResourceLoadStatisticsStore::hasStorageAccessForFrame):
+        (WebKit::WebResourceLoadStatisticsStore::requestStorageAccess):
+        (WebKit::WebResourceLoadStatisticsStore::grantStorageAccess):
+        (WebKit::WebResourceLoadStatisticsStore::applicationWillTerminate):
+        (WebKit::WebResourceLoadStatisticsStore::logFrameNavigation):
+        (WebKit::WebResourceLoadStatisticsStore::logWebSocketLoading):
+        (WebKit::WebResourceLoadStatisticsStore::logSubresourceLoading):
+        (WebKit::WebResourceLoadStatisticsStore::logSubresourceRedirect):
+        (WebKit::WebResourceLoadStatisticsStore::hasHadUserInteraction):
+        (WebKit::WebResourceLoadStatisticsStore::removePrevalentDomains):
+        (WebKit::WebResourceLoadStatisticsStore::networkSession):
+        (WebKit::WebResourceLoadStatisticsStore::invalidateAndCancel):
+        (WebKit::WebResourceLoadStatisticsStore::sendDiagnosticMessageWithValue const):
+        (WebKit::WebResourceLoadStatisticsStore::notifyPageStatisticsTelemetryFinished const):
+
 2019-08-07  Ryan Haddad  <ryanhad...@apple.com>
 
         Unreviewed, rolling out r248330.

Modified: trunk/Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsMemoryStore.cpp (248371 => 248372)


--- trunk/Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsMemoryStore.cpp	2019-08-07 17:19:08 UTC (rev 248371)
+++ trunk/Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsMemoryStore.cpp	2019-08-07 17:22:38 UTC (rev 248372)
@@ -82,11 +82,15 @@
 
 bool ResourceLoadStatisticsMemoryStore::isEmpty() const
 {
+    ASSERT(!RunLoop::isMain());
+
     return m_resourceStatisticsMap.isEmpty();
 }
 
 void ResourceLoadStatisticsMemoryStore::setPersistentStorage(ResourceLoadStatisticsPersistentStorage& persistentStorage)
 {
+    ASSERT(!RunLoop::isMain());
+
     m_persistentStorage = makeWeakPtr(persistentStorage);
 }
 
@@ -100,6 +104,8 @@
 
 void ResourceLoadStatisticsMemoryStore::incrementRecordsDeletedCountForDomains(HashSet<RegistrableDomain>&& domainsWithDeletedWebsiteData)
 {
+    ASSERT(!RunLoop::isMain());
+
     for (auto& domain : domainsWithDeletedWebsiteData) {
         auto& statistic = ensureResourceStatisticsForRegistrableDomain(domain);
         ++statistic.dataRecordsRemoved;
@@ -172,6 +178,8 @@
 
 void ResourceLoadStatisticsMemoryStore::classifyPrevalentResources()
 {
+    ASSERT(!RunLoop::isMain());
+
     for (auto& resourceStatistic : m_resourceStatisticsMap.values()) {
         if (shouldSkip(resourceStatistic.registrableDomain))
             continue;
@@ -189,6 +197,8 @@
 
 void ResourceLoadStatisticsMemoryStore::syncStorageIfNeeded()
 {
+    ASSERT(!RunLoop::isMain());
+
     if (m_persistentStorage)
         m_persistentStorage->scheduleOrWriteMemoryStore(ResourceLoadStatisticsPersistentStorage::ForceImmediateWrite::No);
 }
@@ -195,6 +205,8 @@
 
 void ResourceLoadStatisticsMemoryStore::syncStorageImmediately()
 {
+    ASSERT(!RunLoop::isMain());
+
     if (m_persistentStorage)
         m_persistentStorage->scheduleOrWriteMemoryStore(ResourceLoadStatisticsPersistentStorage::ForceImmediateWrite::Yes);
 }
@@ -315,6 +327,8 @@
 
 void ResourceLoadStatisticsMemoryStore::grandfatherDataForDomains(const HashSet<RegistrableDomain>& domains)
 {
+    ASSERT(!RunLoop::isMain());
+
     for (auto& domain : domains) {
         auto& statistic = ensureResourceStatisticsForRegistrableDomain(domain);
         statistic.grandfathered = true;
@@ -323,6 +337,8 @@
 
 Vector<RegistrableDomain> ResourceLoadStatisticsMemoryStore::ensurePrevalentResourcesForDebugMode()
 {
+    ASSERT(!RunLoop::isMain());
+
     if (!debugModeEnabled())
         return { };

Modified: trunk/Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsStore.cpp (248371 => 248372)


--- trunk/Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsStore.cpp	2019-08-07 17:19:08 UTC (rev 248371)
+++ trunk/Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsStore.cpp	2019-08-07 17:22:38 UTC (rev 248372)
@@ -445,6 +445,8 @@
 
 Optional<Seconds> ResourceLoadStatisticsStore::statisticsEpirationTime() const
 {
+    ASSERT(!RunLoop::isMain());
+
     if (m_parameters.timeToLiveUserInteraction)
         return WallTime::now().secondsSinceEpoch() - m_parameters.timeToLiveUserInteraction.value();
     
@@ -475,6 +477,8 @@
 
 void ResourceLoadStatisticsStore::mergeOperatingDates(Vector<OperatingDate>&& newDates)
 {
+    ASSERT(!RunLoop::isMain());
+
     m_operatingDates = mergeOperatingDates(m_operatingDates, WTFMove(newDates));
 }

Modified: trunk/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp (248371 => 248372)


--- trunk/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp	2019-08-07 17:19:08 UTC (rev 248371)
+++ trunk/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp	2019-08-07 17:22:38 UTC (rev 248372)
@@ -306,10 +306,14 @@
 
 bool WebResourceLoadStatisticsStore::hasStorageAccessForFrame(const RegistrableDomain& resourceDomain, const RegistrableDomain& firstPartyDomain, uint64_t frameID, PageIdentifier pageID)
 {
-    if (m_networkSession) {
-        if (auto* storageSession = m_networkSession->networkStorageSession())
-            return storageSession->hasStorageAccess(resourceDomain, firstPartyDomain, frameID, pageID);
-    }
+    ASSERT(RunLoop::isMain());
+
+    if (!m_networkSession)
+        return false;
+
+    if (auto* storageSession = m_networkSession->networkStorageSession())
+        return storageSession->hasStorageAccess(resourceDomain, firstPartyDomain, frameID, pageID);
+
     return false;
 }
 
@@ -329,6 +333,8 @@
 
 void WebResourceLoadStatisticsStore::requestStorageAccess(const RegistrableDomain& subFrameDomain, const RegistrableDomain& topFrameDomain, uint64_t frameID, PageIdentifier pageID, CompletionHandler<void(StorageAccessWasGranted, StorageAccessPromptWasShown)>&& completionHandler)
 {
+    ASSERT(RunLoop::isMain());
+
     if (subFrameDomain == topFrameDomain) {
         completionHandler(StorageAccessWasGranted::Yes, StorageAccessPromptWasShown::No);
         return;
@@ -409,6 +415,8 @@
 
 StorageAccessWasGranted WebResourceLoadStatisticsStore::grantStorageAccess(const RegistrableDomain& resourceDomain, const RegistrableDomain& firstPartyDomain, Optional<uint64_t> frameID, PageIdentifier pageID)
 {
+    ASSERT(RunLoop::isMain());
+
     bool isStorageGranted = false;
 
     if (m_networkSession) {
@@ -454,6 +462,7 @@
 
 void WebResourceLoadStatisticsStore::applicationWillTerminate()
 {
+    ASSERT(RunLoop::isMain());
     flushAndDestroyPersistentStore();
 }
 
@@ -513,6 +522,8 @@
 
 void WebResourceLoadStatisticsStore::logFrameNavigation(const RegistrableDomain& targetDomain, const RegistrableDomain& topFrameDomain, const RegistrableDomain& sourceDomain, bool isRedirect, bool isMainFrame)
 {
+    ASSERT(RunLoop::isMain());
+
     postTask([this, targetDomain = targetDomain.isolatedCopy(), topFrameDomain = topFrameDomain.isolatedCopy(), sourceDomain = sourceDomain.isolatedCopy(), isRedirect, isMainFrame] {
         if (m_statisticsStore)
             m_statisticsStore->logFrameNavigation(targetDomain, topFrameDomain, sourceDomain, isRedirect, isMainFrame);
@@ -521,6 +532,8 @@
 
 void WebResourceLoadStatisticsStore::logWebSocketLoading(const RegistrableDomain& targetDomain, const RegistrableDomain& topFrameDomain, WallTime lastSeen, CompletionHandler<void()>&& completionHandler)
 {
+    ASSERT(RunLoop::isMain());
+
     postTask([this, targetDomain = targetDomain.isolatedCopy(), topFrameDomain = topFrameDomain.isolatedCopy(), lastSeen, completionHandler = WTFMove(completionHandler)]() mutable {
         if (m_statisticsStore)
             m_statisticsStore->logSubresourceLoading(targetDomain, topFrameDomain, lastSeen);
@@ -531,6 +544,8 @@
 
 void WebResourceLoadStatisticsStore::logSubresourceLoading(const SubResourceDomain& targetDomain, const TopFrameDomain& topFrameDomain, WallTime lastSeen, CompletionHandler<void()>&& completionHandler)
 {
+    ASSERT(RunLoop::isMain());
+
     postTask([this, targetDomain = targetDomain.isolatedCopy(), topFrameDomain = topFrameDomain.isolatedCopy(), lastSeen, completionHandler = WTFMove(completionHandler)]() mutable {
         if (m_statisticsStore)
             m_statisticsStore->logSubresourceLoading(targetDomain, topFrameDomain, lastSeen);
@@ -541,6 +556,8 @@
 
 void WebResourceLoadStatisticsStore::logSubresourceRedirect(const RegistrableDomain& sourceDomain, const RegistrableDomain& targetDomain, CompletionHandler<void()>&& completionHandler)
 {
+    ASSERT(RunLoop::isMain());
+
     postTask([this, sourceDomain = sourceDomain.isolatedCopy(), targetDomain = targetDomain.isolatedCopy(), completionHandler = WTFMove(completionHandler)]() mutable {
         if (m_statisticsStore)
             m_statisticsStore->logSubresourceRedirect(sourceDomain, targetDomain);
@@ -584,6 +601,8 @@
 
 void WebResourceLoadStatisticsStore::hasHadUserInteraction(const RegistrableDomain& domain, CompletionHandler<void(bool)>&& completionHandler)
 {
+    ASSERT(RunLoop::isMain());
+
     postTask([this, domain, completionHandler = WTFMove(completionHandler)]() mutable {
         bool hadUserInteraction = m_statisticsStore ? m_statisticsStore->hasHadUserInteraction(domain, OperatingDatesWindow::Long) : false;
         postTaskReply([hadUserInteraction, completionHandler = WTFMove(completionHandler)]() mutable {
@@ -954,10 +973,12 @@
 
 void WebResourceLoadStatisticsStore::removePrevalentDomains(const Vector<RegistrableDomain>& domains)
 {
-    if (m_networkSession) {
-        if (auto* storageSession = m_networkSession->networkStorageSession())
-            storageSession->removePrevalentDomains(domains);
-    }
+    ASSERT(RunLoop::isMain());
+    if (!m_networkSession)
+        return;
+
+    if (auto* storageSession = m_networkSession->networkStorageSession())
+        storageSession->removePrevalentDomains(domains);
 }
 
 void WebResourceLoadStatisticsStore::callRemoveDomainsHandler(const Vector<RegistrableDomain>& domains)
@@ -1020,11 +1041,13 @@
 
 NetworkSession* WebResourceLoadStatisticsStore::networkSession()
 {
+    ASSERT(RunLoop::isMain());
     return m_networkSession.get();
 }
 
 void WebResourceLoadStatisticsStore::invalidateAndCancel()
 {
+    ASSERT(RunLoop::isMain());
     m_networkSession = nullptr;
 }
 
@@ -1054,6 +1077,7 @@
 
 void WebResourceLoadStatisticsStore::sendDiagnosticMessageWithValue(const String& message, const String& description, unsigned value, unsigned sigDigits, WebCore::ShouldSample shouldSample) const
 {
+    ASSERT(RunLoop::isMain());
     if (m_networkSession)
         const_cast<WebResourceLoadStatisticsStore*>(this)->networkSession()->logDiagnosticMessageWithValue(message, description, value, sigDigits, shouldSample);
 }
@@ -1060,6 +1084,7 @@
 
 void WebResourceLoadStatisticsStore::notifyPageStatisticsTelemetryFinished(unsigned totalPrevalentResources, unsigned totalPrevalentResourcesWithUserInteraction, unsigned top3SubframeUnderTopFrameOrigins) const
 {
+    ASSERT(RunLoop::isMain());
     if (m_networkSession)
         const_cast<WebResourceLoadStatisticsStore*>(this)->networkSession()->notifyPageStatisticsTelemetryFinished(totalPrevalentResources, totalPrevalentResourcesWithUserInteraction, top3SubframeUnderTopFrameOrigins);
 }

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to