Enable subpixel positioning for internal display with dsf larger than 1.0

ash/display/display_manager_unittest.cc

Index: ash/display/display_manager_unittest.cc
diff --git a/ash/display/display_manager_unittest.cc b/ash/display/display_manager_unittest.cc
index 2d3fc3e185920c9ce9196ec2f3398dbc1d5b649f..d705806618d7f6029cf146375e8b5e3f10eb11d4 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,80 @@ TEST_F(ScreenShutdownTest, ScreenAfterShutdown) {
   UpdateDisplay("500x300,800x400");
 }
 
+
+#if defined(OS_CHROMEOS)
+template <const char* VALUE, bool INTERNAL>

[Daniel Erat, 2014/08/20 04:20:59]

i don't love using a template here instead of putting all of this inside of a
single test with a helper method that does the setup and returns whether
subpixel positioning is enabled. is the issue that you can't change the switches
repeatedly within a single test?

[oshima, 2014/08/20 18:36:21]

Can you elaborate why you don't like this?

It's possible to create a helper method which does init process, but I prefer
the test works in the same way as others. I think that's more important than
avoiding template.

+class DisplayManagerFontTest : public test::AshTestBase {
+ public:
+  DisplayManagerFontTest() {}
+  virtual ~DisplayManagerFontTest() {}
+
+  virtual void SetUp() OVERRIDE {
+    gfx::ClearFontRenderParamsCacheForTest();
+    base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
+    if (INTERNAL)
+      command_line->AppendSwitch(switches::kAshUseFirstDisplayAsInternal);
+    command_line->AppendSwitchASCII(switches::kAshHostWindowBounds, VALUE);
+    test::AshTestBase::SetUp();
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(DisplayManagerFontTest);
+};
+
+const char DSF100[] = "1000x800*1.00";
+const char DSF125[] = "1000x800*1.25";
+const char DSF200[] = "1000x800*2.00";
+typedef DisplayManagerFontTest<DSF100, true> DisplayManagerFontTestInternal100;
+typedef DisplayManagerFontTest<DSF125, true> DisplayManagerFontTestInternal125;
+typedef DisplayManagerFontTest<DSF200, true> DisplayManagerFontTestInternal200;
+
+bool IsTextSubpixelPoisitioningEnabled() {

[Daniel Erat, 2014/08/20 04:20:59]

you have the same typo as before here; should be "Positioning" (starts with
"Pos" instead of "Pois")

[oshima, 2014/08/20 18:36:20]

Done.

+  gfx::FontRenderParams params =
+      gfx::GetFontRenderParams(gfx::FontRenderParamsQuery(false), NULL);
+  return params.subpixel_positioning;
+}
+
+TEST_F(DisplayManagerFontTestInternal100, TestDsf100) {
+  ASSERT_EQ(1.0f,

[Daniel Erat, 2014/08/20 04:20:59]

all of these should be ASSERT_DOUBLE_EQ, i think

[oshima, 2014/08/20 18:36:21]

Done.

+            Shell::GetScreen()->GetPrimaryDisplay().device_scale_factor());
+  EXPECT_FALSE(IsTextSubpixelPoisitioningEnabled());
+}
+
+TEST_F(DisplayManagerFontTestInternal125, TestDsf125) {
+  ASSERT_EQ(1.25f,
+            Shell::GetScreen()->GetPrimaryDisplay().device_scale_factor());
+  EXPECT_TRUE(IsTextSubpixelPoisitioningEnabled());
+}
+
+TEST_F(DisplayManagerFontTestInternal200, TestDsf200) {
+  ASSERT_EQ(2.0f,
+            Shell::GetScreen()->GetPrimaryDisplay().device_scale_factor());
+  EXPECT_TRUE(IsTextSubpixelPoisitioningEnabled());
+}
+
+typedef DisplayManagerFontTest<DSF100, false> DisplayManagerFontTestExternal100;
+typedef DisplayManagerFontTest<DSF125, false> DisplayManagerFontTestExternal125;
+typedef DisplayManagerFontTest<DSF200, false> DisplayManagerFontTestExternal200;
+
+TEST_F(DisplayManagerFontTestExternal100, TestDsf100) {
+  ASSERT_EQ(1.0f,
+            Shell::GetScreen()->GetPrimaryDisplay().device_scale_factor());
+  EXPECT_FALSE(IsTextSubpixelPoisitioningEnabled());
+}
+
+TEST_F(DisplayManagerFontTestExternal125, TestDsf125) {
+  ASSERT_EQ(1.25f,
+            Shell::GetScreen()->GetPrimaryDisplay().device_scale_factor());
+  EXPECT_FALSE(IsTextSubpixelPoisitioningEnabled());
+}
+
+TEST_F(DisplayManagerFontTestExternal200, TestDsf200) {
+  ASSERT_EQ(2.0f,
+            Shell::GetScreen()->GetPrimaryDisplay().device_scale_factor());
+  EXPECT_FALSE(IsTextSubpixelPoisitioningEnabled());
+}
+
+#endif  // OS_CHROMEOS
+
 }  // namespace ash