Changeset 241565 in webkit

Timestamp:

Feb 14, 2019 2:55:24 PM (5 years ago)

Author:

youenn@apple.com

Message:

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:

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• NetworkProcess/cache/CacheStorageEngineCaches.cpp (modified) (3 diffs)
• NetworkProcess/cache/CacheStorageEngineCaches.h (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.cpp

@@ -344 +344 @@
 }
 
+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)
 {
@@ -357 +364 @@
     cache.clearMemoryRepresentation();
 
-    if (m_caches.findMatching([](const auto& item) { return item.isActive(); }) == notFound)
+    if (!hasActiveCache())
         clearMemoryRepresentation();
 }
@@ -595 +602 @@
 {
     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();

trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.h

@@ -97 +97 @@
     void notifyCachesOfRequestSpaceEnd();
 
+    bool hasActiveCache() const;
+
     bool m_isInitialized { false };
     bool m_isRequestingSpace { false };