ASSERTION FAILED: m_caches.isEmpty() || !m_pendingInitializationCallbacks.isEmpty...
authoryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Feb 2019 22:55:24 +0000 (22:55 +0000)
committeryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Feb 2019 22:55:24 +0000 (22:55 +0000)
https://bugs.webkit.org/show_bug.cgi?id=188393
<rdar://problem/43025665>

Reviewed by Alex Christensen.

In case Caches::dispose is called, clearMemoryRepresentation might be called if there is no active cache.
We also ensure to not clear the memory representation if there is any remaining removed cache.
Update the clearMemoryRepresentation assertion to take that into account.

In case a Caches is cleared twice, the clearMemoryRepresentation assertion will assert while it should not.
In that case m_storage is null the second time. Update the assertion accordingly.

* NetworkProcess/cache/CacheStorageEngineCaches.cpp:
(WebKit::CacheStorage::Caches::hasActiveCache const):
(WebKit::CacheStorage::Caches::dispose):
(WebKit::CacheStorage::Caches::clearMemoryRepresentation):
* NetworkProcess/cache/CacheStorageEngineCaches.h:

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

Source/WebKit/ChangeLog
Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.cpp
Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.h

index 885a378..11da96a 100644 (file)
@@ -1,3 +1,24 @@
+2019-02-14  Youenn Fablet  <youenn@apple.com>
+
+        ASSERTION FAILED: m_caches.isEmpty() || !m_pendingInitializationCallbacks.isEmpty() in WebKit::CacheStorage::Caches::clearMemoryRepresentation()
+        https://bugs.webkit.org/show_bug.cgi?id=188393
+        <rdar://problem/43025665>
+
+        Reviewed by Alex Christensen.
+
+        In case Caches::dispose is called, clearMemoryRepresentation might be called if there is no active cache.
+        We also ensure to not clear the memory representation if there is any remaining removed cache.
+        Update the clearMemoryRepresentation assertion to take that into account.
+
+        In case a Caches is cleared twice, the clearMemoryRepresentation assertion will assert while it should not.
+        In that case m_storage is null the second time. Update the assertion accordingly.
+
+        * NetworkProcess/cache/CacheStorageEngineCaches.cpp:
+        (WebKit::CacheStorage::Caches::hasActiveCache const):
+        (WebKit::CacheStorage::Caches::dispose):
+        (WebKit::CacheStorage::Caches::clearMemoryRepresentation):
+        * NetworkProcess/cache/CacheStorageEngineCaches.h:
+
 2019-02-14  Brian Burg  <bburg@apple.com>
 
         [Mac] WebInspectorUI.framework does not need to be soft-linked anymore
index 1f608cf..474cf1c 100644 (file)
@@ -343,6 +343,13 @@ void Caches::remove(uint64_t identifier, CacheIdentifierCallback&& callback)
     });
 }
 
+bool Caches::hasActiveCache() const
+{
+    if (m_removedCaches.size())
+        return true;
+    return m_caches.findMatching([](const auto& item) { return item.isActive(); }) != notFound;
+}
+
 void Caches::dispose(Cache& cache)
 {
     auto position = m_removedCaches.findMatching([&](const auto& item) { return item.identifier() == cache.identifier(); });
@@ -356,7 +363,7 @@ void Caches::dispose(Cache& cache)
     ASSERT(m_caches.findMatching([&](const auto& item) { return item.identifier() == cache.identifier(); }) != notFound);
     cache.clearMemoryRepresentation();
 
-    if (m_caches.findMatching([](const auto& item) { return item.isActive(); }) == notFound)
+    if (!hasActiveCache())
         clearMemoryRepresentation();
 }
 
@@ -594,7 +601,7 @@ void Caches::removeCacheEntry(const NetworkCache::Key& key)
 void Caches::clearMemoryRepresentation()
 {
     if (!m_isInitialized) {
-        ASSERT(m_caches.isEmpty() || !m_pendingInitializationCallbacks.isEmpty());
+        ASSERT(!m_storage || !hasActiveCache() || !m_pendingInitializationCallbacks.isEmpty());
         // m_storage might not be null in case Caches is being initialized. This is fine as nullify it below is a memory optimization.
         m_caches.clear();
         return;
index 489622c..8903aa0 100644 (file)
@@ -96,6 +96,8 @@ private:
 
     void notifyCachesOfRequestSpaceEnd();
 
+    bool hasActiveCache() const;
+
     bool m_isInitialized { false };
     bool m_isRequestingSpace { false };
     Engine* m_engine { nullptr };