Title: [201690] trunk/Source/WebCore
- Revision
- 201690
- Author
- [email protected]
- Date
- 2016-06-04 16:02:48 -0700 (Sat, 04 Jun 2016)
Log Message
leaks seen in fast/css/variables tests
https://bugs.webkit.org/show_bug.cgi?id=150728
Reviewed by Anders Carlsson.
Fixes leaks seen running fast/css/variables tests with leak checking turned on.
* css/CSSPrimitiveValue.cpp:
(WebCore::isStringType): Added. For debugging purposes so we catch cases where we
are not treating strings consistently between construction and destruction.
(WebCore::CSSPrimitiveValue::CSSPrimitiveValue): Assert isStringType returns true.
(WebCore::CSSPrimitiveValue::cleanup): Added CSS_DIMENSION and CSS_PARSER_IDENTIFIER
to the list of types that have to decrement the reference count of the string we own.
Both types are passed to the string constructor above.
* css/CSSValueList.cpp:
(WebCore::CSSValueList::buildParserValueListSubstitutingVariables): Restructured the
code so we destroy any CSSParserValue that we don't use. This is needed because of the
peculiar requirements of CSSParserValue: it has a be a struct without a destructor so
it can be used in the CSS grammar, so we have to destroy it explicitly. Ideally we would
minimize any use of it outside the CSSParser itself, but as long as we are using it, we
need to do this explicit destruction.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (201689 => 201690)
--- trunk/Source/WebCore/ChangeLog 2016-06-04 22:48:07 UTC (rev 201689)
+++ trunk/Source/WebCore/ChangeLog 2016-06-04 23:02:48 UTC (rev 201690)
@@ -1,3 +1,28 @@
+2016-06-04 Darin Adler <[email protected]>
+
+ leaks seen in fast/css/variables tests
+ https://bugs.webkit.org/show_bug.cgi?id=150728
+
+ Reviewed by Anders Carlsson.
+
+ Fixes leaks seen running fast/css/variables tests with leak checking turned on.
+
+ * css/CSSPrimitiveValue.cpp:
+ (WebCore::isStringType): Added. For debugging purposes so we catch cases where we
+ are not treating strings consistently between construction and destruction.
+ (WebCore::CSSPrimitiveValue::CSSPrimitiveValue): Assert isStringType returns true.
+ (WebCore::CSSPrimitiveValue::cleanup): Added CSS_DIMENSION and CSS_PARSER_IDENTIFIER
+ to the list of types that have to decrement the reference count of the string we own.
+ Both types are passed to the string constructor above.
+
+ * css/CSSValueList.cpp:
+ (WebCore::CSSValueList::buildParserValueListSubstitutingVariables): Restructured the
+ code so we destroy any CSSParserValue that we don't use. This is needed because of the
+ peculiar requirements of CSSParserValue: it has a be a struct without a destructor so
+ it can be used in the CSS grammar, so we have to destroy it explicitly. Ideally we would
+ minimize any use of it outside the CSSParser itself, but as long as we are using it, we
+ need to do this explicit destruction.
+
2016-06-04 Anders Carlsson <[email protected]>
32-bit build fix
Modified: trunk/Source/WebCore/css/CSSPrimitiveValue.cpp (201689 => 201690)
--- trunk/Source/WebCore/css/CSSPrimitiveValue.cpp 2016-06-04 22:48:07 UTC (rev 201689)
+++ trunk/Source/WebCore/css/CSSPrimitiveValue.cpp 2016-06-04 23:02:48 UTC (rev 201690)
@@ -64,18 +64,15 @@
{
switch (unitType) {
case CSSPrimitiveValue::CSS_CALC:
+ case CSSPrimitiveValue::CSS_CALC_PERCENTAGE_WITH_LENGTH:
case CSSPrimitiveValue::CSS_CALC_PERCENTAGE_WITH_NUMBER:
- case CSSPrimitiveValue::CSS_CALC_PERCENTAGE_WITH_LENGTH:
+ case CSSPrimitiveValue::CSS_CHS:
case CSSPrimitiveValue::CSS_CM:
case CSSPrimitiveValue::CSS_DEG:
case CSSPrimitiveValue::CSS_DIMENSION:
-#if ENABLE(CSS_IMAGE_RESOLUTION) || ENABLE(RESOLUTION_MEDIA_QUERY)
- case CSSPrimitiveValue::CSS_DPPX:
- case CSSPrimitiveValue::CSS_DPI:
- case CSSPrimitiveValue::CSS_DPCM:
-#endif
case CSSPrimitiveValue::CSS_EMS:
case CSSPrimitiveValue::CSS_EXS:
+ case CSSPrimitiveValue::CSS_FR:
case CSSPrimitiveValue::CSS_GRAD:
case CSSPrimitiveValue::CSS_HZ:
case CSSPrimitiveValue::CSS_IN:
@@ -83,53 +80,126 @@
case CSSPrimitiveValue::CSS_MM:
case CSSPrimitiveValue::CSS_MS:
case CSSPrimitiveValue::CSS_NUMBER:
+ case CSSPrimitiveValue::CSS_PC:
case CSSPrimitiveValue::CSS_PERCENTAGE:
- case CSSPrimitiveValue::CSS_PC:
case CSSPrimitiveValue::CSS_PT:
case CSSPrimitiveValue::CSS_PX:
case CSSPrimitiveValue::CSS_RAD:
case CSSPrimitiveValue::CSS_REMS:
- case CSSPrimitiveValue::CSS_CHS:
case CSSPrimitiveValue::CSS_S:
case CSSPrimitiveValue::CSS_TURN:
- case CSSPrimitiveValue::CSS_VW:
case CSSPrimitiveValue::CSS_VH:
+ case CSSPrimitiveValue::CSS_VMAX:
case CSSPrimitiveValue::CSS_VMIN:
- case CSSPrimitiveValue::CSS_VMAX:
- case CSSPrimitiveValue::CSS_FR:
+ case CSSPrimitiveValue::CSS_VW:
return true;
+ case CSSPrimitiveValue::CSS_DPCM:
+ case CSSPrimitiveValue::CSS_DPI:
+ case CSSPrimitiveValue::CSS_DPPX:
+#if ENABLE(CSS_IMAGE_RESOLUTION) || ENABLE(RESOLUTION_MEDIA_QUERY)
+ return true;
+#else
+ return false;
+#endif
case CSSPrimitiveValue::CSS_ATTR:
case CSSPrimitiveValue::CSS_COUNTER:
case CSSPrimitiveValue::CSS_COUNTER_NAME:
-#if ENABLE(DASHBOARD_SUPPORT)
- case CSSPrimitiveValue::CSS_DASHBOARD_REGION:
-#endif
-#if !ENABLE(CSS_IMAGE_RESOLUTION) && !ENABLE(RESOLUTION_MEDIA_QUERY)
- case CSSPrimitiveValue::CSS_DPPX:
- case CSSPrimitiveValue::CSS_DPI:
- case CSSPrimitiveValue::CSS_DPCM:
-#endif
+ case CSSPrimitiveValue::CSS_FONT_FAMILY:
case CSSPrimitiveValue::CSS_IDENT:
- case CSSPrimitiveValue::CSS_PROPERTY_ID:
- case CSSPrimitiveValue::CSS_VALUE_ID:
case CSSPrimitiveValue::CSS_PAIR:
case CSSPrimitiveValue::CSS_PARSER_HEXCOLOR:
case CSSPrimitiveValue::CSS_PARSER_IDENTIFIER:
case CSSPrimitiveValue::CSS_PARSER_INTEGER:
case CSSPrimitiveValue::CSS_PARSER_OPERATOR:
case CSSPrimitiveValue::CSS_PARSER_WHITESPACE:
+ case CSSPrimitiveValue::CSS_PROPERTY_ID:
+ case CSSPrimitiveValue::CSS_QUAD:
case CSSPrimitiveValue::CSS_RECT:
- case CSSPrimitiveValue::CSS_QUAD:
+ case CSSPrimitiveValue::CSS_RGBCOLOR:
+ case CSSPrimitiveValue::CSS_SHAPE:
+ case CSSPrimitiveValue::CSS_STRING:
+ case CSSPrimitiveValue::CSS_UNICODE_RANGE:
+ case CSSPrimitiveValue::CSS_UNKNOWN:
+ case CSSPrimitiveValue::CSS_URI:
+ case CSSPrimitiveValue::CSS_VALUE_ID:
#if ENABLE(CSS_SCROLL_SNAP)
case CSSPrimitiveValue::CSS_LENGTH_REPEAT:
#endif
+#if ENABLE(DASHBOARD_SUPPORT)
+ case CSSPrimitiveValue::CSS_DASHBOARD_REGION:
+#endif
+ return false;
+ }
+
+ ASSERT_NOT_REACHED();
+ return false;
+}
+
+#if !ASSERT_DISABLED
+
+static inline bool isStringType(CSSPrimitiveValue::UnitTypes type)
+{
+ switch (type) {
+ case CSSPrimitiveValue::CSS_STRING:
+ case CSSPrimitiveValue::CSS_URI:
+ case CSSPrimitiveValue::CSS_ATTR:
+ case CSSPrimitiveValue::CSS_COUNTER_NAME:
+ case CSSPrimitiveValue::CSS_DIMENSION:
+ case CSSPrimitiveValue::CSS_PARSER_HEXCOLOR:
+ case CSSPrimitiveValue::CSS_PARSER_IDENTIFIER:
+ case CSSPrimitiveValue::CSS_PARSER_WHITESPACE:
+ return true;
+ case CSSPrimitiveValue::CSS_CALC:
+ case CSSPrimitiveValue::CSS_CALC_PERCENTAGE_WITH_LENGTH:
+ case CSSPrimitiveValue::CSS_CALC_PERCENTAGE_WITH_NUMBER:
+ case CSSPrimitiveValue::CSS_CHS:
+ case CSSPrimitiveValue::CSS_CM:
+ case CSSPrimitiveValue::CSS_COUNTER:
+ case CSSPrimitiveValue::CSS_DEG:
+ case CSSPrimitiveValue::CSS_DPCM:
+ case CSSPrimitiveValue::CSS_DPI:
+ case CSSPrimitiveValue::CSS_DPPX:
+ case CSSPrimitiveValue::CSS_EMS:
+ case CSSPrimitiveValue::CSS_EXS:
+ case CSSPrimitiveValue::CSS_FONT_FAMILY:
+ case CSSPrimitiveValue::CSS_FR:
+ case CSSPrimitiveValue::CSS_GRAD:
+ case CSSPrimitiveValue::CSS_HZ:
+ case CSSPrimitiveValue::CSS_IDENT:
+ case CSSPrimitiveValue::CSS_IN:
+ case CSSPrimitiveValue::CSS_KHZ:
+ case CSSPrimitiveValue::CSS_MM:
+ case CSSPrimitiveValue::CSS_MS:
+ case CSSPrimitiveValue::CSS_NUMBER:
+ case CSSPrimitiveValue::CSS_PAIR:
+ case CSSPrimitiveValue::CSS_PARSER_INTEGER:
+ case CSSPrimitiveValue::CSS_PARSER_OPERATOR:
+ case CSSPrimitiveValue::CSS_PC:
+ case CSSPrimitiveValue::CSS_PERCENTAGE:
+ case CSSPrimitiveValue::CSS_PROPERTY_ID:
+ case CSSPrimitiveValue::CSS_PT:
+ case CSSPrimitiveValue::CSS_PX:
+ case CSSPrimitiveValue::CSS_QUAD:
+ case CSSPrimitiveValue::CSS_RAD:
+ case CSSPrimitiveValue::CSS_RECT:
+ case CSSPrimitiveValue::CSS_REMS:
case CSSPrimitiveValue::CSS_RGBCOLOR:
+ case CSSPrimitiveValue::CSS_S:
case CSSPrimitiveValue::CSS_SHAPE:
- case CSSPrimitiveValue::CSS_STRING:
- case CSSPrimitiveValue::CSS_FONT_FAMILY:
+ case CSSPrimitiveValue::CSS_TURN:
case CSSPrimitiveValue::CSS_UNICODE_RANGE:
case CSSPrimitiveValue::CSS_UNKNOWN:
- case CSSPrimitiveValue::CSS_URI:
+ case CSSPrimitiveValue::CSS_VALUE_ID:
+ case CSSPrimitiveValue::CSS_VH:
+ case CSSPrimitiveValue::CSS_VMAX:
+ case CSSPrimitiveValue::CSS_VMIN:
+ case CSSPrimitiveValue::CSS_VW:
+#if ENABLE(DASHBOARD_SUPPORT)
+ case CSSPrimitiveValue::CSS_DASHBOARD_REGION:
+#endif
+#if ENABLE(CSS_SCROLL_SNAP)
+ case CSSPrimitiveValue::CSS_LENGTH_REPEAT:
+#endif
return false;
}
@@ -137,6 +207,8 @@
return false;
}
+#endif // !ASSERT_DISABLED
+
CSSPrimitiveValue::UnitCategory CSSPrimitiveValue::unitCategory(CSSPrimitiveValue::UnitTypes type)
{
// Here we violate the spec (http://www.w3.org/TR/DOM-Level-2-Style/css.html#CSS-CSSPrimitiveValue) and allow conversions
@@ -267,11 +339,12 @@
m_value.num = num;
}
-CSSPrimitiveValue::CSSPrimitiveValue(const String& str, UnitTypes type)
+CSSPrimitiveValue::CSSPrimitiveValue(const String& string, UnitTypes type)
: CSSValue(PrimitiveClass)
{
+ ASSERT(isStringType(type));
m_primitiveUnitType = type;
- if ((m_value.string = str.impl()))
+ if ((m_value.string = string.impl()))
m_value.string->ref();
}
@@ -447,13 +520,17 @@
void CSSPrimitiveValue::cleanup()
{
- switch (static_cast<UnitTypes>(m_primitiveUnitType)) {
+ auto type = static_cast<UnitTypes>(m_primitiveUnitType);
+ switch (type) {
case CSS_STRING:
case CSS_URI:
case CSS_ATTR:
case CSS_COUNTER_NAME:
+ case CSS_DIMENSION:
case CSS_PARSER_HEXCOLOR:
+ case CSS_PARSER_IDENTIFIER:
case CSS_PARSER_WHITESPACE:
+ ASSERT(isStringType(type));
if (m_value.string)
m_value.string->deref();
break;
@@ -526,13 +603,12 @@
case CSS_FR:
case CSS_IDENT:
case CSS_RGBCOLOR:
- case CSS_DIMENSION:
case CSS_UNKNOWN:
case CSS_UNICODE_RANGE:
case CSS_PARSER_OPERATOR:
- case CSS_PARSER_IDENTIFIER:
case CSS_PROPERTY_ID:
case CSS_VALUE_ID:
+ ASSERT(!isStringType(type));
break;
}
m_primitiveUnitType = 0;
Modified: trunk/Source/WebCore/css/CSSValueList.cpp (201689 => 201690)
--- trunk/Source/WebCore/css/CSSValueList.cpp 2016-06-04 22:48:07 UTC (rev 201689)
+++ trunk/Source/WebCore/css/CSSValueList.cpp 2016-06-04 23:02:48 UTC (rev 201690)
@@ -247,33 +247,45 @@
bool CSSValueList::buildParserValueListSubstitutingVariables(CSSParserValueList* parserList, const CustomPropertyValueMap& customProperties) const
{
- for (unsigned i = 0; i < m_values.size(); ++i) {
- CSSParserValue result;
- result.id = CSSValueInvalid;
- switch (m_values[i]->classType()) {
- case FunctionClass:
- if (!downcast<CSSFunctionValue>(*m_values[i].ptr()).buildParserValueSubstitutingVariables(&result, customProperties))
+ for (auto& value : m_values) {
+ switch (value.get().classType()) {
+ case FunctionClass: {
+ CSSParserValue result;
+ result.id = CSSValueInvalid;
+ if (!downcast<CSSFunctionValue>(value.get()).buildParserValueSubstitutingVariables(&result, customProperties)) {
+ WebCore::destroy(result);
return false;
+ }
parserList->addValue(result);
break;
- case ValueListClass:
- if (!downcast<CSSValueList>(*m_values[i].ptr()).buildParserValueSubstitutingVariables(&result, customProperties))
+ }
+ case ValueListClass: {
+ CSSParserValue result;
+ result.id = CSSValueInvalid;
+ if (!downcast<CSSValueList>(value.get()).buildParserValueSubstitutingVariables(&result, customProperties)) {
+ WebCore::destroy(result);
return false;
+ }
parserList->addValue(result);
break;
+ }
case VariableClass: {
- if (!downcast<CSSVariableValue>(*m_values[i].ptr()).buildParserValueListSubstitutingVariables(parserList, customProperties))
+ if (!downcast<CSSVariableValue>(value.get()).buildParserValueListSubstitutingVariables(parserList, customProperties))
return false;
break;
}
- case PrimitiveClass:
+ case PrimitiveClass: {
// FIXME: Will have to change this if we start preserving invalid tokens.
- if (downcast<CSSPrimitiveValue>(*m_values[i].ptr()).buildParserValue(&result))
+ CSSParserValue result;
+ result.id = CSSValueInvalid;
+ if (downcast<CSSPrimitiveValue>(value.get()).buildParserValue(&result))
parserList->addValue(result);
+ else
+ WebCore::destroy(result);
break;
+ }
default:
ASSERT_NOT_REACHED();
- break;
return false;
}
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
