Attachment 357342 Details for Bug 192714 – Patch v2

[patch] Patch v2

bug-192714-20181215021101.patch (text/plain), 8.68 KB, created by Adrian Perez on 2018-12-14 14:12:33 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Adrian Perez

Created: 2018-12-14 14:12:33 PST

Size: 8.68 KB

patch

obsolete

>Subversion Revision: 239229
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 9a2e520b8591048a7c19f73cf1ad4791408810f1..63c388c7d1078aa5cbc31efac6c5e2ce3cc5d24a 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,35 @@
>+2018-12-14  Adrian Perez de Castro  <aperez@igalia.com>
>+
>+        Unify SharedMemory factory functions
>+        https://bugs.webkit.org/show_bug.cgi?id=192714
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This unifies SharedMemory so in the following way, across platforms:
>+
>+        - SharedMemory::create() is removed, to avoid ambiguity.
>+        - SharedMemory::allocate() always allocates a new block of shared memory.
>+        - SharedMemory::wrapMap() always creates a SharedMemory object which refers to an
>+          existing region of memory resulting from memory-mapping a file. This is now
>+          private and can be only used from NetworkCache::Data.
>+
>+        The platform-specific (Windows) SharedMemory::adopt(), which is used internally
>+        only, is marked as private.
>+
>+        * NetworkProcess/cache/NetworkCacheDataCocoa.mm:
>+        (WebKit::NetworkCache::Data::tryCreateSharedMemory const): Use SharedMemory::wrapMap().
>+        * Platform/SharedMemory.h: Make SharedMemory::adopt() private, remove the definition of
>+        SharedMemory::create(), and make SharedMemory::wrapMap() private.
>+        * Platform/cocoa/SharedMemoryCocoa.cpp:
>+        (WebKit::SharedMemory::wrapMap): Renamed from ::create().
>+        * Platform/unix/SharedMemoryUnix.cpp:
>+        (WebKit::SharedMemory::allocate): Renamed from ::create().
>+        * Platform/win/SharedMemoryWin.cpp:
>+        (WebKit::SharedMemory::allocate): Renamed from ::create()
>+        * UIProcess/API/APIContentRuleListStore.cpp:
>+        (API::createExtension): Use NetworkCache::Data::tryCreateSharedMemory() instead of
>+        SharedMemory::create().
>+
> 2018-12-14  Adrian Perez de Castro  <aperez@igalia.com>
> 
>         [SOUP] Unreviewed build fix after r239219
>diff --git a/Source/WebKit/NetworkProcess/cache/NetworkCacheDataCocoa.mm b/Source/WebKit/NetworkProcess/cache/NetworkCacheDataCocoa.mm
>index c81f858aa0a235e8d68985d1e79f791ef3118899..6a39f88700879443d669f3b29a03b28eaaf6522b 100644
>--- a/Source/WebKit/NetworkProcess/cache/NetworkCacheDataCocoa.mm
>+++ b/Source/WebKit/NetworkProcess/cache/NetworkCacheDataCocoa.mm
>@@ -108,7 +108,7 @@ RefPtr<SharedMemory> Data::tryCreateSharedMemory() const
>     if (isNull() || !isMap())
>         return nullptr;
> 
>-    return SharedMemory::create(const_cast<uint8_t*>(data()), m_size, SharedMemory::Protection::ReadOnly);
>+    return SharedMemory::wrapMap(const_cast<uint8_t*>(data()), m_size, SharedMemory::Protection::ReadOnly);
> }
> 
> }
>diff --git a/Source/WebKit/Platform/SharedMemory.h b/Source/WebKit/Platform/SharedMemory.h
>index 3eadb15f72a25472c7b560577f924e87b2df5065..dd3adcd4ddcfecdf49b5dce6787e7285c25d3889 100644
>--- a/Source/WebKit/Platform/SharedMemory.h
>+++ b/Source/WebKit/Platform/SharedMemory.h
>@@ -52,6 +52,10 @@ class MachSendRight;
> 
> namespace WebKit {
> 
>+namespace NetworkCache {
>+class Data;
>+};
>+
> class SharedMemory : public RefCounted<SharedMemory> {
> public:
>     enum class Protection {
>@@ -90,14 +94,7 @@ public:
>     };
> 
>     static RefPtr<SharedMemory> allocate(size_t);
>-    static RefPtr<SharedMemory> create(void*, size_t, Protection);
>     static RefPtr<SharedMemory> map(const Handle&, Protection);
>-#if USE(UNIX_DOMAIN_SOCKETS)
>-    static RefPtr<SharedMemory> wrapMap(void*, size_t, int fileDescriptor);
>-#endif
>-#if OS(WINDOWS)
>-    static RefPtr<SharedMemory> adopt(HANDLE, size_t, Protection);
>-#endif
> 
>     ~SharedMemory();
> 
>@@ -120,6 +117,13 @@ public:
> private:
> #if OS(DARWIN)
>     WTF::MachSendRight createSendRight(Protection) const;
>+    static RefPtr<SharedMemory> wrapMap(void*, size_t, Protection);
>+#endif
>+#if USE(UNIX_DOMAIN_SOCKETS)
>+    static RefPtr<SharedMemory> wrapMap(void*, size_t, int fileDescriptor);
>+#endif
>+#if OS(WINDOWS)
>+    static RefPtr<SharedMemory> adopt(HANDLE, size_t, Protection);
> #endif
> 
>     size_t m_size;
>@@ -136,6 +140,11 @@ private:
> #elif OS(WINDOWS)
>     HANDLE m_handle;
> #endif
>+
>+    // NetworkCache::Data::tryCreateSharedMemory() is the only function
>+    // the ::wrapMap(), so mark it as friend and keep the factory private,
>+    // to make sure it is not used accidentally by any other code.
>+    friend class NetworkCache::Data;
> };
> 
> };
>diff --git a/Source/WebKit/Platform/cocoa/SharedMemoryCocoa.cpp b/Source/WebKit/Platform/cocoa/SharedMemoryCocoa.cpp
>index 363ad32a70ac40c28f40f7692a775c73033b7da0..e02889125f24fa666fae1bbdbce63ba02842513c 100644
>--- a/Source/WebKit/Platform/cocoa/SharedMemoryCocoa.cpp
>+++ b/Source/WebKit/Platform/cocoa/SharedMemoryCocoa.cpp
>@@ -157,7 +157,7 @@ static WTF::MachSendRight makeMemoryEntry(size_t size, vm_offset_t offset, Share
>     return WTF::MachSendRight::adopt(port);
> }
> 
>-RefPtr<SharedMemory> SharedMemory::create(void* data, size_t size, Protection protection)
>+RefPtr<SharedMemory> SharedMemory::wrapMap(void* data, size_t size, Protection protection)
> {
>     ASSERT(size);
> 
>diff --git a/Source/WebKit/Platform/unix/SharedMemoryUnix.cpp b/Source/WebKit/Platform/unix/SharedMemoryUnix.cpp
>index 520e7975992ed9dee48a0577e0ab2950d94dc5ed..c09b74a3b6c1d267e17be7bf9f299292c4afdd38 100644
>--- a/Source/WebKit/Platform/unix/SharedMemoryUnix.cpp
>+++ b/Source/WebKit/Platform/unix/SharedMemoryUnix.cpp
>@@ -148,7 +148,7 @@ static int createSharedMemory()
>     return fileDescriptor;
> }
> 
>-RefPtr<SharedMemory> SharedMemory::create(void* address, size_t size, Protection protection)
>+RefPtr<SharedMemory> SharedMemory::allocate(size_t size)
> {
>     int fileDescriptor = createSharedMemory();
>     if (fileDescriptor == -1) {
>@@ -163,7 +163,7 @@ RefPtr<SharedMemory> SharedMemory::create(void* address, size_t size, Protection
>         }
>     }
> 
>-    void* data = mmap(address, size, accessModeMMap(protection), MAP_SHARED, fileDescriptor, 0);
>+    void* data = mmap(nullptr, size, accessModeMMap(SharedMemory::Protection::ReadWrite), MAP_SHARED, fileDescriptor, 0);
>     if (data == MAP_FAILED) {
>         closeWithRetry(fileDescriptor);
>         return nullptr;
>@@ -176,11 +176,6 @@ RefPtr<SharedMemory> SharedMemory::create(void* address, size_t size, Protection
>     return instance;
> }
> 
>-RefPtr<SharedMemory> SharedMemory::allocate(size_t size)
>-{
>-    return SharedMemory::create(nullptr, size, SharedMemory::Protection::ReadWrite);
>-}
>-
> RefPtr<SharedMemory> SharedMemory::map(const Handle& handle, Protection protection)
> {
>     ASSERT(!handle.isNull());
>diff --git a/Source/WebKit/Platform/win/SharedMemoryWin.cpp b/Source/WebKit/Platform/win/SharedMemoryWin.cpp
>index 2d32c3313cd432fadbd7c4676fa0752d50e8ee29..61df19a25906e94be259162b7d6e8bd32d13c81f 100644
>--- a/Source/WebKit/Platform/win/SharedMemoryWin.cpp
>+++ b/Source/WebKit/Platform/win/SharedMemoryWin.cpp
>@@ -132,16 +132,11 @@ static DWORD protectAttribute(SharedMemory::Protection protection)
> 
> RefPtr<SharedMemory> SharedMemory::allocate(size_t size)
> {
>-    return SharedMemory::create(nullptr, size, SharedMemory::Protection::ReadWrite);
>-}
>-
>-RefPtr<SharedMemory> SharedMemory::create(void* address, size_t size, Protection protection)
>-{
>-    HANDLE handle = ::CreateFileMappingW(INVALID_HANDLE_VALUE, 0, protectAttribute(protection), 0, size, 0);
>+    HANDLE handle = ::CreateFileMappingW(INVALID_HANDLE_VALUE, 0, protectAttribute(SharedMemory::Protection::ReadWrite), 0, size, 0);
>     if (!handle)
>         return nullptr;
> 
>-    void* baseAddress = ::MapViewOfFileEx(handle, FILE_MAP_ALL_ACCESS, 0, 0, size, address);
>+    void* baseAddress = ::MapViewOfFileEx(handle, FILE_MAP_ALL_ACCESS, 0, 0, size, nullptr);
>     if (!baseAddress) {
>         ::CloseHandle(handle);
>         return nullptr;
>diff --git a/Source/WebKit/UIProcess/API/APIContentRuleListStore.cpp b/Source/WebKit/UIProcess/API/APIContentRuleListStore.cpp
>index 0778cd66bab8d8c1571c078658ab3b852316200b..4e91fda918ccbb76c1324f8e1a99d82d4459ac60 100644
>--- a/Source/WebKit/UIProcess/API/APIContentRuleListStore.cpp
>+++ b/Source/WebKit/UIProcess/API/APIContentRuleListStore.cpp
>@@ -369,7 +369,8 @@ static std::error_code compiledToFile(WTF::String&& json, Vector<WebCore::Conten
> 
> static Ref<API::ContentRuleList> createExtension(const WTF::String& identifier, const ContentRuleListMetaData& metaData, const WebKit::NetworkCache::Data& fileData)
> {
>-    auto sharedMemory = WebKit::SharedMemory::create(const_cast<uint8_t*>(fileData.data()), fileData.size(), WebKit::SharedMemory::Protection::ReadOnly);
>+    auto sharedMemory = fileData.tryCreateSharedMemory();
>+    ASSERT(sharedMemory);
>     const size_t headerAndSourceSize = ContentRuleListFileHeaderSize + metaData.sourceSize;
>     auto compiledContentRuleListData = WebKit::WebCompiledContentRuleListData(
>         WTFMove(sharedMemory),

Actions: View | Formatted Diff | Diff

Attachments on bug 192714: 357339 | 357342 | 357418 | 357419