Diff
Modified: trunk/Source/WebKit/ChangeLog (261671 => 261672)
--- trunk/Source/WebKit/ChangeLog 2020-05-14 02:41:19 UTC (rev 261671)
+++ trunk/Source/WebKit/ChangeLog 2020-05-14 02:42:58 UTC (rev 261672)
@@ -1,3 +1,47 @@
+2020-05-13 David Kilzer <[email protected]>
+
+ [IPC] Use templates to reduce duplicate code in IPC::Decoder and IPC::Encoder classes
+ <https://webkit.org/b/211861>
+ <rdar://problem/62360390>
+
+ Reviewed by Alex Christensen.
+
+ In broad strokes, this change fixes the following issues:
+ - Change `unsigned` type to `size_t` for `alignment` parameters
+ for consistency on 32-bit platforms.
+ - Templatize Encoder::encode(), Decoder::decode() and
+ Decoder::operator>>() methods to reduce duplicate code. This
+ deleted code in source files, added template methods to header
+ files, and required a few changes to existing templates in
+ header files to disambiguate std::is_arithmetic parameter
+ types.
+ - Use `typename E` for all template methods that handle enum
+ types.
+ - Use `typename T` for all other template method types.
+ - Move WARN_UNUSED_RETURN to same line as `bool` return type.
+ - Add FIXME comments to validate enum values.
+
+ * Platform/IPC/Decoder.cpp:
+ (IPC::roundUpToAlignment):
+ (IPC::Decoder::alignBufferPosition):
+ (IPC::Decoder::bufferIsLargeEnoughToContain const):
+ (IPC::Decoder::decodeFixedLengthData):
+ (IPC::decodeValueFromBuffer): Delete.
+ (IPC::Decoder::getOptional): Delete.
+ (IPC::Decoder::operator>>): Delete.
+ (IPC::Decoder::decode): Delete.
+ * Platform/IPC/Decoder.h:
+ (IPC::Decoder::decode):
+ (IPC::Decoder::operator>>):
+ (IPC::Decoder::decodeEnum):
+ * Platform/IPC/Encoder.cpp:
+ (IPC::roundUpToAlignment):
+ (IPC::Encoder::grow):
+ (IPC::Encoder::encodeFixedLengthData):
+ (IPC::copyValueToBuffer): Delete.
+ (IPC::Encoder::encode): Delete.
+ * Platform/IPC/Encoder.h:
+
2020-05-13 Devin Rousso <[email protected]>
Web Inspector: `RuntimeEnabledFeatures` should also be enabled when remotely inspecting
Modified: trunk/Source/WebKit/Platform/IPC/Decoder.cpp (261671 => 261672)
--- trunk/Source/WebKit/Platform/IPC/Decoder.cpp 2020-05-14 02:41:19 UTC (rev 261671)
+++ trunk/Source/WebKit/Platform/IPC/Decoder.cpp 2020-05-14 02:42:58 UTC (rev 261672)
@@ -133,7 +133,7 @@
return Decoder::create(wrappedMessage.data(), wrappedMessage.size(), nullptr, WTFMove(attachments));
}
-static inline const uint8_t* roundUpToAlignment(const uint8_t* ptr, unsigned alignment)
+static inline const uint8_t* roundUpToAlignment(const uint8_t* ptr, size_t alignment)
{
// Assert that the alignment is a power of 2.
ASSERT(alignment && !(alignment & (alignment - 1)));
@@ -151,7 +151,7 @@
return bufferEnd >= alignedPosition && bufferStart <= alignedPosition && static_cast<size_t>(bufferEnd - alignedPosition) >= size;
}
-bool Decoder::alignBufferPosition(unsigned alignment, size_t size)
+bool Decoder::alignBufferPosition(size_t alignment, size_t size)
{
const uint8_t* alignedPosition = roundUpToAlignment(m_bufferPos, alignment);
if (!alignedBufferIsLargeEnoughToContain(alignedPosition, m_buffer, m_bufferEnd, size)) {
@@ -164,12 +164,12 @@
return true;
}
-bool Decoder::bufferIsLargeEnoughToContain(unsigned alignment, size_t size) const
+bool Decoder::bufferIsLargeEnoughToContain(size_t alignment, size_t size) const
{
return alignedBufferIsLargeEnoughToContain(roundUpToAlignment(m_bufferPos, alignment), m_buffer, m_bufferEnd, size);
}
-bool Decoder::decodeFixedLengthData(uint8_t* data, size_t size, unsigned alignment)
+bool Decoder::decodeFixedLengthData(uint8_t* data, size_t size, size_t alignment)
{
if (!alignBufferPosition(alignment, size))
return false;
@@ -196,165 +196,6 @@
return true;
}
-template<typename Type>
-static void decodeValueFromBuffer(Type& value, const uint8_t*& bufferPosition)
-{
- memcpy(&value, bufferPosition, sizeof(value));
- bufferPosition += sizeof(Type);
-}
-
-template<typename Type>
-Decoder& Decoder::getOptional(Optional<Type>& optional)
-{
- Type result;
- if (!alignBufferPosition(sizeof(result), sizeof(result)))
- return *this;
-
- decodeValueFromBuffer(result, m_bufferPos);
- optional = result;
- return *this;
-}
-
-Decoder& Decoder::operator>>(Optional<bool>& optional)
-{
- return getOptional(optional);
-}
-
-Decoder& Decoder::operator>>(Optional<uint8_t>& optional)
-{
- return getOptional(optional);
-}
-
-Decoder& Decoder::operator>>(Optional<uint16_t>& optional)
-{
- return getOptional(optional);
-}
-
-Decoder& Decoder::operator>>(Optional<uint32_t>& optional)
-{
- return getOptional(optional);
-}
-
-Decoder& Decoder::operator>>(Optional<uint64_t>& optional)
-{
- return getOptional(optional);
-}
-
-Decoder& Decoder::operator>>(Optional<int16_t>& optional)
-{
- return getOptional(optional);
-}
-
-Decoder& Decoder::operator>>(Optional<int32_t>& optional)
-{
- return getOptional(optional);
-}
-
-Decoder& Decoder::operator>>(Optional<int64_t>& optional)
-{
- return getOptional(optional);
-}
-
-Decoder& Decoder::operator>>(Optional<float>& optional)
-{
- return getOptional(optional);
-}
-
-Decoder& Decoder::operator>>(Optional<double>& optional)
-{
- return getOptional(optional);
-}
-
-bool Decoder::decode(bool& result)
-{
- if (!alignBufferPosition(sizeof(result), sizeof(result)))
- return false;
-
- decodeValueFromBuffer(result, m_bufferPos);
- return true;
-}
-
-bool Decoder::decode(uint8_t& result)
-{
- if (!alignBufferPosition(sizeof(result), sizeof(result)))
- return false;
-
- decodeValueFromBuffer(result, m_bufferPos);
- return true;
-}
-
-bool Decoder::decode(uint16_t& result)
-{
- if (!alignBufferPosition(sizeof(result), sizeof(result)))
- return false;
-
- decodeValueFromBuffer(result, m_bufferPos);
- return true;
-}
-
-bool Decoder::decode(uint32_t& result)
-{
- if (!alignBufferPosition(sizeof(result), sizeof(result)))
- return false;
-
- decodeValueFromBuffer(result, m_bufferPos);
- return true;
-}
-
-bool Decoder::decode(uint64_t& result)
-{
- if (!alignBufferPosition(sizeof(result), sizeof(result)))
- return false;
-
- decodeValueFromBuffer(result, m_bufferPos);
- return true;
-}
-
-bool Decoder::decode(int16_t& result)
-{
- if (!alignBufferPosition(sizeof(result), sizeof(result)))
- return false;
-
- decodeValueFromBuffer(result, m_bufferPos);
- return true;
-}
-
-bool Decoder::decode(int32_t& result)
-{
- if (!alignBufferPosition(sizeof(result), sizeof(result)))
- return false;
-
- decodeValueFromBuffer(result, m_bufferPos);
- return true;
-}
-
-bool Decoder::decode(int64_t& result)
-{
- if (!alignBufferPosition(sizeof(result), sizeof(result)))
- return false;
-
- decodeValueFromBuffer(result, m_bufferPos);
- return true;
-}
-
-bool Decoder::decode(float& result)
-{
- if (!alignBufferPosition(sizeof(result), sizeof(result)))
- return false;
-
- decodeValueFromBuffer(result, m_bufferPos);
- return true;
-}
-
-bool Decoder::decode(double& result)
-{
- if (!alignBufferPosition(sizeof(result), sizeof(result)))
- return false;
-
- decodeValueFromBuffer(result, m_bufferPos);
- return true;
-}
-
bool Decoder::removeAttachment(Attachment& attachment)
{
if (m_attachments.isEmpty())
Modified: trunk/Source/WebKit/Platform/IPC/Decoder.h (261671 => 261672)
--- trunk/Source/WebKit/Platform/IPC/Decoder.h 2020-05-14 02:41:19 UTC (rev 261671)
+++ trunk/Source/WebKit/Platform/IPC/Decoder.h 2020-05-14 02:42:58 UTC (rev 261672)
@@ -75,35 +75,29 @@
WARN_UNUSED_RETURN bool isValid() const { return m_bufferPos != nullptr; }
void markInvalid() { m_bufferPos = nullptr; }
- WARN_UNUSED_RETURN bool decodeFixedLengthData(uint8_t*, size_t, unsigned alignment);
+ WARN_UNUSED_RETURN bool decodeFixedLengthData(uint8_t* data, size_t, size_t alignment);
// The data in the data reference here will only be valid for the lifetime of the ArgumentDecoder object.
WARN_UNUSED_RETURN bool decodeVariableLengthByteArray(DataReference&);
- WARN_UNUSED_RETURN bool decode(bool&);
- Decoder& operator>>(Optional<bool>&);
- WARN_UNUSED_RETURN bool decode(uint8_t&);
- Decoder& operator>>(Optional<uint8_t>&);
- WARN_UNUSED_RETURN bool decode(uint16_t&);
- Decoder& operator>>(Optional<uint16_t>&);
- WARN_UNUSED_RETURN bool decode(uint32_t&);
- Decoder& operator>>(Optional<uint32_t>&);
- WARN_UNUSED_RETURN bool decode(uint64_t&);
- Decoder& operator>>(Optional<uint64_t>&);
- WARN_UNUSED_RETURN bool decode(int16_t&);
- Decoder& operator>>(Optional<int16_t>&);
- WARN_UNUSED_RETURN bool decode(int32_t&);
- Decoder& operator>>(Optional<int32_t>&);
- WARN_UNUSED_RETURN bool decode(int64_t&);
- Decoder& operator>>(Optional<int64_t>&);
- WARN_UNUSED_RETURN bool decode(float&);
- Decoder& operator>>(Optional<float>&);
- WARN_UNUSED_RETURN bool decode(double&);
- Decoder& operator>>(Optional<double>&);
+ template<typename T, std::enable_if_t<std::is_arithmetic<T>::value>* = nullptr>
+ WARN_UNUSED_RETURN bool decode(T& value)
+ {
+ return decodeFixedLengthData(reinterpret_cast<uint8_t*>(&value), sizeof(T), alignof(T));
+ }
- template<typename E, typename = std::enable_if_t<std::is_enum<E>::value>> WARN_UNUSED_RETURN
- bool decode(E& e)
+ template<typename T, std::enable_if_t<std::is_arithmetic<T>::value>* = nullptr>
+ Decoder& operator>>(Optional<T>& optional)
{
+ T result;
+ if (decodeFixedLengthData(reinterpret_cast<uint8_t*>(&result), sizeof(T), alignof(T)))
+ optional = result;
+ return *this;
+ }
+
+ template<typename E, std::enable_if_t<std::is_enum<E>::value>* = nullptr>
+ WARN_UNUSED_RETURN bool decode(E& enumValue)
+ {
typename std::underlying_type<E>::type value;
if (!decode(value))
return false;
@@ -110,11 +104,11 @@
if (!WTF::isValidEnum<E>(value))
return false;
- e = static_cast<E>(value);
+ enumValue = static_cast<E>(value);
return true;
}
- template<typename E, typename = std::enable_if_t<std::is_enum<E>::value>>
+ template<typename E, std::enable_if_t<std::is_enum<E>::value>* = nullptr>
Decoder& operator>>(Optional<E>& optional)
{
Optional<typename std::underlying_type<E>::type> value;
@@ -124,14 +118,14 @@
return *this;
}
- template<typename T> WARN_UNUSED_RETURN
- bool decodeEnum(T& result)
+ template<typename E>
+ WARN_UNUSED_RETURN bool decodeEnum(E& result)
{
- typename std::underlying_type<T>::type value;
+ // FIXME: Remove this after migrating all uses of this function to decode() or operator>>() with WTF::isValidEnum check.
+ typename std::underlying_type<E>::type value;
if (!decode(value))
return false;
-
- result = static_cast<T>(value);
+ result = static_cast<E>(value);
return true;
}
@@ -146,14 +140,14 @@
return bufferIsLargeEnoughToContain(alignof(T), numElements * sizeof(T));
}
- template<typename T, std::enable_if_t<!std::is_enum<T>::value && UsesLegacyDecoder<T>::value>* = nullptr> WARN_UNUSED_RETURN
- bool decode(T& t)
+ template<typename T, std::enable_if_t<!std::is_enum<T>::value && !std::is_arithmetic<T>::value && UsesLegacyDecoder<T>::value>* = nullptr>
+ WARN_UNUSED_RETURN bool decode(T& t)
{
return ArgumentCoder<T>::decode(*this, t);
}
- template<typename T, std::enable_if_t<!std::is_enum<T>::value && !UsesLegacyDecoder<T>::value>* = nullptr> WARN_UNUSED_RETURN
- bool decode(T& t)
+ template<typename T, std::enable_if_t<!std::is_enum<T>::value && !std::is_arithmetic<T>::value && !UsesLegacyDecoder<T>::value>* = nullptr>
+ WARN_UNUSED_RETURN bool decode(T& t)
{
Optional<T> optional;
*this >> optional;
@@ -170,7 +164,7 @@
return *this;
}
- template<typename T, std::enable_if_t<!std::is_enum<T>::value && !UsesModernDecoder<T>::value>* = nullptr>
+ template<typename T, std::enable_if_t<!std::is_enum<T>::value && !std::is_arithmetic<T>::value && !UsesModernDecoder<T>::value>* = nullptr>
Decoder& operator>>(Optional<T>& optional)
{
T t;
@@ -187,9 +181,8 @@
static const bool isIPCDecoder = true;
private:
- bool alignBufferPosition(unsigned alignment, size_t);
- bool bufferIsLargeEnoughToContain(unsigned alignment, size_t) const;
- template<typename Type> Decoder& getOptional(Optional<Type>&);
+ bool alignBufferPosition(size_t alignment, size_t);
+ bool bufferIsLargeEnoughToContain(size_t alignment, size_t) const;
const uint8_t* m_buffer;
const uint8_t* m_bufferPos;
Modified: trunk/Source/WebKit/Platform/IPC/Encoder.cpp (261671 => 261672)
--- trunk/Source/WebKit/Platform/IPC/Encoder.cpp 2020-05-14 02:41:19 UTC (rev 261671)
+++ trunk/Source/WebKit/Platform/IPC/Encoder.cpp 2020-05-14 02:42:58 UTC (rev 261672)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2010 Apple Inc. All rights reserved.
+ * Copyright (C) 2010-2020 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -138,7 +138,7 @@
addAttachment(WTFMove(attachment));
}
-static inline size_t roundUpToAlignment(size_t value, unsigned alignment)
+static inline size_t roundUpToAlignment(size_t value, size_t alignment)
{
return ((value + alignment - 1) / alignment) * alignment;
}
@@ -172,7 +172,7 @@
*this << m_destinationID;
}
-uint8_t* Encoder::grow(unsigned alignment, size_t size)
+uint8_t* Encoder::grow(size_t alignment, size_t size)
{
size_t alignedSize = roundUpToAlignment(m_bufferSize, alignment);
reserve(alignedSize + size);
@@ -185,7 +185,7 @@
return m_buffer + alignedSize;
}
-void Encoder::encodeFixedLengthData(const uint8_t* data, size_t size, unsigned alignment)
+void Encoder::encodeFixedLengthData(const uint8_t* data, size_t size, size_t alignment)
{
ASSERT(!(reinterpret_cast<uintptr_t>(data) % alignment));
@@ -199,72 +199,6 @@
encodeFixedLengthData(dataReference.data(), dataReference.size(), 1);
}
-template<typename Type>
-static void copyValueToBuffer(Type value, uint8_t* bufferPosition)
-{
- memcpy(bufferPosition, &value, sizeof(Type));
-}
-
-void Encoder::encode(bool n)
-{
- uint8_t* buffer = grow(sizeof(n), sizeof(n));
- copyValueToBuffer(n, buffer);
-}
-
-void Encoder::encode(uint8_t n)
-{
- uint8_t* buffer = grow(sizeof(n), sizeof(n));
- copyValueToBuffer(n, buffer);
-}
-
-void Encoder::encode(uint16_t n)
-{
- uint8_t* buffer = grow(sizeof(n), sizeof(n));
- copyValueToBuffer(n, buffer);
-}
-
-void Encoder::encode(uint32_t n)
-{
- uint8_t* buffer = grow(sizeof(n), sizeof(n));
- copyValueToBuffer(n, buffer);
-}
-
-void Encoder::encode(uint64_t n)
-{
- uint8_t* buffer = grow(sizeof(n), sizeof(n));
- copyValueToBuffer(n, buffer);
-}
-
-void Encoder::encode(int16_t n)
-{
- uint8_t* buffer = grow(sizeof(n), sizeof(n));
- copyValueToBuffer(n, buffer);
-}
-
-void Encoder::encode(int32_t n)
-{
- uint8_t* buffer = grow(sizeof(n), sizeof(n));
- copyValueToBuffer(n, buffer);
-}
-
-void Encoder::encode(int64_t n)
-{
- uint8_t* buffer = grow(sizeof(n), sizeof(n));
- copyValueToBuffer(n, buffer);
-}
-
-void Encoder::encode(float n)
-{
- uint8_t* buffer = grow(sizeof(n), sizeof(n));
- copyValueToBuffer(n, buffer);
-}
-
-void Encoder::encode(double n)
-{
- uint8_t* buffer = grow(sizeof(n), sizeof(n));
- copyValueToBuffer(n, buffer);
-}
-
void Encoder::addAttachment(Attachment&& attachment)
{
m_attachments.append(WTFMove(attachment));
Modified: trunk/Source/WebKit/Platform/IPC/Encoder.h (261671 => 261672)
--- trunk/Source/WebKit/Platform/IPC/Encoder.h 2020-05-14 02:41:19 UTC (rev 261671)
+++ trunk/Source/WebKit/Platform/IPC/Encoder.h 2020-05-14 02:42:58 UTC (rev 261672)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2010 Apple Inc. All rights reserved.
+ * Copyright (C) 2010-2020 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -58,24 +58,27 @@
void wrapForTesting(std::unique_ptr<Encoder>);
- void encodeFixedLengthData(const uint8_t*, size_t, unsigned alignment);
+ void encodeFixedLengthData(const uint8_t* data, size_t, size_t alignment);
void encodeVariableLengthByteArray(const DataReference&);
- template<typename T> void encodeEnum(T t)
+ template<typename E>
+ void encodeEnum(E enumValue)
{
- encode(static_cast<typename std::underlying_type<T>::type>(t));
+ // FIXME: Remove this after migrating all uses of this function to encode() or operator<<() with WTF::isValidEnum check.
+ encode(static_cast<typename std::underlying_type<E>::type>(enumValue));
}
- template<typename T, std::enable_if_t<!std::is_enum<typename std::remove_const_t<std::remove_reference_t<T>>>::value>* = nullptr>
+ template<typename T, std::enable_if_t<!std::is_enum<typename std::remove_const_t<std::remove_reference_t<T>>>::value && !std::is_arithmetic<typename std::remove_const_t<std::remove_reference_t<T>>>::value>* = nullptr>
void encode(T&& t)
{
ArgumentCoder<typename std::remove_const<typename std::remove_reference<T>::type>::type>::encode(*this, std::forward<T>(t));
}
- template<typename T, std::enable_if_t<std::is_enum<T>::value>* = nullptr>
- Encoder& operator<<(T&& t)
+ template<typename E, std::enable_if_t<std::is_enum<E>::value>* = nullptr>
+ Encoder& operator<<(E&& enumValue)
{
- encode(static_cast<typename std::underlying_type<T>::type>(t));
+ ASSERT(WTF::isValidEnum<E>(static_cast<typename std::underlying_type<E>::type>(enumValue)));
+ encode(static_cast<typename std::underlying_type<E>::type>(enumValue));
return *this;
}
@@ -86,6 +89,12 @@
return *this;
}
+ template<typename T, std::enable_if_t<std::is_arithmetic<T>::value>* = nullptr>
+ void encode(T value)
+ {
+ encodeFixedLengthData(reinterpret_cast<const uint8_t*>(&value), sizeof(T), alignof(T));
+ }
+
uint8_t* buffer() const { return m_buffer; }
size_t bufferSize() const { return m_bufferSize; }
@@ -94,28 +103,16 @@
static const bool isIPCEncoder = true;
- void encode(uint64_t);
-
private:
void reserve(size_t);
- uint8_t* grow(unsigned alignment, size_t);
+ uint8_t* grow(size_t alignment, size_t);
- void encode(bool);
- void encode(uint8_t);
- void encode(uint16_t);
- void encode(uint32_t);
- void encode(int16_t);
- void encode(int32_t);
- void encode(int64_t);
- void encode(float);
- void encode(double);
-
- template<typename E>
- auto encode(E value) -> std::enable_if_t<std::is_enum<E>::value>
+ template<typename E, std::enable_if_t<std::is_enum<E>::value>* = nullptr>
+ void encode(E enumValue)
{
- ASSERT(WTF::isValidEnum<E>(static_cast<typename std::underlying_type<E>::type>(value)));
- encode(static_cast<typename std::underlying_type<E>::type>(value));
+ ASSERT(WTF::isValidEnum<E>(static_cast<typename std::underlying_type<E>::type>(enumValue)));
+ encode(static_cast<typename std::underlying_type<E>::type>(enumValue));
}
void encodeHeader();
