REGRESSION (r170576): Storage leaks in parsing of CSS image sizes
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 2 Feb 2015 18:12:32 +0000 (18:12 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 2 Feb 2015 18:12:32 +0000 (18:12 +0000)
https://bugs.webkit.org/show_bug.cgi?id=141026

Reviewed by Anders Carlsson.

* css/CSSGrammar.y.in: Fixed all the shift/reduce conflicts caused
by the ENABLE_PICTURE_SIZES code by removing all the redundant
maybe_space which caused them. Rearranged the productions for
ENABLE_PICTURE_SIZES to tighten up the code quite a bit. Changed
the code to build up the source size vector as a Vector instead of
a special class, and use the SourceSize struct from inside the
CSSParser class.'

* css/CSSParser.cpp:
(WebCore::CSSParser::setupParser): Changed this to take a StringView.
In the future we can change all the parsing functions to take StringView,
since they don't work with the String in place.
(WebCore::CSSParser::parseSizesAttribute): Changed to return a vector
of SourceSize instead of a SourceSizeList. This is better because it's
a real CSS data structure that does not contain a CSSParserValue.
(WebCore::CSSParser::sourceSize): Added. Helper that creates a
SourceSize, mapping parser data structures into real CSS ones.

* css/CSSParser.h: Updated for changes above.

* css/MediaQuery.cpp:
(WebCore::MediaQuery::MediaQuery): Use std::make_unique and the copy
constructor directly instead of using a MediaQuery::copy function.

* css/MediaQueryExp.cpp: Streamlined the class a little bit.
* css/MediaQueryExp.h: Removed unneeded includes. Moved functions out
of the class body so the class is easier to read. Removed the unneeded
copy function.

* css/SourceSizeList.cpp:
(WebCore::SourceSize::match): Changed to use WTF::move instead
of releasing and then re-creating the unique_ptr.
(WebCore::computeLength): Added a comment to explain this function
is using an incorrect strategy. Also added some type checking code
to handle cases where a null or non-primitive CSS value might be
returned. Probably dead code, but we don't want to risk a bad cast.
Worthe cleaning up when we fix the strategy.
(WebCore::SourceSizeList::getEffectiveSize): Updated since the
vector now contains actual SourceSize objects rather than pointers
to SourceSize objects on the heap.

* css/SourceSizeList.h: Changed the CSSParserValue argument to be
an rvalue reference to make it clearer that we take ownership of it
when it's moved in. Added a move constructor and a destructor. Added
comments explaining that it's not correct design to use a
CSSParserValue here, outside the parser. Changed SourceSizeList's
append function to move a SourceSize in rather than a unique_ptr.
Made getEffectiveSize private. Moved the various inline functions to
the bottom of the file to make the class definitions easier to read.

* css/SourceSizeList.cpp: Made almost everything about this private
to this source file instead of public in the header.
(WebCore::match): Made this a free function instead of a member function
and made it take the media query expression as an argument.
(WebCore::computeLength): Changed the argument type to CSSValue*,
rather than using CSSParserValue here outside the parser.
(WebCore::parseSizesAttribute): Streamlined and simplified this.
Now that the parser builds the list in the correct order, there was
no need to iterate backwards any more so we could use a modern for
loop.

* css/SourceSizeList.h: Removed almost everything in this header.

* html/HTMLImageElement.cpp:
(WebCore::HTMLImageElement::parseAttribute): Call the
parseSizesAttribute function as free function since it's no longer
a member of a SourceSizeList class.

* html/parser/HTMLPreloadScanner.cpp:
(WebCore::TokenPreloadScanner::StartTagScanner::processAttributes):
Ditto.

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

Source/WebCore/ChangeLog
Source/WebCore/css/CSSGrammar.y.in
Source/WebCore/css/CSSParser.cpp
Source/WebCore/css/CSSParser.h
Source/WebCore/css/MediaQuery.cpp
Source/WebCore/css/MediaQueryExp.cpp
Source/WebCore/css/MediaQueryExp.h
Source/WebCore/css/SourceSizeList.cpp
Source/WebCore/css/SourceSizeList.h
Source/WebCore/html/HTMLImageElement.cpp
Source/WebCore/html/parser/HTMLPreloadScanner.cpp

index 6db065b..10b49f1 100644 (file)
@@ -1,5 +1,85 @@
 2015-02-02  Darin Adler  <darin@apple.com>
 
+        REGRESSION (r170576): Storage leaks in parsing of CSS image sizes
+        https://bugs.webkit.org/show_bug.cgi?id=141026
+
+        Reviewed by Anders Carlsson.
+
+        * css/CSSGrammar.y.in: Fixed all the shift/reduce conflicts caused
+        by the ENABLE_PICTURE_SIZES code by removing all the redundant
+        maybe_space which caused them. Rearranged the productions for
+        ENABLE_PICTURE_SIZES to tighten up the code quite a bit. Changed
+        the code to build up the source size vector as a Vector instead of
+        a special class, and use the SourceSize struct from inside the
+        CSSParser class.'
+
+        * css/CSSParser.cpp:
+        (WebCore::CSSParser::setupParser): Changed this to take a StringView.
+        In the future we can change all the parsing functions to take StringView,
+        since they don't work with the String in place.
+        (WebCore::CSSParser::parseSizesAttribute): Changed to return a vector
+        of SourceSize instead of a SourceSizeList. This is better because it's
+        a real CSS data structure that does not contain a CSSParserValue.
+        (WebCore::CSSParser::sourceSize): Added. Helper that creates a
+        SourceSize, mapping parser data structures into real CSS ones.
+
+        * css/CSSParser.h: Updated for changes above.
+
+        * css/MediaQuery.cpp:
+        (WebCore::MediaQuery::MediaQuery): Use std::make_unique and the copy
+        constructor directly instead of using a MediaQuery::copy function.
+
+        * css/MediaQueryExp.cpp: Streamlined the class a little bit.
+        * css/MediaQueryExp.h: Removed unneeded includes. Moved functions out
+        of the class body so the class is easier to read. Removed the unneeded
+        copy function.
+
+        * css/SourceSizeList.cpp:
+        (WebCore::SourceSize::match): Changed to use WTF::move instead
+        of releasing and then re-creating the unique_ptr.
+        (WebCore::computeLength): Added a comment to explain this function
+        is using an incorrect strategy. Also added some type checking code
+        to handle cases where a null or non-primitive CSS value might be
+        returned. Probably dead code, but we don't want to risk a bad cast.
+        Worthe cleaning up when we fix the strategy.
+        (WebCore::SourceSizeList::getEffectiveSize): Updated since the
+        vector now contains actual SourceSize objects rather than pointers
+        to SourceSize objects on the heap.
+
+        * css/SourceSizeList.h: Changed the CSSParserValue argument to be
+        an rvalue reference to make it clearer that we take ownership of it
+        when it's moved in. Added a move constructor and a destructor. Added
+        comments explaining that it's not correct design to use a
+        CSSParserValue here, outside the parser. Changed SourceSizeList's
+        append function to move a SourceSize in rather than a unique_ptr.
+        Made getEffectiveSize private. Moved the various inline functions to
+        the bottom of the file to make the class definitions easier to read.
+
+
+        * css/SourceSizeList.cpp: Made almost everything about this private
+        to this source file instead of public in the header.
+        (WebCore::match): Made this a free function instead of a member function
+        and made it take the media query expression as an argument.
+        (WebCore::computeLength): Changed the argument type to CSSValue*,
+        rather than using CSSParserValue here outside the parser.
+        (WebCore::parseSizesAttribute): Streamlined and simplified this.
+        Now that the parser builds the list in the correct order, there was
+        no need to iterate backwards any more so we could use a modern for
+        loop.
+
+        * css/SourceSizeList.h: Removed almost everything in this header.
+
+        * html/HTMLImageElement.cpp:
+        (WebCore::HTMLImageElement::parseAttribute): Call the
+        parseSizesAttribute function as free function since it's no longer
+        a member of a SourceSizeList class.
+
+        * html/parser/HTMLPreloadScanner.cpp:
+        (WebCore::TokenPreloadScanner::StartTagScanner::processAttributes):
+        Ditto.
+
+2015-02-02  Darin Adler  <darin@apple.com>
+
         Fix some leaks found by the leak bot
         https://bugs.webkit.org/show_bug.cgi?id=141149
 
index 442fad7..345ca0c 100644 (file)
@@ -1,6 +1,6 @@
 /*
  *  Copyright (C) 2002-2003 Lars Knoll (knoll@kde.org)
- *  Copyright (C) 2004-2014 Apple Inc. All rights reserved.
+ *  Copyright (C) 2004-2015 Apple Inc. All rights reserved.
  *  Copyright (C) 2006 Alexey Proskuryakov (ap@nypop.com)
  *  Copyright (C) 2008 Eric Seidel <eric@webkit.org>
  *  Copyright (C) 2012 Intel Corporation. All rights reserved.
@@ -104,18 +104,10 @@ static bool selectorListDoesNotMatchAnyPseudoElement(const Vector<std::unique_pt
 %}
 
 #if ENABLE_CSS_GRID_LAYOUT
-#if ENABLE_PICTURE_SIZES
-%expect 36
-#else
 %expect 32
-#endif
-#else
-#if ENABLE_PICTURE_SIZES
-%expect 35
 #else
 %expect 31
 #endif
-#endif
 
 %nonassoc LOWEST_PREC
 
@@ -265,23 +257,21 @@ static bool selectorListDoesNotMatchAnyPseudoElement(const Vector<std::unique_pt
 %type <mediaQueryRestrictor> maybe_media_restrictor
 
 %union { MediaQueryExp* mediaQueryExp; }
-%type <mediaQueryExp> media_query_exp
-%destructor { delete $$; } media_query_exp
+%type <mediaQueryExp> media_query_exp base_media_query_exp
+%destructor { delete $$; } media_query_exp base_media_query_exp
 
 #if ENABLE_PICTURE_SIZES
-%union { MediaQueryExp* mediaCondition; }
-%type <mediaQueryExp> media_condition
-%destructor { delete $$; } media_condition
 
-%union { SourceSize* sourceSize; }
-%type <sourceSize> source_size
-%destructor { delete $$; } source_size
-
-%union { SourceSizeList* sourceSizeList; }
+%union { Vector<CSSParser::SourceSize>* sourceSizeList; }
 %type <sourceSizeList> source_size_list
 %destructor { delete $$; } source_size_list
 
+%type <mediaQueryExp> maybe_source_media_query_exp
+%destructor { delete $$; } maybe_source_media_query_exp
+
 %type <value> source_size_length
+%destructor { destroy($$); } source_size_length
+
 #endif
 
 %union { Vector<std::unique_ptr<MediaQueryExp>>* mediaQueryExpList; }
@@ -298,12 +288,12 @@ static bool selectorListDoesNotMatchAnyPseudoElement(const Vector<std::unique_pt
 %type <keyframeRuleList> keyframes_rule
 %destructor { delete $$; } keyframes_rule
 
-// These two parser values never need to be destroyed because they are never functions or value lists.
-%type <value> key unary_term
+// These parser values never need to be destroyed because they are never functions or value lists.
+%type <value> calc_func_term key unary_term
 
 // These parser values need to be destroyed because they might be functions.
-%type <value> calc_func_term calc_function function min_or_max_function term
-%destructor { destroy($$); } calc_func_term calc_function function min_or_max_function term
+%type <value> calc_function function min_or_max_function term
+%destructor { destroy($$); } calc_function function min_or_max_function term
 
 %union { CSSPropertyID id; }
 %type <id> property
@@ -334,10 +324,13 @@ static bool selectorListDoesNotMatchAnyPseudoElement(const Vector<std::unique_pt
 %destructor { delete $$; } calc_func_expr calc_func_expr_list calc_func_paren_expr expr key_list maybe_media_value valid_calc_func_expr valid_expr
 
 #if ENABLE_CSS_SELECTORS_LEVEL4
+
 %type <string> lang_range
+
 %union { Vector<CSSParserString>* stringList; }
 %type <stringList> comma_separated_lang_ranges
 %destructor { delete $$; } comma_separated_lang_ranges
+
 #endif
 
 %type <string> min_or_max
@@ -348,11 +341,13 @@ static bool selectorListDoesNotMatchAnyPseudoElement(const Vector<std::unique_pt
 %type <location> error_location
 
 #if ENABLE_CSS_GRID_LAYOUT
+
 %type <valueList> ident_list
 %destructor { delete $$; } ident_list
 
 %type <value> track_names_list
 %destructor { destroy($$); } track_names_list
+
 #endif
 
 %token SUPPORTS_AND
@@ -577,55 +572,50 @@ string_or_uri: STRING | URI ;
 maybe_media_value: /*empty*/ { $$ = nullptr; } | ':' maybe_space expr maybe_space { $$ = $3; } ;
 
 #if ENABLE_PICTURE_SIZES
-media_condition:
-    maybe_space '(' maybe_space IDENT maybe_space maybe_media_value ')' maybe_space {
-        std::unique_ptr<CSSParserValueList> mediaValue($6);
-        $4.lower();
-        $$ = new MediaQueryExp($4, mediaValue.get());
-    }
-    ;
 
 webkit_source_size_list:
-    WEBKIT_SIZESATTR_SYM WHITESPACE source_size_list '}' { parser->m_sourceSizeList = std::unique_ptr<SourceSizeList>($3); };
+    WEBKIT_SIZESATTR_SYM WHITESPACE source_size_list maybe_space '}' {
+        parser->m_sourceSizeList = std::unique_ptr<Vector<CSSParser::SourceSize>>($3);
+    }
+    ;
 
 source_size_list:
-    maybe_space source_size maybe_space {
-        $$ = new SourceSizeList();
-        $$->append(std::unique_ptr<SourceSize>($2));
+    maybe_source_media_query_exp source_size_length {
+        $$ = new Vector<CSSParser::SourceSize>;
+        $$->append(parser->sourceSize(std::unique_ptr<MediaQueryExp>($1), $2));
     }
-    | maybe_space source_size maybe_space ',' maybe_space source_size_list maybe_space {
-        $$ = $6;
-        $$->append(std::unique_ptr<SourceSize>($2));
-    };
-
-source_size_length:
-    unary_term {
+    | source_size_list maybe_space ',' maybe_space maybe_source_media_query_exp source_size_length {
         $$ = $1;
+        $$->append(parser->sourceSize(std::unique_ptr<MediaQueryExp>($5), $6));
     }
-    | calc_function {
-        $$ = $1;
-    };
+    ;
 
-source_size:
-    media_condition source_size_length {
-        $$ = new SourceSize(std::unique_ptr<MediaQueryExp>($1), $2);
+maybe_source_media_query_exp:
+    /* empty */ {
+        $$ = new MediaQueryExp;
     }
-    | source_size_length {
-        $$ = new SourceSize(std::make_unique<MediaQueryExp>(emptyString(), nullptr), $1);
-    };
+    | base_media_query_exp maybe_space;
+
+source_size_length: unary_term | calc_function;
+
 #endif
 
+base_media_query_exp: '(' maybe_space IDENT maybe_space maybe_media_value ')' {
+        std::unique_ptr<CSSParserValueList> mediaValue($5);
+        $3.lower();
+        $$ = new MediaQueryExp($3, mediaValue.get());
+    }
+    ;
+
 media_query_exp:
-    maybe_media_restrictor maybe_space '(' maybe_space IDENT maybe_space maybe_media_value ')' maybe_space {
-        // If restrictor is specified, media query expression is invalid.
-        // Create empty media query expression and continue parsing media query.
-        std::unique_ptr<CSSParserValueList> mediaValue($7);
-        if ($1 != MediaQuery::None)
-            $$ = new MediaQueryExp(emptyString(), nullptr);
-        else {
-            $5.lower();
-            $$ = new MediaQueryExp($5, mediaValue.get());
-        }
+    maybe_media_restrictor maybe_space base_media_query_exp maybe_space {
+        if ($1 != MediaQuery::None) {
+            // If restrictor is specified, media query expression is invalid.
+            // Create empty media query expression and continue parsing media query.
+            delete $3;
+            $$ = new MediaQueryExp;
+        } else
+            $$ = $3;
     }
     ;
 
@@ -1915,4 +1905,3 @@ error_recovery:
     ;
 
 %%
-
index b7e236a..100c756 100644 (file)
@@ -377,7 +377,7 @@ void CSSParserString::lower()
     makeLower(characters16(), characters16(), length());
 }
 
-void CSSParser::setupParser(const char* prefix, unsigned prefixLength, const String& string, const char* suffix, unsigned suffixLength)
+void CSSParser::setupParser(const char* prefix, unsigned prefixLength, StringView string, const char* suffix, unsigned suffixLength)
 {
     m_parsedTextPrefixLength = prefixLength;
     unsigned stringLength = string.length();
@@ -1480,18 +1480,43 @@ std::unique_ptr<MediaQuery> CSSParser::parseMediaQuery(const String& string)
 }
 
 #if ENABLE(PICTURE_SIZES)
-std::unique_ptr<SourceSizeList> CSSParser::parseSizesAttribute(const String& string)
+
+Vector<CSSParser::SourceSize> CSSParser::parseSizesAttribute(StringView string)
 {
+    Vector<SourceSize> result;
+
     if (string.isEmpty())
-        return nullptr;
+        return result;
 
-    ASSERT(!m_sourceSizeList.get());
+    ASSERT(!m_sourceSizeList);
 
     setupParser("@-webkit-sizesattr ", string, "}");
     cssyyparse(this);
 
-    return WTF::move(m_sourceSizeList);
+    if (!m_sourceSizeList)
+        return result;
+
+    result = WTF::move(*m_sourceSizeList);
+    m_sourceSizeList = nullptr;
+    return result;
 }
+
+CSSParser::SourceSize CSSParser::sourceSize(std::unique_ptr<MediaQueryExp>&& expression, CSSParserValue& parserValue)
+{
+    RefPtr<CSSValue> value;
+    if (isCalculation(parserValue)) {
+        auto* args = parserValue.function->args.get();
+        if (args && args->size())
+            value = CSSCalcValue::create(parserValue.function->name, *args, CalculationRangeNonNegative);
+    }
+    if (!value)
+        value = parserValue.createCSSValue();
+    // FIXME: Using a named local for the result here to work around an MSVC bug.
+    // With the other compilers, this works without explicitly stating the type name SourceSize or using a local.
+    SourceSize result { WTF::move(expression), value };
+    return result;
+}
+
 #endif
 
 static inline void filterProperties(bool important, const CSSParser::ParsedPropertyVector& input, Vector<CSSProperty, 256>& output, size_t& unusedEntries, std::bitset<numCSSProperties>& seenProperties)
index f9f6f76..46902a9 100644 (file)
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2003 Lars Knoll (knoll@kde.org)
- * Copyright (C) 2004, 2005, 2006, 2008, 2009, 2010 Apple Inc. All rights reserved.
+ * Copyright (C) 2004-2010, 2015 Apple Inc. All rights reserved.
  * Copyright (C) 2008 Eric Seidel <eric@webkit.org>
  * Copyright (C) 2009 - 2010  Torch Mobile (Beijing) Co. Ltd. All rights reserved.
  *
@@ -117,9 +117,6 @@ public:
     bool parseDeclaration(MutableStyleProperties*, const String&, PassRefPtr<CSSRuleSourceData>, StyleSheetContents* contextStyleSheet);
     static Ref<ImmutableStyleProperties> parseInlineStyleDeclaration(const String&, Element*);
     std::unique_ptr<MediaQuery> parseMediaQuery(const String&);
-#if ENABLE(PICTURE_SIZES)
-    std::unique_ptr<SourceSizeList> parseSizesAttribute(const String&);
-#endif
 
     void addPropertyWithPrefixingVariant(CSSPropertyID, PassRefPtr<CSSValue>, bool important, bool implicit = false);
     void addProperty(CSSPropertyID, PassRefPtr<CSSValue>, bool important, bool implicit = false);
@@ -138,6 +135,15 @@ public:
 
     PassRefPtr<CSSValue> parseBackgroundColor();
 
+#if ENABLE(PICTURE_SIZES)
+    struct SourceSize {
+        std::unique_ptr<MediaQueryExp> expression;
+        RefPtr<CSSValue> length;
+    };
+    Vector<SourceSize> parseSizesAttribute(StringView);
+    SourceSize sourceSize(std::unique_ptr<MediaQueryExp>&&, CSSParserValue&);
+#endif
+
     // FIXME: Maybe these two methods could be combined into one.
     bool parseMaskImage(CSSParserValueList&, RefPtr<CSSValue>&);
     bool parseFillImage(CSSParserValueList&, RefPtr<CSSValue>&);
@@ -366,7 +372,7 @@ public:
     RefPtr<StyleKeyframe> m_keyframe;
     std::unique_ptr<MediaQuery> m_mediaQuery;
 #if ENABLE(PICTURE_SIZES)
-    std::unique_ptr<SourceSizeList> m_sourceSizeList;
+    std::unique_ptr<Vector<SourceSize>> m_sourceSizeList;
 #endif
     std::unique_ptr<CSSParserValueList> m_valueList;
     bool m_supportsCondition;
@@ -514,11 +520,11 @@ private:
     void recheckAtKeyword(const UChar* str, int len);
 
     template<unsigned prefixLength, unsigned suffixLength>
-    inline void setupParser(const char (&prefix)[prefixLength], const String& string, const char (&suffix)[suffixLength])
+    void setupParser(const char (&prefix)[prefixLength], StringView string, const char (&suffix)[suffixLength])
     {
         setupParser(prefix, prefixLength - 1, string, suffix, suffixLength - 1);
     }
-    void setupParser(const char* prefix, unsigned prefixLength, const String&, const char* suffix, unsigned suffixLength);
+    void setupParser(const char* prefix, unsigned prefixLength, StringView, const char* suffix, unsigned suffixLength);
     bool inShorthand() const { return m_inParseShorthand; }
 
     bool validateWidth(ValueWithCalculation&);
index 93d88f3..aebc0eb 100644 (file)
@@ -110,7 +110,7 @@ MediaQuery::MediaQuery(const MediaQuery& o)
     , m_serializationCache(o.m_serializationCache)
 {
     for (unsigned i = 0; i < m_expressions->size(); ++i)
-        (*m_expressions)[i] = o.m_expressions->at(i)->copy();
+        (*m_expressions)[i] = std::make_unique<MediaQueryExp>(*o.m_expressions->at(i));
 }
 
 MediaQuery::~MediaQuery()
index c80d974..a29510b 100644 (file)
@@ -1,9 +1,7 @@
 /*
- * CSS Media Query
- *
  * Copyright (C) 2006 Kimmo Kinnunen <kimmo.t.kinnunen@nokia.com>.
  * Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies).
- * Copyright (C) 2013 Apple Inc. All rights reserved.
+ * Copyright (C) 2013, 2015 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -162,8 +160,6 @@ static inline bool featureWithoutValue(const AtomicString& mediaFeature)
 
 MediaQueryExp::MediaQueryExp(const AtomicString& mediaFeature, CSSParserValueList* valueList)
     : m_mediaFeature(mediaFeature)
-    , m_value(0)
-    , m_isValid(false)
 {
     // Initialize media query expression that must have 1 or more values.
     if (valueList) {
@@ -228,10 +224,6 @@ MediaQueryExp::MediaQueryExp(const AtomicString& mediaFeature, CSSParserValueLis
         m_isValid = true;
 }
 
-MediaQueryExp::~MediaQueryExp()
-{
-}
-
 String MediaQueryExp::serialize() const
 {
     if (!m_serializationCache.isNull())
@@ -246,7 +238,7 @@ String MediaQueryExp::serialize() const
     }
     result.append(')');
 
-    const_cast<MediaQueryExp*>(this)->m_serializationCache = result.toString();
+    m_serializationCache = result.toString();
     return m_serializationCache;
 }
 
index bdef545..7c38397 100644 (file)
@@ -1,8 +1,7 @@
 /*
- * CSS Media Query
- *
  * Copyright (C) 2006 Kimmo Kinnunen <kimmo.t.kinnunen@nokia.com>.
  * Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies).
+ * Copyright (C) 2015 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
 #include "CSSValue.h"
 #include "MediaFeatureNames.h"
 #include <memory>
-#include <wtf/RefPtr.h>
 #include <wtf/text/AtomicString.h>
 
 namespace WebCore {
+
 class CSSParserValueList;
 
 class MediaQueryExp {
     WTF_MAKE_FAST_ALLOCATED;
 public:
-    MediaQueryExp(const AtomicString& mediaFeature, CSSParserValueList* values);
-    ~MediaQueryExp();
-
-    AtomicString mediaFeature() const { return m_mediaFeature; }
-
-    CSSValue* value() const { return m_value.get(); }
-
-    bool operator==(const MediaQueryExp& other) const
-    {
-        return (other.m_mediaFeature == m_mediaFeature)
-            && ((!other.m_value && !m_value)
-                || (other.m_value && m_value && other.m_value->equals(*m_value)));
-    }
+    explicit MediaQueryExp(const AtomicString& mediaFeature = emptyAtom, CSSParserValueList* values = nullptr);
 
-    bool isValid() const { return m_isValid; }
+    const AtomicString& mediaFeature() const;
+    CSSValue* value() const;
 
-    bool isViewportDependent() const { return m_mediaFeature == MediaFeatureNames::widthMediaFeature
-                                            || m_mediaFeature == MediaFeatureNames::heightMediaFeature
-                                            || m_mediaFeature == MediaFeatureNames::min_widthMediaFeature
-                                            || m_mediaFeature == MediaFeatureNames::min_heightMediaFeature
-                                            || m_mediaFeature == MediaFeatureNames::max_widthMediaFeature
-                                            || m_mediaFeature == MediaFeatureNames::max_heightMediaFeature
-                                            || m_mediaFeature == MediaFeatureNames::orientationMediaFeature
-                                            || m_mediaFeature == MediaFeatureNames::aspect_ratioMediaFeature
-                                            || m_mediaFeature == MediaFeatureNames::min_aspect_ratioMediaFeature
-                                            || m_mediaFeature == MediaFeatureNames::max_aspect_ratioMediaFeature;  }
+    bool isValid() const;
+    bool isViewportDependent() const;
 
     String serialize() const;
 
-    std::unique_ptr<MediaQueryExp> copy() const { return std::make_unique<MediaQueryExp>(*this); }
+    bool operator==(const MediaQueryExp&) const;
 
 private:
     AtomicString m_mediaFeature;
     RefPtr<CSSValue> m_value;
-    bool m_isValid;
-    String m_serializationCache;
+    bool m_isValid { false };
+    mutable String m_serializationCache;
 };
 
+inline const AtomicString& MediaQueryExp::mediaFeature() const
+{
+    return m_mediaFeature;
+}
+
+inline CSSValue* MediaQueryExp::value() const
+{
+    return m_value.get();
+}
+
+inline bool MediaQueryExp::operator==(const MediaQueryExp& other) const
+{
+    return (other.m_mediaFeature == m_mediaFeature)
+        && ((!other.m_value && !m_value)
+            || (other.m_value && m_value && other.m_value->equals(*m_value)));
+}
+
+inline bool MediaQueryExp::isValid() const
+{
+    return m_isValid;
+}
+
+inline bool MediaQueryExp::isViewportDependent() const
+{
+    return m_mediaFeature == MediaFeatureNames::widthMediaFeature
+        || m_mediaFeature == MediaFeatureNames::heightMediaFeature
+        || m_mediaFeature == MediaFeatureNames::min_widthMediaFeature
+        || m_mediaFeature == MediaFeatureNames::min_heightMediaFeature
+        || m_mediaFeature == MediaFeatureNames::max_widthMediaFeature
+        || m_mediaFeature == MediaFeatureNames::max_heightMediaFeature
+        || m_mediaFeature == MediaFeatureNames::orientationMediaFeature
+        || m_mediaFeature == MediaFeatureNames::aspect_ratioMediaFeature
+        || m_mediaFeature == MediaFeatureNames::min_aspect_ratioMediaFeature
+        || m_mediaFeature == MediaFeatureNames::max_aspect_ratioMediaFeature;
+}
+
 } // namespace
 
 #endif
index 80fd2ff..62487a9 100644 (file)
@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2014 Yoav Weiss <yoav@yoav.ws>
+ * Copyright (C) 2015 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
 
 #include "CSSParser.h"
 #include "CSSToLengthConversionData.h"
-#include "Document.h"
 #include "MediaList.h"
 #include "MediaQuery.h"
 #include "MediaQueryEvaluator.h"
-#include "RenderObject.h"
+#include "MediaQueryExp.h"
 #include "RenderStyle.h"
 #include "RenderView.h"
 
 namespace WebCore {
 
-#if ENABLE(PICTURE_SIZES)
-bool SourceSize::match(RenderStyle& style, Frame* frame)
+#if !ENABLE(PICTURE_SIZES)
+
+unsigned parseSizesAttribute(StringView, RenderView*, Frame*)
+{
+    return 0;
+}
+
+#else
+
+static bool match(std::unique_ptr<MediaQueryExp>&& expression, RenderStyle& style, Frame* frame)
 {
-    if (m_mediaExp->mediaFeature().isEmpty())
+    if (expression->mediaFeature().isEmpty())
         return true;
+
     auto expList = std::make_unique<Vector<std::unique_ptr<MediaQueryExp>>>();
-    expList->append(m_mediaExp.release());
+    expList->append(WTF::move(expression));
 
     RefPtr<MediaQuerySet> mediaQuerySet = MediaQuerySet::create();
     mediaQuerySet->addMediaQuery(std::make_unique<MediaQuery>(MediaQuery::None, "all", WTF::move(expList)));
@@ -47,60 +56,28 @@ bool SourceSize::match(RenderStyle& style, Frame* frame)
     return mediaQueryEvaluator.eval(mediaQuerySet.get());
 }
 
-static unsigned computeLength(CSSParserValue& value, RenderStyle& style, RenderView* view)
+static unsigned computeLength(CSSValue* value, RenderStyle& style, RenderView* view)
 {
     CSSToLengthConversionData conversionData(&style, &style, view);
-    if (CSSParser::isCalculation(value)) {
-        CSSParserValueList* args = value.function->args.get();
-        if (args && args->size()) {
-            RefPtr<CSSCalcValue> calcValue = CSSCalcValue::create(value.function->name, *args, CalculationRangeNonNegative);
-            Length length(calcValue->createCalculationValue(conversionData));
-            RefPtr<CSSPrimitiveValue> primitiveValue = CSSPrimitiveValue::create(length, &style);
-            return primitiveValue->computeLength<unsigned>(conversionData);
-        }
+    if (is<CSSPrimitiveValue>(value))
+        return downcast<CSSPrimitiveValue>(*value).computeLength<unsigned>(conversionData);
+    if (is<CSSCalcValue>(value)) {
+        Length length(downcast<CSSCalcValue>(*value).createCalculationValue(conversionData));
+        return CSSPrimitiveValue::create(length, &style)->computeLength<unsigned>(conversionData);
     }
-    RefPtr<CSSValue> cssValue = value.createCSSValue();
-    RefPtr<CSSPrimitiveValue> primitiveValue = downcast<CSSPrimitiveValue>(cssValue.get());
-    return primitiveValue->computeLength<unsigned>(conversionData);
+    return 0;
 }
 
-static unsigned defaultValue(RenderStyle& style, RenderView* view)
-{
-    const unsigned defaultSizesAttributeValueInVW = 100;
-
-    CSSParserValue value;
-    value.id = CSSValueInvalid;
-    value.isInt = true;
-    value.fValue = defaultSizesAttributeValueInVW;
-    value.unit = CSSPrimitiveValue::CSS_VW;
-
-    return computeLength(value, style, view);
-}
-
-unsigned SourceSize::length(RenderStyle& style, RenderView* view)
-{
-    return computeLength(m_length, style, view);
-}
-
-unsigned SourceSizeList::parseSizesAttribute(const String& sizesAttribute, RenderView* view, Frame* frame)
+unsigned parseSizesAttribute(StringView sizesAttribute, RenderView* view, Frame* frame)
 {
     if (!view)
         return 0;
-    CSSParser parser(CSSStrictMode);
-    std::unique_ptr<SourceSizeList> sourceSizeList = parser.parseSizesAttribute(sizesAttribute);
-    if (!sourceSizeList)
-        return defaultValue(view->style(), view);
-    return sourceSizeList->getEffectiveSize(view->style(), view, frame);
-}
-
-unsigned SourceSizeList::getEffectiveSize(RenderStyle& style, RenderView* view, Frame* frame)
-{
-    for (int i = m_list.size() - 1; i >= 0; --i) {
-        SourceSize* sourceSize = m_list[i].get();
-        if (sourceSize->match(style, frame))
-            return sourceSize->length(style, view);
+    RenderStyle& style = view->style();
+    for (auto& sourceSize : CSSParser(CSSStrictMode).parseSizesAttribute(sizesAttribute)) {
+        if (match(WTF::move(sourceSize.expression), style, frame))
+            return computeLength(sourceSize.length.get(), style, view);
     }
-    return defaultValue(style, view);
+    return computeLength(CSSPrimitiveValue::create(100, CSSPrimitiveValue::CSS_VW).ptr(), style, view);
 }
 
 #endif
index da1697b..4f71a69 100644 (file)
@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2014 Yoav Weiss <yoav@yoav.ws>
+ * Copyright (C) 2015 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
 #ifndef SourceSizeList_h
 #define SourceSizeList_h
 
-#if ENABLE(PICTURE_SIZES)
-
-#include "CSSParserValues.h"
-#include "MediaQueryExp.h"
-#include <memory>
+#include <wtf/Forward.h>
 
 namespace WebCore {
 
-class RenderStyle;
-class RenderView;
 class Frame;
+class RenderView;
 
-class SourceSize {
-public:
-    SourceSize(std::unique_ptr<MediaQueryExp> mediaExp, const CSSParserValue& length)
-        : m_mediaExp(WTF::move(mediaExp))
-        , m_length(length)
-    {
-    }
-
-    bool match(RenderStyle&, Frame*);
-    unsigned length(RenderStyle&, RenderView*);
-
-private:
-    std::unique_ptr<MediaQueryExp> m_mediaExp;
-    CSSParserValue m_length;
-};
-
-class SourceSizeList {
-public:
-    void append(std::unique_ptr<SourceSize> sourceSize)
-    {
-        m_list.append(WTF::move(sourceSize));
-    }
-
-    static unsigned parseSizesAttribute(const String& sizesAttribute, RenderView*, Frame*);
-    unsigned getEffectiveSize(RenderStyle&, RenderView*, Frame*);
-
-private:
-    Vector<std::unique_ptr<SourceSize>> m_list;
-};
+unsigned parseSizesAttribute(StringView sizesAttribute, RenderView*, Frame*);
 
 } // namespace WebCore
 
-#endif // ENABLE(PICTURE_SIZES)
-
 #endif // SourceSizeList_h
-
index b7cea7c..554b541 100644 (file)
@@ -143,10 +143,7 @@ void HTMLImageElement::parseAttribute(const QualifiedName& name, const AtomicStr
         if (is<RenderImage>(renderer()))
             downcast<RenderImage>(*renderer()).updateAltText();
     } else if (name == srcAttr || name == srcsetAttr) {
-        unsigned sourceSize = 0;
-#if ENABLE(PICTURE_SIZES)
-        sourceSize = SourceSizeList::parseSizesAttribute(fastGetAttribute(sizesAttr), document().renderView(), document().frame());
-#endif
+        unsigned sourceSize = parseSizesAttribute(fastGetAttribute(sizesAttr).string(), document().renderView(), document().frame());
         ImageCandidate candidate = bestFitSourceForImageAttributes(document().deviceScaleFactor(), fastGetAttribute(srcAttr), fastGetAttribute(srcsetAttr), sourceSize);
         setBestFitURLAndDPRFromImageCandidate(candidate);
         m_imageLoader.updateFromElementIgnoringPreviousError();
index 47031b1..f0e88e7 100644 (file)
@@ -107,12 +107,7 @@ public:
 
         // Resolve between src and srcSet if we have them.
         if (!m_srcSetAttribute.isEmpty()) {
-            unsigned sourceSize = 0;
-#if ENABLE(PICTURE_SIZES)
-            sourceSize = SourceSizeList::parseSizesAttribute(m_sizesAttribute, document.renderView(), document.frame());
-#else
-            UNUSED_PARAM(document);
-#endif
+            unsigned sourceSize = parseSizesAttribute(m_sizesAttribute, document.renderView(), document.frame());
             ImageCandidate imageCandidate = bestFitSourceForImageAttributes(m_deviceScaleFactor, m_urlToLoad, m_srcSetAttribute, sourceSize);
             setUrlToLoad(imageCandidate.string.toString(), true);
         }