Attachment 339820 Details for Bug 185422 – patch

[patch] patch

cache-no-blob-class-a.patch (text/plain), 9.67 KB, created by Antti Koivisto on 2018-05-08 07:33:13 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Antti Koivisto

Created: 2018-05-08 07:33:13 PDT

Size: 9.67 KB

patch

obsolete

>Index: Source/WebKit/ChangeLog
>===================================================================
>--- Source/WebKit/ChangeLog	(revision 231483)
>+++ Source/WebKit/ChangeLog	(working copy)
>@@ -1,3 +1,44 @@
>+2018-05-08  Antti Koivisto  <antti@apple.com>
>+
>+        Don't use mapped cache files in case of Class A/B protected app
>+        https://bugs.webkit.org/show_bug.cgi?id=185422
>+        <rdar://problem/34001688>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Currently we don't use shared memory maps in these cases. This still leaves us open for crashes
>+        in the network process when the device is locked.
>+
>+        This patch disables use of blob storage (mapped cache files) in apps that use class A/B protection.
>+        Normally we use blobs for resources > 16KB. Since use of shared memory is already disabled,
>+        the only optimization lost for these apps is body data deduplication.
>+
>+        Any existing cache entries with blobs are ignored and deleted. New entries are created with
>+        body data inlined with the metadata.
>+
>+        * NetworkProcess/cache/NetworkCache.cpp:
>+        (WebKit::NetworkCache::Cache::store):
>+        * NetworkProcess/cache/NetworkCache.h:
>+        (WebKit::NetworkCache::Cache::canUseSharedMemoryForBodyData const): Deleted.
>+        * NetworkProcess/cache/NetworkCacheEntry.cpp:
>+        (WebKit::NetworkCache::Entry::initializeShareableResourceHandleFromStorageRecord const):
>+
>+            Remove the code the prevented use of shared memory in these cases. Non-mapped Data objects
>+            are never shareable.
>+
>+        (WebKit::NetworkCache::Entry::setNeedsValidation):
>+        * NetworkProcess/cache/NetworkCacheFileSystem.cpp:
>+        (WebKit::NetworkCache::isSafeToUseMemoryMapForPath):
>+        (WebKit::NetworkCache::canUseSharedMemoryForPath): Deleted.
>+        * NetworkProcess/cache/NetworkCacheFileSystem.h:
>+        * NetworkProcess/cache/NetworkCacheStorage.cpp:
>+        (WebKit::NetworkCache::Storage::Storage):
>+        (WebKit::NetworkCache::Storage::mayContainBlob const):
>+        (WebKit::NetworkCache::Storage::shouldStoreBodyAsBlob):
>+        (WebKit::NetworkCache::shouldStoreBodyAsBlob): Deleted.
>+        * NetworkProcess/cache/NetworkCacheStorage.h:
>+        (WebKit::NetworkCache::Storage::canUseSharedMemoryForBodyData const): Deleted.
>+
> 2018-05-07  Carlos Garcia Campos  <cgarcia@igalia.com>
> 
>         Unreviewed. Add missing exit not included in r231298.
>Index: Source/WebKit/NetworkProcess/cache/NetworkCache.cpp
>===================================================================
>--- Source/WebKit/NetworkProcess/cache/NetworkCache.cpp	(revision 231438)
>+++ Source/WebKit/NetworkProcess/cache/NetworkCache.cpp	(working copy)
>@@ -427,15 +427,13 @@ std::unique_ptr<Entry> Cache::store(cons
>     auto cacheEntry = makeEntry(request, response, WTFMove(responseData));
>     auto record = cacheEntry->encodeAsStorageRecord();
> 
>-    m_storage->store(record, [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)](const Data& bodyData) {
>+    m_storage->store(record, [protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)](const Data& bodyData) {
>         MappedBody mappedBody;
> #if ENABLE(SHAREABLE_RESOURCE)
>-        if (canUseSharedMemoryForBodyData()) {
>-            if (auto sharedMemory = bodyData.tryCreateSharedMemory()) {
>-                mappedBody.shareableResource = ShareableResource::create(sharedMemory.releaseNonNull(), 0, bodyData.size());
>-                ASSERT(mappedBody.shareableResource);
>-                mappedBody.shareableResource->createHandle(mappedBody.shareableResourceHandle);
>-            }
>+        if (auto sharedMemory = bodyData.tryCreateSharedMemory()) {
>+            mappedBody.shareableResource = ShareableResource::create(sharedMemory.releaseNonNull(), 0, bodyData.size());
>+            ASSERT(mappedBody.shareableResource);
>+            mappedBody.shareableResource->createHandle(mappedBody.shareableResourceHandle);
>         }
> #endif
>         completionHandler(mappedBody);
>Index: Source/WebKit/NetworkProcess/cache/NetworkCache.h
>===================================================================
>--- Source/WebKit/NetworkProcess/cache/NetworkCache.h	(revision 231438)
>+++ Source/WebKit/NetworkProcess/cache/NetworkCache.h	(working copy)
>@@ -128,7 +128,6 @@ public:
>     void dumpContentsToFile();
> 
>     String recordsPath() const;
>-    bool canUseSharedMemoryForBodyData() const { return m_storage->canUseSharedMemoryForBodyData(); }
> 
> #if ENABLE(NETWORK_CACHE_SPECULATIVE_REVALIDATION)
>     SpeculativeLoadManager* speculativeLoadManager() { return m_speculativeLoadManager.get(); }
>Index: Source/WebKit/NetworkProcess/cache/NetworkCacheEntry.cpp
>===================================================================
>--- Source/WebKit/NetworkProcess/cache/NetworkCacheEntry.cpp	(revision 231438)
>+++ Source/WebKit/NetworkProcess/cache/NetworkCacheEntry.cpp	(working copy)
>@@ -146,7 +146,7 @@ std::unique_ptr<Entry> Entry::decodeStor
> void Entry::initializeShareableResourceHandleFromStorageRecord() const
> {
>     auto* cache = NetworkProcess::singleton().cache();
>-    if (!cache || !cache->canUseSharedMemoryForBodyData())
>+    if (!cache)
>         return;
> 
>     auto sharedMemory = m_sourceStorageRecord.body.tryCreateSharedMemory();
>@@ -195,13 +195,6 @@ bool Entry::needsValidation() const
> 
> void Entry::setNeedsValidation(bool value)
> {
>-    if (value) {
>-        // Validation keeps the entry alive waiting for the network response. Pull data from a mapped file into a buffer early
>-        // to protect against map disappearing due to device becoming locked.
>-        // FIXME: Cache files should be Class B/C, or we shoudn't use mapped files at all in these cases.
>-        if (!NetworkProcess::singleton().cache()->canUseSharedMemoryForBodyData())
>-            buffer();
>-    }
>     m_response.setSource(value ? WebCore::ResourceResponse::Source::DiskCacheAfterValidation : WebCore::ResourceResponse::Source::DiskCache);
> }
> 
>Index: Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystem.cpp
>===================================================================
>--- Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystem.cpp	(revision 231438)
>+++ Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystem.cpp	(working copy)
>@@ -146,7 +146,7 @@ void updateFileModificationTimeIfNeeded(
> #endif
> }
> 
>-bool canUseSharedMemoryForPath(const String& path)
>+bool isSafeToUseMemoryMapForPath(const String& path)
> {
> #if PLATFORM(IOS) && !PLATFORM(IOS_SIMULATOR)
>     struct {
>Index: Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystem.h
>===================================================================
>--- Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystem.h	(revision 231438)
>+++ Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystem.h	(working copy)
>@@ -42,7 +42,7 @@ struct FileTimes {
> FileTimes fileTimes(const String& path);
> void updateFileModificationTimeIfNeeded(const String& path);
> 
>-bool canUseSharedMemoryForPath(const String& path);
>+bool isSafeToUseMemoryMapForPath(const String& path);
> 
> }
> }
>Index: Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.cpp
>===================================================================
>--- Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.cpp	(revision 231438)
>+++ Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.cpp	(working copy)
>@@ -219,7 +219,7 @@ Storage::Storage(const String& baseDirec
>     , m_recordsPath(makeRecordsDirectoryPath(baseDirectoryPath))
>     , m_mode(mode)
>     , m_salt(salt)
>-    , m_canUseSharedMemoryForBodyData(canUseSharedMemoryForPath(baseDirectoryPath))
>+    , 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))
>@@ -348,6 +348,8 @@ 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());
> }
> 
>@@ -750,8 +752,10 @@ void Storage::dispatchPendingWriteOperat
>     }
> }
> 
>-static bool shouldStoreBodyAsBlob(const Data& bodyData)
>+bool Storage::shouldStoreBodyAsBlob(const Data& bodyData)
> {
>+    if (!m_canUseBlobsForForBodyData)
>+        return false;
>     const size_t maximumInlineBodySize { 16 * 1024 };
>     return bodyData.size() > maximumInlineBodySize;
> }
>Index: Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.h
>===================================================================
>--- Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.h	(revision 231438)
>+++ Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.h	(working copy)
>@@ -101,8 +101,6 @@ public:
> 
>     const Salt& salt() const { return m_salt; }
> 
>-    bool canUseSharedMemoryForBodyData() const { return m_canUseSharedMemoryForBodyData; }
>-
>     ~Storage();
> 
>     void writeWithoutWaiting() { m_initialWriteDelay = 0_s; };
>@@ -130,6 +128,7 @@ private:
>     void dispatchPendingWriteOperations();
>     void finishWriteOperation(WriteOperation&);
> 
>+    bool shouldStoreBodyAsBlob(const Data& bodyData);
>     std::optional<BlobStorage::Blob> storeBodyAsBlob(WriteOperation&);
>     Data encodeRecord(const Record&, std::optional<BlobStorage::Blob>);
>     void readRecord(ReadOperation&, const Data&);
>@@ -152,7 +151,7 @@ private:
>     
>     const Mode m_mode;
>     const Salt m_salt;
>-    const bool m_canUseSharedMemoryForBodyData;
>+    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 185422: 339820