Parse media attributes and CSSOM media text as media_query_list.

Source/core/css/MediaList.cpp

Index: Source/core/css/MediaList.cpp
diff --git a/Source/core/css/MediaList.cpp b/Source/core/css/MediaList.cpp
index 6e650fd8c725a2273779e52a0cc636276724e77a..c2d06161ad47be77ad5d33aeb4e336efc69c010e 100644
--- a/Source/core/css/MediaList.cpp
+++ b/Source/core/css/MediaList.cpp
@@ -55,21 +55,18 @@ namespace WebCore {
  */
 
 MediaQuerySet::MediaQuerySet()
-    : m_parserMode(MediaQueryNormalMode)
-    , m_lastLine(0)
+    : m_lastLine(0)
 {
 }
 
-MediaQuerySet::MediaQuerySet(const String& mediaString, MediaQueryParserMode mode)
-    : m_parserMode(mode)
-    , m_lastLine(0)
+MediaQuerySet::MediaQuerySet(const String& mediaString)
+    : m_lastLine(0)
 {
     set(mediaString);
 }
 
 MediaQuerySet::MediaQuerySet(const MediaQuerySet& o)
     : RefCounted<MediaQuerySet>()
-    , m_parserMode(o.m_parserMode)
     , m_lastLine(o.m_lastLine)
     , m_queries(o.m_queries.size())
 {
@@ -81,53 +78,20 @@ MediaQuerySet::~MediaQuerySet()
 {
 }
 
-PassOwnPtr<MediaQuery> MediaQuerySet::parseMediaQuery(const String& queryString, MediaQueryParserMode mode)
+PassRefPtr<MediaQuerySet> MediaQuerySet::parseMediaQueryList(const String& mediaString)

[SeRya, 2013/06/10 12:11:47]

It could be static. And MediaQuerySet::create could be like this:

return parseMediaQueryList(mediaString);

And MediaQuerySet::MediaQuerySet(const String& mediaString) could be removed.

[rune, 2013/06/10 19:53:07]

Fixed. Made a "create" method out of parseMediaQueryList as Kenneth proposed.

 {
-    CSSParser parser(CSSStrictMode);
-    OwnPtr<MediaQuery> parsedQuery = parser.parseMediaQuery(queryString);
-
-    if (parsedQuery)
-        return parsedQuery.release();
-
-    switch (mode) {
-    case MediaQueryNormalMode:
-        return adoptPtr(new MediaQuery(MediaQuery::None, "not all", nullptr));
-    case MediaQueryStrictMode:
-        break;
-    default:
-        ASSERT_NOT_REACHED();
-        break;
-    }
-    return nullptr;
-}
-
-void MediaQuerySet::parseMediaQueryList(const String& mediaString, MediaQueryParserMode mode, Vector<OwnPtr<MediaQuery> >& result)
-{
-    if (mediaString.isEmpty()) {
-        result.clear();
-        return;
-    }
-
-    Vector<String> list;
-    // FIXME: This is too simple as it shouldn't split when the ',' is inside
-    // other allowed matching pairs such as (), [], {}, "", and ''.
-    mediaString.split(',', /* allowEmptyEntries */ true, list);
+    if (mediaString.isEmpty())
+        return MediaQuerySet::create();
 
-    result.reserveInitialCapacity(list.size());
-
-    for (unsigned i = 0; i < list.size(); ++i) {
-        String queryString = list[i].stripWhiteSpace();
-        OwnPtr<MediaQuery> parsedQuery = parseMediaQuery(queryString, mode);
-        if (parsedQuery)
-            result.uncheckedAppend(parsedQuery.release());
-    }
+    CSSParser parser(CSSStrictMode);
+    return parser.parseMediaQueryList(mediaString);
 }
 
 bool MediaQuerySet::set(const String& mediaString)
 {
-    Vector<OwnPtr<MediaQuery> > result;
-    parseMediaQueryList(mediaString, parserMode(), result);
-    m_queries.swap(result);
+    RefPtr<MediaQuerySet> result = parseMediaQueryList(mediaString);
+    ASSERT(result);

[SeRya, 2013/06/10 12:11:47]

I think this ASSERT says nothing once next line wold cause core dump.

[rune, 2013/06/10 19:53:07]

This is way it's done across Source/core. ASSERT followed by accessing a pointer
without a null check when the invariant is that it should never be null.

I have removed them, though since I made a create method instead, as Kenneth
suggested. A create method normally has the convention that it doesn't return
null (unless there's an out-of-memory situation). Also, there is already an
ASSERT in CSSParser::parseMediaQueryList that covers the invariant.

+    m_queries.swap(result->m_queries);
     return true;
 }
 
@@ -136,14 +100,14 @@ bool MediaQuerySet::add(const String& queryString)
     // To "parse a media query" for a given string means to follow "the parse
     // a media query list" steps and return "null" if more than one media query
     // is returned, or else the returned media query.
-    Vector<OwnPtr<MediaQuery> > queries;
-    parseMediaQueryList(queryString, MediaQueryStrictMode, queries);
+    RefPtr<MediaQuerySet> result = parseMediaQueryList(queryString);
+    ASSERT(result);

[SeRya, 2013/06/10 12:11:47]

ditto

[rune, 2013/06/10 19:53:07]

Done.

 
     // Only continue if exactly one media query is found, as described above.
-    if (queries.size() != 1)
+    if (result->m_queries.size() != 1)
         return true;
 
-    OwnPtr<MediaQuery> newQuery = queries[0].release();
+    OwnPtr<MediaQuery> newQuery = result->m_queries[0].release();
     ASSERT(newQuery);
 
     // If comparing with any of the media queries in the collection of media
@@ -163,14 +127,14 @@ bool MediaQuerySet::remove(const String& queryStringToRemove)
     // To "parse a media query" for a given string means to follow "the parse
     // a media query list" steps and return "null" if more than one media query
     // is returned, or else the returned media query.
-    Vector<OwnPtr<MediaQuery> > queries;
-    parseMediaQueryList(queryStringToRemove, MediaQueryStrictMode, queries);
+    RefPtr<MediaQuerySet> result = parseMediaQueryList(queryStringToRemove);
+    ASSERT(result);

[SeRya, 2013/06/10 12:11:47]

ditto

[rune, 2013/06/10 19:53:07]

Done.

 
     // Only continue if exactly one media query is found, as described above.
-    if (queries.size() != 1)
+    if (result->m_queries.size() != 1)
         return true;
 
-    OwnPtr<MediaQuery> newQuery = queries[0].release();
+    OwnPtr<MediaQuery> newQuery = result->m_queries[0].release();
     ASSERT(newQuery);
 
     // Remove any media query from the collection of media queries for which