Use WTF::Variant for WebPreferencesStore::Value
authorachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 2 Aug 2018 21:48:21 +0000 (21:48 +0000)
committerachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 2 Aug 2018 21:48:21 +0000 (21:48 +0000)
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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@234516 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/ChangeLog
Source/WebKit/Platform/IPC/ArgumentCoders.h
Source/WebKit/Platform/IPC/Decoder.cpp
Source/WebKit/Platform/IPC/Decoder.h
Source/WebKit/Platform/IPC/mac/ConnectionMac.mm
Source/WebKit/Scripts/PreferencesTemplates/WebPreferencesStoreDefaultsMap.cpp.erb
Source/WebKit/Shared/WebPreferencesStore.cpp
Source/WebKit/Shared/WebPreferencesStore.h

index 2923169..5c8d045 100644 (file)
@@ -1,5 +1,52 @@
 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
         https://bugs.webkit.org/show_bug.cgi?id=188133
 
index 49d951a..6cadb1c 100644 (file)
@@ -515,6 +515,68 @@ template<typename ValueType, typename ErrorType> struct ArgumentCoder<Expected<V
     }
 };
 
+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 eac7725..57b18a2 100644 (file)
@@ -44,7 +44,7 @@ static const uint8_t* copyBuffer(const uint8_t* buffer, size_t bufferSize)
     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 bb1632f..b788244 100644 (file)
@@ -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 e154a52..433d0a9 100644 (file)
@@ -472,7 +472,7 @@ static std::unique_ptr<Decoder> createMessageDecoder(mach_msg_header_t* header)
     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 a0e5250..9dcb045 100644 (file)
@@ -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 6f28d77..b4ee0ea 100644 (file)
@@ -42,72 +42,6 @@ static BoolOverridesMap& boolTestRunnerOverridesMap()
     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::Encoder& encoder) const
 
 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::removeTestRunnerOverrides()
     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 d6f19da..73bea66 100644 (file)
@@ -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;