Let page zoom affect resolution and -webkit-device-pixel-ratio MQs.

Let page zoom affect resolution and -webkit-device-pixel-ratio MQs.

In order to make them consistent with the recent change for
window.devicePixelRatio (r156040).

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

[johnme, 2013-08-14 15:14:48]

LGTM - thanks Rune. Emil, what do you say?

[eae, 2013-08-14 15:20:49]

https://codereview.chromium.org/23192002/diff/1/Source/core/css/MediaQueryEva...
File Source/core/css/MediaQueryEvaluator.cpp (right):

https://codereview.chromium.org/23192002/diff/1/Source/core/css/MediaQueryEva...
Source/core/css/MediaQueryEvaluator.cpp:284: actualResolution =
frame->resolutionDPPX();
Why not use devicePixelRatio directly here?

https://codereview.chromium.org/23192002/diff/1/Source/core/page/Frame.cpp
File Source/core/page/Frame.cpp (right):

https://codereview.chromium.org/23192002/diff/1/Source/core/page/Frame.cpp#ne...
Source/core/page/Frame.cpp:712: float Frame::resolutionDPPX() const
I don't like the name of this method, why not call it devicePixelRatio instead?
Also, it should use (and return) a double, not a float.

[rune, 2013-08-14 15:59:34]

https://codereview.chromium.org/23192002/diff/1/Source/core/css/MediaQueryEva...
File Source/core/css/MediaQueryEvaluator.cpp (right):

https://codereview.chromium.org/23192002/diff/1/Source/core/css/MediaQueryEva...
Source/core/css/MediaQueryEvaluator.cpp:284: actualResolution =
frame->resolutionDPPX();
On 2013/08/14 15:20:49, eae wrote:
> Why not use devicePixelRatio directly here?

Do you mean the one in DOMWindow?

It felt more natural to do implement devicePixelRatio/resolutionDPPX in Frame
instead of going Frame -> DOMWindow -> Frame.

https://codereview.chromium.org/23192002/diff/1/Source/core/page/Frame.cpp
File Source/core/page/Frame.cpp (right):

https://codereview.chromium.org/23192002/diff/1/Source/core/page/Frame.cpp#ne...
Source/core/page/Frame.cpp:712: float Frame::resolutionDPPX() const
On 2013/08/14 15:20:49, eae wrote:
> I don't like the name of this method, why not call it devicePixelRatio
instead?

OK.

> Also, it should use (and return) a double, not a float.

If so, I'll need to clampTo<float> in the media query evaluation to avoid test
failures. I tried to use a double value return from Frame::resolutionDPPX in
combination with getDoubleValue instead of getFloatValue in evalResolution, but
that caused tests to fail. I haven't investigated it closely.

[rune, 2013-08-14 16:04:43]

https://codereview.chromium.org/23192002/diff/1/Source/core/page/Frame.cpp
File Source/core/page/Frame.cpp (right):

https://codereview.chromium.org/23192002/diff/1/Source/core/page/Frame.cpp#ne...
Source/core/page/Frame.cpp:712: float Frame::resolutionDPPX() const
On 2013/08/14 15:59:34, rune wrote:
> On 2013/08/14 15:20:49, eae wrote:
> > I don't like the name of this method, why not call it devicePixelRatio
> instead?
> 
> OK.
> 
> > Also, it should use (and return) a double, not a float.
> 
> If so, I'll need to clampTo<float> in the media query evaluation to avoid test
> failures. I tried to use a double value return from Frame::resolutionDPPX in
> combination with getDoubleValue instead of getFloatValue in evalResolution,
but
> that caused tests to fail. I haven't investigated it closely.

Done.

[eae, 2013-08-14 16:06:03]

On 2013/08/14 15:59:34, rune wrote:
>
https://codereview.chromium.org/23192002/diff/1/Source/core/css/MediaQueryEva...
> File Source/core/css/MediaQueryEvaluator.cpp (right):
> 
>
https://codereview.chromium.org/23192002/diff/1/Source/core/css/MediaQueryEva...
> Source/core/css/MediaQueryEvaluator.cpp:284: actualResolution =
> frame->resolutionDPPX();
> On 2013/08/14 15:20:49, eae wrote:
> > Why not use devicePixelRatio directly here?
> 
> Do you mean the one in DOMWindow?
> 
> It felt more natural to do implement devicePixelRatio/resolutionDPPX in Frame
> instead of going Frame -> DOMWindow -> Frame.
> 
> https://codereview.chromium.org/23192002/diff/1/Source/core/page/Frame.cpp
> File Source/core/page/Frame.cpp (right):
> 
>
https://codereview.chromium.org/23192002/diff/1/Source/core/page/Frame.cpp#ne...
> Source/core/page/Frame.cpp:712: float Frame::resolutionDPPX() const
> On 2013/08/14 15:20:49, eae wrote:
> > I don't like the name of this method, why not call it devicePixelRatio
> instead?
> 
> OK.

Cool.

> 
> > Also, it should use (and return) a double, not a float.
> 
> If so, I'll need to clampTo<float> in the media query evaluation to avoid test
> failures. I tried to use a double value return from Frame::resolutionDPPX in
> combination with getDoubleValue instead of getFloatValue in evalResolution,
but
> that caused tests to fail. I haven't investigated it closely.

devicePixelRatio is defined as returning a double while css uses floats. I'd
rather compute it as a double and then clamp it to a float for media queries
than compute it as a float and upcast it to a double as that might reduce
precision.

[rune, 2013-08-14 16:15:20]

eae: are you OK with patch set 2?

[eae, 2013-08-14 16:33:38]

LGTM

Looks good with the following change.

https://codereview.chromium.org/23192002/diff/9001/Source/core/page/Frame.cpp
File Source/core/page/Frame.cpp (right):

https://codereview.chromium.org/23192002/diff/9001/Source/core/page/Frame.cpp...
Source/core/page/Frame.cpp:717: double resolution = m_page->deviceScaleFactor();
Please rename this to ratio or scaleFactor.

[rune, 2013-08-14 19:11:22]

https://codereview.chromium.org/23192002/diff/9001/Source/core/page/Frame.cpp
File Source/core/page/Frame.cpp (right):

https://codereview.chromium.org/23192002/diff/9001/Source/core/page/Frame.cpp...
Source/core/page/Frame.cpp:717: double resolution = m_page->deviceScaleFactor();
On 2013/08/14 16:33:39, eae wrote:
> Please rename this to ratio or scaleFactor.

Done.

[commit-bot: I haz the power, 2013-08-14 19:11:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rune@opera.com/23192002/18001

[commit-bot: I haz the power, 2013-08-14 20:41:22]

Change committed as 156113