Source/core/css/StylePropertySerializer.cpp - Issue 1164573002: CSSValue Immediates: Change CSSValue to an object instead of a pointer

Unified Diff: Source/core/css/StylePropertySerializer.cpp

Issue 1164573002: CSSValue Immediates: Change CSSValue to an object instead of a pointer (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master

Patch Set: Some small fixes to (hopefully) fix some broken tests Created 5 years, 7 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

Index: Source/core/css/StylePropertySerializer.cpp

diff --git a/Source/core/css/StylePropertySerializer.cpp b/Source/core/css/StylePropertySerializer.cpp

index 2a0095a41d56fc5071da4b11ecee5277f8212cab..7e13bf557396a20686f69b82f65a1458b4a90f3e 100644

--- a/Source/core/css/StylePropertySerializer.cpp

+++ b/Source/core/css/StylePropertySerializer.cpp

@@ -55,7 +55,7 @@ StylePropertySerializer::StylePropertySetForSerializer::StylePropertySetForSeria

continue;

if (static_cast<unsigned>(m_allIndex) >= i)

continue;

- if (property.value()->equals(*allProperty.value())

+ if (property.value().equals(allProperty.value())

&& property.isImportant() == allProperty.isImportant())

continue;

m_needToExpandAll = true;

@@ -128,11 +128,11 @@ int StylePropertySerializer::StylePropertySetForSerializer::findPropertyIndex(CS

return propertyID - firstCSSProperty;

}

-const CSSValue* StylePropertySerializer::StylePropertySetForSerializer::getPropertyCSSValue(CSSPropertyID propertyID) const

+const NullableCSSValue StylePropertySerializer::StylePropertySetForSerializer::getPropertyCSSValue(CSSPropertyID propertyID) const

{

int index = findPropertyIndex(propertyID);

if (index == -1)

- return 0;

+ return nullptr;

StylePropertySerializer::PropertyValueForSerializer value = propertyAt(index);

return value.value();

}

@@ -142,7 +142,7 @@ String StylePropertySerializer::StylePropertySetForSerializer::getPropertyValue(

if (!hasExpandedAllProperty())

return m_propertySet.getPropertyValue(propertyID);

- const CSSValue* value = getPropertyCSSValue(propertyID);

+ const NullableCSSValue value = getPropertyCSSValue(propertyID);

if (!value)

return String();

return value->cssText();

@@ -338,7 +338,7 @@ String StylePropertySerializer::asText() const

shorthandPropertyID = CSSPropertyWebkitMask;

break;

case CSSPropertyAll:

- result.append(getPropertyText(propertyID, property.value()->cssText(), property.isImportant(), numDecls++));

+ result.append(getPropertyText(propertyID, property.value().cssText(), property.isImportant(), numDecls++));

continue;

default:

break;

@@ -361,9 +361,9 @@ String StylePropertySerializer::asText() const

} else {

// We should not show "initial" when the "initial" is implicit.

// If explicit "initial", we need to show.

- if (property.value()->isImplicitInitialValue())

+ if (property.value().isImplicitInitialValue())

continue;

- value = property.value()->cssText();

+ value = property.value().cssText();

}

result.append(getPropertyText(propertyID, value, property.isImportant(), numDecls++));

@@ -449,7 +449,7 @@ String StylePropertySerializer::getPropertyValue(CSSPropertyID propertyID) const

case CSSPropertyWebkitTextStroke:

return getShorthandValue(webkitTextStrokeShorthand());

case CSSPropertyMarker: {

- if (const CSSValue* value = m_propertySet.getPropertyCSSValue(CSSPropertyMarkerStart))

+ if (const NullableCSSValue value = m_propertySet.getPropertyCSSValue(CSSPropertyMarkerStart))

return value->cssText();

return String();

}

@@ -462,8 +462,8 @@ String StylePropertySerializer::getPropertyValue(CSSPropertyID propertyID) const

String StylePropertySerializer::borderSpacingValue(const StylePropertyShorthand& shorthand) const

{

- const CSSValue* horizontalValue = m_propertySet.getPropertyCSSValue(shorthand.properties()[0]);

- const CSSValue* verticalValue = m_propertySet.getPropertyCSSValue(shorthand.properties()[1]);

+ const NullableCSSValue horizontalValue = m_propertySet.getPropertyCSSValue(shorthand.properties()[0]);

+ const NullableCSSValue verticalValue = m_propertySet.getPropertyCSSValue(shorthand.properties()[1]);

// While standard border-spacing property does not allow specifying border-spacing-vertical without

// specifying border-spacing-horizontal <http://www.w3.org/TR/CSS21/tables.html#separated-borders>,

@@ -508,7 +508,7 @@ void StylePropertySerializer::appendFontLonghandValueIfExplicit(CSSPropertyID pr

if (prefix && !result.isEmpty())

result.append(prefix);

- String value = m_propertySet.propertyAt(foundPropertyIndex).value()->cssText();

+ String value = m_propertySet.propertyAt(foundPropertyIndex).value().cssText();

result.append(value);

if (!commonValue.isNull() && commonValue != value)

commonValue = String();

@@ -526,7 +526,7 @@ String StylePropertySerializer::fontValue() const

if (fontSizeProperty.isImplicit() || fontFamilyProperty.isImplicit())

return emptyString();

- String commonValue = fontSizeProperty.value()->cssText();

+ String commonValue = fontSizeProperty.value().cssText();

StringBuilder result;

appendFontLonghandValueIfExplicit(CSSPropertyFontStyle, result, commonValue);

appendFontLonghandValueIfExplicit(CSSPropertyFontVariant, result, commonValue);

@@ -534,11 +534,11 @@ String StylePropertySerializer::fontValue() const

appendFontLonghandValueIfExplicit(CSSPropertyFontStretch, result, commonValue);

if (!result.isEmpty())

result.append(' ');

- result.append(fontSizeProperty.value()->cssText());

+ result.append(fontSizeProperty.value().cssText());

appendFontLonghandValueIfExplicit(CSSPropertyLineHeight, result, commonValue);

if (!result.isEmpty())

result.append(' ');

- result.append(fontFamilyProperty.value()->cssText());

+ result.append(fontFamilyProperty.value().cssText());

if (isInitialOrInherit(commonValue))

return commonValue;

return result.toString();

@@ -560,39 +560,35 @@ String StylePropertySerializer::get4Values(const StylePropertyShorthand& shortha

PropertyValueForSerializer bottom = m_propertySet.propertyAt(bottomValueIndex);

PropertyValueForSerializer left = m_propertySet.propertyAt(leftValueIndex);

- // All 4 properties must be specified.

- if (!top.value() || !right.value() || !bottom.value() || !left.value())

- return String();

if (top.isImportant() != right.isImportant() || right.isImportant() != bottom.isImportant() || bottom.isImportant() != left.isImportant())

return String();

if (top.isInherited() && right.isInherited() && bottom.isInherited() && left.isInherited())

return getValueName(CSSValueInherit);

- unsigned numInitial = top.value()->isInitialValue() + right.value()->isInitialValue() + bottom.value()->isInitialValue() + left.value()->isInitialValue();

+ unsigned numInitial = top.value().isInitialValue() + right.value().isInitialValue() + bottom.value().isInitialValue() + left.value().isInitialValue();

if (numInitial == 4)

return getValueName(CSSValueInitial);

if (numInitial > 0)

return String();

- bool showLeft = !right.value()->equals(*left.value());

- bool showBottom = !top.value()->equals(*bottom.value()) || showLeft;

- bool showRight = !top.value()->equals(*right.value()) || showBottom;

+ bool showLeft = !right.value().equals(left.value());

+ bool showBottom = !top.value().equals(bottom.value()) || showLeft;

+ bool showRight = !top.value().equals(right.value()) || showBottom;

StringBuilder result;

- result.append(top.value()->cssText());

+ result.append(top.value().cssText());

if (showRight) {

result.append(' ');

- result.append(right.value()->cssText());

+ result.append(right.value().cssText());

}

if (showBottom) {

result.append(' ');

- result.append(bottom.value()->cssText());

+ result.append(bottom.value().cssText());

}

if (showLeft) {

result.append(' ');

- result.append(left.value()->cssText());

+ result.append(left.value().cssText());

}

return result.toString();

}

@@ -603,7 +599,7 @@ String StylePropertySerializer::getLayeredShorthandValue(const StylePropertyShor

const unsigned size = shorthand.length();

// Begin by collecting the properties into an array.

- WillBeHeapVector<const CSSValue*> values(size);

+ WillBeHeapVector<NullableCSSValue> values(size);

size_t numLayers = 0;

for (unsigned i = 0; i < size; ++i) {

@@ -633,7 +629,7 @@ String StylePropertySerializer::getLayeredShorthandValue(const StylePropertyShor

bool useSingleWordShorthand = false;

bool foundPositionYCSSProperty = false;

for (unsigned j = 0; j < size; j++) {

- const CSSValue* value = 0;

+ NullableCSSValue value;

if (values[j]) {

if (values[j]->isBaseValueList()) {

value = toCSSValueList(values[j])->itemWithBoundsCheck(i);

@@ -643,10 +639,10 @@ String StylePropertySerializer::getLayeredShorthandValue(const StylePropertyShor

// Color only belongs in the last layer.

if (shorthand.properties()[j] == CSSPropertyBackgroundColor) {

if (i != numLayers - 1)

- value = 0;

+ value = nullptr;

} else if (i) {

// Other singletons only belong in the first layer.

- value = 0;

+ value = nullptr;

}

@@ -660,12 +656,10 @@ String StylePropertySerializer::getLayeredShorthandValue(const StylePropertyShor

// BUG 49055: make sure the value was not reset in the layer check just above.

if ((j < size - 1 && shorthand.properties()[j + 1] == CSSPropertyBackgroundRepeatY && value)

|| (j < size - 1 && shorthand.properties()[j + 1] == CSSPropertyWebkitMaskRepeatY && value)) {

- const CSSValue* yValue = 0;

- const CSSValue* nextValue = values[j + 1];

- if (nextValue->isValueList())

+ NullableCSSValue nextValue = values[j + 1];

+ NullableCSSValue yValue = nextValue;

+ if (nextValue && nextValue->isValueList())

yValue = toCSSValueList(nextValue)->item(i);

- else

- yValue = nextValue;

// background-repeat-x(y) or mask-repeat-x(y) may be like this : "initial, repeat". We can omit the implicit initial values

// before starting to compare their values.

@@ -757,7 +751,7 @@ String StylePropertySerializer::getShorthandValue(const StylePropertyShorthand&

StringBuilder result;

for (unsigned i = 0; i < shorthand.length(); ++i) {

if (!m_propertySet.isPropertyImplicit(shorthand.properties()[i])) {

- const CSSValue* value = m_propertySet.getPropertyCSSValue(shorthand.properties()[i]);

+ const NullableCSSValue value = m_propertySet.getPropertyCSSValue(shorthand.properties()[i]);

if (!value)

return String();

String valueText = value->cssText();

@@ -786,7 +780,7 @@ String StylePropertySerializer::getCommonValue(const StylePropertyShorthand& sho

String res;

bool lastPropertyWasImportant = false;

for (unsigned i = 0; i < shorthand.length(); ++i) {

- const CSSValue* value = m_propertySet.getPropertyCSSValue(shorthand.properties()[i]);

+ const NullableCSSValue value = m_propertySet.getPropertyCSSValue(shorthand.properties()[i]);

// FIXME: CSSInitialValue::cssText should generate the right value.

if (!value)

return String();

@@ -857,8 +851,8 @@ static void appendBackgroundRepeatValue(StringBuilder& builder, const CSSValue&

String StylePropertySerializer::backgroundRepeatPropertyValue() const

{

- const CSSValue* repeatX = m_propertySet.getPropertyCSSValue(CSSPropertyBackgroundRepeatX);

- const CSSValue* repeatY = m_propertySet.getPropertyCSSValue(CSSPropertyBackgroundRepeatY);

+ const NullableCSSValue repeatX = m_propertySet.getPropertyCSSValue(CSSPropertyBackgroundRepeatX);

+ const NullableCSSValue repeatY = m_propertySet.getPropertyCSSValue(CSSPropertyBackgroundRepeatY);

if (!repeatX || !repeatY)

return String();

if (m_propertySet.propertyIsImportant(CSSPropertyBackgroundRepeatX) != m_propertySet.propertyIsImportant(CSSPropertyBackgroundRepeatY))

@@ -890,9 +884,9 @@ String StylePropertySerializer::backgroundRepeatPropertyValue() const

if (i)

builder.appendLiteral(", ");

- const CSSValue* xValue = repeatXList ? repeatXList->item(i % repeatXList->length()) : repeatX;

- const CSSValue* yValue = repeatYList ? repeatYList->item(i % repeatYList->length()) : repeatY;

- appendBackgroundRepeatValue(builder, *xValue, *yValue);

+ const CSSValue xValue = repeatXList ? repeatXList->item(i % repeatXList->length()) : *repeatX;

+ const CSSValue yValue = repeatYList ? repeatYList->item(i % repeatYList->length()) : *repeatY;

+ appendBackgroundRepeatValue(builder, xValue, yValue);

}

return builder.toString();

}

@@ -906,7 +900,8 @@ void StylePropertySerializer::appendBackgroundPropertyAsText(StringBuilder& resu

return;

}

if (shorthandHasOnlyInitialOrInheritedValue(backgroundShorthand())) {

- const CSSValue* value = m_propertySet.getPropertyCSSValue(CSSPropertyBackgroundImage);

+ const NullableCSSValue value = m_propertySet.getPropertyCSSValue(CSSPropertyBackgroundImage);

+ ASSERT(value);

bool isImportant = m_propertySet.propertyIsImportant(CSSPropertyBackgroundImage);

result.append(getPropertyText(CSSPropertyBackground, value->cssText(), isImportant, numDecls++));

return;

@@ -924,7 +919,7 @@ void StylePropertySerializer::appendBackgroundPropertyAsText(StringBuilder& resu

for (unsigned i = 0; i < WTF_ARRAY_LENGTH(backgroundPropertyIds); ++i) {

CSSPropertyID propertyID = backgroundPropertyIds[i];

- const CSSValue* value = m_propertySet.getPropertyCSSValue(propertyID);

+ const NullableCSSValue value = m_propertySet.getPropertyCSSValue(propertyID);

if (!value)

continue;

result.append(getPropertyText(propertyID, value->cssText(), m_propertySet.propertyIsImportant(propertyID), numDecls++));

@@ -935,7 +930,8 @@ void StylePropertySerializer::appendBackgroundPropertyAsText(StringBuilder& resu

// would not work in Firefox (<rdar://problem/5143183>)

// It would be a better solution if background-position was CSS_PAIR.

if (shorthandHasOnlyInitialOrInheritedValue(backgroundPositionShorthand())) {

- const CSSValue* value = m_propertySet.getPropertyCSSValue(CSSPropertyBackgroundPositionX);

+ const NullableCSSValue value = m_propertySet.getPropertyCSSValue(CSSPropertyBackgroundPositionX);

+ ASSERT(value);

bool isImportant = m_propertySet.propertyIsImportant(CSSPropertyBackgroundPositionX);

result.append(getPropertyText(CSSPropertyBackgroundPosition, value->cssText(), isImportant, numDecls++));

} else if (isPropertyShorthandAvailable(backgroundPositionShorthand())) {

@@ -945,14 +941,14 @@ void StylePropertySerializer::appendBackgroundPropertyAsText(StringBuilder& resu

result.append(getPropertyText(CSSPropertyBackgroundPosition, positionValue, isImportant, numDecls++));

} else {

// should check background-position-x or background-position-y.

- if (const CSSValue* value = m_propertySet.getPropertyCSSValue(CSSPropertyBackgroundPositionX)) {

- if (!value->isImplicitInitialValue()) {

+ if (const NullableCSSValue value = m_propertySet.getPropertyCSSValue(CSSPropertyBackgroundPositionX)) {

+ if (value && !value->isImplicitInitialValue()) {

bool isImportant = m_propertySet.propertyIsImportant(CSSPropertyBackgroundPositionX);

result.append(getPropertyText(CSSPropertyBackgroundPositionX, value->cssText(), isImportant, numDecls++));

}

- if (const CSSValue* value = m_propertySet.getPropertyCSSValue(CSSPropertyBackgroundPositionY)) {

- if (!value->isImplicitInitialValue()) {

+ if (const NullableCSSValue value = m_propertySet.getPropertyCSSValue(CSSPropertyBackgroundPositionY)) {

+ if (value && !value->isImplicitInitialValue()) {

bool isImportant = m_propertySet.propertyIsImportant(CSSPropertyBackgroundPositionY);

result.append(getPropertyText(CSSPropertyBackgroundPositionY, value->cssText(), isImportant, numDecls++));

}

@@ -970,7 +966,7 @@ bool StylePropertySerializer::isPropertyShorthandAvailable(const StylePropertySh

bool isImportant = m_propertySet.propertyIsImportant(shorthand.properties()[0]);

for (unsigned i = 0; i < shorthand.length(); ++i) {

- const CSSValue* value = m_propertySet.getPropertyCSSValue(shorthand.properties()[i]);

+ const NullableCSSValue value = m_propertySet.getPropertyCSSValue(shorthand.properties()[i]);

if (!value || (value->isInitialValue() && !value->isImplicitInitialValue()) || value->isInheritedValue())

return false;

if (isImportant != m_propertySet.propertyIsImportant(shorthand.properties()[i]))

@@ -986,7 +982,7 @@ bool StylePropertySerializer::shorthandHasOnlyInitialOrInheritedValue(const Styl

bool isInitialValue = true;

bool isInheritedValue = true;

for (unsigned i = 0; i < shorthand.length(); ++i) {

- const CSSValue* value = m_propertySet.getPropertyCSSValue(shorthand.properties()[i]);

+ const NullableCSSValue value = m_propertySet.getPropertyCSSValue(shorthand.properties()[i]);

if (!value)

return false;

if (!value->isInitialValue())

« Source/core/css/CSSValue.h ('K') | « Source/core/css/StylePropertySerializer.h ('k') | Source/core/css/StylePropertySet.h » ('j') | Source/core/css/parser/CSSParserImpl.cpp » ('J')