2nd try

* Use DisplayManager::IsMirrored to check mirroring state
* Split CycleDisplay shortcut into AddRemoveDisplay and ToggleMirrorMode
* A few minor cleanups that will make it easy to implement
  compositor based mirroing.

BUG=239776
TEST=none no functional change.
R=jamescook@chromium.org, mukai@chromium.org, sky@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=200208

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

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

11 files changed, 118 insertions, 105 deletions

diff --git a/ash/accelerators/accelerator_controller.cc b/ash/accelerators/accelerator_controller.cc
index daa013a..43e2dac 100644
--- a/ash/accelerators/accelerator_controller.cc
+++ b/ash/accelerators/accelerator_controller.cc
@@ -525,8 +525,11 @@ bool AcceleratorController::PerformAction(int action,
       HandleCycleWindowLinear(CYCLE_FORWARD);
       return true;
 #if defined(OS_CHROMEOS)
-    case CYCLE_DISPLAY_MODE:
-      Shell::GetInstance()->display_controller()->CycleDisplayMode();
+    case ADD_REMOVE_DISPLAY:
+      Shell::GetInstance()->display_manager()->AddRemoveDisplay();
+      return true;
+    case TOGGLE_MIRROR_MODE:
+      Shell::GetInstance()->display_controller()->ToggleMirrorMode();
       return true;
     case LOCK_SCREEN:
       if (key_code == ui::VKEY_L)
@@ -852,7 +855,7 @@ bool AcceleratorController::PerformAction(int action,
     case TOGGLE_ROOT_WINDOW_FULL_SCREEN:
       return HandleToggleRootWindowFullScreen();
     case DEBUG_TOGGLE_DEVICE_SCALE_FACTOR:
-      internal::DisplayManager::ToggleDisplayScaleFactor();
+      Shell::GetInstance()->display_manager()->ToggleDisplayScaleFactor();
       return true;
     case DEBUG_TOGGLE_SHOW_DEBUG_BORDERS:
       ash::debug::ToggleShowDebugBorders();
diff --git a/ash/accelerators/accelerator_controller_unittest.cc b/ash/accelerators/accelerator_controller_unittest.cc
index 292cc3e..933d0a5 100644
--- a/ash/accelerators/accelerator_controller_unittest.cc
+++ b/ash/accelerators/accelerator_controller_unittest.cc
@@ -1205,26 +1205,38 @@ TEST_F(AcceleratorControllerTest, ReservedAccelerators) {
 
 #if defined(OS_CHROMEOS)
 TEST_F(AcceleratorControllerTest, DisallowedAtModalWindow) {
-  std::set<AcceleratorAction> allActions;
+  std::set<AcceleratorAction> all_actions;
   for (size_t i = 0 ; i < kAcceleratorDataLength; ++i)
-    allActions.insert(kAcceleratorData[i].action);
+    all_actions.insert(kAcceleratorData[i].action);
+#if !defined(NDEBUG)
+  std::set<AcceleratorAction> all_desktop_actions;
+  for (size_t i = 0 ; i < kDesktopAcceleratorDataLength; ++i)
+    all_desktop_actions.insert(kDesktopAcceleratorData[i].action);
+#endif
+
   std::set<AcceleratorAction> actionsAllowedAtModalWindow;
   for (size_t k = 0 ; k < kActionsAllowedAtModalWindowLength; ++k)
     actionsAllowedAtModalWindow.insert(kActionsAllowedAtModalWindow[k]);
   for (std::set<AcceleratorAction>::const_iterator it =
            actionsAllowedAtModalWindow.begin();
        it != actionsAllowedAtModalWindow.end(); ++it) {
-    EXPECT_FALSE(allActions.find(*it) == allActions.end())
+    EXPECT_TRUE(all_actions.find(*it) != all_actions.end()
+
+#if !defined(NDEBUG)
+                || all_desktop_actions.find(*it) != all_desktop_actions.end()
+#endif
+                )
         << " action from kActionsAllowedAtModalWindow"
-        << " not found in kAcceleratorData. action: " << *it;
+        << " not found in kAcceleratorData or kDesktopAcceleratorData. "
+        << "action: " << *it;
   }
   scoped_ptr<aura::Window> window(
       CreateTestWindowInShellWithBounds(gfx::Rect(5, 5, 20, 20)));
   const ui::Accelerator dummy;
   wm::ActivateWindow(window.get());
   Shell::GetInstance()->SimulateModalWindowOpenForTesting(true);
-  for (std::set<AcceleratorAction>::const_iterator it = allActions.begin();
-       it != allActions.end(); ++it) {
+  for (std::set<AcceleratorAction>::const_iterator it = all_actions.begin();
+       it != all_actions.end(); ++it) {
     if (actionsAllowedAtModalWindow.find(*it) ==
         actionsAllowedAtModalWindow.end()) {
       EXPECT_TRUE(GetController()->PerformAction(*it, dummy))
diff --git a/ash/accelerators/accelerator_table.cc b/ash/accelerators/accelerator_table.cc
index d9a6813..ec32f2c 100644
--- a/ash/accelerators/accelerator_table.cc
+++ b/ash/accelerators/accelerator_table.cc
@@ -44,7 +44,7 @@ const AcceleratorData kAcceleratorData[] = {
   { true, ui::VKEY_KBD_BRIGHTNESS_DOWN, ui::EF_NONE, KEYBOARD_BRIGHTNESS_DOWN },
   { true, ui::VKEY_KBD_BRIGHTNESS_UP, ui::EF_NONE, KEYBOARD_BRIGHTNESS_UP },
   // Maximize button.
-  { true, ui::VKEY_MEDIA_LAUNCH_APP2, ui::EF_CONTROL_DOWN, CYCLE_DISPLAY_MODE },
+  { true, ui::VKEY_MEDIA_LAUNCH_APP2, ui::EF_CONTROL_DOWN, TOGGLE_MIRROR_MODE },
   { true, ui::VKEY_MEDIA_LAUNCH_APP2, ui::EF_ALT_DOWN, SWAP_PRIMARY_DISPLAY },
   // Cycle windows button.
   { true, ui::VKEY_MEDIA_LAUNCH_APP1, ui::EF_CONTROL_DOWN, TAKE_SCREENSHOT },
@@ -186,6 +186,8 @@ const AcceleratorData kDesktopAcceleratorData[] = {
   { true, ui::VKEY_L, ui::EF_ALT_DOWN, LOCK_SCREEN },
   { true, ui::VKEY_POWER, ui::EF_SHIFT_DOWN, LOCK_PRESSED },
   { false, ui::VKEY_POWER, ui::EF_SHIFT_DOWN, LOCK_RELEASED },
+  { true, ui::VKEY_D, ui::EF_CONTROL_DOWN | ui::EF_SHIFT_DOWN,
+    ADD_REMOVE_DISPLAY },
 #endif
   // Extra shortcut for display swaping as alt-f4 is taken on linux desktop.
   { true, ui::VKEY_S, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN,
@@ -264,8 +266,9 @@ const AcceleratorAction kActionsAllowedAtLoginOrLockScreen[] = {
   VOLUME_MUTE,
   VOLUME_UP,
 #if defined(OS_CHROMEOS)
-  CYCLE_DISPLAY_MODE,
+  ADD_REMOVE_DISPLAY,
   DISABLE_GPU_WATCHDOG,
+  TOGGLE_MIRROR_MODE,
 #endif
 #if defined(OS_CHROMEOS) && !defined(NDEBUG)
   POWER_PRESSED,
@@ -315,8 +318,11 @@ const AcceleratorAction kActionsAllowedAtModalWindow[] = {
   VOLUME_MUTE,
   VOLUME_UP,
 #if defined(OS_CHROMEOS)
-  CYCLE_DISPLAY_MODE,
+#if !defined(NDEBUG)
+  ADD_REMOVE_DISPLAY,
+#endif
   LOCK_SCREEN,
+  TOGGLE_MIRROR_MODE,
 #endif
 };
 
@@ -382,8 +388,9 @@ const AcceleratorAction kActionsAllowedInAppMode[] = {
   VOLUME_MUTE,
   VOLUME_UP,
 #if defined(OS_CHROMEOS)
-  CYCLE_DISPLAY_MODE,
+  ADD_REMOVE_DISPLAY,
   DISABLE_GPU_WATCHDOG,
+  TOGGLE_MIRROR_MODE,
 #endif  // defined(OS_CHROMEOS)
 };
 
diff --git a/ash/accelerators/accelerator_table.h b/ash/accelerators/accelerator_table.h
index 70e57d8d..ee635f4 100644
--- a/ash/accelerators/accelerator_table.h
+++ b/ash/accelerators/accelerator_table.h
@@ -98,7 +98,8 @@ enum AcceleratorAction {
   WINDOW_SNAP_LEFT,
   WINDOW_SNAP_RIGHT,
 #if defined(OS_CHROMEOS)
-  CYCLE_DISPLAY_MODE,
+  ADD_REMOVE_DISPLAY,
+  TOGGLE_MIRROR_MODE,
   DISABLE_GPU_WATCHDOG,
   LOCK_SCREEN,
   OPEN_CROSH,
diff --git a/ash/display/display_controller.cc b/ash/display/display_controller.cc
index 4be9ca4..c2f981f 100644
--- a/ash/display/display_controller.cc
+++ b/ash/display/display_controller.cc
@@ -620,7 +620,7 @@ DisplayLayout DisplayController::GetRegisteredDisplayLayout(
   return iter != paired_layouts_.end() ? iter->second : default_display_layout_;
 }
 
-void DisplayController::CycleDisplayMode() {
+void DisplayController::ToggleMirrorMode() {
   if (limiter_) {
     if  (limiter_->IsThrottled())
       return;
@@ -629,17 +629,13 @@ void DisplayController::CycleDisplayMode() {
 #if defined(OS_CHROMEOS) && defined(USE_X11)
   Shell* shell = Shell::GetInstance();
   internal::DisplayManager* display_manager = GetDisplayManager();
-  if (!base::chromeos::IsRunningOnChromeOS()) {
-    internal::DisplayManager::CycleDisplay();
-  } else if (display_manager->num_connected_displays() > 1) {
-    chromeos::OutputState new_state = display_manager->IsMirrored() ?
-       chromeos::STATE_DUAL_EXTENDED : chromeos::STATE_DUAL_MIRROR;
+  if (display_manager->num_connected_displays() > 1) {
     internal::OutputConfiguratorAnimation* animation =
         shell->output_configurator_animation();
     animation->StartFadeOutAnimation(base::Bind(
-        base::IgnoreResult(&chromeos::OutputConfigurator::SetDisplayMode),
-        base::Unretained(shell->output_configurator()),
-        new_state));
+        base::IgnoreResult(&internal::DisplayManager::SetMirrorMode),
+        base::Unretained(display_manager),
+        !display_manager->IsMirrored()));
   }
 #endif
 }
@@ -904,7 +900,7 @@ void DisplayController::OnDisplayRemoved(const gfx::Display& display) {
   base::MessageLoop::current()->DeleteSoon(FROM_HERE, controller);
 }
 
-aura::RootWindow* DisplayController::CreateRootWindowForDisplay(
+aura::RootWindow* DisplayController::AddRootWindowForDisplay(
     const gfx::Display& display) {
   static int root_window_count = 0;
   const internal::DisplayInfo& display_info =
@@ -922,23 +918,18 @@ aura::RootWindow* DisplayController::CreateRootWindowForDisplay(
   root_window->AddRootWindowObserver(GetDisplayManager());
   root_window->SetProperty(internal::kDisplayIdKey, display.id());
   root_window->Init();
-  return root_window;
-}
 
-aura::RootWindow* DisplayController::AddRootWindowForDisplay(
-    const gfx::Display& display) {
-  aura::RootWindow* root = CreateRootWindowForDisplay(display);
-  root_windows_[display.id()] = root;
-  SetDisplayPropertiesOnHostWindow(root, display);
+  root_windows_[display.id()] = root_window;
+  SetDisplayPropertiesOnHostWindow(root_window, display);
 
 #if defined(OS_CHROMEOS)
   static bool force_constrain_pointer_to_root =
       CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kAshConstrainPointerToRoot);
   if (base::chromeos::IsRunningOnChromeOS() || force_constrain_pointer_to_root)
-    root->ConfineCursorToWindow();
+    root_window->ConfineCursorToWindow();
 #endif
-  return root;
+  return root_window;
 }
 
 void DisplayController::UpdateDisplayBoundsForLayout() {
diff --git a/ash/display/display_controller.h b/ash/display/display_controller.h
index ae6e3af..d44567a 100644
--- a/ash/display/display_controller.h
+++ b/ash/display/display_controller.h
@@ -132,8 +132,8 @@ class ASH_EXPORT DisplayController : public gfx::DisplayObserver {
   // Returns the root window for |display_id|.
   aura::RootWindow* GetRootWindowForDisplayId(int64 id);
 
-  // Cycles display mode.
-  void CycleDisplayMode();
+  // Toggle mirror mode.
+  void ToggleMirrorMode();
 
   // Swap primary and secondary display.
   void SwapPrimaryDisplay();
@@ -211,9 +211,6 @@ class ASH_EXPORT DisplayController : public gfx::DisplayObserver {
  private:
   friend class internal::DisplayManager;
 
-  // Create a root window for given |display|.
-  aura::RootWindow* CreateRootWindowForDisplay(const gfx::Display& display);
-
   // Creates a root window for |display| and stores it in the |root_windows_|
   // map.
   aura::RootWindow* AddRootWindowForDisplay(const gfx::Display& display);
diff --git a/ash/display/display_manager.cc b/ash/display/display_manager.cc
index 81d7b88..c776e79 100644
--- a/ash/display/display_manager.cc
+++ b/ash/display/display_manager.cc
@@ -39,6 +39,7 @@
 
 #if defined(OS_CHROMEOS)
 #include "base/chromeos/chromeos_version.h"
+#include "chromeos/display/output_configurator.h"
 #endif
 
 #if defined(OS_WIN)
@@ -142,16 +143,6 @@ DisplayManager::~DisplayManager() {
 }
 
 // static
-void DisplayManager::CycleDisplay() {
-  Shell::GetInstance()->display_manager()->CycleDisplayImpl();
-}
-
-// static
-void DisplayManager::ToggleDisplayScaleFactor() {
-  Shell::GetInstance()->display_manager()->ScaleDisplayImpl();
-}
-
-// static
 float DisplayManager::GetNextUIScale(const DisplayInfo& info, bool up) {
   float scale = info.ui_scale();
   std::vector<float> scales = GetScalesForDisplay(info);
@@ -624,35 +615,22 @@ void DisplayManager::OnRootWindowResized(const aura::RootWindow* root,
   }
 }
 
-int64 DisplayManager::GetDisplayIdForUIScaling() const {
-  // UI Scaling is effective only on internal display.
-  int64 display_id = gfx::Display::InternalDisplayId();
-#if defined(OS_WIN)
-  display_id = first_display_id();
-#endif
-  return display_id;
-}
+void DisplayManager::SetMirrorMode(bool mirrored) {
+  if (num_connected_displays() <= 1)
+    return;
 
-void DisplayManager::Init() {
-  // TODO(oshima): Move this logic to DisplayChangeObserver.
-  const string size_str = CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
-      switches::kAshHostWindowBounds);
-  vector<string> parts;
-  base::SplitString(size_str, ',', &parts);
-  for (vector<string>::const_iterator iter = parts.begin();
-       iter != parts.end(); ++iter) {
-    AddDisplayFromSpec(*iter);
+#if defined(OS_CHROMEOS)
+  if (base::chromeos::IsRunningOnChromeOS()) {
+    chromeos::OutputState new_state = mirrored ?
+        chromeos::STATE_DUAL_MIRROR : chromeos::STATE_DUAL_EXTENDED;
+    Shell::GetInstance()->output_configurator()->SetDisplayMode(new_state);
+  } else {
+    // TODO(oshima): Compositor based mirroring.
   }
-  if (displays_.empty())
-    AddDisplayFromSpec(std::string() /* default */);
-  first_display_id_ = displays_[0].id();
-  CommandLine* command_line = CommandLine::ForCurrentProcess();
-  if (command_line->HasSwitch(switches::kAshUseFirstDisplayAsInternal))
-    gfx::Display::SetInternalDisplayId(first_display_id_);
-  num_connected_displays_ = displays_.size();
+#endif
 }
 
-void DisplayManager::CycleDisplayImpl() {
+void DisplayManager::AddRemoveDisplay() {
   DCHECK(!displays_.empty());
   std::vector<DisplayInfo> new_display_info_list;
   new_display_info_list.push_back(
@@ -671,7 +649,7 @@ void DisplayManager::CycleDisplayImpl() {
   UpdateDisplays(new_display_info_list);
 }
 
-void DisplayManager::ScaleDisplayImpl() {
+void DisplayManager::ToggleDisplayScaleFactor() {
   DCHECK(!displays_.empty());
   std::vector<DisplayInfo> new_display_info_list;
   for (DisplayList::const_iterator iter = displays_.begin();
@@ -684,6 +662,34 @@ void DisplayManager::ScaleDisplayImpl() {
   UpdateDisplays(new_display_info_list);
 }
 
+int64 DisplayManager::GetDisplayIdForUIScaling() const {
+  // UI Scaling is effective only on internal display.
+  int64 display_id = gfx::Display::InternalDisplayId();
+#if defined(OS_WIN)
+  display_id = first_display_id();
+#endif
+  return display_id;
+}
+
+void DisplayManager::Init() {
+  // TODO(oshima): Move this logic to DisplayChangeObserver.
+  const string size_str = CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+      switches::kAshHostWindowBounds);
+  vector<string> parts;
+  base::SplitString(size_str, ',', &parts);
+  for (vector<string>::const_iterator iter = parts.begin();
+       iter != parts.end(); ++iter) {
+    AddDisplayFromSpec(*iter);
+  }
+  if (displays_.empty())
+    AddDisplayFromSpec(std::string() /* default */);
+  first_display_id_ = displays_[0].id();
+  CommandLine* command_line = CommandLine::ForCurrentProcess();
+  if (command_line->HasSwitch(switches::kAshUseFirstDisplayAsInternal))
+    gfx::Display::SetInternalDisplayId(first_display_id_);
+  num_connected_displays_ = displays_.size();
+}
+
 gfx::Display& DisplayManager::FindDisplayForRootWindow(
     const aura::RootWindow* root_window) {
   int64 id = root_window->GetProperty(kDisplayIdKey);
diff --git a/ash/display/display_manager.h b/ash/display/display_manager.h
index ffd4e18..e75a797 100644
--- a/ash/display/display_manager.h
+++ b/ash/display/display_manager.h
@@ -39,11 +39,6 @@ class ASH_EXPORT DisplayManager : public aura::RootWindowObserver {
   DisplayManager();
   virtual ~DisplayManager();
 
-  // Used to emulate display change when run in a desktop environment instead
-  // of on a device.
-  static void CycleDisplay();
-  static void ToggleDisplayScaleFactor();
-
   // Returns next valid UI scale.
   static float GetNextUIScale(const DisplayInfo& info, bool up);
 
@@ -166,6 +161,14 @@ class ASH_EXPORT DisplayManager : public aura::RootWindowObserver {
   virtual void OnRootWindowResized(const aura::RootWindow* root,
                                    const gfx::Size& new_size) OVERRIDE;
 
+  // Change the mirror mode.
+  void SetMirrorMode(bool mirrored);
+
+  // Used to emulate display change when run in a desktop environment instead
+  // of on a device.
+  void AddRemoveDisplay();
+  void ToggleDisplayScaleFactor();
+
  private:
   FRIEND_TEST_ALL_PREFIXES(ExtendedDesktopTest, ConvertPoint);
   FRIEND_TEST_ALL_PREFIXES(DisplayManagerTest, TestNativeDisplaysChanged);
@@ -184,8 +187,6 @@ class ASH_EXPORT DisplayManager : public aura::RootWindowObserver {
   }
 
   void Init();
-  void CycleDisplayImpl();
-  void ScaleDisplayImpl();
 
   gfx::Display& FindDisplayForRootWindow(const aura::RootWindow* root);
   gfx::Display& FindDisplayForId(int64 id);
diff --git a/ash/display/display_manager_unittest.cc b/ash/display/display_manager_unittest.cc
index cfe6458..0706b0e 100644
--- a/ash/display/display_manager_unittest.cc
+++ b/ash/display/display_manager_unittest.cc
@@ -205,18 +205,18 @@ TEST_F(DisplayManagerTest, NativeDisplayTest) {
 TEST_F(DisplayManagerTest, EmulatorTest) {
   EXPECT_EQ(1U, display_manager()->GetNumDisplays());
 
-  DisplayManager::CycleDisplay();
+  display_manager()->AddRemoveDisplay();
   // Update primary and add seconary.
   EXPECT_EQ(2U, display_manager()->GetNumDisplays());
   EXPECT_EQ("0 1 0", GetCountSummary());
   reset();
 
-  DisplayManager::CycleDisplay();
+  display_manager()->AddRemoveDisplay();
   EXPECT_EQ(1U, display_manager()->GetNumDisplays());
   EXPECT_EQ("0 0 1", GetCountSummary());
   reset();
 
-  DisplayManager::CycleDisplay();
+  display_manager()->AddRemoveDisplay();
   EXPECT_EQ(2U, display_manager()->GetNumDisplays());
   EXPECT_EQ("0 1 0", GetCountSummary());
   reset();
diff --git a/ash/display/event_transformation_handler.cc b/ash/display/event_transformation_handler.cc
index 52cb647..cb33891 100644
--- a/ash/display/event_transformation_handler.cc
+++ b/ash/display/event_transformation_handler.cc
@@ -68,6 +68,9 @@ void EventTransformationHandler::OnTouchEvent(ui::TouchEvent* event) {
   OutputConfigurator* output_configurator =
       ash::Shell::GetInstance()->output_configurator();
 
+  // Check output_configurator's output_state instead of checking
+  // DisplayManager::IsMirrored() because the compositor based mirroring
+  // won't cause the scaling issue.
   if (output_configurator->output_state() != chromeos::STATE_DUAL_MIRROR)
     return;
 
diff --git a/ash/system/chromeos/tray_display.cc b/ash/system/chromeos/tray_display.cc
index f3d3297..bfe814a 100644
--- a/ash/system/chromeos/tray_display.cc
+++ b/ash/system/chromeos/tray_display.cc
@@ -54,28 +54,20 @@ class DisplayView : public ash::internal::ActionableView {
   virtual ~DisplayView() {}
 
   void Update() {
-    chromeos::OutputState state =
-        base::chromeos::IsRunningOnChromeOS() ?
-        Shell::GetInstance()->output_configurator()->output_state() :
-        InferOutputState();
-    switch (state) {
-      case chromeos::STATE_INVALID:
-      case chromeos::STATE_HEADLESS:
-      case chromeos::STATE_SINGLE:
-        SetVisible(false);
-        return;
-      case chromeos::STATE_DUAL_MIRROR:
-        label_->SetText(l10n_util::GetStringFUTF16(
-            IDS_ASH_STATUS_TRAY_DISPLAY_MIRRORING, GetExternalDisplayName()));
-        SetVisible(true);
-        return;
-      case chromeos::STATE_DUAL_EXTENDED:
-        label_->SetText(l10n_util::GetStringFUTF16(
-            IDS_ASH_STATUS_TRAY_DISPLAY_EXTENDED, GetExternalDisplayName()));
-        SetVisible(true);
-        return;
+    DisplayManager* display_manager = Shell::GetInstance()->display_manager();
+    if (display_manager->num_connected_displays() == 1) {
+      // TODO(oshima|mukai): Support single display mode for overscan alignment.
+      SetVisible(false);
+      return;
+    }
+    SetVisible(true);
+    if (display_manager->IsMirrored()) {
+      label_->SetText(l10n_util::GetStringFUTF16(
+          IDS_ASH_STATUS_TRAY_DISPLAY_MIRRORING, GetExternalDisplayName()));
+    } else {
+      label_->SetText(l10n_util::GetStringFUTF16(
+          IDS_ASH_STATUS_TRAY_DISPLAY_EXTENDED, GetExternalDisplayName()));
     }
-    NOTREACHED() << "Unhandled state " << state;
   }
 
   chromeos::OutputState InferOutputState() const {