Skip to content
Permalink
Browse files
LayoutTest imported/w3c/web-platform-tests/service-workers/service-wo…
…rker/fetch-event-within-sw.https.html is a flaky failure

https://bugs.webkit.org/show_bug.cgi?id=179248
<rdar://problem/35377756>

Patch by Youenn Fablet <youenn@apple.com> on 2018-03-01
Reviewed by Chris Dumez.

Source/WebKit:

WebKitTestRunner is clearing caches for every test but there might still be some on-going cache activity due to a previous test.
In that case, the activity might try to open the Caches object at the same time the files are deleted by the clearing task.
If the new test is trying to open the same caches, it will also receive the same error, hence the console log message.

To fix that issue, we clear the initialization pending callbacks when clearing the caches.
This prevents the new test to receive the error since the new test should only start some cache activity after the cache clear task is done.
Made refactoring to append the first callback into the list of pending callbacks.

* NetworkProcess/cache/CacheStorageEngineCaches.cpp:
(WebKit::CacheStorage::Caches::initialize):
(WebKit::CacheStorage::Caches::initializeSize):
(WebKit::CacheStorage::Caches::clear):
* NetworkProcess/cache/CacheStorageEngineCaches.h:

LayoutTests:

* TestExpectations:

Canonical link: https://commits.webkit.org/198938@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@229151 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
youennf authored and webkit-commit-queue committed Mar 1, 2018
1 parent 26b3012 commit 5f37455c7ae3247b8f53a2f6f67a726c6fbf9a0b
@@ -1,3 +1,13 @@
2018-03-01 Youenn Fablet <youenn@apple.com>

LayoutTest imported/w3c/web-platform-tests/service-workers/service-worker/fetch-event-within-sw.https.html is a flaky failure
https://bugs.webkit.org/show_bug.cgi?id=179248
<rdar://problem/35377756>

Reviewed by Chris Dumez.

* TestExpectations:

2018-03-01 Youenn Fablet <youenn@apple.com>

Add API test to validate setting of service worker and cache storage directories
@@ -176,7 +176,6 @@ imported/w3c/web-platform-tests/service-workers/service-worker/navigation-redire
# Different failure string each test run due to a random number being dumped in test output
imported/w3c/web-platform-tests/service-workers/service-worker/fetch-request-css-images.https.html [ Skip ]

webkit.org/b/179248 imported/w3c/web-platform-tests/service-workers/service-worker/fetch-event-within-sw.https.html [ Pass Failure ]
imported/w3c/web-platform-tests/service-workers/service-worker/websocket.https.html [ Pass Failure ]

webkit.org/b/181901 imported/w3c/web-platform-tests/service-workers/service-worker/fetch-cors-xhr.https.html [ DumpJSConsoleLogInStdErr Pass Failure ]
@@ -1,3 +1,25 @@
2018-03-01 Youenn Fablet <youenn@apple.com>

LayoutTest imported/w3c/web-platform-tests/service-workers/service-worker/fetch-event-within-sw.https.html is a flaky failure
https://bugs.webkit.org/show_bug.cgi?id=179248
<rdar://problem/35377756>

Reviewed by Chris Dumez.

WebKitTestRunner is clearing caches for every test but there might still be some on-going cache activity due to a previous test.
In that case, the activity might try to open the Caches object at the same time the files are deleted by the clearing task.
If the new test is trying to open the same caches, it will also receive the same error, hence the console log message.

To fix that issue, we clear the initialization pending callbacks when clearing the caches.
This prevents the new test to receive the error since the new test should only start some cache activity after the cache clear task is done.
Made refactoring to append the first callback into the list of pending callbacks.

* NetworkProcess/cache/CacheStorageEngineCaches.cpp:
(WebKit::CacheStorage::Caches::initialize):
(WebKit::CacheStorage::Caches::initializeSize):
(WebKit::CacheStorage::Caches::clear):
* NetworkProcess/cache/CacheStorageEngineCaches.h:

2018-03-01 Youenn Fablet <youenn@apple.com>

Add API test to validate setting of service worker and cache storage directories
@@ -144,13 +144,13 @@ void Caches::initialize(WebCore::DOMCacheEngine::CompletionCallback&& callback)
callback(Error::WriteDisk);
return;
}

m_pendingInitializationCallbacks.append(WTFMove(callback));
m_storage = storage.releaseNonNull();
m_storage->writeWithoutWaiting();

storeOrigin([this, callback = WTFMove(callback)] (std::optional<Error>&& error) mutable {
storeOrigin([this] (std::optional<Error>&& error) mutable {
if (error) {
callback(Error::WriteDisk);

auto pendingCallbacks = WTFMove(m_pendingInitializationCallbacks);
for (auto& callback : pendingCallbacks)
callback(Error::WriteDisk);
@@ -159,12 +159,10 @@ void Caches::initialize(WebCore::DOMCacheEngine::CompletionCallback&& callback)
return;
}

readCachesFromDisk([this, callback = WTFMove(callback)](Expected<Vector<Cache>, Error>&& result) mutable {
readCachesFromDisk([this](Expected<Vector<Cache>, Error>&& result) mutable {
makeDirty();

if (!result.has_value()) {
callback(result.error());

auto pendingCallbacks = WTFMove(m_pendingInitializationCallbacks);
for (auto& callback : pendingCallbacks)
callback(result.error());
@@ -174,29 +172,25 @@ void Caches::initialize(WebCore::DOMCacheEngine::CompletionCallback&& callback)
}
m_caches = WTFMove(result.value());

initializeSize(WTFMove(callback));
initializeSize();
});
});
}

void Caches::initializeSize(WebCore::DOMCacheEngine::CompletionCallback&& callback)
void Caches::initializeSize()
{
if (!m_storage) {
callback(Error::Internal);

auto pendingCallbacks = WTFMove(m_pendingInitializationCallbacks);
for (auto& callback : pendingCallbacks)
callback(Error::Internal);
return;
}

uint64_t size = 0;
m_storage->traverse({ }, 0, [protectedThis = makeRef(*this), this, protectedStorage = makeRef(*m_storage), callback = WTFMove(callback), size](const auto* storage, const auto& information) mutable {
m_storage->traverse({ }, 0, [protectedThis = makeRef(*this), this, protectedStorage = makeRef(*m_storage), size](const auto* storage, const auto& information) mutable {
if (!storage) {
m_size = size;
m_isInitialized = true;
callback(std::nullopt);

auto pendingCallbacks = WTFMove(m_pendingInitializationCallbacks);
for (auto& callback : pendingCallbacks)
callback(std::nullopt);
@@ -225,6 +219,10 @@ void Caches::clear(CompletionHandler<void()>&& completionHandler)
return;
}

auto pendingCallbacks = WTFMove(m_pendingInitializationCallbacks);
for (auto& callback : pendingCallbacks)
callback(Error::Internal);

if (m_engine)
m_engine->removeFile(cachesListFilename(m_rootPath));
if (m_storage) {
@@ -80,7 +80,7 @@ class Caches : public RefCounted<Caches> {
private:
Caches(Engine&, WebCore::ClientOrigin&&, String&& rootPath, uint64_t quota);

void initializeSize(WebCore::DOMCacheEngine::CompletionCallback&&);
void initializeSize();
void readCachesFromDisk(WTF::Function<void(Expected<Vector<Cache>, WebCore::DOMCacheEngine::Error>&&)>&&);
void writeCachesToDisk(WebCore::DOMCacheEngine::CompletionCallback&&);

0 comments on commit 5f37455

Please sign in to comment.