Changeset 252412 in webkit

Timestamp:

Nov 13, 2019, 9:31:49 AM (7 years ago)

Author:

youenn@apple.com

Message:

[ iOS ]: Layout Test http/tests/IndexedDB/storage-limit-1.https.html is a Flaky Failure
​https://bugs.webkit.org/show_bug.cgi?id=203275
<rdar://problem/56516249>

Reviewed by Alex Christensen.

Source/WebKit:

Fix flakiness by clearing the storage of each cache when the cache is being cleared.
This ensures that the storage salt gets recreated if needed.

To further improve repeatability, make sure that initialize based tasks happen after clear tasks are complete.
For that purpose, add a clear task counter and append initialize callbacks to a Vector if counter is not zero.
Increment counter at clear task creation and decrement counter at completion time.
If counter is back to 0, we can safely process the pending clear tasks.

Covered by unflaked test.

• NetworkProcess/cache/CacheStorageEngine.cpp:

(WebKit::CacheStorage::Engine::~Engine):
(WebKit::CacheStorage::Engine::initialize):
(WebKit::CacheStorage::CompletionHandler<void):
(WebKit::CacheStorage::Engine::clearAllCaches):
(WebKit::CacheStorage::Engine::clearCachesForOrigin):

• NetworkProcess/cache/CacheStorageEngine.h:
• NetworkProcess/cache/CacheStorageEngineCaches.cpp:

(WebKit::CacheStorage::Caches::clearMemoryRepresentation):

LayoutTests:

• platform/ios-wk2/TestExpectations:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/ios-wk2/TestExpectations (modified) (1 diff)
• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp (modified) (4 diffs)
• Source/WebKit/NetworkProcess/cache/CacheStorageEngine.h (modified) (2 diffs)
• Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2019-11-13  Youenn Fablet  <youenn@apple.com>
+
+        [ iOS ]: Layout Test http/tests/IndexedDB/storage-limit-1.https.html is a Flaky Failure
+        https://bugs.webkit.org/show_bug.cgi?id=203275
+        <rdar://problem/56516249>
+
+        Reviewed by Alex Christensen.
+
+        * platform/ios-wk2/TestExpectations:
+
 2019-11-13  Per Arne Vollan  <pvollan@apple.com>
 

trunk/LayoutTests/platform/ios-wk2/TestExpectations

@@ -1352 +1352 @@
 webkit.org/b/203264 [ Release ] editing/pasteboard/smart-paste-paragraph-004.html [ Pass Failure ]
 
-# <rdar://problem/56516249> REGRESSION (r250936?) [ iOS ]: Layout Test http/tests/IndexedDB/storage-limit-1.https.html is a Flaky Failure (203275)
-webkit.org/b/203275 http/tests/IndexedDB/storage-limit-1.https.html [ Pass Failure ]
-
 # <rdar://problem/56590026> [iOS] fast/forms/contenteditable- font-optical-size.html landed flaky (203371)
 webkit.org/b/203371 fast/forms/contenteditable-font-optical-size.html [ Pass Failure ]

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-11-13  Youenn Fablet  <youenn@apple.com>
+
+        [ iOS ]: Layout Test http/tests/IndexedDB/storage-limit-1.https.html is a Flaky Failure
+        https://bugs.webkit.org/show_bug.cgi?id=203275
+        <rdar://problem/56516249>
+
+        Reviewed by Alex Christensen.
+
+        Fix flakiness by clearing the storage of each cache when the cache is being cleared.
+        This ensures that the storage salt gets recreated if needed.
+
+        To further improve repeatability, make sure that initialize based tasks happen after clear tasks are complete.
+        For that purpose, add a clear task counter and append initialize callbacks to a Vector if counter is not zero.
+        Increment counter at clear task creation and decrement counter at completion time.
+        If counter is back to 0, we can safely process the pending clear tasks.
+
+        Covered by unflaked test.
+
+        * NetworkProcess/cache/CacheStorageEngine.cpp:
+        (WebKit::CacheStorage::Engine::~Engine):
+        (WebKit::CacheStorage::Engine::initialize):
+        (WebKit::CacheStorage::CompletionHandler<void):
+        (WebKit::CacheStorage::Engine::clearAllCaches):
+        (WebKit::CacheStorage::Engine::clearCachesForOrigin):
+        * NetworkProcess/cache/CacheStorageEngine.h:
+        * NetworkProcess/cache/CacheStorageEngineCaches.cpp:
+        (WebKit::CacheStorage::Caches::clearMemoryRepresentation):
+
 2019-11-13  Chris Dumez  <cdumez@apple.com>
 

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

@@ -63 +63 @@
         caches->detach();
 
+    auto pendingClearCallbacks = WTFMove(m_pendingClearCallbacks);
+    for (auto& callback : pendingClearCallbacks)
+        callback(Error::Internal);
+
     auto initializationCallbacks = WTFMove(m_initializationCallbacks);
     for (auto& callback : initializationCallbacks)
@@ -290 +294 @@
 void Engine::initialize(CompletionCallback&& callback)
 {
+    if (m_clearTaskCounter || !m_pendingClearCallbacks.isEmpty()) {
+        m_pendingClearCallbacks.append(WTFMove(callback));
+        return;
+    }
+
     if (m_salt) {
         callback(WTF::nullopt);
@@ -597 +606 @@
 }
 
+CompletionHandler<void()> Engine::createClearTask(CompletionHandler<void()>&& completionHandler)
+{
+    ++m_clearTaskCounter;
+    return [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)]() mutable {
+        completionHandler();
+        if (!--m_clearTaskCounter) {
+            auto callbacks = WTFMove(m_pendingClearCallbacks);
+            for (auto& callback : callbacks)
+                initialize(WTFMove(callback));
+        }
+    };
+}
+
 void Engine::clearAllCaches(CompletionHandler<void()>&& completionHandler)
 {
     ASSERT(RunLoop::isMain());
 
-    auto callbackAggregator = CallbackAggregator::create([this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)]() mutable {
+    auto callbackAggregator = CallbackAggregator::create([this, completionHandler = createClearTask(WTFMove(completionHandler))]() mutable {
         if (!this->shouldPersist())
             return completionHandler();
@@ -630 +652 @@
     ASSERT(RunLoop::isMain());
 
-    auto callbackAggregator = CallbackAggregator::create([this, protectedThis = makeRef(*this), origin, completionHandler = WTFMove(completionHandler)]() mutable {
+    auto callbackAggregator = CallbackAggregator::create([this, origin, completionHandler = createClearTask(WTFMove(completionHandler))]() mutable {
         if (!this->shouldPersist())
             return completionHandler();

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

@@ -133 +133 @@
     void readCache(uint64_t cacheIdentifier, CacheCallback&&);
 
+    CompletionHandler<void()> createClearTask(CompletionHandler<void()>&&);
+
     Cache* cache(uint64_t cacheIdentifier);
 
@@ -147 +149 @@
     HashMap<uint64_t, CompletionHandler<void(const NetworkCache::Data&, int error)>> m_pendingReadCallbacks;
     uint64_t m_pendingCallbacksCounter { 0 };
+    Vector<WebCore::DOMCacheEngine::CompletionCallback> m_pendingClearCallbacks;
+    uint64_t m_clearTaskCounter { 0 };
 };
 

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

@@ -635 +635 @@
 void Caches::clearMemoryRepresentation()
 {
-    if (!m_isInitialized) {
-        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;
-    }
-
     makeDirty();
     m_caches.clear();