Correctly handle system metrics with e.g. --force-device-scale-factor.

Because GetSystemMetrics() always reflects the true current system DPI, it's not
appropriate to scale its result by GetDPIScale(); this results in the wrong
value when GetDPIScale() is forced to return a scale factor different than the
native one.  (It's not 100% clear to me whether the "clamp <=1.25 to 1" code
should still be applied in this case, but I think not.)

However, simply changing GetSystemMetricsInDIP() to use
GetUnforcedDeviceScaleFactor() is insufficient, because that function will
preferentially return |g_device_scale_factor|, which was being initialized to
the result of GetDPIScale() in two places.

This doesn't actually make any sense when you think through it.
* When there was a forced device scale factor, this meant that
  GetUnforcedDeviceScaleFactor() was returning this forced scale.  This makes no
  difference to anything except the updated version of GetSystemMetricsInDIP(),
  which would be unable to access the non-forced scale factor.
* When there wasn't a forced device scale factor, and the unforced DPI was 1.0
  or >1.25, this simply meant that GetUnforcedDeviceScaleFactor() would return,
  as a cached value, the value it would have returned anyway.  The extra caching
  doesn't buy anything meaningful performance-wise since GetDPI() already caches
  internally, so this would have no discernable effect.
* Finally, when there was no forced device scale factor and the DPI was between
  1 and 1.25, this would mean GetUnforcedDeviceScaleFactor() would display the
  same clamping behavior GetDPIScale() would.  Again, this makes no difference
  to anything except the updated version of GetSystemMetricsInDIP().

So, without my change in GetSystemMetricsInDIP(), the two calls to
InitDeviceScaleFactor() I've removed will have no behavioral effect; with that
change, removing them is required in order to get the desired behavior, namely,
what the name "GetUnforcedDeviceScaleFactor()" actually suggests.

BUG=none
TEST=On Windows, running with --force-device-scale-factor=2 should result in a height above the main toolbar that's exactly double the normal height, instead of being shorter.

Review URL: https://codereview.chromium.org/1520283005

Cr-Commit-Position: refs/heads/master@{#365784}

chrome/browser/metro_viewer/chrome_metro_viewer_process_host_aurawin.cc

@@ -134,7 +134,7 @@ void ChromeMetroViewerProcessHost::OnSetTargetSurface(
     float device_scale) {
   HWND hwnd = reinterpret_cast<HWND>(target_surface);
 
-  gfx::InitDeviceScaleFactor(device_scale);
+  gfx::SetDefaultDeviceScaleFactor(device_scale);
   chrome::OpenAsh(hwnd);
   DCHECK(aura::RemoteWindowTreeHostWin::Instance());
   DCHECK_EQ(hwnd, aura::RemoteWindowTreeHostWin::Instance()->remote_window());

components/html_viewer/layout_test_blink_settings_impl.cc

@@ -95,7 +95,7 @@ void LayoutTestBlinkSettingsImpl::ApplySettingsToWebView(
   blink::WebRuntimeFeatures::enableTestOnlyFeatures(true);
 
 #if defined(OS_WIN)
-  gfx::InitDeviceScaleFactor(1.0f);
+  gfx::SetDefaultDeviceScaleFactor(1.0f);
 #endif
 }
 

content/app/content_main_runner.cc

@@ -570,18 +570,13 @@ class ContentMainRunnerImpl : public ContentMainRunner {
 #endif
 
 #if defined(OS_WIN)
-    bool init_device_scale_factor = true;
     if (command_line.HasSwitch(switches::kDeviceScaleFactor)) {
       std::string scale_factor_string = command_line.GetSwitchValueASCII(
           switches::kDeviceScaleFactor);
       double scale_factor = 0;
-      if (base::StringToDouble(scale_factor_string, &scale_factor)) {
-        init_device_scale_factor = false;
-        gfx::InitDeviceScaleFactor(scale_factor);
-      }
+      if (base::StringToDouble(scale_factor_string, &scale_factor))
+        gfx::SetDefaultDeviceScaleFactor(scale_factor);
     }
-    if (init_device_scale_factor)
-      gfx::InitDeviceScaleFactor(gfx::GetDPIScale());
 #endif
 
     if (!GetContentClient())

content/test/blink_test_environment.cc

@@ -107,7 +107,7 @@ void SetUpBlinkTestEnvironment() {
 #endif
 
 #if defined(OS_WIN)
-  gfx::InitDeviceScaleFactor(1.0f);
+  gfx::SetDefaultDeviceScaleFactor(1.0f);
 #endif
 
   // Explicitly initialize the GURL library before spawning any threads.

content/test/content_test_suite.cc

@@ -82,7 +82,7 @@ void ContentTestSuite::Initialize() {
 #endif
 
 #if defined(OS_WIN)
-  gfx::InitDeviceScaleFactor(1.0f);
+  gfx::SetDefaultDeviceScaleFactor(1.0f);
 #endif
 
   ContentTestSuiteBase::Initialize();

ui/aura/demo/demo_main.cc

@@ -129,7 +129,7 @@ int DemoMain() {
   gfx::GLSurface::InitializeOneOff();
 
 #if defined(OS_WIN)
-  gfx::InitDeviceScaleFactor(1.0f);
+  gfx::SetDefaultDeviceScaleFactor(1.0f);
 #endif
 
   // The ContextFactory must exist before any Compositors are created.

ui/base/resource/resource_bundle.cc

@@ -651,11 +651,6 @@ void ResourceBundle::InitSharedInstance(Delegate* delegate) {
     supported_scale_factors.push_back(SCALE_FACTOR_100P);
 #endif
   ui::SetSupportedScaleFactors(supported_scale_factors);
-#if defined(OS_WIN)
-  // Must be called _after_ supported scale factors are set since it
-  // uses them.
-  gfx::InitDeviceScaleFactor(gfx::GetDPIScale());
-#endif
 }
 
 void ResourceBundle::FreeImages() {

ui/base/resource/resource_bundle_unittest.cc

@@ -516,7 +516,7 @@ TEST_F(ResourceBundleImageTest, GetRawDataResource) {
 // via ResourceBundle::GetImageNamed().
 TEST_F(ResourceBundleImageTest, GetImageNamed) {
 #if defined(OS_WIN)
-  gfx::InitDeviceScaleFactor(2.0);
+  gfx::SetDefaultDeviceScaleFactor(2.0);
 #endif
   std::vector<ScaleFactor> supported_factors;
   supported_factors.push_back(SCALE_FACTOR_100P);

ui/base/test/run_all_unittests.cc

@@ -48,7 +48,7 @@ void UIBaseTestSuite::Initialize() {
   base::TestSuite::Initialize();
 
 #if defined(OS_WIN)
-  gfx::InitDeviceScaleFactor(1.0);
+  gfx::SetDefaultDeviceScaleFactor(1.0);
 #endif
 
 #if defined(OS_ANDROID)

ui/compositor/test/test_suite.cc

@@ -30,7 +30,7 @@ void CompositorTestSuite::Initialize() {
   gfx::RegisterPathProvider();
 
 #if defined(OS_WIN)
-  gfx::InitDeviceScaleFactor(1.0f);
+  gfx::SetDefaultDeviceScaleFactor(1.0f);
 #endif
 
   message_loop_.reset(new base::MessageLoopForUI);

ui/gfx/screen_win_unittest.cc

@@ -39,11 +39,11 @@ class ScreenWinTest : public testing::Test {
  private:
   void SetUp() override {
     testing::Test::SetUp();
-    gfx::InitDeviceScaleFactor(1.0);
+    gfx::SetDefaultDeviceScaleFactor(1.0);
   }
 
   void TearDown() override {
-    gfx::InitDeviceScaleFactor(1.0);
+    gfx::SetDefaultDeviceScaleFactor(1.0);
     testing::Test::TearDown();
   }
 };
@@ -62,7 +62,7 @@ TEST_F(ScreenWinTest, SingleDisplay1x) {
 }
 
 TEST_F(ScreenWinTest, SingleDisplay2x) {
-  gfx::InitDeviceScaleFactor(2.0);
+  gfx::SetDefaultDeviceScaleFactor(2.0);
 
   std::vector<MONITORINFOEX> monitor_infos;
   monitor_infos.push_back(CreateMonitorInfo(gfx::Rect(0, 0, 1920, 1200),

ui/gfx/win/dpi.cc

@@ -13,25 +13,22 @@
 
 namespace {
 
-int kDefaultDPI = 96;
+const float kDefaultDPI = 96.f;
 
-float g_device_scale_factor = 0.0f;
+float g_device_scale_factor = 0.f;
 
 float GetUnforcedDeviceScaleFactor() {
-  // If the global device scale factor is initialized use it. This is to ensure
-  // we use the same scale factor across all callsites.
-  if (g_device_scale_factor)
-    return g_device_scale_factor;
-  return static_cast<float>(gfx::GetDPI().width()) /
-      static_cast<float>(kDefaultDPI);
+  return g_device_scale_factor ?
+      g_device_scale_factor :
+      static_cast<float>(gfx::GetDPI().width()) / kDefaultDPI;
 }
 
 }  // namespace
 
 namespace gfx {
 
-void InitDeviceScaleFactor(float scale) {
-  DCHECK_NE(0.0f, scale);
+void SetDefaultDeviceScaleFactor(float scale) {
+  DCHECK_NE(0.f, scale);
   g_device_scale_factor = scale;
 }
 
@@ -56,12 +53,7 @@ float GetDPIScale() {
   if (gfx::Display::HasForceDeviceScaleFactor())
     return gfx::Display::GetForcedDeviceScaleFactor();
   float dpi_scale = GetUnforcedDeviceScaleFactor();
-  if (dpi_scale <= 1.25) {
-    // Force 125% and below to 100% scale. We do this to maintain previous
-    // (non-DPI-aware) behavior where only the font size was boosted.
-    dpi_scale = 1.0;
-  }
-  return dpi_scale;
+  return (dpi_scale <= 1.25f) ? 1.f : dpi_scale;
 }
 
 namespace win {
@@ -101,7 +93,10 @@ Size DIPToScreenSize(const Size& dip_size) {
 }
 
 int GetSystemMetricsInDIP(int metric) {
-  return static_cast<int>(GetSystemMetrics(metric) / GetDPIScale() + 0.5);
+  // The system metrics always reflect the system DPI, not whatever scale we've
+  // forced or decided to use.
+  return static_cast<int>(
+      std::round(GetSystemMetrics(metric) / GetUnforcedDeviceScaleFactor()));
 }
 
 }  // namespace win

ui/gfx/win/dpi.h

@@ -12,17 +12,18 @@
 
 namespace gfx {
 
-// Initialization of the scale factor that should be applied for rendering
-// in this process. Must be called before attempts to call any of the getter
-// methods below in this file, e.g. in the early toolkit/resource bundle setup.
-// This can be called multiple times during various tests, but subsequent calls
-// have no effect.
-GFX_EXPORT void InitDeviceScaleFactor(float scale);
+// Sets the device scale factor that will be used unless overridden on the
+// command line by --force-device-scale-factor.  If this is not called, and that
+// flag is not used, the scale factor used by the DIP conversion functions below
+// will be that returned by GetDPIScale().
+GFX_EXPORT void SetDefaultDeviceScaleFactor(float scale);
 
 GFX_EXPORT Size GetDPI();
 
 // Gets the scale factor of the display. For example, if the display DPI is
-// 96 then the scale factor is 1.0.
+// 96 then the scale factor is 1.0.  This clamps scale factors <= 1.25 to 1.0 to
+// maintain previous (non-DPI-aware) behavior where only the font size was
+// boosted.
 GFX_EXPORT float GetDPIScale();
 
 namespace win {