Attachment 361988 Details for Bug 188393 – Patch

[patch] Patch

bug-188393-20190213204004.patch (text/plain), 3.78 KB, created by youenn fablet on 2019-02-13 20:40:05 PST

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2019-02-13 20:40:05 PST

Size: 3.78 KB

patch

obsolete

>Subversion Revision: 241453
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 4b97c0ff8b8730e9f207b7f7eaf1cfb25d422a51..b1ffc01c1fda163c0a238a81b2365c1fab46ef6b 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,24 @@
>+2019-02-13  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 NOBODY (OOPS!).
>+
>+        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-13  Michael Catanzaro  <mcatanzaro@igalia.com>
> 
>         Fix -Wformat warning from r241401
>diff --git a/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.cpp b/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.cpp
>index 1f608cf525f996b24ea4113600e8e29cb287c27c..474cf1c0df71a9ff2b13c3ddde08955c9620d5c6 100644
>--- a/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.cpp
>+++ b/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.cpp
>@@ -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;
>diff --git a/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.h b/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.h
>index 489622ceed13bc5e83b5b91be824ab088eb614f6..8903aa0cfa03314d5b985100ad229ab5cd798878 100644
>--- a/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.h
>+++ b/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.h
>@@ -96,6 +96,8 @@ private:
> 
>     void notifyCachesOfRequestSpaceEnd();
> 
>+    bool hasActiveCache() const;
>+
>     bool m_isInitialized { false };
>     bool m_isRequestingSpace { false };
>     Engine* m_engine { nullptr };

Actions: View | Formatted Diff | Diff

Attachments on bug 188393: 346741 | 361982 | 361988