Make NetworkCache blobs safe for mmap instead of not using blobs

https://bugs.webkit.org/show_bug.cgi?id=197264 <rdar://problem/49564348> Patch by Alex Christensen <achristensen@webkit.org> on 2019-04-25 Reviewed by Antti Koivisto. This does what r244597 did for WKContentRuleLists but for the NetworkCache's blobs. Those are the two cases where we were calling mmap and seeing crashes in apps with default file protection of NSFileProtectionComplete. * NetworkProcess/cache/NetworkCacheBlobStorage.cpp: (WebKit::NetworkCache::BlobStorage::add): * NetworkProcess/cache/NetworkCacheFileSystem.cpp: (WebKit::NetworkCache::isSafeToUseMemoryMapForPath): Deleted. * NetworkProcess/cache/NetworkCacheFileSystem.h: * NetworkProcess/cache/NetworkCacheFileSystemCocoa.mm: (WebKit::NetworkCache::isSafeToUseMemoryMapForPath): * NetworkProcess/cache/NetworkCacheStorage.cpp: (WebKit::NetworkCache::Storage::Storage): (WebKit::NetworkCache::Storage::synchronize): (WebKit::NetworkCache::Storage::mayContainBlob const): (WebKit::NetworkCache::Storage::shouldStoreBodyAsBlob): (WebKit::NetworkCache::estimateRecordsSize): Deleted. * NetworkProcess/cache/NetworkCacheStorage.h: Canonical link: https://commits.webkit.org/211517@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@244678 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Apr 26, 2019 · 676ca21 · 676ca21
1 parent 0e0b72a
commit 676ca21
Show file tree

Hide file tree

Showing 7 changed files with 33 additions and 24 deletions.
diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
@@ -1,3 +1,30 @@
+2019-04-25  Alex Christensen  <achristensen@webkit.org>
+
+        Make NetworkCache blobs safe for mmap instead of not using blobs
+        https://bugs.webkit.org/show_bug.cgi?id=197264
+        <rdar://problem/49564348>
+
+        Reviewed by Antti Koivisto.
+
+        This does what r244597 did for WKContentRuleLists but for the NetworkCache's blobs.
+        Those are the two cases where we were calling mmap and seeing crashes in apps with
+        default file protection of NSFileProtectionComplete.
+
+        * NetworkProcess/cache/NetworkCacheBlobStorage.cpp:
+        (WebKit::NetworkCache::BlobStorage::add):
+        * NetworkProcess/cache/NetworkCacheFileSystem.cpp:
+        (WebKit::NetworkCache::isSafeToUseMemoryMapForPath): Deleted.
+        * NetworkProcess/cache/NetworkCacheFileSystem.h:
+        * NetworkProcess/cache/NetworkCacheFileSystemCocoa.mm:
+        (WebKit::NetworkCache::isSafeToUseMemoryMapForPath):
+        * NetworkProcess/cache/NetworkCacheStorage.cpp:
+        (WebKit::NetworkCache::Storage::Storage):
+        (WebKit::NetworkCache::Storage::synchronize):
+        (WebKit::NetworkCache::Storage::mayContainBlob const):
+        (WebKit::NetworkCache::Storage::shouldStoreBodyAsBlob):
+        (WebKit::NetworkCache::estimateRecordsSize): Deleted.
+        * NetworkProcess/cache/NetworkCacheStorage.h:
+
 2019-04-25  Simon Fraser  <simon.fraser@apple.com>
 
         REGRESSION (r234330): 3 legacy-animation-engine/compositing tests are flaky failures

diff --git a/Source/WebKit/NetworkProcess/cache/NetworkCacheBlobStorage.cpp b/Source/WebKit/NetworkProcess/cache/NetworkCacheBlobStorage.cpp
@@ -93,7 +93,10 @@ BlobStorage::Blob BlobStorage::add(const String& path, const Data& data)
     if (data.isEmpty())
         return { data, hash };
 
-    auto blobPath = FileSystem::fileSystemRepresentation(blobPathForHash(hash));
+    String blobPathString = blobPathForHash(hash);
+    makeSafeToUseMemoryMapForPath(blobPathString);
+
+    auto blobPath = FileSystem::fileSystemRepresentation(blobPathString);
     auto linkPath = FileSystem::fileSystemRepresentation(path);
     unlink(linkPath.data());
 

diff --git a/Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystem.cpp b/Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystem.cpp
@@ -146,10 +146,6 @@ void updateFileModificationTimeIfNeeded(const String& path)
 }
 
 #if !PLATFORM(IOS_FAMILY)
-bool isSafeToUseMemoryMapForPath(const String&)
-{
-    return true;
-}
 void makeSafeToUseMemoryMapForPath(const String&)
 {
 }

diff --git a/Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystem.h b/Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystem.h
@@ -42,7 +42,6 @@ struct FileTimes {
 FileTimes fileTimes(const String& path);
 void updateFileModificationTimeIfNeeded(const String& path);
 
-bool isSafeToUseMemoryMapForPath(const String&);
 void makeSafeToUseMemoryMapForPath(const String&);
 
 }

diff --git a/Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystemCocoa.mm b/Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystemCocoa.mm
@@ -33,10 +33,8 @@
 namespace NetworkCache {
 
 #if PLATFORM(IOS_FAMILY)
-bool isSafeToUseMemoryMapForPath(const String& path)
+static bool isSafeToUseMemoryMapForPath(const String& path)
 {
-    // FIXME: For the network cache we should either use makeSafeToUseMemoryMapForPath instead of this
-    // or we should listen to UIApplicationProtectedDataWillBecomeUnavailable and unmap files.
     NSError *error = nil;
     NSDictionary<NSFileAttributeKey, id> *attributes = [[NSFileManager defaultManager] attributesOfItemAtPath:path error:&error];
     if (error) {

diff --git a/Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.cpp b/Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.cpp
@@ -228,7 +228,6 @@ Storage::Storage(const String& baseDirectoryPath, Mode mode, Salt salt)
     , m_recordsPath(makeRecordsDirectoryPath(baseDirectoryPath))
     , m_mode(mode)
     , m_salt(salt)
-    , m_canUseBlobsForForBodyData(isSafeToUseMemoryMapForPath(baseDirectoryPath))
     , m_readOperationTimeoutTimer(*this, &Storage::cancelAllReadOperations)
     , m_writeOperationDispatchTimer(*this, &Storage::dispatchPendingWriteOperations)
     , m_ioQueue(WorkQueue::create("com.apple.WebKit.Cache.Storage", WorkQueue::Type::Concurrent))
@@ -295,7 +294,6 @@ void Storage::synchronize()
         auto blobFilter = std::make_unique<ContentsFilter>();
 
         // Most of the disk space usage is in blobs if we are using them. Approximate records file sizes to avoid expensive stat() calls.
-        bool shouldComputeExactRecordsSize = !m_canUseBlobsForForBodyData;
         size_t recordsSize = 0;
         unsigned recordCount = 0;
         unsigned blobCount = 0;
@@ -318,17 +316,10 @@ void Storage::synchronize()
 
             ++recordCount;
 
-            if (shouldComputeExactRecordsSize) {
-                long long fileSize = 0;
-                FileSystem::getFileSize(filePath, fileSize);
-                recordsSize += fileSize;
-            }
-
             recordFilter->add(hash);
         });
 
-        if (!shouldComputeExactRecordsSize)
-            recordsSize = estimateRecordsSize(recordCount, blobCount);
+        recordsSize = estimateRecordsSize(recordCount, blobCount);
 
         m_blobStorage.synchronize();
 
@@ -377,8 +368,6 @@ bool Storage::mayContain(const Key& key) const
 bool Storage::mayContainBlob(const Key& key) const
 {
     ASSERT(RunLoop::isMain());
-    if (!m_canUseBlobsForForBodyData)
-        return false;
     return !m_blobFilter || m_blobFilter->mayContain(key.hash());
 }
 
@@ -794,8 +783,6 @@ void Storage::dispatchPendingWriteOperations()
 
 bool Storage::shouldStoreBodyAsBlob(const Data& bodyData)
 {
-    if (!m_canUseBlobsForForBodyData)
-        return false;
     return bodyData.size() > maximumInlineBodySize;
 }
 

diff --git a/Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.h b/Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.h
@@ -169,7 +169,6 @@ class Storage : public ThreadSafeRefCounted<Storage, WTF::DestructionThread::Mai
 
     const Mode m_mode;
     const Salt m_salt;
-    const bool m_canUseBlobsForForBodyData;
 
     size_t m_capacity { std::numeric_limits<size_t>::max() };
     size_t m_approximateRecordsSize { 0 };