MediaQueryEvaluator getting mediaType from MediaValues

MediaQueryEvaluator getting mediaType from MediaValues

Current MediaQueryEvaluator takes the media type on top of the LocalFrame upon its creation.
That means that media type changes are not taken into account, and users such as
MediaQueryMatcher have to create a new instance whenever the media type may have changed.
This CL fixes that.

BUG=389462
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=177087

[esprehn, 2014-06-27 08:10:32]

This looks good, but I don't think your patch applies.

https://codereview.chromium.org/335313003/diff/40001/Source/core/css/MediaQue...
File Source/core/css/MediaQueryEvaluator.cpp (right):

https://codereview.chromium.org/335313003/diff/40001/Source/core/css/MediaQue...
Source/core/css/MediaQueryEvaluator.cpp:100: if (m_mediaValues.get())
You don't need to call .get(), there's an overload on the RefPtr type.

if (m_mediaValues)

https://codereview.chromium.org/335313003/diff/40001/Source/core/css/MediaVal...
File Source/core/css/MediaValuesCached.cpp (right):

https://codereview.chromium.org/335313003/diff/40001/Source/core/css/MediaVal...
Source/core/css/MediaValuesCached.cpp:66: m_data.mediaType =
mediaType.isolatedCopy();
Why do you need a copy?

[Yoav Weiss, 2014-06-27 08:16:36]

On 2014/06/27 08:10:32, esprehn wrote:
> This looks good, but I don't think your patch applies.

Yeah, had conflicts with the MQMatcher patch. Just rebased it.

> 
>
https://codereview.chromium.org/335313003/diff/40001/Source/core/css/MediaQue...
> File Source/core/css/MediaQueryEvaluator.cpp (right):
> 
>
https://codereview.chromium.org/335313003/diff/40001/Source/core/css/MediaQue...
> Source/core/css/MediaQueryEvaluator.cpp:100: if (m_mediaValues.get())
> You don't need to call .get(), there's an overload on the RefPtr type.
Ok

> 
> if (m_mediaValues)
> 
>
https://codereview.chromium.org/335313003/diff/40001/Source/core/css/MediaVal...
> File Source/core/css/MediaValuesCached.cpp (right):
> 
>
https://codereview.chromium.org/335313003/diff/40001/Source/core/css/MediaVal...
> Source/core/css/MediaValuesCached.cpp:66: m_data.mediaType =
> mediaType.isolatedCopy();
> Why do you need a copy?

MediaValuesCached is used to pass these values to the parser thread (so that
they can be used in the preloadScanner).
Unless we can be certain that this string will not be changed, I think we need
to create a copy here. Do we have any guaranties that the media type is a static
string that won't be changed?

[kenneth.christiansen, 2014-06-27 09:09:32]

lgtm

https://codereview.chromium.org/335313003/diff/60001/Source/core/css/MediaQue...
File Source/core/css/MediaQueryEvaluator.cpp (right):

https://codereview.chromium.org/335313003/diff/60001/Source/core/css/MediaQue...
Source/core/css/MediaQueryEvaluator.cpp:100: if (m_mediaValues.get())
Maybe a comment to why you shouldnt set m_mediaType here

[esprehn, 2014-06-27 10:01:31]

Lgtm, makes sense re the string.

[Yoav Weiss, 2014-06-27 10:07:25]

The CQ bit was checked by yoav@yoav.ws

[commit-bot: I haz the power, 2014-06-27 10:07:42]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/yoav@yoav.ws/335313003/100001

[commit-bot: I haz the power, 2014-06-27 10:34:41]

Change committed as 177087