Revert 168974 - Undo all existing overscan settings before updating to a new overscan settings.

Revert Reason:
DisplayControllerTest.CursorDeviceScaleFactorSwapPrimary fails on Win Aura bot.

If an existing display has overscan setting and SetDisplayOverscan is called for
a different display, the display manager will call OnNativeDisplayChanged(), and
it will re-apply the overscan setting to other displays, so the effect could be
twice or more. We need to undo the existing settings beforehand.

BUG=161097
TEST=confirmed with the repro steps in the bug


Review URL: https://chromiumcodereview.appspot.com/11362259

TBR=mukai@chromium.org
Review URL: https://codereview.chromium.org/11414106

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@168989 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 40 insertions, 78 deletions

diff --git a/ash/display/display_controller.cc b/ash/display/display_controller.cc
index 16328fc..1318f16 100644
--- a/ash/display/display_controller.cc
+++ b/ash/display/display_controller.cc
@@ -449,7 +449,7 @@ void DisplayController::SetPrimaryDisplay(
   displays.push_back(display_manager->GetDisplayForId(primary_display_id));
   displays.push_back(*GetSecondaryDisplay());
   GetDisplayManager()->set_force_bounds_changed(true);
-  GetDisplayManager()->UpdateDisplays(displays);
+  GetDisplayManager()->OnNativeDisplaysChanged(displays);
   GetDisplayManager()->set_force_bounds_changed(false);
 }
 
diff --git a/ash/display/display_manager.cc b/ash/display/display_manager.cc
index 70622ce..431a845 100644
--- a/ash/display/display_manager.cc
+++ b/ash/display/display_manager.cc
@@ -144,19 +144,30 @@ const gfx::Display& DisplayManager::FindDisplayContainingPoint(
 
 void DisplayManager::SetOverscanInsets(int64 display_id,
                                        const gfx::Insets& insets_in_dip) {
-  display_info_[display_id].overscan_insets_in_dip = insets_in_dip;
-
-  // Copies the |displays_| because UpdateDisplays() compares the passed
-  // displays and its internal |displays_|.
   DisplayList displays = displays_;
-  UpdateDisplays(displays);
+  std::map<int64, gfx::Insets>::const_iterator old_overscan =
+      overscan_mapping_.find(display_id);
+  if (old_overscan != overscan_mapping_.end()) {
+    gfx::Insets old_insets = old_overscan->second;
+    for (DisplayList::iterator iter = displays.begin();
+         iter != displays.end(); ++iter) {
+      if (iter->id() == display_id) {
+        // Undo the existing insets before applying the new insets.
+        gfx::Rect bounds = iter->bounds_in_pixel();
+        bounds.Inset(old_insets.Scale(-iter->device_scale_factor()));
+        iter->SetScaleAndBounds(iter->device_scale_factor(), bounds);
+        break;
+      }
+    }
+  }
+  overscan_mapping_[display_id] = insets_in_dip;
+  OnNativeDisplaysChanged(displays);
 }
 
 gfx::Insets DisplayManager::GetOverscanInsets(int64 display_id) const {
-  std::map<int64, DisplayInfo>::const_iterator it =
-      display_info_.find(display_id);
-  return (it != display_info_.end()) ?
-      it->second.overscan_insets_in_dip : gfx::Insets();
+  std::map<int64, gfx::Insets>::const_iterator it =
+      overscan_mapping_.find(display_id);
+  return (it != overscan_mapping_.end()) ? it->second : gfx::Insets();
 }
 
 void DisplayManager::OnNativeDisplaysChanged(
@@ -200,35 +211,13 @@ void DisplayManager::OnNativeDisplaysChanged(
     new_displays = updated_displays;
   }
 
-  for (DisplayList::const_iterator iter = updated_displays.begin();
-       iter != updated_displays.end(); ++iter) {
-    std::map<int64, DisplayInfo>::iterator info =
-        display_info_.find(iter->id());
-    if (info != display_info_.end()) {
-      info->second.original_bounds_in_pixel = iter->bounds_in_pixel();
-    } else {
-      DisplayInfo new_info;
-      display_info_[iter->id()].original_bounds_in_pixel =
-          iter->bounds_in_pixel();
-    }
-  }
-
-  UpdateDisplays(new_displays);
-  RefreshDisplayNames();
-}
-
-void DisplayManager::UpdateDisplays(
-    const std::vector<gfx::Display>& updated_displays) {
-  DisplayList new_displays = updated_displays;
-
   for (DisplayList::iterator iter = new_displays.begin();
        iter != new_displays.end(); ++iter) {
-    std::map<int64, DisplayInfo>::const_iterator info =
-        display_info_.find(iter->id());
-    if (info != display_info_.end()) {
-      gfx::Rect bounds = info->second.original_bounds_in_pixel;
-      bounds.Inset(info->second.overscan_insets_in_dip.Scale(
-          iter->device_scale_factor()));
+    std::map<int64, gfx::Insets>::const_iterator overscan_insets =
+        overscan_mapping_.find(iter->id());
+    if (overscan_insets != overscan_mapping_.end()) {
+      gfx::Rect bounds = iter->bounds_in_pixel();
+      bounds.Inset(overscan_insets->second.Scale(iter->device_scale_factor()));
       iter->SetScaleAndBounds(iter->device_scale_factor(), bounds);
     }
   }
@@ -292,6 +281,7 @@ void DisplayManager::UpdateDisplays(
   }
 
   displays_ = new_displays;
+  RefreshDisplayNames();
 
   // Temporarily add displays to be removed because display object
   // being removed are accessed during shutting down the root.
@@ -389,10 +379,10 @@ std::string DisplayManager::GetDisplayNameFor(
   if (!display.is_valid())
     return l10n_util::GetStringUTF8(IDS_ASH_STATUS_TRAY_UNKNOWN_DISPLAY_NAME);
 
-  std::map<int64, DisplayInfo>::const_iterator iter =
-      display_info_.find(display.id());
-  if (iter != display_info_.end())
-    return iter->second.name;
+  std::map<int64, std::string>::const_iterator iter =
+      display_names_.find(display.id());
+  if (iter != display_names_.end())
+    return iter->second;
 
   return base::StringPrintf("Display %d", static_cast<int>(display.id()));
 }
@@ -545,6 +535,8 @@ void DisplayManager::EnsurePointerInDisplays() {
 }
 
 void DisplayManager::RefreshDisplayNames() {
+  display_names_.clear();
+
 #if defined(OS_CHROMEOS)
   if (!base::chromeos::IsRunningOnChromeOS())
     return;
@@ -563,10 +555,10 @@ void DisplayManager::RefreshDisplayNames() {
             outputs[i], &manufacturer_id, &serial_number, &name)) {
       int64 id = gfx::Display::GetID(manufacturer_id, serial_number);
       if (IsInternalDisplayId(id)) {
-        display_info_[id].name =
+        display_names_[id] =
             l10n_util::GetStringUTF8(IDS_ASH_INTERNAL_DISPLAY_NAME);
       } else {
-        display_info_[id].name = name;
+        display_names_[id] = name;
       }
     }
   }
diff --git a/ash/display/display_manager.h b/ash/display/display_manager.h
index 292da15..a3c8b12 100644
--- a/ash/display/display_manager.h
+++ b/ash/display/display_manager.h
@@ -86,9 +86,6 @@ class ASH_EXPORT DisplayManager : public aura::RootWindowObserver {
   // contains each display's new infomration.
   void OnNativeDisplaysChanged(const std::vector<gfx::Display>& displays);
 
-  // Updates the internal display data and notifies observers about the changes.
-  void UpdateDisplays(const std::vector<gfx::Display>& displays);
-
   // Create a root window for given |display|.
   aura::RootWindow* CreateRootWindowForDisplay(const gfx::Display& display);
 
@@ -130,19 +127,6 @@ class ASH_EXPORT DisplayManager : public aura::RootWindowObserver {
 
   typedef std::vector<gfx::Display> DisplayList;
 
-  // Metadata for each display.
-  struct DisplayInfo {
-    // The cached name of the display.
-    std::string name;
-
-    // The original bounds_in_pixel for the display.  This can be different from
-    // the current one in case of overscan insets.
-    gfx::Rect original_bounds_in_pixel;
-
-    // The overscan insets for the display.
-    gfx::Insets overscan_insets_in_dip;
-  };
-
   void Init();
   void CycleDisplayImpl();
   void ScaleDisplayImpl();
@@ -180,8 +164,11 @@ class ASH_EXPORT DisplayManager : public aura::RootWindowObserver {
 
   bool force_bounds_changed_;
 
-  // The mapping from the display ID to its internal data.
-  std::map<int64, DisplayInfo> display_info_;
+  // The mapping from the display ID to its overscan insets.
+  std::map<int64, gfx::Insets> overscan_mapping_;
+
+  // The cached display's name for the display ID.
+  std::map<int64, std::string> display_names_;
 
   DISALLOW_COPY_AND_ASSIGN(DisplayManager);
 };
diff --git a/ash/display/display_manager_unittest.cc b/ash/display/display_manager_unittest.cc
index e7378b6..12ed8d20 100644
--- a/ash/display/display_manager_unittest.cc
+++ b/ash/display/display_manager_unittest.cc
@@ -234,15 +234,6 @@ TEST_F(DisplayManagerTest, MAYBE_OverscanInsetsTest) {
   EXPECT_EQ("12,514 378x376",
             display_manager()->GetDisplayAt(1)->bounds_in_pixel().ToString());
 
-  // Make sure that SetOverscanInsets() is idempotent.
-  display_manager()->SetOverscanInsets(display1.id(), gfx::Insets());
-  display_manager()->SetOverscanInsets(
-      display2.id(), gfx::Insets(13, 12, 11, 10));
-  EXPECT_EQ("0,0 500x500",
-            display_manager()->GetDisplayAt(0)->bounds_in_pixel().ToString());
-  EXPECT_EQ("12,514 378x376",
-            display_manager()->GetDisplayAt(1)->bounds_in_pixel().ToString());
-
   display_manager()->SetOverscanInsets(
       display2.id(), gfx::Insets(10, 11, 12, 13));
   EXPECT_EQ("0,0 500x500",
@@ -281,14 +272,6 @@ TEST_F(DisplayManagerTest, MAYBE_OverscanInsetsTest) {
   EXPECT_EQ("10,509 376x380",
             display_manager()->GetDisplayAt(1)->bounds_in_pixel().ToString());
   EXPECT_EQ("188x190", display_manager()->GetDisplayAt(1)->size().ToString());
-
-  // Make sure switching primary display applies the overscan offset only once.
-  ash::Shell::GetInstance()->display_controller()->SetPrimaryDisplay(
-      ScreenAsh::GetSecondaryDisplay());
-  EXPECT_EQ("0,0 500x500",
-            ScreenAsh::GetSecondaryDisplay().bounds_in_pixel().ToString());
-  EXPECT_EQ("10,509 376x380", gfx::Screen::GetNativeScreen()->
-            GetPrimaryDisplay().bounds_in_pixel().ToString());
 }
 
 TEST_F(DisplayManagerTest, MAYBE_ZeroOverscanInsets) {