Attachment 346418 Details for Bug 188250 – Patch

[patch] Patch

bug-188250-20180802142643.patch (text/plain), 16.16 KB, created by Alex Christensen on 2018-08-02 14:26:43 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Alex Christensen

Created: 2018-08-02 14:26:43 PDT

Size: 16.16 KB

patch

obsolete

>Index: Source/WebKit/ChangeLog
>===================================================================
>--- Source/WebKit/ChangeLog	(revision 234514)
>+++ Source/WebKit/ChangeLog	(working copy)
>@@ -1,3 +1,50 @@
>+2018-08-02  Alex Christensen  <achristensen@webkit.org>
>+
>+        Use WTF::Variant for WebPreferencesStore::Value
>+        https://bugs.webkit.org/show_bug.cgi?id=188250
>+
>+        Reviewed by Sam Weinig.
>+
>+        It was conceptually a variant. This just uses an actual WTF::Variant.
>+        Since it's encoded and decoded, I wrote variadic template encoders and decoders
>+        like we have for other types (Expected, std::tuple, KeyValuePair, etc.)
>+
>+        * Platform/IPC/ArgumentCoders.h:
>+        (IPC::VariantCoder::encode):
>+        (IPC::VariantCoder::decode):
>+        Introduce ArgumentCoder<Variant>, which encodes which type the Variant currently
>+        holds followed by the encoding of the current type.
>+        * Platform/IPC/Decoder.cpp:
>+        (IPC::Decoder::Decoder):
>+        * Platform/IPC/Decoder.h:
>+        * Platform/IPC/mac/ConnectionMac.mm:
>+        (IPC::createMessageDecoder):
>+        Use move semantics to prevent an unnecessary copy of the Vector<Attachment>
>+        * Scripts/PreferencesTemplates/WebPreferencesStoreDefaultsMap.cpp.erb:
>+        * Shared/WebPreferencesStore.cpp:
>+        (WebKit::WebPreferencesStore::decode):
>+        Use modern std::optional-based decoding supported by the new Variant decoding.
>+        (WebKit::valueForKey):
>+        Use WTF::get and WTF::holds_alternative instead of custom type checks.
>+        (WebKit::WebPreferencesStore::Value::encode const): Deleted.
>+        (WebKit::WebPreferencesStore::Value::decode): Deleted.
>+        (WebKit::as<String>): Deleted.
>+        (WebKit::as<bool>): Deleted.
>+        (WebKit::as<uint32_t>): Deleted.
>+        (WebKit::as<double>): Deleted.
>+        * Shared/WebPreferencesStore.h:
>+        (WebKit::WebPreferencesStore::Value::Value): Deleted.
>+        (WebKit::WebPreferencesStore::Value::operator=): Deleted.
>+        (WebKit::WebPreferencesStore::Value::~Value): Deleted.
>+        (WebKit::WebPreferencesStore::Value::type const): Deleted.
>+        (WebKit::WebPreferencesStore::Value::asString const): Deleted.
>+        (WebKit::WebPreferencesStore::Value::asBool const): Deleted.
>+        (WebKit::WebPreferencesStore::Value::asUInt32 const): Deleted.
>+        (WebKit::WebPreferencesStore::Value::asDouble const): Deleted.
>+        (WebKit::WebPreferencesStore::Value::destroy): Deleted.
>+        (): Deleted.
>+        Use WTF::Variant instead of a custom type/union pair.
>+
> 2018-08-02  Alex Christensen  <achristensen@webkit.org>
> 
>         Check with SafeBrowsing during navigation in WKWebView
>Index: Source/WebKit/Platform/IPC/ArgumentCoders.h
>===================================================================
>--- Source/WebKit/Platform/IPC/ArgumentCoders.h	(revision 234514)
>+++ Source/WebKit/Platform/IPC/ArgumentCoders.h	(working copy)
>@@ -515,6 +515,68 @@ template<typename ValueType, typename Er
>     }
> };
> 
>+template<size_t index, typename... Types>
>+struct VariantCoder {
>+    static void encode(Encoder& encoder, const WTF::Variant<Types...>& variant, unsigned i)
>+    {
>+        if (i == index) {
>+            encoder << WTF::get<index>(variant);
>+            return;
>+        }
>+        VariantCoder<index - 1, Types...>::encode(encoder, variant, i);
>+    }
>+    
>+    static std::optional<WTF::Variant<Types...>> decode(Decoder& decoder, unsigned i)
>+    {
>+        if (i == index) {
>+            std::optional<typename WTF::variant_alternative<index, WTF::Variant<Types...>>::type> optional;
>+            decoder >> optional;
>+            if (!optional)
>+                return std::nullopt;
>+            return { WTFMove(*optional) };
>+        }
>+        return VariantCoder<index - 1, Types...>::decode(decoder, i);
>+    }
>+};
>+
>+template<typename... Types>
>+struct VariantCoder<0, Types...> {
>+    static void encode(Encoder& encoder, const WTF::Variant<Types...>& variant, unsigned i)
>+    {
>+        ASSERT_UNUSED(i, !i);
>+        encoder << WTF::get<0>(variant);
>+    }
>+    
>+    static std::optional<WTF::Variant<Types...>> decode(Decoder& decoder, unsigned i)
>+    {
>+        ASSERT_UNUSED(i, !i);
>+        std::optional<typename WTF::variant_alternative<0, WTF::Variant<Types...>>::type> optional;
>+        decoder >> optional;
>+        if (!optional)
>+            return std::nullopt;
>+        return { WTFMove(*optional) };
>+    }
>+};
>+
>+template<typename... Types> struct ArgumentCoder<WTF::Variant<Types...>> {
>+    static void encode(Encoder& encoder, const WTF::Variant<Types...>& variant)
>+    {
>+        ASSERT(static_cast<unsigned>(variant.index()) == variant.index());
>+        unsigned i = variant.index();
>+        encoder << i;
>+        VariantCoder<sizeof...(Types) - 1, Types...>::encode(encoder, variant, i);
>+    }
>+    
>+    static std::optional<WTF::Variant<Types...>> decode(Decoder& decoder)
>+    {
>+        std::optional<unsigned> i;
>+        decoder >> i;
>+        if (!i)
>+            return std::nullopt;
>+        return VariantCoder<sizeof...(Types) - 1, Types...>::decode(decoder, *i);
>+    }
>+};
>+    
> template<> struct ArgumentCoder<WallTime> {
>     static void encode(Encoder&, const WallTime&);
>     static bool decode(Decoder&, WallTime&);
>Index: Source/WebKit/Platform/IPC/Decoder.cpp
>===================================================================
>--- Source/WebKit/Platform/IPC/Decoder.cpp	(revision 234514)
>+++ Source/WebKit/Platform/IPC/Decoder.cpp	(working copy)
>@@ -44,7 +44,7 @@ static const uint8_t* copyBuffer(const u
>     return bufferCopy;
> }
> 
>-Decoder::Decoder(const uint8_t* buffer, size_t bufferSize, void (*bufferDeallocator)(const uint8_t*, size_t), Vector<Attachment> attachments)
>+Decoder::Decoder(const uint8_t* buffer, size_t bufferSize, void (*bufferDeallocator)(const uint8_t*, size_t), Vector<Attachment>&& attachments)
>     : m_buffer { bufferDeallocator ? buffer : copyBuffer(buffer, bufferSize) }
>     , m_bufferPos { m_buffer }
>     , m_bufferEnd { m_buffer + bufferSize }
>Index: Source/WebKit/Platform/IPC/Decoder.h
>===================================================================
>--- Source/WebKit/Platform/IPC/Decoder.h	(revision 234514)
>+++ Source/WebKit/Platform/IPC/Decoder.h	(working copy)
>@@ -43,7 +43,7 @@ class ImportanceAssertion;
> class Decoder {
>     WTF_MAKE_FAST_ALLOCATED;
> public:
>-    Decoder(const uint8_t* buffer, size_t bufferSize, void (*bufferDeallocator)(const uint8_t*, size_t), Vector<Attachment>);
>+    Decoder(const uint8_t* buffer, size_t bufferSize, void (*bufferDeallocator)(const uint8_t*, size_t), Vector<Attachment>&&);
>     ~Decoder();
> 
>     Decoder(const Decoder&) = delete;
>Index: Source/WebKit/Platform/IPC/mac/ConnectionMac.mm
>===================================================================
>--- Source/WebKit/Platform/IPC/mac/ConnectionMac.mm	(revision 234514)
>+++ Source/WebKit/Platform/IPC/mac/ConnectionMac.mm	(working copy)
>@@ -472,7 +472,7 @@ static std::unique_ptr<Decoder> createMe
>     uint8_t* messageBody = descriptorData;
>     size_t messageBodySize = header->msgh_size - (descriptorData - reinterpret_cast<uint8_t*>(header));
> 
>-    return std::make_unique<Decoder>(messageBody, messageBodySize, nullptr, attachments);
>+    return std::make_unique<Decoder>(messageBody, messageBodySize, nullptr, WTFMove(attachments));
> }
> 
> // The receive buffer size should always include the maximum trailer size.
>Index: Source/WebKit/Scripts/PreferencesTemplates/WebPreferencesStoreDefaultsMap.cpp.erb
>===================================================================
>--- Source/WebKit/Scripts/PreferencesTemplates/WebPreferencesStoreDefaultsMap.cpp.erb	(revision 234514)
>+++ Source/WebKit/Scripts/PreferencesTemplates/WebPreferencesStoreDefaultsMap.cpp.erb	(working copy)
>@@ -31,6 +31,7 @@
> #include "WebPreferencesDefaultValues.h"
> #include "WebPreferencesKeys.h"
> #include <wtf/NeverDestroyed.h>
>+#include <wtf/Variant.h>
> 
> // FIXME: These should added via options in WebPreferences.yaml, rather than hardcoded.
> #include "FontSmoothingLevel.h"
>Index: Source/WebKit/Shared/WebPreferencesStore.cpp
>===================================================================
>--- Source/WebKit/Shared/WebPreferencesStore.cpp	(revision 234514)
>+++ Source/WebKit/Shared/WebPreferencesStore.cpp	(working copy)
>@@ -42,72 +42,6 @@ static BoolOverridesMap& boolTestRunnerO
>     return map;
> }
> 
>-void WebPreferencesStore::Value::encode(IPC::Encoder& encoder) const
>-{
>-    encoder.encodeEnum(m_type);
>-    
>-    switch (m_type) {
>-    case Type::None:
>-        break;
>-    case Type::String:
>-        encoder << m_string;
>-        break;
>-    case Type::Bool:
>-        encoder << m_bool;
>-        break;
>-    case Type::UInt32:
>-        encoder << m_uint32;
>-        break;
>-    case Type::Double:
>-        encoder << m_double;
>-        break;
>-    }
>-}
>-
>-bool WebPreferencesStore::Value::decode(IPC::Decoder& decoder, Value& result)
>-{
>-    Value::Type type;
>-    if (!decoder.decodeEnum(type))
>-        return false;
>-    
>-    switch (type) {
>-    case Type::None:
>-        break;
>-    case Type::String: {
>-        String value;
>-        if (!decoder.decode(value))
>-            return false;
>-        result = Value(value);
>-        break;
>-    }
>-    case Type::Bool: {
>-        bool value;
>-        if (!decoder.decode(value))
>-            return false;
>-        result = Value(value);
>-        break;
>-    }
>-    case Type::UInt32: {
>-        uint32_t value;
>-        if (!decoder.decode(value))
>-            return false;
>-        result = Value(value);
>-        break;
>-    }
>-    case Type::Double: {
>-        double value;
>-        if (!decoder.decode(value))
>-            return false;
>-        result = Value(value);
>-        break;
>-    }
>-    default:
>-        return false;
>-    }
>-
>-    return true;
>-}
>-
> WebPreferencesStore::WebPreferencesStore()
> {
> }
>@@ -120,10 +54,17 @@ void WebPreferencesStore::encode(IPC::En
> 
> bool WebPreferencesStore::decode(IPC::Decoder& decoder, WebPreferencesStore& result)
> {
>-    if (!decoder.decode(result.m_values))
>+    std::optional<HashMap<String, Value>> values;
>+    decoder >> values;
>+    if (!values)
>         return false;
>-    if (!decoder.decode(result.m_overridenDefaults))
>+    result.m_values = WTFMove(*values);
>+
>+    std::optional<HashMap<String, Value>> overridenDefaults;
>+    decoder >> overridenDefaults;
>+    if (!overridenDefaults)
>         return false;
>+    result.m_overridenDefaults = WTFMove(*overridenDefaults);
>     return true;
> }
> 
>@@ -137,37 +78,21 @@ void WebPreferencesStore::removeTestRunn
>     boolTestRunnerOverridesMap().clear();
> }
> 
>-template <typename T> struct ToType { };
>-
>-template<> struct ToType<String> { static const auto value = WebPreferencesStore::Value::Type::String; };
>-template<> struct ToType<bool> { static const auto value = WebPreferencesStore::Value::Type::Bool; };
>-template<> struct ToType<uint32_t> { static const auto value = WebPreferencesStore::Value::Type::UInt32; };
>-template<> struct ToType<double> { static const auto value = WebPreferencesStore::Value::Type::Double; };
>-
>-
>-template<typename MappedType> MappedType as(const WebPreferencesStore::Value&);
>-
>-template<> String as<String>(const WebPreferencesStore::Value& value) { return value.asString(); }
>-template<> bool as<bool>(const WebPreferencesStore::Value& value) { return value.asBool(); }
>-template<> uint32_t as<uint32_t>(const WebPreferencesStore::Value& value) { return value.asUInt32(); }
>-template<> double as<double>(const WebPreferencesStore::Value& value) { return value.asDouble(); }
>-
>-
> template<typename MappedType>
> static MappedType valueForKey(const WebPreferencesStore::ValueMap& values, const WebPreferencesStore::ValueMap& overridenDefaults, const String& key)
> {
>     auto valuesIt = values.find(key);
>-    if (valuesIt != values.end() && valuesIt->value.type() == ToType<MappedType>::value)
>-        return as<MappedType>(valuesIt->value);
>+    if (valuesIt != values.end() && WTF::holds_alternative<MappedType>(valuesIt->value))
>+        return WTF::get<MappedType>(valuesIt->value);
> 
>     auto overridenDefaultsIt = overridenDefaults.find(key);
>-    if (overridenDefaultsIt != overridenDefaults.end() && overridenDefaultsIt->value.type() == ToType<MappedType>::value)
>-        return as<MappedType>(overridenDefaultsIt->value);
>+    if (overridenDefaultsIt != overridenDefaults.end() && WTF::holds_alternative<MappedType>(overridenDefaultsIt->value))
>+        return WTF::get<MappedType>(overridenDefaultsIt->value);
> 
>     auto& defaultsMap = WebPreferencesStore::defaults();
>     auto defaultsIt = defaultsMap.find(key);
>-    if (defaultsIt != defaultsMap.end() && defaultsIt->value.type() == ToType<MappedType>::value)
>-        return as<MappedType>(defaultsIt->value);
>+    if (defaultsIt != defaultsMap.end() && WTF::holds_alternative<MappedType>(defaultsIt->value))
>+        return WTF::get<MappedType>(defaultsIt->value);
> 
>     return MappedType();
> }
>Index: Source/WebKit/Shared/WebPreferencesStore.h
>===================================================================
>--- Source/WebKit/Shared/WebPreferencesStore.h	(revision 234514)
>+++ Source/WebKit/Shared/WebPreferencesStore.h	(working copy)
>@@ -62,119 +62,7 @@ struct WebPreferencesStore {
>     static void overrideBoolValueForKey(const String& key, bool value);
>     static void removeTestRunnerOverrides();
> 
>-    struct Value {
>-        enum class Type {
>-            None,
>-            String,
>-            Bool,
>-            UInt32,
>-            Double,
>-        };
>-
>-        void encode(IPC::Encoder&) const;
>-        static bool decode(IPC::Decoder&, Value&);
>-
>-        explicit Value() : m_type(Type::None) { }
>-        explicit Value(const String& value) : m_type(Type::String), m_string(value) { }
>-        explicit Value(bool value) : m_type(Type::Bool), m_bool(value) { }
>-        explicit Value(uint32_t value) : m_type(Type::UInt32), m_uint32(value) { }
>-        explicit Value(double value) : m_type(Type::Double), m_double(value) { }
>-
>-        Value(Value&& value)
>-            : m_type(value.m_type)
>-        {
>-            switch (m_type) {
>-            case Type::None:
>-                break;
>-            case Type::String:
>-                new (&m_string) String(WTFMove(value.m_string));
>-                break;
>-            case Type::Bool:
>-                m_bool = value.m_bool;
>-                break;
>-            case Type::UInt32:
>-                m_uint32 = value.m_uint32;
>-                break;
>-            case Type::Double:
>-                m_double = value.m_double;
>-                break;
>-            }
>-        }
>-
>-        Value& operator=(const Value& other)
>-        {
>-            if (this == &other)
>-                return *this;
>-                
>-            destroy();
>-
>-            m_type = other.m_type;
>-            switch (m_type) {
>-            case Type::None:
>-                break;
>-            case Type::String:
>-                new (&m_string) String(other.m_string);
>-                break;
>-            case Type::Bool:
>-                m_bool = other.m_bool;
>-                break;
>-            case Type::UInt32:
>-                m_uint32 = other.m_uint32;
>-                break;
>-            case Type::Double:
>-                m_double = other.m_double;
>-                break;
>-            }
>-    
>-            return *this;
>-        }
>-
>-        ~Value()
>-        {
>-            destroy();
>-        }
>-
>-        Type type() const { return m_type; }
>-
>-        String asString() const
>-        {
>-            ASSERT(m_type == Type::String);
>-            return m_string;
>-        }
>-
>-        bool asBool() const
>-        {
>-            ASSERT(m_type == Type::Bool);
>-            return m_bool;
>-        }
>-
>-        uint32_t asUInt32() const
>-        {
>-            ASSERT(m_type == Type::UInt32);
>-            return m_uint32;
>-        }
>-
>-        double asDouble() const
>-        {
>-            ASSERT(m_type == Type::Double);
>-            return m_double;
>-        }
>-
>-    private:
>-        void destroy()
>-        {
>-            if (m_type == Type::String)
>-                m_string.~String();
>-        }
>-
>-        Type m_type;
>-        union {
>-            String m_string;
>-            bool m_bool;
>-            uint32_t m_uint32;
>-            double m_double;
>-        };
>-    };
>+    using Value = Variant<String, bool, uint32_t, double>;
> 
>     typedef HashMap<String, Value> ValueMap;
>     ValueMap m_values;

Actions: View | Formatted Diff | Diff

Attachments on bug 188250: 346358 | 346393 | 346394 | 346395 | 346418