chromeos: wireup more display management for mushrome

ash/mus/bridge/shell_port_mash.cc

Index: ash/mus/bridge/shell_port_mash.cc
diff --git a/ash/mus/bridge/shell_port_mash.cc b/ash/mus/bridge/shell_port_mash.cc
index 5abe4f69cf7e6bb9aa7081a82f6ec0f496708ec7..027574b297dfc71d54cf57c752c9eff5287de356 100644
--- a/ash/mus/bridge/shell_port_mash.cc
+++ b/ash/mus/bridge/shell_port_mash.cc
@@ -49,6 +49,7 @@
 #include "ash/wm/window_resizer.h"
 #include "ash/wm/window_util.h"
 #include "ash/wm/workspace/workspace_event_handler_aura.h"
+#include "ash/wm_display_observer.h"
 #include "ash/wm_window.h"
 #include "base/command_line.h"
 #include "base/memory/ptr_util.h"
@@ -159,6 +160,9 @@ aura::WindowTreeClient* ShellPortMash::window_tree_client() {
 }
 
 void ShellPortMash::Shutdown() {
+  if (added_display_observer_)
+    Shell::Get()->window_tree_host_manager()->RemoveObserver(this);
+
   if (mus_state_)
     mus_state_->pointer_watcher_adapter.reset();
 
@@ -200,6 +204,10 @@ WmWindow* ShellPortMash::GetRootWindowForDisplayId(int64_t display_id) {
 
 const display::ManagedDisplayInfo& ShellPortMash::GetDisplayInfo(
     int64_t display_id) const {
+  // TODO(sky): mash should use this too http://crbug.com/718860.
+  if (GetAshConfig() == Config::MUS)
+    return Shell::Get()->display_manager()->GetDisplayInfo(display_id);
+
   // TODO(mash): implement http://crbug.com/622480.
   NOTIMPLEMENTED();
   static display::ManagedDisplayInfo fake_info;
@@ -207,24 +215,47 @@ const display::ManagedDisplayInfo& ShellPortMash::GetDisplayInfo(
 }
 
 bool ShellPortMash::IsActiveDisplayId(int64_t display_id) const {
+  // TODO(sky): mash should use this too http://crbug.com/718860.
+  if (GetAshConfig() == Config::MUS)
+    return Shell::Get()->display_manager()->IsActiveDisplayId(display_id);
+
   // TODO(mash): implement http://crbug.com/622480.
   NOTIMPLEMENTED();
   return true;
 }
 
 display::Display ShellPortMash::GetFirstDisplay() const {
+  // TODO(sky): mash should use this too http://crbug.com/718860.
+  if (GetAshConfig() == Config::MUS) {
+    return Shell::Get()
+        ->display_manager()
+        ->software_mirroring_display_list()[0];
+  }
+
   // TODO(mash): implement http://crbug.com/622480.
   NOTIMPLEMENTED();
   return display::Screen::GetScreen()->GetPrimaryDisplay();
 }
 
 bool ShellPortMash::IsInUnifiedMode() const {
+  // TODO(sky): mash should use this too http://crbug.com/718860.
+  if (GetAshConfig() == Config::MUS)
+    return Shell::Get()->display_manager()->IsInUnifiedMode();
+
   // TODO(mash): implement http://crbug.com/622480.
   NOTIMPLEMENTED();
   return false;
 }
 
 bool ShellPortMash::IsInUnifiedModeIgnoreMirroring() const {
+  // TODO(sky): mash should use this too http://crbug.com/718860.
+  if (GetAshConfig() == Config::MUS) {
+    return Shell::Get()
+               ->display_manager()
+               ->current_default_multi_display_mode() ==
+           display::DisplayManager::UNIFIED;
+  }
+
   // TODO(mash): implement http://crbug.com/622480.
   NOTIMPLEMENTED();
   return false;
@@ -388,12 +419,26 @@ SessionStateDelegate* ShellPortMash::GetSessionStateDelegate() {
 
 void ShellPortMash::AddDisplayObserver(WmDisplayObserver* observer) {

[msw, 2017/05/11 01:07:17]

Why shouldn't callers use
|Shell::Get()->window_tree_host_manager()->AddObserver(this);| directly? It
seems odd to replicate the functionality with ShellPort as an intermediary.

[sky, 2017/05/11 14:06:52]

Initially we weren't going to use WindowTreeHostManager in mash at all. This
lead to the creation of WmDisplayObserver. Simplified display management means
we can use WindowTreeHostManager, and that WmDisplayObserver is no longer
necessary.

So, you're definitely right that this code can be nuked. I can't do that now
though as mash isn't using simplified display management yet. Or rather I could,
but I would be shifting around the conditionals, and the change would be much
bigger. So, for the time being I've gone this route. Effectively duplicate the
code from classic ash to here, and once I get simplified display management
working for mash I'll nuke this code. Does that make sense?

[msw, 2017/05/11 16:48:06]

Fair enough, but shouldn't the TODOs then say "TODO: Remove WmDisplayObserver
and use WindowTreeHostManager::Observer instead, once mash supports simplified
display management." or similar?

[sky, 2017/05/12 01:49:36]

Indeed you are right. Updated.

   // TODO: need WmDisplayObserver support for mus. http://crbug.com/705831.

[msw, 2017/05/11 01:07:16]

nit: is this obsolete? (or merge this with the mash todo?) (ditto below)

[sky, 2017/05/11 14:06:52]

I'm keeping TODO to remind me to update the bug.

-  NOTIMPLEMENTED();
+  if (GetAshConfig() == Config::MASH) {
+    NOTIMPLEMENTED();
+    return;
+  }
+  // TODO(sky): mash should use this too http://crbug.com/718860.
+  if (!added_display_observer_) {

[msw, 2017/05/11 01:07:16]

would it be reasonable to always add this instance as an observer (in mus
config), presuming you keep the intermediary pattern?

[sky, 2017/05/11 14:06:52]

I believe there are timing issues doing that. ShellPortClassic has the same
code.

+    added_display_observer_ = true;
+    Shell::Get()->window_tree_host_manager()->AddObserver(this);
+  }
+  display_observers_.AddObserver(observer);
 }
 
 void ShellPortMash::RemoveDisplayObserver(WmDisplayObserver* observer) {
   // TODO: need WmDisplayObserver support for mus. http://crbug.com/705831.
-  NOTIMPLEMENTED();
+  // TODO(sky): mash should use this too http://crbug.com/718860.
+  if (GetAshConfig() == Config::MASH) {
+    NOTIMPLEMENTED();
+    return;
+  }
+  display_observers_.RemoveObserver(observer);
 }
 
 void ShellPortMash::AddPointerWatcher(views::PointerWatcher* watcher,
@@ -570,5 +615,15 @@ ShellPortMash::CreateAcceleratorController() {
       mash_state_->accelerator_controller_registrar.get());
 }
 
+void ShellPortMash::OnDisplayConfigurationChanging() {
+  for (auto& observer : display_observers_)
+    observer.OnDisplayConfigurationChanging();
+}
+
+void ShellPortMash::OnDisplayConfigurationChanged() {
+  for (auto& observer : display_observers_)
+    observer.OnDisplayConfigurationChanged();
+}
+
 }  // namespace mus
 }  // namespace ash