Add a function to layout.h for getting ScaleFactor of the native view.

Add a function to layout.h for getting ScaleFactor of the native view.

BUG=125043
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=149775

[mazda, 2012-07-30 18:31:35]

Hi Scott,

Does it make sense to add this function to Screen?
I'd like to get the device scale factor of native view from ThumbnailGenerator
(chrome/browser/tab_contents/thumbnail_generator.cc).

Thanks,

[sky, 2012-08-01 00:15:27]

I think this makes more sense in ui/base/layout.h. Also, shouldn't it return a
ScaleFactor?

[mazda, 2012-08-01 03:47:43]

Thank you for your comments.
I moved the function to ui/base/layout.h and changed it to return ScaleFactor.
Please take another look.

[mazda, 2012-08-02 19:37:55]

ping?

[sky, 2012-08-02 21:10:14]

I'm not familiar enough with the mac side to say if your changes to
layout_mac.mm are correct. I added Nico to review them.

http://codereview.chromium.org/10823067/diff/13003/ui/base/layout_mac.mm
File ui/base/layout_mac.mm (right):

http://codereview.chromium.org/10823067/diff/13003/ui/base/layout_mac.mm#newc...
ui/base/layout_mac.mm:62: supported_scale_factors.clear();
Can you replace 62-66 with supported_scale_factors = scale_factors;

[Nico, 2012-08-02 21:24:24]

mac lgtm

[Nico, 2012-08-02 21:26:52]

wait

http://codereview.chromium.org/10823067/diff/13003/content/browser/renderer_h...
File content/browser/renderer_host/render_widget_host_view_mac.mm (right):

http://codereview.chromium.org/10823067/diff/13003/content/browser/renderer_h...
content/browser/renderer_host/render_widget_host_view_mac.mm:110: return
ui::GetScaleFactorScale(ui::GetScaleFactorForNativeView(view));
doesn't GetScaleFactorForNativeView() return a scale already? Why do you need
the GetSCaleFactorScale() call?

[mazda, 2012-08-02 22:42:24]

Thank you for the comments.

http://codereview.chromium.org/10823067/diff/13003/content/browser/renderer_h...
File content/browser/renderer_host/render_widget_host_view_mac.mm (right):

http://codereview.chromium.org/10823067/diff/13003/content/browser/renderer_h...
content/browser/renderer_host/render_widget_host_view_mac.mm:110: return
ui::GetScaleFactorScale(ui::GetScaleFactorForNativeView(view));
On 2012/08/02 21:26:52, Nico wrote:
> doesn't GetScaleFactorForNativeView() return a scale already? Why do you need
> the GetSCaleFactorScale() call?

GetScaleFactorForNativeView returns ScaleFactor (enum).
Should we expose the version that returns float?

http://codereview.chromium.org/10823067/diff/13003/ui/base/layout_mac.mm
File ui/base/layout_mac.mm (right):

http://codereview.chromium.org/10823067/diff/13003/ui/base/layout_mac.mm#newc...
ui/base/layout_mac.mm:62: supported_scale_factors.clear();
On 2012/08/02 21:10:14, sky wrote:
> Can you replace 62-66 with supported_scale_factors = scale_factors;

Done.

[Nico, 2012-08-02 22:43:16]

http://codereview.chromium.org/10823067/diff/13003/content/browser/renderer_h...
File content/browser/renderer_host/render_widget_host_view_mac.mm (right):

http://codereview.chromium.org/10823067/diff/13003/content/browser/renderer_h...
content/browser/renderer_host/render_widget_host_view_mac.mm:110: return
ui::GetScaleFactorScale(ui::GetScaleFactorForNativeView(view));
On 2012/08/02 22:42:24, mazda wrote:
> On 2012/08/02 21:26:52, Nico wrote:
> > doesn't GetScaleFactorForNativeView() return a scale already? Why do you
need
> > the GetSCaleFactorScale() call?
> 
> GetScaleFactorForNativeView returns ScaleFactor (enum).
> Should we expose the version that returns float?

Ah, ok. Might make sense if it's needed in more places, no need to do it now.

[mazda, 2012-08-02 23:32:10]

sky: Could you approve the CL for ui/ if there's no problem?

[sky, 2012-08-02 23:38:37]

LGTM

[commit-bot: I haz the power, 2012-08-02 23:58:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mazda@chromium.org/10823067/4008

[commit-bot: I haz the power, 2012-08-03 01:35:57]

Change committed as 149775