Skip to content

Commit

Permalink
Unreviewed, reverting 268760@main.
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=263153

Seems to have introduced crashes on Apple Watch

Reverted changeset:

"Use StringView in CSSParserToken and simplify its code"
https://bugs.webkit.org/show_bug.cgi?id=262498
https://commits.webkit.org/268760@main

Canonical link: https://commits.webkit.org/269335@main
  • Loading branch information
webkit-commit-queue authored and cdumez committed Oct 14, 2023
1 parent d490ce5 commit 77de72c
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 34 deletions.
8 changes: 4 additions & 4 deletions Source/WTF/wtf/text/StringView.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -261,15 +261,15 @@ String convertASCIICase(const CharacterType* input, unsigned length)
String StringView::convertToASCIILowercase() const
{
if (m_is8Bit)
return convertASCIICase<ASCIICase::Lower>(static_cast<const LChar*>(m_characters.get()), m_length);
return convertASCIICase<ASCIICase::Lower>(static_cast<const UChar*>(m_characters.get()), m_length);
return convertASCIICase<ASCIICase::Lower>(static_cast<const LChar*>(m_characters), m_length);
return convertASCIICase<ASCIICase::Lower>(static_cast<const UChar*>(m_characters), m_length);
}

String StringView::convertToASCIIUppercase() const
{
if (m_is8Bit)
return convertASCIICase<ASCIICase::Upper>(static_cast<const LChar*>(m_characters.get()), m_length);
return convertASCIICase<ASCIICase::Upper>(static_cast<const UChar*>(m_characters.get()), m_length);
return convertASCIICase<ASCIICase::Upper>(static_cast<const LChar*>(m_characters), m_length);
return convertASCIICase<ASCIICase::Upper>(static_cast<const UChar*>(m_characters), m_length);
}

template<typename CharacterType>
Expand Down
26 changes: 17 additions & 9 deletions Source/WTF/wtf/text/StringView.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
#include <limits.h>
#include <unicode/utypes.h>
#include <wtf/Forward.h>
#include <wtf/Packed.h>
#include <wtf/RetainPtr.h>
#include <wtf/Vector.h>
#include <wtf/text/ASCIILiteral.h>
Expand Down Expand Up @@ -68,6 +67,7 @@ class StringView final {
StringView(const LChar*, unsigned length);
StringView(const UChar*, unsigned length);
StringView(const char*, unsigned length);
StringView(const void*, unsigned length, bool is8bit);
StringView(ASCIILiteral);
ALWAYS_INLINE StringView(std::span<const LChar> characters) : StringView(characters.data(), characters.size()) { }
ALWAYS_INLINE StringView(std::span<const UChar> characters) : StringView(characters.data(), characters.size()) { }
Expand Down Expand Up @@ -95,6 +95,7 @@ class StringView final {
bool is8Bit() const;
const LChar* characters8() const;
const UChar* characters16() const;
const void* rawCharacters() const { return m_characters; }
std::span<const LChar> span8() const { return { characters8(), length() }; }
std::span<const UChar> span16() const { return { characters16(), length() }; }

Expand Down Expand Up @@ -229,13 +230,13 @@ class StringView final {

void clear();

const void* m_characters { nullptr };
unsigned m_length { 0 };
bool m_is8Bit { true };

#if CHECK_STRINGVIEW_LIFETIME
UnderlyingString* m_underlyingString { nullptr };
#endif

PackedPtr<const void> m_characters;
bool m_is8Bit { true };
unsigned m_length { 0 };
};

template<typename CharacterType, size_t inlineCapacity> void append(Vector<CharacterType, inlineCapacity>&, StringView);
Expand Down Expand Up @@ -309,8 +310,8 @@ inline StringView::~StringView()

inline StringView::StringView(StringView&& other)
: m_characters(other.m_characters)
, m_is8Bit(other.m_is8Bit)
, m_length(other.m_length)
, m_is8Bit(other.m_is8Bit)
{
ASSERT(other.underlyingStringIsValid());

Expand All @@ -322,8 +323,8 @@ inline StringView::StringView(StringView&& other)

inline StringView::StringView(const StringView& other)
: m_characters(other.m_characters)
, m_is8Bit(other.m_is8Bit)
, m_length(other.m_length)
, m_is8Bit(other.m_is8Bit)
{
ASSERT(other.underlyingStringIsValid());

Expand Down Expand Up @@ -395,6 +396,13 @@ inline StringView::StringView(const char* characters, unsigned length)
initialize(reinterpret_cast<const LChar*>(characters), length);
}

inline StringView::StringView(const void* characters, unsigned length, bool is8bit)
: m_characters(characters)
, m_length(length)
, m_is8Bit(is8bit)
{
}

inline StringView::StringView(ASCIILiteral string)
{
initialize(string.characters8(), string.length());
Expand Down Expand Up @@ -451,14 +459,14 @@ inline const LChar* StringView::characters8() const
{
ASSERT(is8Bit());
ASSERT(underlyingStringIsValid());
return static_cast<const LChar*>(m_characters.get());
return static_cast<const LChar*>(m_characters);
}

inline const UChar* StringView::characters16() const
{
ASSERT(!is8Bit() || isEmpty());
ASSERT(underlyingStringIsValid());
return static_cast<const UChar*>(m_characters.get());
return static_cast<const UChar*>(m_characters);
}

inline unsigned StringView::hash() const
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/css/CSSVariableData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ template<typename CharacterType> void CSSVariableData::updateBackingStringsInTok
if (!token.hasStringBacking())
continue;
unsigned length = token.value().length();
token.updateString({ currentOffset, length });
token.updateCharacters(currentOffset, length);
currentOffset += length;
}
ASSERT(currentOffset == m_backingString.characters<CharacterType>() + m_backingString.length());
Expand Down
24 changes: 8 additions & 16 deletions Source/WebCore/css/parser/CSSParserToken.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,6 @@
namespace WebCore {
DEFINE_ALLOCATOR_WITH_HEAP_IDENTIFIER(CSSParserToken);

struct SameSizeAsCSSParserToken {
StringView value;
unsigned flags;
double unionValue;
};

static_assert(sizeof(CSSParserToken) == sizeof(SameSizeAsCSSParserToken), "CSSParserToken should stay small");

template<typename CharacterType>
CSSUnitType cssPrimitiveValueUnitFromTrie(const CharacterType* data, unsigned length)
{
Expand Down Expand Up @@ -377,30 +369,30 @@ CSSParserToken::CSSParserToken(CSSParserTokenType type, UChar c)
}

CSSParserToken::CSSParserToken(CSSParserTokenType type, StringView value, BlockType blockType)
: m_value(value)
, m_type(type)
: m_type(type)
, m_blockType(blockType)
{
initValueFromStringView(value);
m_id = -1;
}

CSSParserToken::CSSParserToken(double numericValue, NumericValueType numericValueType, NumericSign sign, StringView originalText)
: m_value(originalText)
, m_type(NumberToken)
: m_type(NumberToken)
, m_blockType(NotBlock)
, m_numericValueType(numericValueType)
, m_numericSign(sign)
, m_unit(static_cast<unsigned>(CSSUnitType::CSS_NUMBER))
, m_numericValue(numericValue)
{
initValueFromStringView(originalText);
}

CSSParserToken::CSSParserToken(HashTokenType type, StringView value)
: m_value(value)
, m_type(HashToken)
: m_type(HashToken)
, m_blockType(NotBlock)
, m_hashTokenType(type)
{
initValueFromStringView(value);
}

static StringView mergeIfAdjacent(StringView a, StringView b)
Expand Down Expand Up @@ -430,7 +422,7 @@ void CSSParserToken::convertToDimensionWithUnit(StringView unit)
m_type = DimensionToken;
m_unit = static_cast<unsigned>(stringToUnitType(unit));
m_nonUnitPrefixLength = string == unit ? 0 : originalNumberTextLength;
m_value = string;
initValueFromStringView(string);
}

void CSSParserToken::convertToPercentage()
Expand Down Expand Up @@ -548,7 +540,7 @@ CSSParserToken CSSParserToken::copyWithUpdatedString(StringView string) const
{
ASSERT(value() == string);
CSSParserToken copy(*this);
copy.m_value = string;
copy.initValueFromStringView(string);
return copy;
}

Expand Down
28 changes: 24 additions & 4 deletions Source/WebCore/css/parser/CSSParserToken.h
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ class CSSParserToken {
void convertToPercentage();

CSSParserTokenType type() const { return static_cast<CSSParserTokenType>(m_type); }
StringView value() const { return m_value; }
StringView value() const { return { m_valueDataCharRaw, m_valueLength, m_valueIs8Bit }; }

UChar delimiter() const;
NumericSign numericSign() const;
Expand All @@ -137,19 +137,31 @@ class CSSParserToken {

void serialize(StringBuilder&, const CSSParserToken* nextToken = nullptr) const;

void updateString(StringView string) { m_value = string; }
template<typename CharacterType>
void updateCharacters(const CharacterType* characters, unsigned length);

CSSParserToken copyWithUpdatedString(StringView) const;

private:
StringView m_value;

void initValueFromStringView(StringView string)
{
m_valueLength = string.length();
m_valueIs8Bit = string.is8Bit();
m_valueDataCharRaw = string.rawCharacters();
}
unsigned m_type : 6; // CSSParserTokenType
unsigned m_blockType : 2; // BlockType
unsigned m_numericValueType : 1; // NumericValueType
unsigned m_numericSign : 2; // NumericSign
unsigned m_unit : 7; // CSSUnitType
unsigned m_nonUnitPrefixLength : 4; // Only for DimensionType, only needs to be long enough for UnicodeRange parsing.

// m_value... is an unpacked StringView so that we can pack it
// tightly with the rest of this object for a smaller object size.
bool m_valueIs8Bit : 1;
unsigned m_valueLength;
const void* m_valueDataCharRaw; // Either LChar* or UChar*.

union {
UChar m_delimiter;
HashTokenType m_hashTokenType;
Expand All @@ -158,4 +170,12 @@ class CSSParserToken {
};
};

template<typename CharacterType>
inline void CSSParserToken::updateCharacters(const CharacterType* characters, unsigned length)
{
m_valueLength = length;
m_valueIs8Bit = (sizeof(CharacterType) == 1);
m_valueDataCharRaw = characters;
}

} // namespace WebCore

0 comments on commit 77de72c

Please sign in to comment.