Get the default device scale factor from monitor

Get the default device scale factor from monitor 

BUG=none
TEST=none


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

[pkotwicz, 2012-04-29 18:35:15]

Cool!

[sadrul, 2012-04-30 15:15:48]

http://codereview.chromium.org/10255020/diff/3001/content/browser/web_content...
File content/browser/web_contents/web_contents_impl.cc (right):

http://codereview.chromium.org/10255020/diff/3001/content/browser/web_content...
content/browser/web_contents/web_contents_impl.cc:516:
prefs.default_device_scale_factor = 1;
From this change, it looks like it won't be possible to set the
default-device-scale-factor in non-aura anymore. Are we sure it's not used on
any other platforms?

http://codereview.chromium.org/10255020/diff/3001/ui/aura/monitor_manager.cc
File ui/aura/monitor_manager.cc (right):

http://codereview.chromium.org/10255020/diff/3001/ui/aura/monitor_manager.cc#...
ui/aura/monitor_manager.cc:34: float device_scale_factor, const std::string&
spec) {
one param in a line

http://codereview.chromium.org/10255020/diff/3001/ui/aura/monitor_manager.cc#...
ui/aura/monitor_manager.cc:65: 
-newline

http://codereview.chromium.org/10255020/diff/3001/ui/aura/monitor_manager.h
File ui/aura/monitor_manager.h (right):

http://codereview.chromium.org/10255020/diff/3001/ui/aura/monitor_manager.h#n...
ui/aura/monitor_manager.h:47: static gfx::Monitor CreateMonitorFromSpec(
It would be very nice to remove the '*' bit from the string-spec. Having
multiple ways of setting the scale-factor from the command-line is rather
confusing.

[oshima, 2012-04-30 17:25:24]

http://codereview.chromium.org/10255020/diff/3001/content/browser/web_content...
File content/browser/web_contents/web_contents_impl.cc (right):

http://codereview.chromium.org/10255020/diff/3001/content/browser/web_content...
content/browser/web_contents/web_contents_impl.cc:516:
prefs.default_device_scale_factor = 1;
On 2012/04/30 15:15:49, sadrul wrote:
> From this change, it looks like it won't be possible to set the
> default-device-scale-factor in non-aura anymore. Are we sure it's not used on
> any other platforms?

Good question. This is not used officially (as this must come from platform, but
not from command line switch), but I guess we're using this to test on other
platforms. I'll fix that.

http://codereview.chromium.org/10255020/diff/3001/ui/aura/monitor_manager.h
File ui/aura/monitor_manager.h (right):

http://codereview.chromium.org/10255020/diff/3001/ui/aura/monitor_manager.h#n...
ui/aura/monitor_manager.h:47: static gfx::Monitor CreateMonitorFromSpec(
On 2012/04/30 15:15:49, sadrul wrote:
> It would be very nice to remove the '*' bit from the string-spec. Having
> multiple ways of setting the scale-factor from the command-line is rather
> confusing.
This allow us to test multi densities environment, so we should remove default
scale factor if we have to remove one of them. I can't do this now however
because the device is using this value now. We need to fix powerd to report
correct DPI first.

[oshima, 2012-04-30 18:23:31]

Moved the code to get default device scale factor to ui/gfx/monitor. PTAL.

This actually looks better, so thank you for pointing this out.

[sadrul, 2012-04-30 18:56:59]

LGTM with a nit and a note:

http://codereview.chromium.org/10255020/diff/8006/ui/gfx/gfx_switches.h
File ui/gfx/gfx_switches.h (right):

http://codereview.chromium.org/10255020/diff/8006/ui/gfx/gfx_switches.h#newco...
ui/gfx/gfx_switches.h:13: UI_EXPORT extern const char
kDefaultDeviceScaleFactor[];
Perhaps it makes sense to move this into ui_base_switches?

http://codereview.chromium.org/10255020/diff/8006/ui/gfx/monitor.cc
File ui/gfx/monitor.cc (right):

http://codereview.chromium.org/10255020/diff/8006/ui/gfx/monitor.cc#newcode20
ui/gfx/monitor.cc:20: base::StringToDouble(
Check the return value of StringToDouble?

[oshima, 2012-04-30 19:41:13]

http://codereview.chromium.org/10255020/diff/8006/ui/gfx/gfx_switches.h
File ui/gfx/gfx_switches.h (right):

http://codereview.chromium.org/10255020/diff/8006/ui/gfx/gfx_switches.h#newco...
ui/gfx/gfx_switches.h:13: UI_EXPORT extern const char
kDefaultDeviceScaleFactor[];
On 2012/04/30 18:57:00, sadrul wrote:
> Perhaps it makes sense to move this into ui_base_switches?

Didn't know about the file. Thanks. Done.

http://codereview.chromium.org/10255020/diff/8006/ui/gfx/monitor.cc
File ui/gfx/monitor.cc (right):

http://codereview.chromium.org/10255020/diff/8006/ui/gfx/monitor.cc#newcode20
ui/gfx/monitor.cc:20: base::StringToDouble(
On 2012/04/30 18:57:00, sadrul wrote:
> Check the return value of StringToDouble?

Done.

[oshima, 2012-04-30 19:42:17]

+ben for OWNERS review

[Ben Goodger (Google), 2012-04-30 21:54:36]

LGTM

http://codereview.chromium.org/10255020/diff/19005/ui/aura/monitor_manager.h
File ui/aura/monitor_manager.h (right):

http://codereview.chromium.org/10255020/diff/19005/ui/aura/monitor_manager.h#...
ui/aura/monitor_manager.h:45: //  The device scale factor can be specifeid by
"*", like "1280x780*2",
specified

[oshima, 2012-04-30 23:13:32]

http://codereview.chromium.org/10255020/diff/19005/ui/aura/monitor_manager.h
File ui/aura/monitor_manager.h (right):

http://codereview.chromium.org/10255020/diff/19005/ui/aura/monitor_manager.h#...
ui/aura/monitor_manager.h:45: //  The device scale factor can be specifeid by
"*", like "1280x780*2",
On 2012/04/30 21:54:36, Ben Goodger (Google) wrote:
> specified

Done.

[commit-bot: I haz the power, 2012-05-01 03:32:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/oshima@chromium.org/10255020/21032

[commit-bot: I haz the power, 2012-05-01 04:55:10]

Change committed as 134677