Cleanup: Enable 125DSFForUIScaling on by default

* Fix typo
* Introduced Scoped125DSFForUIScaling to temporarily disable it for testing.
* Make sure id isn't invalid when checking if it's internal.
 This problem exists only in unit tests, not in production.

BUG=None

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

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

ash/display/display_info.cc

@@ -24,7 +24,7 @@
 namespace ash {
 namespace {
 
-bool use_125_dsf_for_ui_scaling = false;
+bool use_125_dsf_for_ui_scaling = true;
 
 // Check the content of |spec| and fill |bounds| and |device_scale_factor|.
 // Returns true when |bounds| is found.
@@ -61,6 +61,11 @@ struct DisplayModeSorter {
   bool is_internal;
 };
 
+bool IsInternalDisplayId(int64 id) {
+  return id == gfx::Display::InternalDisplayId() &&
+         id != gfx::Display::kInvalidDisplayID;
+}
+
 }  // namespace
 
 DisplayMode::DisplayMode()
@@ -84,12 +89,11 @@ DisplayMode::DisplayMode(const gfx::Size& size,
 gfx::Size DisplayMode::GetSizeInDIP(bool is_internal) const {
   gfx::SizeF size_dip(size);
   size_dip.Scale(ui_scale);
-  // DSF=1.25 is special. The screen is drawn with DSF=1.25 in some mode but it
-  // doesn't affect the screen size computation.
-  if (!(use_125_dsf_for_ui_scaling && is_internal) ||
-      device_scale_factor != 1.25f) {
-    size_dip.Scale(1.0f / device_scale_factor);
-  }
+  // DSF=1.25 is special on internal display. The screen is drawn with DSF=1.25
+  // but it doesn't affect the screen size computation.
+  if (use_125_dsf_for_ui_scaling && is_internal && device_scale_factor == 1.25f)
+    return gfx::ToFlooredSize(size_dip);
+  size_dip.Scale(1.0f / device_scale_factor);
   return gfx::ToFlooredSize(size_dip);
 }
 
@@ -105,11 +109,6 @@ DisplayInfo DisplayInfo::CreateFromSpec(const std::string& spec) {
   return CreateFromSpecWithID(spec, gfx::Display::kInvalidDisplayID);
 }
 
-// static
-void DisplayInfo::SetUse125DSFForUIScaling(bool enable) {
-  use_125_dsf_for_ui_scaling = enable;
-}
-
 // static
 DisplayInfo DisplayInfo::CreateFromSpecWithID(const std::string& spec,
                                               int64 id) {
@@ -231,6 +230,11 @@ DisplayInfo DisplayInfo::CreateFromSpecWithID(const std::string& spec,
   return display_info;
 }
 
+// static
+void DisplayInfo::SetUse125DSFForUIScalingForTest(bool enable) {
+  use_125_dsf_for_ui_scaling = enable;
+}
+
 DisplayInfo::DisplayInfo()
     : id_(gfx::Display::kInvalidDisplayID),
       has_overscan_(false),
@@ -321,7 +325,7 @@ void DisplayInfo::SetBounds(const gfx::Rect& new_bounds_in_native) {
 }
 
 float DisplayInfo::GetEffectiveDeviceScaleFactor() const {
-  if (Use125DSFRorUIScaling() && device_scale_factor_ == 1.25f)
+  if (Use125DSFForUIScaling() && device_scale_factor_ == 1.25f)
     return (configured_ui_scale_ == 0.8f) ? 1.25f : 1.0f;
   if (device_scale_factor_ == configured_ui_scale_)
     return 1.0f;
@@ -329,7 +333,7 @@ float DisplayInfo::GetEffectiveDeviceScaleFactor() const {
 }
 
 float DisplayInfo::GetEffectiveUIScale() const {
-  if (Use125DSFRorUIScaling() && device_scale_factor_ == 1.25f)
+  if (Use125DSFForUIScaling() && device_scale_factor_ == 1.25f)
     return (configured_ui_scale_ == 0.8f) ? 1.0f : configured_ui_scale_;
   if (device_scale_factor_ == configured_ui_scale_)
     return 1.0f;
@@ -367,7 +371,7 @@ void DisplayInfo::SetDisplayModes(
     const std::vector<DisplayMode>& display_modes) {
   display_modes_ = display_modes;
   std::sort(display_modes_.begin(), display_modes_.end(),
-            DisplayModeSorter(id_ == gfx::Display::InternalDisplayId()));
+            DisplayModeSorter(IsInternalDisplayId(id_)));
 }
 
 gfx::Size DisplayInfo::GetNativeModeSize() const {
@@ -429,8 +433,8 @@ bool DisplayInfo::IsColorProfileAvailable(
                    profile) != available_color_profiles_.end();
 }
 
-bool DisplayInfo::Use125DSFRorUIScaling() const {
-  return use_125_dsf_for_ui_scaling && id_ == gfx::Display::InternalDisplayId();
+bool DisplayInfo::Use125DSFForUIScaling() const {
+  return use_125_dsf_for_ui_scaling && IsInternalDisplayId(id_);
 }
 
 }  // namespace ash

ash/display/display_info.h

@@ -86,14 +86,14 @@ class ASH_EXPORT DisplayInfo {
   static DisplayInfo CreateFromSpecWithID(const std::string& spec,
                                           int64 id);
 
-  DisplayInfo();
-  DisplayInfo(int64 id, const std::string& name, bool has_overscan);
-  ~DisplayInfo();
-
   // When this is set to true on the device whose internal display has
   // 1.25 dsf, Chrome uses 1.0f as a default scale factor, and uses
   // dsf 1.25 when UI scaling is set to 0.8f.
-  static void SetUse125DSFForUIScaling(bool enable);
+  static void SetUse125DSFForUIScalingForTest(bool enable);
+
+  DisplayInfo();
+  DisplayInfo(int64 id, const std::string& name, bool has_overscan);
+  ~DisplayInfo();
 
   int64 id() const { return id_; }
 
@@ -235,7 +235,7 @@ class ASH_EXPORT DisplayInfo {
  private:
   // Returns true if this display should use DSF=1.25 for UI scaling; i.e.
   // SetUse125DSFForUIScaling(true) is called and this is the internal display.
-  bool Use125DSFRorUIScaling() const;
+  bool Use125DSFForUIScaling() const;
 
   int64 id_;
   std::string name_;

ash/display/display_info_unittest.cc

@@ -110,23 +110,19 @@ TEST_F(DisplayInfoTest, DisplayModeGetSizeInDIPHiDPI) {
 }
 
 TEST_F(DisplayInfoTest, DisplayModeGetSizeInDIP125) {
-  DisplayInfo::SetUse125DSFForUIScaling(true);
   gfx::Size size(1920, 1080);
   EXPECT_EQ("2400x1350", GetModeSizeInDIP(size, 1.25f, 1.25f, true));
   EXPECT_EQ("1920x1080", GetModeSizeInDIP(size, 1.25f, 1.0f, true));
   EXPECT_EQ("1536x864", GetModeSizeInDIP(size, 1.25f, 0.8f, true));
   EXPECT_EQ("1200x675", GetModeSizeInDIP(size, 1.25f, 0.625f, true));
   EXPECT_EQ("960x540", GetModeSizeInDIP(size, 1.25f, 0.5f, true));
-  DisplayInfo::SetUse125DSFForUIScaling(false);
 }
 
 TEST_F(DisplayInfoTest, DisplayModeGetSizeForExternal4K) {
-  DisplayInfo::SetUse125DSFForUIScaling(true);
   gfx::Size size(3840, 2160);
   EXPECT_EQ("1920x1080", GetModeSizeInDIP(size, 2.0f, 1.0f, false));
   EXPECT_EQ("3072x1728", GetModeSizeInDIP(size, 1.25f, 1.0f, false));
   EXPECT_EQ("3840x2160", GetModeSizeInDIP(size, 1.0f, 1.0f, false));
-  DisplayInfo::SetUse125DSFForUIScaling(false);
 }
 
 }  // namespace ash

ash/display/display_manager.cc

@@ -136,10 +136,6 @@ DisplayManager::DisplayManager()
       registered_internal_display_rotation_(gfx::Display::ROTATE_0),
       weak_ptr_factory_(this) {
 #if defined(OS_CHROMEOS)
-  // Enable only on the device so that DisplayManagerFontTest passes.
-  if (base::SysInfo::IsRunningOnChromeOS())
-    DisplayInfo::SetUse125DSFForUIScaling(true);
-
   change_display_upon_host_resize_ = !base::SysInfo::IsRunningOnChromeOS();
 #endif
   gfx::Screen::SetScreenInstance(gfx::SCREEN_TYPE_ALTERNATE, screen_.get());

ash/display/display_manager_unittest.cc

@@ -942,7 +942,7 @@ TEST_F(DisplayManagerTest, Rotate) {
 }
 
 TEST_F(DisplayManagerTest, UIScale) {
-  DisplayInfo::SetUse125DSFForUIScaling(false);
+  test::ScopedDisable125DSFForUIScaling disable;
 
   UpdateDisplay("1280x800");
   int64 display_id = Shell::GetScreen()->GetPrimaryDisplay().id();
@@ -1113,9 +1113,7 @@ TEST_F(DisplayManagerTest, UIScaleWithDisplayMode) {
       display_manager()->GetActiveModeForDisplayId(display_id)));
 }
 
-TEST_F(DisplayManagerTest, Use125DSFRorUIScaling) {
-  DisplayInfo::SetUse125DSFForUIScaling(true);
-
+TEST_F(DisplayManagerTest, Use125DSFForUIScaling) {
   int64 display_id = Shell::GetScreen()->GetPrimaryDisplay().id();
   test::DisplayManagerTestApi(display_manager())
       .SetInternalDisplayId(display_id);
@@ -1138,8 +1136,6 @@ TEST_F(DisplayManagerTest, Use125DSFRorUIScaling) {
   EXPECT_EQ(1.0f, GetDisplayInfoAt(0).GetEffectiveDeviceScaleFactor());
   EXPECT_EQ(1.25f, GetDisplayInfoAt(0).GetEffectiveUIScale());
   EXPECT_EQ("2400x1350", GetDisplayForId(display_id).size().ToString());
-
-  DisplayInfo::SetUse125DSFForUIScaling(false);
 }
 
 TEST_F(DisplayManagerTest, UIScaleInUnifiedMode) {
@@ -1727,6 +1723,7 @@ TEST_F(DisplayManagerFontTest, TextSubpixelPositioningWithDsf100Internal) {
 }
 
 TEST_F(DisplayManagerFontTest, TextSubpixelPositioningWithDsf125Internal) {
+  test::ScopedDisable125DSFForUIScaling disable;
   FontTestHelper helper(1.25f, FontTestHelper::INTERNAL);
   ASSERT_DOUBLE_EQ(
       1.25f, Shell::GetScreen()->GetPrimaryDisplay().device_scale_factor());
@@ -1776,7 +1773,6 @@ TEST_F(DisplayManagerFontTest, TextSubpixelPositioningWithDsf200External) {
 
 TEST_F(DisplayManagerFontTest,
        TextSubpixelPositioningWithDsf125InternalWithScaling) {
-  DisplayInfo::SetUse125DSFForUIScaling(true);
   FontTestHelper helper(1.25f, FontTestHelper::INTERNAL);
   ASSERT_DOUBLE_EQ(
       1.0f, Shell::GetScreen()->GetPrimaryDisplay().device_scale_factor());
@@ -1790,8 +1786,6 @@ TEST_F(DisplayManagerFontTest,
       1.25f, Shell::GetScreen()->GetPrimaryDisplay().device_scale_factor());
   EXPECT_TRUE(IsTextSubpixelPositioningEnabled());
   EXPECT_EQ(gfx::FontRenderParams::HINTING_NONE, GetFontHintingParams());
-
-  DisplayInfo::SetUse125DSFForUIScaling(false);
 }
 
 TEST_F(DisplayManagerTest, CheckInitializationOfRotationProperty) {

ash/test/display_manager_test_api.cc

@@ -169,5 +169,13 @@ void DisplayManagerTestApi::SetAvailableColorProfiles(
       profiles);
 }
 
+ScopedDisable125DSFForUIScaling::ScopedDisable125DSFForUIScaling() {
+  DisplayInfo::SetUse125DSFForUIScalingForTest(false);
+}
+
+ScopedDisable125DSFForUIScaling::~ScopedDisable125DSFForUIScaling() {
+  DisplayInfo::SetUse125DSFForUIScalingForTest(true);
+}
+
 }  // namespace test
 }  // namespace ash

ash/test/display_manager_test_api.h

@@ -66,6 +66,15 @@ class DisplayManagerTestApi {
   DISALLOW_COPY_AND_ASSIGN(DisplayManagerTestApi);
 };
 
+class ScopedDisable125DSFForUIScaling {
+ public:
+  ScopedDisable125DSFForUIScaling();
+  ~ScopedDisable125DSFForUIScaling();
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ScopedDisable125DSFForUIScaling);
+};
+
 }  // namespace test
 }  // namespace ash