Remember the configured device scale factor for external displays.

Users can specify the device scale factor for some external
displays. Now, it remembers the configured device scale factor
to the local storage, and recovers to the specified value when
reconnected.

This also fixes the bug when the user specifies the resolution
and the device scale factor at the same time (DSF was ignored
beforehand).

BUG=396704
R=oshima@chromium.org
TEST=manually, unit_tests

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

Cr-Commit-Position: refs/heads/master@{#288412}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@288412 0039d316-1c4b-4281-b951-d872f2087c98

ash/display/display_change_observer_chromeos.cc

@@ -191,10 +191,16 @@ void DisplayChangeObserver::OnDisplayModeChanged(
       continue;
 
     float device_scale_factor = 1.0f;
-    if (!ui::IsDisplaySizeBlackListed(state.display->physical_size())) {
+    if (state.display->type() == ui::DISPLAY_CONNECTION_TYPE_INTERNAL) {
       device_scale_factor =
           FindDeviceScaleFactor((kInchInMm * mode_info->size().width() /
                                  state.display->physical_size().width()));
+    } else {
+      DisplayMode mode;
+      if (Shell::GetInstance()->display_manager()->GetSelectedModeForDisplayId(
+              state.display->display_id(), &mode)) {
+        device_scale_factor = mode.device_scale_factor;
+      }
     }
     gfx::Rect display_bounds(state.display->origin(), mode_info->size());
 

ash/display/display_info.cc

@@ -26,6 +26,24 @@ namespace {
 // TODO(oshima): This feature is obsolete. Remove this after m38.
 bool allow_upgrade_to_high_dpi = false;
 
+// Check the content of |spec| and fill |bounds| and |device_scale_factor|.
+// Returns true when |bounds| is found.
+bool GetDisplayBounds(
+    const std::string& spec, gfx::Rect* bounds, float* device_scale_factor) {
+  int width = 0;
+  int height = 0;
+  int x = 0;
+  int y = 0;
+  if (sscanf(spec.c_str(), "%dx%d*%f",
+             &width, &height, device_scale_factor) >= 2 ||
+      sscanf(spec.c_str(), "%d+%d-%dx%d*%f", &x, &y, &width, &height,
+             device_scale_factor) >= 4) {
+    bounds->SetRect(x, y, width, height);
+    return true;
+  }
+  return false;
+}
+
 }
 
 DisplayMode::DisplayMode()
@@ -126,14 +144,8 @@ DisplayInfo DisplayInfo::CreateFromSpecWithID(const std::string& spec,
     }
   }
 
-  int x = 0, y = 0, width, height;
   float device_scale_factor = 1.0f;
-  if (sscanf(main_spec.c_str(), "%dx%d*%f",
-             &width, &height, &device_scale_factor) >= 2 ||
-      sscanf(main_spec.c_str(), "%d+%d-%dx%d*%f", &x, &y, &width, &height,
-             &device_scale_factor) >= 4) {
-    bounds_in_native.SetRect(x, y, width, height);
-  } else {
+  if (!GetDisplayBounds(main_spec, &bounds_in_native, &device_scale_factor)) {
 #if defined(OS_WIN)
     if (gfx::IsHighDPIEnabled()) {
       device_scale_factor = gfx::GetDPIScale();
@@ -150,23 +162,23 @@ DisplayInfo DisplayInfo::CreateFromSpecWithID(const std::string& spec,
     std::string resolution_list = parts[1];
     count = Tokenize(resolution_list, "|", &parts);
     for (size_t i = 0; i < count; ++i) {
-      std::string resolution = parts[i];
-      int width, height;
-      float refresh_rate = 0.0f;
-      if (sscanf(resolution.c_str(),
-                 "%dx%d%%%f",
-                 &width,
-                 &height,
-                 &refresh_rate) >= 2) {
-        if (width * height >= largest_area &&
-            refresh_rate > highest_refresh_rate) {
+      DisplayMode mode;
+      gfx::Rect mode_bounds;
+      std::vector<std::string> resolution;
+      Tokenize(parts[i], "%", &resolution);
+      if (GetDisplayBounds(
+              resolution[0], &mode_bounds, &mode.device_scale_factor)) {
+        mode.size = mode_bounds.size();
+        if (resolution.size() > 1)
+          sscanf(resolution[1].c_str(), "%f", &mode.refresh_rate);
+        if (mode.size.GetArea() >= largest_area &&
+            mode.refresh_rate > highest_refresh_rate) {
           // Use mode with largest area and highest refresh rate as native.
-          largest_area = width * height;
-          highest_refresh_rate = refresh_rate;
+          largest_area = mode.size.GetArea();
+          highest_refresh_rate = mode.refresh_rate;
           native_mode = i;
         }
-        display_modes.push_back(
-            DisplayMode(gfx::Size(width, height), refresh_rate, false, false));
+        display_modes.push_back(mode);
       }
     }
     display_modes[native_mode].native = true;

ash/display/display_info_unittest.cc

@@ -44,17 +44,27 @@ TEST_F(DisplayInfoTest, CreateFromSpec) {
   EXPECT_EQ(1.5f, info.configured_ui_scale());
 
   info = DisplayInfo::CreateFromSpecWithID(
-      "200x200#300x200|200x200%59.9|100x100%60", 10);
+      "200x200#300x200|200x200%59.9|100x100%60|150x100*2|150x150*1.25%30", 10);
   EXPECT_EQ("0,0 200x200", info.bounds_in_native().ToString());
-  EXPECT_EQ(3u, info.display_modes().size());
+  EXPECT_EQ(5u, info.display_modes().size());
   EXPECT_EQ("300x200", info.display_modes()[0].size.ToString());
   EXPECT_EQ("200x200", info.display_modes()[1].size.ToString());
   EXPECT_EQ("100x100", info.display_modes()[2].size.ToString());
+  EXPECT_EQ("150x100", info.display_modes()[3].size.ToString());
+  EXPECT_EQ("150x150", info.display_modes()[4].size.ToString());
   EXPECT_EQ(59.9f, info.display_modes()[1].refresh_rate);
   EXPECT_EQ(60.0f, info.display_modes()[2].refresh_rate);
+  EXPECT_EQ(30.0f, info.display_modes()[4].refresh_rate);
+  EXPECT_EQ(1.0f, info.display_modes()[0].device_scale_factor);
+  EXPECT_EQ(1.0f, info.display_modes()[1].device_scale_factor);
+  EXPECT_EQ(1.0f, info.display_modes()[2].device_scale_factor);
+  EXPECT_EQ(2.0f, info.display_modes()[3].device_scale_factor);
+  EXPECT_EQ(1.25f, info.display_modes()[4].device_scale_factor);
   EXPECT_TRUE(info.display_modes()[0].native);
   EXPECT_FALSE(info.display_modes()[1].native);
   EXPECT_FALSE(info.display_modes()[2].native);
+  EXPECT_FALSE(info.display_modes()[3].native);
+  EXPECT_FALSE(info.display_modes()[4].native);
 }
 
 }  // namespace ash

ash/display/display_manager.cc

@@ -530,6 +530,7 @@ void DisplayManager::RegisterDisplayProperty(
     float ui_scale,
     const gfx::Insets* overscan_insets,
     const gfx::Size& resolution_in_pixels,
+    float device_scale_factor,
     ui::ColorCalibrationProfile color_profile) {
   if (display_info_.find(display_id) == display_info_.end())
     display_info_[display_id] = DisplayInfo(display_id, std::string(), false);
@@ -544,10 +545,12 @@ void DisplayManager::RegisterDisplayProperty(
   if (overscan_insets)
     display_info_[display_id].SetOverscanInsets(*overscan_insets);
   if (!resolution_in_pixels.IsEmpty()) {
+    DCHECK(!IsInternalDisplayId(display_id));
     // Default refresh rate, until OnNativeDisplaysChanged() updates us with the
     // actual display info, is 60 Hz.
-    display_modes_[display_id] =
-        DisplayMode(resolution_in_pixels, 60.0f, false, false);
+    DisplayMode mode(resolution_in_pixels, 60.0f, false, false);
+    mode.device_scale_factor = device_scale_factor;
+    display_modes_[display_id] = mode;
   }
 }
 

ash/display/display_manager.h

@@ -182,6 +182,7 @@ class ASH_EXPORT DisplayManager
                                float ui_scale,
                                const gfx::Insets* overscan_insets,
                                const gfx::Size& resolution_in_pixels,
+                               float device_scale_factor,
                                ui::ColorCalibrationProfile color_profile);
 
   // Returns the display mode of |display_id| which is currently used.

chrome/browser/chromeos/display/display_preferences.cc

@@ -166,6 +166,11 @@ void LoadDisplayProperties() {
     dict_value->GetInteger("height", &height);
     gfx::Size resolution_in_pixels(width, height);
 
+    float device_scale_factor = 1.0;
+    int dsf_value = 0;
+    if (dict_value->GetInteger("device-scale-factor", &dsf_value))
+      device_scale_factor = static_cast<float>(dsf_value) / 1000.0f;
+
     gfx::Insets insets;
     if (ValueToInsets(*dict_value, &insets))
       insets_to_set = &insets;
@@ -179,6 +184,7 @@ void LoadDisplayProperties() {
                                                  ui_scale,
                                                  insets_to_set,
                                                  resolution_in_pixels,
+                                                 device_scale_factor,
                                                  color_profile);
   }
 }
@@ -238,6 +244,9 @@ void StoreCurrentDisplayProperties() {
         !mode.native) {
       property_value->SetInteger("width", mode.size.width());
       property_value->SetInteger("height", mode.size.height());
+      property_value->SetInteger(
+          "device-scale-factor",
+          static_cast<int>(mode.device_scale_factor * 1000));
     }
     if (!info.overscan_insets_in_dip().empty())
       InsetsToValue(info.overscan_insets_in_dip(), property_value.get());

chrome/browser/chromeos/display/display_preferences_unittest.cc

@@ -199,7 +199,7 @@ TEST_F(DisplayPreferencesTest, BasicStores) {
   ash::DisplayManager* display_manager =
       ash::Shell::GetInstance()->display_manager();
 
-  UpdateDisplay("200x200*2, 400x300#400x400|300x200");
+  UpdateDisplay("200x200*2, 400x300#400x400|300x200*1.25");
   int64 id1 = gfx::Screen::GetNativeScreen()->GetPrimaryDisplay().id();
   gfx::Display::SetInternalDisplayId(id1);
   int64 id2 = ash::ScreenUtil::GetSecondaryDisplay().id();
@@ -292,13 +292,15 @@ TEST_F(DisplayPreferencesTest, BasicStores) {
   EXPECT_FALSE(property->GetString("color_profile_name", &color_profile));
 
   // Resolution is saved only when the resolution is set
-  // by DisplayManager::SetDisplayResolution
+  // by DisplayManager::SetDisplayMode
   width = 0;
   height = 0;
   EXPECT_FALSE(property->GetInteger("width", &width));
   EXPECT_FALSE(property->GetInteger("height", &height));
 
-  display_manager->SetDisplayResolution(id2, gfx::Size(300, 200));
+  ash::DisplayMode mode(gfx::Size(300, 200), 60.0f, false, true);
+  mode.device_scale_factor = 1.25f;
+  display_manager->SetDisplayMode(id2, mode);
 
   display_controller->SetPrimaryDisplayId(id2);
 
@@ -311,11 +313,15 @@ TEST_F(DisplayPreferencesTest, BasicStores) {
 
   // External display's resolution must be stored this time because
   // it's not best.
+  int device_scale_factor = 0;
   EXPECT_TRUE(properties->GetDictionary(base::Int64ToString(id2), &property));
   EXPECT_TRUE(property->GetInteger("width", &width));
   EXPECT_TRUE(property->GetInteger("height", &height));
+  EXPECT_TRUE(property->GetInteger(
+      "device-scale-factor", &device_scale_factor));
   EXPECT_EQ(300, width);
   EXPECT_EQ(200, height);
+  EXPECT_EQ(1250, device_scale_factor);
 
   // The layout remains the same.
   EXPECT_TRUE(displays->GetDictionary(key, &layout_value));
@@ -364,7 +370,7 @@ TEST_F(DisplayPreferencesTest, BasicStores) {
 
   // Set new display's selected resolution.
   display_manager->RegisterDisplayProperty(
-      id2 + 1, gfx::Display::ROTATE_0, 1.0f, NULL, gfx::Size(500, 400),
+      id2 + 1, gfx::Display::ROTATE_0, 1.0f, NULL, gfx::Size(500, 400), 1.0f,
       ui::COLOR_PROFILE_STANDARD);
 
   UpdateDisplay("200x200*2, 600x500#600x500|500x400");
@@ -390,7 +396,7 @@ TEST_F(DisplayPreferencesTest, BasicStores) {
 
   // Set yet another new display's selected resolution.
   display_manager->RegisterDisplayProperty(
-      id2 + 1, gfx::Display::ROTATE_0, 1.0f, NULL, gfx::Size(500, 400),
+      id2 + 1, gfx::Display::ROTATE_0, 1.0f, NULL, gfx::Size(500, 400), 1.0f,
       ui::COLOR_PROFILE_STANDARD);
   // Disconnect 2nd display first to generate new id for external display.
   UpdateDisplay("200x200*2");
@@ -452,9 +458,9 @@ TEST_F(DisplayPreferencesTest, PreventStore) {
           ResolutionNotificationController::kNotificationId));
 
   // Once the notification is removed, the specified resolution will be stored
-  // by SetDisplayResolution.
-  ash::Shell::GetInstance()->display_manager()->SetDisplayResolution(
-      id, gfx::Size(300, 200));
+  // by SetDisplayMode.
+  ash::Shell::GetInstance()->display_manager()->SetDisplayMode(
+      id, ash::DisplayMode(gfx::Size(300, 200), 60.0f, false, true));
   UpdateDisplay("300x200#500x400|400x300|300x200");
 
   property = NULL;