Diff
Modified: trunk/LayoutTests/ChangeLog (227767 => 227768)
--- trunk/LayoutTests/ChangeLog 2018-01-30 01:38:23 UTC (rev 227767)
+++ trunk/LayoutTests/ChangeLog 2018-01-30 01:41:22 UTC (rev 227768)
@@ -1,3 +1,14 @@
+2018-01-29 Youenn Fablet <[email protected]>
+
+ Cache API should make sure to resolve caches.open promises in the same order as called
+ https://bugs.webkit.org/show_bug.cgi?id=182193
+ <rdar://problem/36930363>
+
+ Reviewed by Chris Dumez.
+
+ * http/wpt/cache-storage/cache-open.https-expected.txt: Added.
+ * http/wpt/cache-storage/cache-open.https.html: Added.
+
2018-01-29 Matt Lewis <[email protected]>
Marked imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/module/errorhandling.html as flaky.
Added: trunk/LayoutTests/http/wpt/cache-storage/cache-open-delete-in-parallel.https-expected.txt (0 => 227768)
--- trunk/LayoutTests/http/wpt/cache-storage/cache-open-delete-in-parallel.https-expected.txt (rev 0)
+++ trunk/LayoutTests/http/wpt/cache-storage/cache-open-delete-in-parallel.https-expected.txt 2018-01-30 01:41:22 UTC (rev 227768)
@@ -0,0 +1,4 @@
+
+PASS Testing ordering of opening cache promises resolution
+PASS Testing ordering of deleting cache promises resolution
+
Added: trunk/LayoutTests/http/wpt/cache-storage/cache-open-delete-in-parallel.https.html (0 => 227768)
--- trunk/LayoutTests/http/wpt/cache-storage/cache-open-delete-in-parallel.https.html (rev 0)
+++ trunk/LayoutTests/http/wpt/cache-storage/cache-open-delete-in-parallel.https.html 2018-01-30 01:41:22 UTC (rev 227768)
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>Cache Storage: testing open and delete in parallel</title>
+<script src=""
+<script src=""
+</head>
+<body>
+<script>
+promise_test(async test => {
+ await caches.delete("test");
+
+ var result = "FAIL";
+ caches.open("test").then(()=> result = "PASS");
+ await caches.open("test");
+
+ assert_equals(result, "PASS", "Open resolution order");
+}, "Testing ordering of opening cache promises resolution");
+
+promise_test(async test => {
+ await caches.open("test");
+
+ var result = "FAIL";
+ caches.delete("test").then((value)=> {
+ result = value ? "PASS" : "FAIL: delete did not return true";
+ });
+ assert_false(await caches.delete("test"));
+
+ assert_equals(result, "PASS", "Delete resolution order");
+}, "Testing ordering of deleting cache promises resolution");
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (227767 => 227768)
--- trunk/Source/WebCore/ChangeLog 2018-01-30 01:38:23 UTC (rev 227767)
+++ trunk/Source/WebCore/ChangeLog 2018-01-30 01:41:22 UTC (rev 227768)
@@ -1,3 +1,17 @@
+2018-01-29 Youenn Fablet <[email protected]>
+
+ Cache API should make sure to resolve caches.open promises in the same order as called
+ https://bugs.webkit.org/show_bug.cgi?id=182193
+ <rdar://problem/36930363>
+
+ Reviewed by Chris Dumez.
+
+ Covered by LayoutTests/http/wpt/cache-storage/cache-open-delete-in-parallel.https.html.
+
+ * Modules/cache/DOMCacheStorage.cpp:
+ (WebCore::DOMCacheStorage::doRemove): Removed optimization consisting in removing the cache from DOMCacheStorage object synchronously.
+ This optimization prevents going to the network process to try deleting the cache.
+
2018-01-29 Jiewen Tan <[email protected]>
[WebAuthN] Add a compile-time feature flag
Modified: trunk/Source/WebCore/Modules/cache/DOMCacheStorage.cpp (227767 => 227768)
--- trunk/Source/WebCore/Modules/cache/DOMCacheStorage.cpp 2018-01-30 01:38:23 UTC (rev 227767)
+++ trunk/Source/WebCore/Modules/cache/DOMCacheStorage.cpp 2018-01-30 01:41:22 UTC (rev 227768)
@@ -233,11 +233,10 @@
else {
if (result.value().hadStorageError)
logConsolePersistencyError(scriptExecutionContext(), name);
- promise.resolve(true);
+ promise.resolve(!!result.value().identifier);
}
}
});
- m_caches.remove(position);
}
void DOMCacheStorage::keys(KeysPromise&& promise)
Modified: trunk/Source/WebKit/ChangeLog (227767 => 227768)
--- trunk/Source/WebKit/ChangeLog 2018-01-30 01:38:23 UTC (rev 227767)
+++ trunk/Source/WebKit/ChangeLog 2018-01-30 01:41:22 UTC (rev 227768)
@@ -1,3 +1,30 @@
+2018-01-29 Youenn Fablet <[email protected]>
+
+ Cache API should make sure to resolve caches.open promises in the same order as called
+ https://bugs.webkit.org/show_bug.cgi?id=182193
+ <rdar://problem/36930363>
+
+ Reviewed by Chris Dumez.
+
+ Covered by added test.
+ Whenever opening/removing a cache requires writing to disk, wait to finish the task
+ until any disk writing task is done.
+ Applying this strategy when clearing data so that we also clear data that is pending to be written.
+ For removing cache, we now return whether a cache was actually deleted by returning zero as removed cache identifier.
+ WebCore uses that information to return true/false as promise resolution value.
+
+ * NetworkProcess/cache/CacheStorageEngine.cpp:
+ (WebKit::CacheStorage::Engine::retrieveCaches):
+ * NetworkProcess/cache/CacheStorageEngineCaches.cpp:
+ (WebKit::CacheStorage::Caches::clear):
+ (WebKit::CacheStorage::Caches::open):
+ (WebKit::CacheStorage::Caches::remove):
+ (WebKit::CacheStorage::Caches::writeCachesToDisk):
+ (WebKit::CacheStorage::Caches::cacheInfos):
+ (WebKit::CacheStorage::Caches::cacheInfos const): Deleted.
+ * NetworkProcess/cache/CacheStorageEngineCaches.h:
+ (WebKit::CacheStorage::Caches::createWeakPtr):
+
2018-01-29 Alex Christensen <[email protected]>
Clean up API after bugs 178240 and 176474
Modified: trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp (227767 => 227768)
--- trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp 2018-01-30 01:38:23 UTC (rev 227767)
+++ trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp 2018-01-30 01:41:22 UTC (rev 227768)
@@ -138,7 +138,7 @@
return;
}
- callback(cachesOrError.value().get().cacheInfos(updateCounter));
+ cachesOrError.value().get().cacheInfos(updateCounter, WTFMove(callback));
});
}
Modified: trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.cpp (227767 => 227768)
--- trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.cpp 2018-01-30 01:38:23 UTC (rev 227767)
+++ trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.cpp 2018-01-30 01:41:22 UTC (rev 227768)
@@ -50,6 +50,11 @@
return WebCore::FileSystem::pathByAppendingComponent(cachesRootPath, ASCIILiteral("origin"));
}
+Caches::~Caches()
+{
+ ASSERT(m_pendingWritingCachesToDiskCallbacks.isEmpty());
+}
+
void Caches::retrieveOriginFromDirectory(const String& folderPath, WorkQueue& queue, WTF::CompletionHandler<void(std::optional<WebCore::ClientOrigin>&&)>&& completionHandler)
{
queue.dispatch([completionHandler = WTFMove(completionHandler), folderPath = folderPath.isolatedCopy()]() mutable {
@@ -196,10 +201,18 @@
{
m_engine = nullptr;
m_rootPath = { };
+ clearPendingWritingCachesToDiskCallbacks();
}
void Caches::clear(CompletionHandler<void()>&& completionHandler)
{
+ if (m_isWritingCachesToDisk) {
+ m_pendingWritingCachesToDiskCallbacks.append([this, completionHandler = WTFMove(completionHandler)] (auto&& error) mutable {
+ this->clear(WTFMove(completionHandler));
+ });
+ return;
+ }
+
if (m_engine)
m_engine->removeFile(cachesListFilename(m_rootPath));
if (m_storage) {
@@ -211,9 +224,17 @@
return;
}
clearMemoryRepresentation();
+ clearPendingWritingCachesToDiskCallbacks();
completionHandler();
}
+void Caches::clearPendingWritingCachesToDiskCallbacks()
+{
+ auto pendingWritingCachesToDiskCallbacks = WTFMove(m_pendingWritingCachesToDiskCallbacks);
+ for (auto& callback : pendingWritingCachesToDiskCallbacks)
+ callback(Error::Internal);
+}
+
Cache* Caches::find(const String& name)
{
auto position = m_caches.findMatching([&](const auto& item) { return item.name() == name; });
@@ -235,6 +256,17 @@
ASSERT(m_isInitialized);
ASSERT(m_engine);
+ if (m_isWritingCachesToDisk) {
+ m_pendingWritingCachesToDiskCallbacks.append([this, name, callback = WTFMove(callback)] (auto&& error) mutable {
+ if (error) {
+ callback(makeUnexpected(error.value()));
+ return;
+ }
+ this->open(name, WTFMove(callback));
+ });
+ return;
+ }
+
if (auto* cache = find(name)) {
cache->open([cacheIdentifier = cache->identifier(), callback = WTFMove(callback)](std::optional<Error>&& error) mutable {
if (error) {
@@ -261,11 +293,22 @@
ASSERT(m_isInitialized);
ASSERT(m_engine);
+ if (m_isWritingCachesToDisk) {
+ m_pendingWritingCachesToDiskCallbacks.append([this, identifier, callback = WTFMove(callback)] (auto&& error) mutable {
+ if (error) {
+ callback(makeUnexpected(error.value()));
+ return;
+ }
+ this->remove(identifier, WTFMove(callback));
+ });
+ return;
+ }
+
auto position = m_caches.findMatching([&](const auto& item) { return item.identifier() == identifier; });
if (position == notFound) {
ASSERT(m_removedCaches.findMatching([&](const auto& item) { return item.identifier() == identifier; }) != notFound);
- callback(CacheIdentifierOperationResult { identifier, false });
+ callback(CacheIdentifierOperationResult { 0, false });
return;
}
@@ -371,6 +414,7 @@
void Caches::writeCachesToDisk(CompletionCallback&& callback)
{
+ ASSERT(!m_isWritingCachesToDisk);
ASSERT(m_isInitialized);
if (!shouldPersist()) {
callback(std::nullopt);
@@ -385,8 +429,12 @@
return;
}
- m_engine->writeFile(cachesListFilename(m_rootPath), encodeCacheNames(m_caches), [callback = WTFMove(callback)](std::optional<Error>&& error) mutable {
+ m_isWritingCachesToDisk = true;
+ m_engine->writeFile(cachesListFilename(m_rootPath), encodeCacheNames(m_caches), [this, protectedThis = makeRef(*this), callback = WTFMove(callback)](std::optional<Error>&& error) mutable {
+ m_isWritingCachesToDisk = false;
callback(WTFMove(error));
+ while (!m_pendingWritingCachesToDiskCallbacks.isEmpty() && !m_isWritingCachesToDisk)
+ m_pendingWritingCachesToDiskCallbacks.takeFirst()(std::nullopt);
});
}
@@ -515,8 +563,19 @@
return m_volatileSalt.value();
}
-CacheInfos Caches::cacheInfos(uint64_t updateCounter) const
+void Caches::cacheInfos(uint64_t updateCounter, CacheInfosCallback&& callback)
{
+ if (m_isWritingCachesToDisk) {
+ m_pendingWritingCachesToDiskCallbacks.append([this, updateCounter, callback = WTFMove(callback)] (auto&& error) mutable {
+ if (error) {
+ callback(makeUnexpected(error.value()));
+ return;
+ }
+ this->cacheInfos(updateCounter, WTFMove(callback));
+ });
+ return;
+ }
+
Vector<CacheInfo> cacheInfos;
if (isDirty(updateCounter)) {
cacheInfos.reserveInitialCapacity(m_caches.size());
@@ -523,7 +582,7 @@
for (auto& cache : m_caches)
cacheInfos.uncheckedAppend(CacheInfo { cache.identifier(), cache.name() });
}
- return { WTFMove(cacheInfos), m_updateCounter };
+ callback(CacheInfos { WTFMove(cacheInfos), m_updateCounter });
}
void Caches::appendRepresentation(StringBuilder& builder) const
Modified: trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.h (227767 => 227768)
--- trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.h 2018-01-30 01:38:23 UTC (rev 227767)
+++ trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.h 2018-01-30 01:41:22 UTC (rev 227768)
@@ -29,6 +29,7 @@
#include "NetworkCacheStorage.h"
#include <WebCore/ClientOrigin.h>
#include <wtf/CompletionHandler.h>
+#include <wtf/Deque.h>
namespace WebKit {
@@ -39,6 +40,7 @@
class Caches : public RefCounted<Caches> {
public:
static Ref<Caches> create(Engine& engine, WebCore::ClientOrigin&& origin, String&& rootPath, uint64_t quota) { return adoptRef(*new Caches { engine, WTFMove(origin), WTFMove(rootPath), quota }); }
+ ~Caches();
static void retrieveOriginFromDirectory(const String& folderPath, WorkQueue&, WTF::CompletionHandler<void(std::optional<WebCore::ClientOrigin>&&)>&&);
@@ -50,7 +52,7 @@
void detach();
bool isInitialized() const { return m_isInitialized; }
- WebCore::DOMCacheEngine::CacheInfos cacheInfos(uint64_t updateCounter) const;
+ void cacheInfos(uint64_t updateCounter, WebCore::DOMCacheEngine::CacheInfosCallback&&);
Cache* find(uint64_t identifier);
void appendRepresentation(StringBuilder&) const;
@@ -86,6 +88,7 @@
static std::optional<WebCore::ClientOrigin> readOrigin(const NetworkCache::Data&);
Cache* find(const String& name);
+ void clearPendingWritingCachesToDiskCallbacks();
void makeDirty() { ++m_updateCounter; }
bool isDirty(uint64_t updateCounter) const;
@@ -103,6 +106,8 @@
HashMap<NetworkCache::Key, WebCore::DOMCacheEngine::Record> m_volatileStorage;
mutable std::optional<NetworkCache::Salt> m_volatileSalt;
Vector<WebCore::DOMCacheEngine::CompletionCallback> m_pendingInitializationCallbacks;
+ bool m_isWritingCachesToDisk { false };
+ Deque<CompletionHandler<void(std::optional<WebCore::DOMCacheEngine::Error>)>> m_pendingWritingCachesToDiskCallbacks;
};
} // namespace CacheStorage
