Enable subpixel positioning for internal display with dsf larger than 1.0 Remove enable-browser-text-subpixel-positioning flag.

BUG=388844
TEST=covered by tests
TBR=oshima@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#291227}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@291227 0039d316-1c4b-4281-b951-d872f2087c98
(cherry picked from commit 68d6080e31b22baeec8c1fd7d06b8960256aa0a1)

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

Cr-Commit-Position: refs/branch-heads/2125@{#94}
Cr-Branched-From: b68026d94bda36dd106a3d91a098719f952a9477-refs/heads/master@{#290040}

10 files changed, 170 insertions, 11 deletions

diff --git a/ash/display/display_manager.cc b/ash/display/display_manager.cc
index 6d64f36..f3b7a20 100644
--- a/ash/display/display_manager.cc
+++ b/ash/display/display_manager.cc
@@ -29,6 +29,7 @@
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/display.h"
 #include "ui/gfx/display_observer.h"
+#include "ui/gfx/font_render_params.h"
 #include "ui/gfx/rect.h"
 #include "ui/gfx/screen.h"
 #include "ui/gfx/size_conversions.h"
@@ -183,6 +184,10 @@ DisplayManager::DisplayManager()
 }
 
 DisplayManager::~DisplayManager() {
+#if defined(OS_CHROMEOS)
+  // Reset the font params.
+  gfx::SetFontRenderParamsDeviceScaleFactor(1.0f);
+#endif
 }
 
 // static
@@ -262,6 +267,17 @@ void DisplayManager::InitDefaultDisplay() {
   OnNativeDisplaysChanged(info_list);
 }
 
+void DisplayManager::InitFontParams() {
+#if defined(OS_CHROMEOS)
+  if (!HasInternalDisplay())
+    return;
+  const DisplayInfo& display_info =
+      GetDisplayInfo(gfx::Display::InternalDisplayId());
+  gfx::SetFontRenderParamsDeviceScaleFactor(
+      display_info.device_scale_factor());
+#endif  // OS_CHROMEOS
+}
+
 // static
 void DisplayManager::UpdateDisplayBoundsForLayoutById(
     const DisplayLayout& layout,
diff --git a/ash/display/display_manager.h b/ash/display/display_manager.h
index 9c57fde21..4be4bbd 100644
--- a/ash/display/display_manager.h
+++ b/ash/display/display_manager.h
@@ -122,6 +122,10 @@ class ASH_EXPORT DisplayManager
   // Initialize default display.
   void InitDefaultDisplay();
 
+  // Initializes font related params that depends on display
+  // configuration.
+  void InitFontParams();
+
   // True if the given |display| is currently connected.
   bool IsActiveDisplay(const gfx::Display& display) const;
 
diff --git a/ash/display/display_manager_unittest.cc b/ash/display/display_manager_unittest.cc
index 2d3fc3e..ff26076 100644
--- a/ash/display/display_manager_unittest.cc
+++ b/ash/display/display_manager_unittest.cc
@@ -4,6 +4,7 @@
 
 #include "ash/display/display_manager.h"
 
+#include "ash/ash_switches.h"
 #include "ash/display/display_controller.h"
 #include "ash/display/display_layout_store.h"
 #include "ash/screen_util.h"
@@ -11,6 +12,7 @@
 #include "ash/test/ash_test_base.h"
 #include "ash/test/display_manager_test_api.h"
 #include "ash/test/mirror_window_test_api.h"
+#include "base/command_line.h"
 #include "base/format_macros.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
@@ -20,6 +22,7 @@
 #include "ui/events/test/event_generator.h"
 #include "ui/gfx/display.h"
 #include "ui/gfx/display_observer.h"
+#include "ui/gfx/font_render_params.h"
 #include "ui/gfx/screen.h"
 #include "ui/gfx/screen_type_delegate.h"
 
@@ -1381,4 +1384,96 @@ TEST_F(ScreenShutdownTest, ScreenAfterShutdown) {
   UpdateDisplay("500x300,800x400");
 }
 
+
+#if defined(OS_CHROMEOS)
+namespace {
+
+// A helper class that sets the display configuration and starts ash.
+// This is to make sure the font configuration happens during ash
+// initialization process.
+class FontTestHelper : public test::AshTestBase {
+ public:
+  enum DisplayType {
+    INTERNAL,
+    EXTERNAL
+  };
+
+  FontTestHelper(float scale, DisplayType display_type) {
+    gfx::ClearFontRenderParamsCacheForTest();
+    base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
+    if (display_type == INTERNAL)
+      command_line->AppendSwitch(switches::kAshUseFirstDisplayAsInternal);
+    command_line->AppendSwitchASCII(switches::kAshHostWindowBounds,
+                                    StringPrintf("1000x800*%f", scale));
+    SetUp();
+  }
+
+  virtual ~FontTestHelper() {
+    TearDown();
+  }
+
+  // test::AshTestBase:
+  virtual void TestBody() OVERRIDE {
+    NOTREACHED();
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(FontTestHelper);
+};
+
+
+bool IsTextSubpixelPositioningEnabled() {
+  gfx::FontRenderParams params =
+      gfx::GetFontRenderParams(gfx::FontRenderParamsQuery(false), NULL);
+  return params.subpixel_positioning;
+}
+
+}  // namespace
+
+typedef testing::Test DisplayManagerFontTest;
+
+TEST_F(DisplayManagerFontTest, TextSubpixelPositioningWithDsf100Internal) {
+  FontTestHelper helper(1.0f, FontTestHelper::INTERNAL);
+  ASSERT_DOUBLE_EQ(
+      1.0f, Shell::GetScreen()->GetPrimaryDisplay().device_scale_factor());
+  EXPECT_FALSE(IsTextSubpixelPositioningEnabled());
+}
+
+TEST_F(DisplayManagerFontTest, TextSubpixelPositioningWithDsf125Internal) {
+  FontTestHelper helper(1.25f, FontTestHelper::INTERNAL);
+  ASSERT_DOUBLE_EQ(
+      1.25f, Shell::GetScreen()->GetPrimaryDisplay().device_scale_factor());
+  EXPECT_TRUE(IsTextSubpixelPositioningEnabled());
+}
+
+TEST_F(DisplayManagerFontTest, TextSubpixelPositioningWithDsf200Internal) {
+  FontTestHelper helper(2.0f, FontTestHelper::INTERNAL);
+  ASSERT_DOUBLE_EQ(
+      2.0f, Shell::GetScreen()->GetPrimaryDisplay().device_scale_factor());
+  EXPECT_TRUE(IsTextSubpixelPositioningEnabled());
+}
+
+TEST_F(DisplayManagerFontTest, TextSubpixelPositioningWithDsf100External) {
+  FontTestHelper helper(1.0f, FontTestHelper::EXTERNAL);
+  ASSERT_DOUBLE_EQ(
+      1.0f, Shell::GetScreen()->GetPrimaryDisplay().device_scale_factor());
+  EXPECT_FALSE(IsTextSubpixelPositioningEnabled());
+}
+
+TEST_F(DisplayManagerFontTest, TextSubpixelPositioningWithDsf125External) {
+  FontTestHelper helper(1.25f, FontTestHelper::EXTERNAL);
+  ASSERT_DOUBLE_EQ(
+      1.25f, Shell::GetScreen()->GetPrimaryDisplay().device_scale_factor());
+  EXPECT_FALSE(IsTextSubpixelPositioningEnabled());
+}
+
+TEST_F(DisplayManagerFontTest, TextSubpixelPositioningWithDsf200External) {
+  FontTestHelper helper(2.0f, FontTestHelper::EXTERNAL);
+  ASSERT_DOUBLE_EQ(
+      2.0f, Shell::GetScreen()->GetPrimaryDisplay().device_scale_factor());
+  EXPECT_FALSE(IsTextSubpixelPositioningEnabled());
+}
+
+#endif  // OS_CHROMEOS
+
 }  // namespace ash
diff --git a/ash/shell.cc b/ash/shell.cc
index a047748..8254610 100644
--- a/ash/shell.cc
+++ b/ash/shell.cc
@@ -852,6 +852,8 @@ void Shell::Init(const ShellInitParams& init_params) {
   if (!display_initialized)
     display_manager_->InitDefaultDisplay();
 
+  display_manager_->InitFontParams();
+
   // Install the custom factory first so that views::FocusManagers for Tray,
   // Shelf, and WallPaper could be created by the factory.
   views::FocusManagerFactory::Install(new AshFocusManagerFactory);
diff --git a/chrome/browser/chromeos/login/chrome_restart_request.cc b/chrome/browser/chromeos/login/chrome_restart_request.cc
index c02904b..7e6aeae 100644
--- a/chrome/browser/chromeos/login/chrome_restart_request.cc
+++ b/chrome/browser/chromeos/login/chrome_restart_request.cc
@@ -205,7 +205,6 @@ std::string DeriveCommandLine(const GURL& start_url,
     chromeos::switches::kNaturalScrollDefault,
     chromeos::switches::kSystemInDevMode,
     policy::switches::kDeviceManagementUrl,
-    ::switches::kEnableBrowserTextSubpixelPositioning,
     ::switches::kEnableWebkitTextSubpixelPositioning,
     wm::switches::kWindowAnimationsDisabled,
   };
diff --git a/ui/gfx/font_render_params.h b/ui/gfx/font_render_params.h
index cd7c0a4..4ec8d35 100644
--- a/ui/gfx/font_render_params.h
+++ b/ui/gfx/font_render_params.h
@@ -98,6 +98,13 @@ GFX_EXPORT FontRenderParams GetFontRenderParams(
 // out what's going on here.
 GFX_EXPORT void ClearFontRenderParamsCacheForTest();
 
+#if defined(OS_CHROMEOS)
+// Sets the device scale factor for FontRenderParams to decide
+// if it should enable subpixel positioning.
+GFX_EXPORT void SetFontRenderParamsDeviceScaleFactor(
+    float device_scale_factor);
+#endif
+
 }  // namespace gfx
 
 #endif  // UI_GFX_FONT_RENDER_PARAMS_H_
diff --git a/ui/gfx/font_render_params_linux.cc b/ui/gfx/font_render_params_linux.cc
index f02636e..a198358 100644
--- a/ui/gfx/font_render_params_linux.cc
+++ b/ui/gfx/font_render_params_linux.cc
@@ -23,6 +23,12 @@ namespace gfx {
 
 namespace {
 
+#if defined(OS_CHROMEOS)
+// A device scale factor for an internal display (if any)
+// that is used to determine if subpixel positioning should be used.
+float device_scale_factor_for_internal_display = 1.0f;
+#endif
+
 // Keyed by hashes of FontRenderParamQuery structs from
 // HashFontRenderParamsQuery().
 typedef base::MRUCache<uint32, FontRenderParams> Cache;
@@ -43,6 +49,14 @@ struct SynchronizedCache {
 base::LazyInstance<SynchronizedCache>::Leaky g_synchronized_cache =
     LAZY_INSTANCE_INITIALIZER;
 
+bool IsBrowserTextSubpixelPositioningEnabled() {
+#if defined(OS_CHROMEOS)
+  return device_scale_factor_for_internal_display > 1.0f;
+#else
+  return false;
+#endif
+}
+
 // Converts Fontconfig FC_HINT_STYLE to FontRenderParams::Hinting.
 FontRenderParams::Hinting ConvertFontconfigHintStyle(int hint_style) {
   switch (hint_style) {
@@ -176,7 +190,6 @@ FontRenderParams GetFontRenderParams(const FontRenderParamsQuery& query,
   if (delegate)
     params = delegate->GetDefaultFontRenderParams();
   QueryFontconfig(query, &params, family_out);
-
   if (!params.antialiasing) {
     // Cairo forces full hinting when antialiasing is disabled, since anything
     // less than that looks awful; do the same here. Requesting subpixel
@@ -187,10 +200,11 @@ FontRenderParams GetFontRenderParams(const FontRenderParamsQuery& query,
   } else {
     // Fontconfig doesn't support configuring subpixel positioning; check a
     // flag.
-    params.subpixel_positioning = CommandLine::ForCurrentProcess()->HasSwitch(
+    params.subpixel_positioning =
         query.for_web_contents ?
-        switches::kEnableWebkitTextSubpixelPositioning :
-        switches::kEnableBrowserTextSubpixelPositioning);
+        CommandLine::ForCurrentProcess()->HasSwitch(
+            switches::kEnableWebkitTextSubpixelPositioning) :
+        IsBrowserTextSubpixelPositioningEnabled();
 
     // To enable subpixel positioning, we need to disable hinting.
     if (params.subpixel_positioning)
@@ -217,4 +231,10 @@ void ClearFontRenderParamsCacheForTest() {
   synchronized_cache->cache.Clear();
 }
 
+#if defined(OS_CHROMEOS)
+void SetFontRenderParamsDeviceScaleFactor(float device_scale_factor) {
+  device_scale_factor_for_internal_display = device_scale_factor;
+}
+#endif
+
 }  // namespace gfx
diff --git a/ui/gfx/font_render_params_linux_unittest.cc b/ui/gfx/font_render_params_linux_unittest.cc
index f89c396..46c82b4 100644
--- a/ui/gfx/font_render_params_linux_unittest.cc
+++ b/ui/gfx/font_render_params_linux_unittest.cc
@@ -273,6 +273,28 @@ TEST_F(FontRenderParamsTest, ForceFullHintingWhenAntialiasingIsDisabled) {
   EXPECT_FALSE(params.subpixel_positioning);
 }
 
+#if defined(OS_CHROMEOS)
+TEST_F(FontRenderParamsTest, ForceSubpixelPositioning) {
+  {
+    FontRenderParams params =
+        GetFontRenderParams(FontRenderParamsQuery(false), NULL);
+    EXPECT_TRUE(params.antialiasing);
+    EXPECT_FALSE(params.subpixel_positioning);
+    SetFontRenderParamsDeviceScaleFactor(1.0f);
+  }
+  ClearFontRenderParamsCacheForTest();
+  SetFontRenderParamsDeviceScaleFactor(1.25f);
+  // Subpixel positioning should be forced.
+  {
+    FontRenderParams params =
+        GetFontRenderParams(FontRenderParamsQuery(false), NULL);
+    EXPECT_TRUE(params.antialiasing);
+    EXPECT_TRUE(params.subpixel_positioning);
+    SetFontRenderParamsDeviceScaleFactor(1.0f);
+  }
+}
+#endif
+
 TEST_F(FontRenderParamsTest, OnlySetConfiguredValues) {
   // Configure the LinuxFontDelegate (which queries GtkSettings on desktop
   // Linux) to request subpixel rendering.
diff --git a/ui/gfx/switches.cc b/ui/gfx/switches.cc
index 6f3da80..f3f3294 100644
--- a/ui/gfx/switches.cc
+++ b/ui/gfx/switches.cc
@@ -15,11 +15,6 @@ const char kDisableArbitraryScaleFactorInImageSkia[] =
 // Disables the HarfBuzz port of RenderText on all platforms.
 const char kDisableHarfBuzzRenderText[] = "disable-harfbuzz-rendertext";
 
-// Let text glyphs have X-positions that aren't snapped to the pixel grid in
-// the browser UI.
-const char kEnableBrowserTextSubpixelPositioning[] =
-    "enable-browser-text-subpixel-positioning";
-
 // Enables the HarfBuzz port of RenderText on all platforms.
 const char kEnableHarfBuzzRenderText[] = "enable-harfbuzz-rendertext";
 
diff --git a/ui/gfx/switches.h b/ui/gfx/switches.h
index 48b5633e..e968a49 100644
--- a/ui/gfx/switches.h
+++ b/ui/gfx/switches.h
@@ -12,7 +12,6 @@ namespace switches {
 GFX_EXPORT extern const char kAllowArbitraryScaleFactorInImageSkia[];
 GFX_EXPORT extern const char kDisableArbitraryScaleFactorInImageSkia[];
 GFX_EXPORT extern const char kDisableHarfBuzzRenderText[];
-GFX_EXPORT extern const char kEnableBrowserTextSubpixelPositioning[];
 GFX_EXPORT extern const char kEnableHarfBuzzRenderText[];
 GFX_EXPORT extern const char kEnableWebkitTextSubpixelPositioning[];
 GFX_EXPORT extern const char kForceDeviceScaleFactor[];