- Revision
- 237071
- Author
- [email protected]
- Date
- 2018-10-12 10:57:20 -0700 (Fri, 12 Oct 2018)
Log Message
Cache API tests are flaky due to file writing failing from time to time
https://bugs.webkit.org/show_bug.cgi?id=190321
Reviewed by Chris Dumez.
Make NetworkCache::Storage::store callback return an error in case of writing failure.
Use this to surface this error at Cache API level.
Minor clean-up to make Storage::clear take a completion handler.
Make also sure to create the folder before writing the file in CacheStorageEngine.
As can be seen from some logging, it does happen that writing the 'origin' file sometimes fail with Posix error 9,
which might mean the folder is not present.
Changes are covered by current tests, flaky tests like http/wpt/cache-storage/cache-put-keys.https.any.worker.html
will show a "failed writing data to the file system" error message.
* NetworkProcess/cache/CacheStorageEngine.cpp:
(WebKit::CacheStorage::Engine::writeFile):
* NetworkProcess/cache/CacheStorageEngine.h:
* NetworkProcess/cache/CacheStorageEngineCaches.cpp:
(WebKit::CacheStorage::Caches::storeOrigin):
(WebKit::CacheStorage::Caches::writeCachesToDisk):
(WebKit::CacheStorage::Caches::writeRecord):
* NetworkProcess/cache/NetworkCacheStorage.cpp:
(WebKit::NetworkCache::Storage::WriteOperation::WriteOperation):
(WebKit::NetworkCache::Storage::dispatchWriteOperation):
(WebKit::NetworkCache::Storage::finishWriteOperation):
(WebKit::NetworkCache::Storage::store):
(WebKit::NetworkCache::Storage::clear):
* NetworkProcess/cache/NetworkCacheStorage.h:
(WebKit::NetworkCache::Storage::store):
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (237070 => 237071)
--- trunk/Source/WebKit/ChangeLog 2018-10-12 17:14:12 UTC (rev 237070)
+++ trunk/Source/WebKit/ChangeLog 2018-10-12 17:57:20 UTC (rev 237071)
@@ -1,3 +1,38 @@
+2018-10-12 Youenn Fablet <[email protected]>
+
+ Cache API tests are flaky due to file writing failing from time to time
+ https://bugs.webkit.org/show_bug.cgi?id=190321
+
+ Reviewed by Chris Dumez.
+
+ Make NetworkCache::Storage::store callback return an error in case of writing failure.
+ Use this to surface this error at Cache API level.
+
+ Minor clean-up to make Storage::clear take a completion handler.
+
+ Make also sure to create the folder before writing the file in CacheStorageEngine.
+ As can be seen from some logging, it does happen that writing the 'origin' file sometimes fail with Posix error 9,
+ which might mean the folder is not present.
+
+ Changes are covered by current tests, flaky tests like http/wpt/cache-storage/cache-put-keys.https.any.worker.html
+ will show a "failed writing data to the file system" error message.
+
+ * NetworkProcess/cache/CacheStorageEngine.cpp:
+ (WebKit::CacheStorage::Engine::writeFile):
+ * NetworkProcess/cache/CacheStorageEngine.h:
+ * NetworkProcess/cache/CacheStorageEngineCaches.cpp:
+ (WebKit::CacheStorage::Caches::storeOrigin):
+ (WebKit::CacheStorage::Caches::writeCachesToDisk):
+ (WebKit::CacheStorage::Caches::writeRecord):
+ * NetworkProcess/cache/NetworkCacheStorage.cpp:
+ (WebKit::NetworkCache::Storage::WriteOperation::WriteOperation):
+ (WebKit::NetworkCache::Storage::dispatchWriteOperation):
+ (WebKit::NetworkCache::Storage::finishWriteOperation):
+ (WebKit::NetworkCache::Storage::store):
+ (WebKit::NetworkCache::Storage::clear):
+ * NetworkProcess/cache/NetworkCacheStorage.h:
+ (WebKit::NetworkCache::Storage::store):
+
2018-10-11 Youenn Fablet <[email protected]>
IOS 12 - Service worker cache not shared when added to homescreen
Modified: trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp (237070 => 237071)
--- trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp 2018-10-12 17:14:12 UTC (rev 237070)
+++ trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp 2018-10-12 17:57:20 UTC (rev 237071)
@@ -391,7 +391,12 @@
}
m_pendingWriteCallbacks.add(++m_pendingCallbacksCounter, WTFMove(callback));
- m_ioQueue->dispatch([this, weakThis = makeWeakPtr(this), identifier = m_pendingCallbacksCounter, data = "" filename = filename.isolatedCopy()] () mutable {
+ m_ioQueue->dispatch([this, weakThis = makeWeakPtr(this), identifier = m_pendingCallbacksCounter, data = "" filename = filename.isolatedCopy()]() mutable {
+
+ String directoryPath = WebCore::FileSystem::directoryName(filename);
+ if (!WebCore::FileSystem::fileExists(directoryPath))
+ WebCore::FileSystem::makeAllDirectories(directoryPath);
+
auto channel = IOChannel::open(filename, IOChannel::Type::Create);
channel->write(0, data, nullptr, [this, weakThis = WTFMove(weakThis), identifier](int error) mutable {
ASSERT(RunLoop::isMain());
Modified: trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.cpp (237070 => 237071)
--- trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.cpp 2018-10-12 17:14:12 UTC (rev 237070)
+++ trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.cpp 2018-10-12 17:57:20 UTC (rev 237071)
@@ -502,7 +502,12 @@
return;
}
- m_storage->store(Cache::encode(recordInformation, record), { }, [protectedStorage = makeRef(*m_storage), callback = WTFMove(callback)]() {
+ m_storage->store(Cache::encode(recordInformation, record), { }, [protectedStorage = makeRef(*m_storage), callback = WTFMove(callback)](int error) {
+ if (error) {
+ RELEASE_LOG_ERROR(CacheStorage, "Caches::writeRecord failed with error %d", error);
+ callback(Error::WriteDisk);
+ return;
+ }
callback(std::nullopt);
});
}
Modified: trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.cpp (237070 => 237071)
--- trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.cpp 2018-10-12 17:14:12 UTC (rev 237070)
+++ trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.cpp 2018-10-12 17:57:20 UTC (rev 237071)
@@ -106,7 +106,7 @@
struct Storage::WriteOperation {
WTF_MAKE_FAST_ALLOCATED;
public:
- WriteOperation(Storage& storage, const Record& record, MappedBodyHandler&& mappedBodyHandler, CompletionHandler<void()>&& completionHandler)
+ WriteOperation(Storage& storage, const Record& record, MappedBodyHandler&& mappedBodyHandler, CompletionHandler<void(int)>&& completionHandler)
: storage(storage)
, record(record)
, mappedBodyHandler(WTFMove(mappedBodyHandler))
@@ -116,7 +116,7 @@
const Record record;
const MappedBodyHandler mappedBodyHandler;
- CompletionHandler<void()> completionHandler;
+ CompletionHandler<void(int)> completionHandler;
std::atomic<unsigned> activeCount { 0 };
};
@@ -822,7 +822,7 @@
channel->write(0, recordData, nullptr, [this, &writeOperation, recordSize](int error) {
// On error the entry still stays in the contents filter until next synchronization.
m_approximateRecordsSize += recordSize;
- finishWriteOperation(writeOperation);
+ finishWriteOperation(writeOperation, error);
LOG(NetworkCacheStorage, "(NetworkProcess) write complete error=%d", error);
});
@@ -829,7 +829,7 @@
});
}
-void Storage::finishWriteOperation(WriteOperation& writeOperation)
+void Storage::finishWriteOperation(WriteOperation& writeOperation, int error)
{
ASSERT(RunLoop::isMain());
ASSERT(writeOperation.activeCount);
@@ -841,7 +841,7 @@
auto protectedThis = makeRef(*this);
if (writeOperation.completionHandler)
- writeOperation.completionHandler();
+ writeOperation.completionHandler(error);
m_activeWriteOperations.remove(&writeOperation);
dispatchPendingWriteOperations();
@@ -879,7 +879,7 @@
dispatchPendingReadOperations();
}
-void Storage::store(const Record& record, MappedBodyHandler&& mappedBodyHandler, CompletionHandler<void()>&& completionHandler)
+void Storage::store(const Record& record, MappedBodyHandler&& mappedBodyHandler, CompletionHandler<void(int)>&& completionHandler)
{
ASSERT(RunLoop::isMain());
ASSERT(!record.key.isNull());
Modified: trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.h (237070 => 237071)
--- trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.h 2018-10-12 17:14:12 UTC (rev 237070)
+++ trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.h 2018-10-12 17:57:20 UTC (rev 237071)
@@ -82,7 +82,7 @@
void retrieve(const Key&, unsigned priority, RetrieveCompletionHandler&&);
using MappedBodyHandler = Function<void (const Data& mappedBody)>;
- void store(const Record&, MappedBodyHandler&&, CompletionHandler<void()>&& = { });
+ void store(const Record&, MappedBodyHandler&&, CompletionHandler<void(int)>&& = { });
void remove(const Key&);
void remove(const Vector<Key>&, CompletionHandler<void()>&&);
@@ -144,7 +144,7 @@
struct WriteOperation;
void dispatchWriteOperation(std::unique_ptr<WriteOperation>);
void dispatchPendingWriteOperations();
- void finishWriteOperation(WriteOperation&);
+ void finishWriteOperation(WriteOperation&, int error = 0);
bool shouldStoreBodyAsBlob(const Data& bodyData);
std::optional<BlobStorage::Blob> storeBodyAsBlob(WriteOperation&);
