Make NetworkCache blobs safe for mmap instead of not using blobs
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 26 Apr 2019 06:06:10 +0000 (06:06 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 26 Apr 2019 06:06:10 +0000 (06:06 +0000)
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:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@244678 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/ChangeLog
Source/WebKit/NetworkProcess/cache/NetworkCacheBlobStorage.cpp
Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystem.cpp
Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystem.h
Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystemCocoa.mm
Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.cpp
Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.h

index b6265ab..42dc4f4 100644 (file)
@@ -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
index 9538da6..bf9ff23 100644 (file)
@@ -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());
 
index 7f7b466..1cd67ac 100644 (file)
@@ -146,10 +146,6 @@ void updateFileModificationTimeIfNeeded(const String& path)
 }
 
 #if !PLATFORM(IOS_FAMILY)
-bool isSafeToUseMemoryMapForPath(const String&)
-{
-    return true;
-}
 void makeSafeToUseMemoryMapForPath(const String&)
 {
 }
index cd8995c..4470923 100644 (file)
@@ -42,7 +42,6 @@ struct FileTimes {
 FileTimes fileTimes(const String& path);
 void updateFileModificationTimeIfNeeded(const String& path);
 
-bool isSafeToUseMemoryMapForPath(const String&);
 void makeSafeToUseMemoryMapForPath(const String&);
 
 }
index 9f68fe8..812196d 100644 (file)
@@ -33,10 +33,8 @@ namespace WebKit {
 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) {
index 58be2ca..06a813d 100644 (file)
@@ -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;
 }
 
index 6eb3357..617b2b3 100644 (file)
@@ -169,7 +169,6 @@ private:
     
     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 };