Attachment 368243 Details for Bug 197264 – Patch

[patch] Patch

bug-197264-20190425102220.patch (text/plain), 7.10 KB, created by Alex Christensen on 2019-04-25 10:22:21 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Alex Christensen

Created: 2019-04-25 10:22:21 PDT

Size: 7.10 KB

patch

obsolete

>Index: Source/WebKit/ChangeLog
>===================================================================
>--- Source/WebKit/ChangeLog	(revision 244629)
>+++ Source/WebKit/ChangeLog	(working copy)
>@@ -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-24  Commit Queue  <commit-queue@webkit.org>
> 
>         Unreviewed, rolling out r244228.
>Index: Source/WebKit/NetworkProcess/cache/NetworkCacheBlobStorage.cpp
>===================================================================
>--- Source/WebKit/NetworkProcess/cache/NetworkCacheBlobStorage.cpp	(revision 244596)
>+++ Source/WebKit/NetworkProcess/cache/NetworkCacheBlobStorage.cpp	(working copy)
>@@ -93,7 +93,10 @@ BlobStorage::Blob BlobStorage::add(const
>     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: Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystem.cpp
>===================================================================
>--- Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystem.cpp	(revision 244597)
>+++ Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystem.cpp	(working copy)
>@@ -146,10 +146,6 @@ void updateFileModificationTimeIfNeeded(
> }
> 
> #if !PLATFORM(IOS_FAMILY)
>-bool isSafeToUseMemoryMapForPath(const String&)
>-{
>-    return true;
>-}
> void makeSafeToUseMemoryMapForPath(const String&)
> {
> }
>Index: Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystem.h
>===================================================================
>--- Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystem.h	(revision 244597)
>+++ Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystem.h	(working copy)
>@@ -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: Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystemCocoa.mm
>===================================================================
>--- Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystemCocoa.mm	(revision 244597)
>+++ Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystemCocoa.mm	(working copy)
>@@ -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: Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.cpp
>===================================================================
>--- Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.cpp	(revision 244596)
>+++ Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.cpp	(working copy)
>@@ -228,7 +228,6 @@ Storage::Storage(const String& baseDirec
>     , 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();
> 
>@@ -375,8 +366,6 @@ bool Storage::mayContain(const Key& key)
> bool Storage::mayContainBlob(const Key& key) const
> {
>     ASSERT(RunLoop::isMain());
>-    if (!m_canUseBlobsForForBodyData)
>-        return false;
>     return !m_blobFilter || m_blobFilter->mayContain(key.hash());
> }
> 
>@@ -792,8 +781,6 @@ void Storage::dispatchPendingWriteOperat
> 
> bool Storage::shouldStoreBodyAsBlob(const Data& bodyData)
> {
>-    if (!m_canUseBlobsForForBodyData)
>-        return false;
>     return bodyData.size() > maximumInlineBodySize;
> }
> 
>Index: Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.h
>===================================================================
>--- Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.h	(revision 244596)
>+++ Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.h	(working copy)
>@@ -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 };

Actions: View | Formatted Diff | Diff

Attachments on bug 197264: 368203 | 368213 | 368243