Changeset 244678 in webkit

Timestamp:

Apr 25, 2019 11:06:10 PM (5 years ago)

Author:

commit-queue@webkit.org

Message:

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:

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• NetworkProcess/cache/NetworkCacheBlobStorage.cpp (modified) (1 diff)
• NetworkProcess/cache/NetworkCacheFileSystem.cpp (modified) (1 diff)
• NetworkProcess/cache/NetworkCacheFileSystem.h (modified) (1 diff)
• NetworkProcess/cache/NetworkCacheFileSystemCocoa.mm (modified) (1 diff)
• NetworkProcess/cache/NetworkCacheStorage.cpp (modified) (5 diffs)
• NetworkProcess/cache/NetworkCacheStorage.h (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheBlobStorage.cpp

@@ -94 +94 @@
         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());

trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystem.cpp

@@ -147 +147 @@
 
 #if !PLATFORM(IOS_FAMILY)
-bool isSafeToUseMemoryMapForPath(const String&)
-{
-    return true;
-}
 void makeSafeToUseMemoryMapForPath(const String&)
 {

trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystem.h

@@ -43 +43 @@
 void updateFileModificationTimeIfNeeded(const String& path);
 
-bool isSafeToUseMemoryMapForPath(const String&);
 void makeSafeToUseMemoryMapForPath(const String&);
 

trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystemCocoa.mm

@@ -34 +34 @@
 
 #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];

trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.cpp

@@ -229 +229 @@
     , m_mode(mode)
     , m_salt(salt)
-    , m_canUseBlobsForForBodyData(isSafeToUseMemoryMapForPath(baseDirectoryPath))
     , m_readOperationTimeoutTimer(*this, &Storage::cancelAllReadOperations)
     , m_writeOperationDispatchTimer(*this, &Storage::dispatchPendingWriteOperations)
@@ -296 +295 @@
 
         // 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;
@@ -319 +317 @@
             ++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();
@@ -378 +369 @@
 {
     ASSERT(RunLoop::isMain());
-    if (!m_canUseBlobsForForBodyData)
-        return false;
     return !m_blobFilter || m_blobFilter->mayContain(key.hash());
 }
@@ -795 +784 @@
 bool Storage::shouldStoreBodyAsBlob(const Data& bodyData)
 {
-    if (!m_canUseBlobsForForBodyData)
-        return false;
     return bodyData.size() > maximumInlineBodySize;
 }

trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.h

@@ -170 +170 @@
     const Mode m_mode;
     const Salt m_salt;
-    const bool m_canUseBlobsForForBodyData;
 
     size_t m_capacity { std::numeric_limits<size_t>::max() };