Make calls to AtomicString(const String&) explicit in css/

Make calls to AtomicString(const String&) explicit in css/

Make calls to AtomicString(const String&) explicit in css/ and avoid useless
conversions from String to AtomicString when possible.

This is one step towards making the AtomicString(const String&) constructor
explicit. This CL takes care only of the css/ part so that it is more
easily reviewable.

R=eseidel, abarth
BUG=323739
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=163461

[eseidel, 2013-12-09 18:24:20]

lgtm

I'm glad your straightening this all out.  I think we should consider renaming
AtomicString to Atom or something shorter at some point.

Also dglazkov and ojan are very useful for css/ reviews these days.

https://codereview.chromium.org/100463004/diff/1/Source/core/css/MediaQueryEv...
File Source/core/css/MediaQueryEvaluator.h (right):

https://codereview.chromium.org/100463004/diff/1/Source/core/css/MediaQueryEv...
Source/core/css/MediaQueryEvaluator.h:75: bool mediaTypeMatch(const String&
mediaTypeToMatch) const;
I would have expected media queries to be atomic strings.  Aren't they a very
small set of values?

[Inactive, 2013-12-09 18:49:32]

https://codereview.chromium.org/100463004/diff/1/Source/core/css/MediaQueryEv...
File Source/core/css/MediaQueryEvaluator.h (right):

https://codereview.chromium.org/100463004/diff/1/Source/core/css/MediaQueryEv...
Source/core/css/MediaQueryEvaluator.h:75: bool mediaTypeMatch(const String&
mediaTypeToMatch) const;
On 2013/12/09 18:24:21, eseidel wrote:
> I would have expected media queries to be atomic strings.  Aren't they a very
> small set of values?

Media queries are not really a small set of values. However, this particular
code is about media types, and those are definitely a small set of values and
using AtomicString definitely makes sense:
http://www.w3.org/TR/CSS2/media.html#media-types

I switched to String here because MediaQuery::mediaType() currently returns a
String but I will look into making it return an AtomicString instead, thanks.

[kenneth.r.christiansen, 2013-12-09 19:08:58]

> I switched to String here because MediaQuery::mediaType() currently returns a
> String but I will look into making it return an AtomicString instead, thanks.

The set of media types is pretty restricted (8 or so, and many are deprecated in
level 4, down to 3)

[Inactive, 2013-12-09 19:10:58]

https://codereview.chromium.org/100463004/diff/1/Source/core/css/MediaQueryEv...
File Source/core/css/MediaQueryEvaluator.h (right):

https://codereview.chromium.org/100463004/diff/1/Source/core/css/MediaQueryEv...
Source/core/css/MediaQueryEvaluator.h:75: bool mediaTypeMatch(const String&
mediaTypeToMatch) const;
On 2013/12/09 18:49:32, Chris Dumez wrote:
> On 2013/12/09 18:24:21, eseidel wrote:
> > I would have expected media queries to be atomic strings.  Aren't they a
very
> > small set of values?
> 
> Media queries are not really a small set of values. However, this particular
> code is about media types, and those are definitely a small set of values and
> using AtomicString definitely makes sense:
> http://www.w3.org/TR/CSS2/media.html#media-types
> 
> I switched to String here because MediaQuery::mediaType() currently returns a
> String but I will look into making it return an AtomicString instead, thanks.

Done. I kept this as an AtomicString.

https://codereview.chromium.org/100463004/diff/20001/Source/core/css/MediaQue...
File Source/core/css/MediaQuery.h (right):

https://codereview.chromium.org/100463004/diff/20001/Source/core/css/MediaQue...
Source/core/css/MediaQuery.h:54: const AtomicString& mediaType() const { return
m_mediaType; }
We now use an AtomicString for the mediaType as the set of values is limited.

[Inactive, 2013-12-09 19:44:03]

Eric, could you please take a final look? Thanks.

[eseidel, 2013-12-09 19:57:47]

lgtm

[commit-bot: I haz the power, 2013-12-09 20:01:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ch.dumez@samsung.com/100463004/20001

[commit-bot: I haz the power, 2013-12-09 22:10:05]

Change committed as 163461