mus: Changes SetDisplayRoot() to create actual display

It now takes enough information for mus to create the display. Further
we now assume mus has not created the display at the time
SetDisplayRoot() is called.

BUG=708279
TEST=covered by tests
R=kylechar@chromium.org, nasko@chromium.org

Review-Url: https://codereview.chromium.org/2829733002
Cr-Commit-Position: refs/heads/master@{#466057}
diff --git a/services/ui/display/screen_manager.h b/services/ui/display/screen_manager.h
index 9252194..017b40d 100644
--- a/services/ui/display/screen_manager.h
+++ b/services/ui/display/screen_manager.h
@@ -16,6 +16,8 @@
 
 namespace display {
 
+class ScreenBase;
+
 // ScreenManager provides the necessary functionality to configure all
 // attached physical displays.
 class ScreenManager {
@@ -40,6 +42,8 @@
   // Handle requests from the platform to close a display.
   virtual void RequestCloseDisplay(int64_t display_id) = 0;
 
+  virtual display::ScreenBase* GetScreen() = 0;
+
  private:
   static ScreenManager* instance_;  // Instance is not owned.
 
diff --git a/services/ui/display/screen_manager_forwarding.cc b/services/ui/display/screen_manager_forwarding.cc
index c3bfd52..c218d27 100644
--- a/services/ui/display/screen_manager_forwarding.cc
+++ b/services/ui/display/screen_manager_forwarding.cc
@@ -8,6 +8,7 @@
 
 #include "base/bind.h"
 #include "services/service_manager/public/cpp/binder_registry.h"
+#include "ui/display/screen_base.h"
 #include "ui/display/types/display_constants.h"
 #include "ui/display/types/display_snapshot_mojo.h"
 #include "ui/display/types/native_display_delegate.h"
@@ -35,10 +36,8 @@
 
 }  // namespace
 
-// TODO(sky/kylechar): Change ScreenManager::Create() to make a
-// ScreenManagerForwarding in mus mode.
-
-ScreenManagerForwarding::ScreenManagerForwarding() : binding_(this) {}
+ScreenManagerForwarding::ScreenManagerForwarding()
+    : screen_(base::MakeUnique<display::ScreenBase>()), binding_(this) {}
 
 ScreenManagerForwarding::~ScreenManagerForwarding() {
   if (native_display_delegate_)
@@ -56,6 +55,10 @@
 
 void ScreenManagerForwarding::RequestCloseDisplay(int64_t display_id) {}
 
+display::ScreenBase* ScreenManagerForwarding::GetScreen() {
+  return screen_.get();
+}
+
 void ScreenManagerForwarding::OnConfigurationChanged() {
   if (observer_.is_bound())
     observer_->OnConfigurationChanged();
diff --git a/services/ui/display/screen_manager_forwarding.h b/services/ui/display/screen_manager_forwarding.h
index d3a0f5b..9eac783 100644
--- a/services/ui/display/screen_manager_forwarding.h
+++ b/services/ui/display/screen_manager_forwarding.h
@@ -36,6 +36,7 @@
   void AddInterfaces(service_manager::BinderRegistry* registry) override;
   void Init(ScreenManagerDelegate* delegate) override;
   void RequestCloseDisplay(int64_t display_id) override;
+  display::ScreenBase* GetScreen() override;
 
   // NativeDisplayObserver:
   void OnConfigurationChanged() override;
@@ -80,6 +81,7 @@
       const mojom::NativeDisplayDelegate::ConfigureCallback& callback,
       bool status);
 
+  std::unique_ptr<display::ScreenBase> screen_;
   mojo::Binding<mojom::NativeDisplayDelegate> binding_;
   mojom::NativeDisplayObserverPtr observer_;
 
diff --git a/services/ui/display/screen_manager_ozone_external.cc b/services/ui/display/screen_manager_ozone_external.cc
index 45a5027..5bc97b3 100644
--- a/services/ui/display/screen_manager_ozone_external.cc
+++ b/services/ui/display/screen_manager_ozone_external.cc
@@ -7,6 +7,7 @@
 #include <memory>
 
 #include "services/service_manager/public/cpp/binder_registry.h"
+#include "ui/display/screen_base.h"
 #include "ui/display/types/display_constants.h"
 
 namespace display {
@@ -16,7 +17,8 @@
   return base::MakeUnique<ScreenManagerOzoneExternal>();
 }
 
-ScreenManagerOzoneExternal::ScreenManagerOzoneExternal() {}
+ScreenManagerOzoneExternal::ScreenManagerOzoneExternal()
+    : screen_(base::MakeUnique<display::ScreenBase>()) {}
 
 ScreenManagerOzoneExternal::~ScreenManagerOzoneExternal() {}
 
@@ -27,4 +29,8 @@
 
 void ScreenManagerOzoneExternal::RequestCloseDisplay(int64_t display_id) {}
 
+display::ScreenBase* ScreenManagerOzoneExternal::GetScreen() {
+  return screen_.get();
+}
+
 }  // namespace display
diff --git a/services/ui/display/screen_manager_ozone_external.h b/services/ui/display/screen_manager_ozone_external.h
index 41fe466..f4af7d4f 100644
--- a/services/ui/display/screen_manager_ozone_external.h
+++ b/services/ui/display/screen_manager_ozone_external.h
@@ -26,6 +26,9 @@
   void AddInterfaces(service_manager::BinderRegistry* registry) override;
   void Init(ScreenManagerDelegate* delegate) override;
   void RequestCloseDisplay(int64_t display_id) override;
+  display::ScreenBase* GetScreen() override;
+
+  std::unique_ptr<display::ScreenBase> screen_;
 
   DISALLOW_COPY_AND_ASSIGN(ScreenManagerOzoneExternal);
 };
diff --git a/services/ui/display/screen_manager_ozone_internal.cc b/services/ui/display/screen_manager_ozone_internal.cc
index 854ef3a..63ca244 100644
--- a/services/ui/display/screen_manager_ozone_internal.cc
+++ b/services/ui/display/screen_manager_ozone_internal.cc
@@ -170,6 +170,10 @@
   fake_display_controller_->RemoveDisplay(display_id);
 }
 
+display::ScreenBase* ScreenManagerOzoneInternal::GetScreen() {
+  return screen_;
+}
+
 void ScreenManagerOzoneInternal::ToggleAddRemoveDisplay() {
   if (!fake_display_controller_)
     return;
diff --git a/services/ui/display/screen_manager_ozone_internal.h b/services/ui/display/screen_manager_ozone_internal.h
index 61c9920..d2774f6 100644
--- a/services/ui/display/screen_manager_ozone_internal.h
+++ b/services/ui/display/screen_manager_ozone_internal.h
@@ -52,6 +52,7 @@
   void AddInterfaces(service_manager::BinderRegistry* registry) override;
   void Init(ScreenManagerDelegate* delegate) override;
   void RequestCloseDisplay(int64_t display_id) override;
+  display::ScreenBase* GetScreen() override;
 
   // mojom::TestDisplayController:
   void ToggleAddRemoveDisplay() override;
diff --git a/services/ui/display/screen_manager_stub_internal.cc b/services/ui/display/screen_manager_stub_internal.cc
index e513b00..335d343 100644
--- a/services/ui/display/screen_manager_stub_internal.cc
+++ b/services/ui/display/screen_manager_stub_internal.cc
@@ -12,6 +12,7 @@
 #include "base/threading/thread_task_runner_handle.h"
 #include "services/service_manager/public/cpp/binder_registry.h"
 #include "services/ui/display/viewport_metrics.h"
+#include "ui/display/screen_base.h"
 #include "ui/gfx/geometry/dip_util.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/size.h"
@@ -46,7 +47,8 @@
 }
 
 ScreenManagerStubInternal::ScreenManagerStubInternal()
-    : weak_ptr_factory_(this) {}
+    : screen_(base::MakeUnique<display::ScreenBase>()),
+      weak_ptr_factory_(this) {}
 
 ScreenManagerStubInternal::~ScreenManagerStubInternal() {}
 
@@ -80,4 +82,8 @@
   }
 }
 
+display::ScreenBase* ScreenManagerStubInternal::GetScreen() {
+  return screen_.get();
+}
+
 }  // namespace display
diff --git a/services/ui/display/screen_manager_stub_internal.h b/services/ui/display/screen_manager_stub_internal.h
index bca5ab9..ea68fa1 100644
--- a/services/ui/display/screen_manager_stub_internal.h
+++ b/services/ui/display/screen_manager_stub_internal.h
@@ -13,6 +13,8 @@
 
 namespace display {
 
+class ScreenBase;
+
 // ScreenManagerStubInternal provides the necessary functionality to configure a
 // fixed 1024x768 display for non-ozone platforms.
 class ScreenManagerStubInternal : public ScreenManager {
@@ -28,12 +30,15 @@
   void AddInterfaces(service_manager::BinderRegistry* registry) override;
   void Init(ScreenManagerDelegate* delegate) override;
   void RequestCloseDisplay(int64_t display_id) override;
+  display::ScreenBase* GetScreen() override;
 
   // Sample display information.
   Display display_;
 
   ScreenManagerDelegate* delegate_ = nullptr;
 
+  std::unique_ptr<ScreenBase> screen_;
+
   base::WeakPtrFactory<ScreenManagerStubInternal> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ScreenManagerStubInternal);
diff --git a/services/ui/public/interfaces/window_manager.mojom b/services/ui/public/interfaces/window_manager.mojom
index aa340ba6..2d25904 100644
--- a/services/ui/public/interfaces/window_manager.mojom
+++ b/services/ui/public/interfaces/window_manager.mojom
@@ -16,6 +16,7 @@
 import "ui/events/mojo/event.mojom";
 import "ui/events/mojo/event_constants.mojom";
 import "ui/gfx/geometry/mojo/geometry.mojom";
+import "ui/display/mojo/display.mojom";
 
 // WindowManager is used when a WindowTreeClient attempts to modify
 // a property of the embed root. When this happens WindowTree calls the
@@ -273,8 +274,13 @@
   // Sets the root of a particular display. This is only applicable when the
   // WindowTree was created with a value of false for
   // |automatically_create_display_roots| (see
-  // WindowManagerWindowTreeFactory::CreateWindowTree() for details).
-  SetDisplayRoot(int64 display_id, uint32 window_id) => (bool success);
+  // WindowManagerWindowTreeFactory::CreateWindowTree() for details). On success
+  // this responds with the FrameSinkId of the root. On failure the FrameSinkId
+  // is not valid.
+  SetDisplayRoot(display.mojom.Display display,
+                 WmViewportMetrics viewport_metrics,
+                 bool is_primary_display,
+                 uint32 window_id) => (cc.mojom.FrameSinkId? frame_sink_id);
 
   // The window manager has completed a request with the specific change id.
   WmResponse(uint32 change_id, bool response);
diff --git a/services/ui/public/interfaces/window_manager_constants.mojom b/services/ui/public/interfaces/window_manager_constants.mojom
index 7863f76..0919ede 100644
--- a/services/ui/public/interfaces/window_manager_constants.mojom
+++ b/services/ui/public/interfaces/window_manager_constants.mojom
@@ -65,3 +65,10 @@
   DRAG,
   UNKNOWN,
 };
+
+// Used when the window manager explicitly adds a new display.
+struct WmViewportMetrics {
+  gfx.mojom.Rect bounds_in_pixels;
+  float device_scale_factor;
+  float ui_scale_factor;
+};
diff --git a/services/ui/service.cc b/services/ui/service.cc
index b7ff9691..3a85575 100644
--- a/services/ui/service.cc
+++ b/services/ui/service.cc
@@ -52,6 +52,7 @@
 #include "ui/base/x/x11_util.h"  // nogncheck
 #include "ui/platform_window/x11/x11_window.h"
 #elif defined(USE_OZONE)
+#include "services/ui/display/screen_manager_forwarding.h"
 #include "ui/events/ozone/layout/keyboard_layout_engine.h"
 #include "ui/events/ozone/layout/keyboard_layout_engine_manager.h"
 #include "ui/ozone/public/ozone_platform.h"
@@ -86,7 +87,6 @@
 
 Service::Service()
     : test_config_(false),
-      screen_manager_(display::ScreenManager::Create()),
       ime_registrar_(&ime_server_) {}
 
 Service::~Service() {
@@ -220,8 +220,6 @@
   if (input_device_server_.IsRegisteredAsObserver())
     input_device_server_.AddInterface(&registry_);
 
-  screen_manager_->AddInterfaces(&registry_);
-
 #if defined(USE_OZONE)
   ui::OzonePlatform::GetInstance()->AddInterfaces(&registry_);
 #endif
@@ -235,7 +233,10 @@
 }
 
 void Service::StartDisplayInit() {
-  screen_manager_->Init(window_server_->display_manager());
+  DCHECK(!is_gpu_ready_);  // This should only be called once.
+  is_gpu_ready_ = true;
+  if (screen_manager_)
+    screen_manager_->Init(window_server_->display_manager());
 }
 
 void Service::OnFirstDisplayReady() {
@@ -261,6 +262,30 @@
   return test_config_;
 }
 
+void Service::OnWillCreateTreeForWindowManager(
+    bool automatically_create_display_roots) {
+  if (screen_manager_config_ != ScreenManagerConfig::UNKNOWN)
+    return;
+
+  DVLOG(3) << "OnWillCreateTreeForWindowManager "
+           << automatically_create_display_roots;
+  screen_manager_config_ = automatically_create_display_roots
+                               ? ScreenManagerConfig::INTERNAL
+                               : ScreenManagerConfig::FORWARDING;
+  if (screen_manager_config_ == ScreenManagerConfig::FORWARDING) {
+#if defined(USE_OZONE) && defined(OS_CHROMEOS)
+    screen_manager_ = base::MakeUnique<display::ScreenManagerForwarding>();
+#else
+    CHECK(false);
+#endif
+  } else {
+    screen_manager_ = display::ScreenManager::Create();
+  }
+  screen_manager_->AddInterfaces(&registry_);
+  if (is_gpu_ready_)
+    screen_manager_->Init(window_server_->display_manager());
+}
+
 void Service::Create(const service_manager::Identity& remote_identity,
                      mojom::AccessibilityManagerRequest request) {
   UserState* user_state = GetUserState(remote_identity);
diff --git a/services/ui/service.h b/services/ui/service.h
index 92e5f60..9d88bfd 100644
--- a/services/ui/service.h
+++ b/services/ui/service.h
@@ -83,6 +83,20 @@
   ~Service() override;
 
  private:
+  // How the ScreenManager is configured.
+  enum ScreenManagerConfig {
+    // Initial state.
+    UNKNOWN,
+
+    // ScreenManager runs locally.
+    INTERNAL,
+
+    // Used when the window manager supplies a value of false for
+    // |automatically_create_display_roots|. In this config the ScreenManager
+    // is configured to forward calls.
+    FORWARDING,
+  };
+
   // Holds InterfaceRequests received before the first WindowTreeHost Display
   // has been established.
   struct PendingRequest;
@@ -111,6 +125,8 @@
   void OnFirstDisplayReady() override;
   void OnNoMoreDisplays() override;
   bool IsTestConfig() const override;
+  void OnWillCreateTreeForWindowManager(
+      bool automatically_create_display_roots) override;
 
   // service_manager::InterfaceFactory<mojom::AccessibilityManager>
   // implementation.
@@ -198,6 +214,10 @@
 
   service_manager::BinderRegistry registry_;
 
+  // Set to true in StartDisplayInit().
+  bool is_gpu_ready_ = false;
+  ScreenManagerConfig screen_manager_config_ = ScreenManagerConfig::UNKNOWN;
+
   DISALLOW_COPY_AND_ASSIGN(Service);
 };
 
diff --git a/services/ui/ws/display_manager.cc b/services/ui/ws/display_manager.cc
index 2bde158..286993e 100644
--- a/services/ui/ws/display_manager.cc
+++ b/services/ui/ws/display_manager.cc
@@ -77,6 +77,12 @@
   pending_displays_.insert(display);
 }
 
+void DisplayManager::AddDisplayForWindowManager(
+    const display::Display& display,
+    const display::ViewportMetrics& metrics) {
+  OnDisplayAdded(display, metrics);
+}
+
 void DisplayManager::DestroyDisplay(Display* display) {
   if (pending_displays_.count(display)) {
     pending_displays_.erase(display);
diff --git a/services/ui/ws/display_manager.h b/services/ui/ws/display_manager.h
index 3d2d370..4fe8c9c 100644
--- a/services/ui/ws/display_manager.h
+++ b/services/ui/ws/display_manager.h
@@ -47,6 +47,9 @@
   // Adds/removes a Display. DisplayManager owns the Displays.
   // TODO(sky): make add take a scoped_ptr.
   void AddDisplay(Display* display);
+  // Called when the window manager explicitly adds a new display.
+  void AddDisplayForWindowManager(const display::Display& display,
+                                  const display::ViewportMetrics& metrics);
   void DestroyDisplay(Display* display);
   void DestroyAllDisplays();
   const std::set<Display*>& displays() { return displays_; }
diff --git a/services/ui/ws/test_utils.cc b/services/ui/ws/test_utils.cc
index 50cbcd27..dea18697 100644
--- a/services/ui/ws/test_utils.cc
+++ b/services/ui/ws/test_utils.cc
@@ -142,6 +142,10 @@
   display::Screen::SetScreenInstance(screen_.get());
 }
 
+display::ScreenBase* TestScreenManager::GetScreen() {
+  return screen_.get();
+}
+
 // TestPlatformDisplayFactory  -------------------------------------------------
 
 TestPlatformDisplayFactory::TestPlatformDisplayFactory(
diff --git a/services/ui/ws/test_utils.h b/services/ui/ws/test_utils.h
index bc7d52d..ac8c2fb 100644
--- a/services/ui/ws/test_utils.h
+++ b/services/ui/ws/test_utils.h
@@ -68,6 +68,7 @@
   void AddInterfaces(service_manager::BinderRegistry* registry) override {}
   void Init(display::ScreenManagerDelegate* delegate) override;
   void RequestCloseDisplay(int64_t display_id) override {}
+  display::ScreenBase* GetScreen() override;
 
  private:
   display::ScreenManagerDelegate* delegate_ = nullptr;
@@ -127,9 +128,12 @@
   void StartPointerWatcher(bool want_moves);
   void StopPointerWatcher();
 
-  bool ProcessSetDisplayRoot(int64_t display_id,
+  bool ProcessSetDisplayRoot(const display::Display& display_to_create,
+                             const mojom::WmViewportMetrics& viewport_metrics,
+                             bool is_primary_display,
                              const ClientWindowId& client_window_id) {
-    return tree_->ProcessSetDisplayRoot(display_id, client_window_id);
+    return tree_->ProcessSetDisplayRoot(display_to_create, viewport_metrics,
+                                        is_primary_display, client_window_id);
   }
 
  private:
diff --git a/services/ui/ws/window_manager_window_tree_factory.cc b/services/ui/ws/window_manager_window_tree_factory.cc
index 0f9589b..89711f2 100644
--- a/services/ui/ws/window_manager_window_tree_factory.cc
+++ b/services/ui/ws/window_manager_window_tree_factory.cc
@@ -7,6 +7,7 @@
 #include "base/bind.h"
 #include "services/ui/ws/window_manager_window_tree_factory_set.h"
 #include "services/ui/ws/window_server.h"
+#include "services/ui/ws/window_server_delegate.h"
 #include "services/ui/ws/window_tree.h"
 
 namespace ui {
@@ -38,6 +39,10 @@
   if (binding_.is_bound())
     binding_.Close();
 
+  window_manager_window_tree_factory_set_->window_server()
+      ->delegate()
+      ->OnWillCreateTreeForWindowManager(automatically_create_display_roots);
+
   SetWindowTree(GetWindowServer()->CreateTreeForWindowManager(
       user_id_, std::move(window_tree_request), std::move(window_tree_client),
       automatically_create_display_roots));
diff --git a/services/ui/ws/window_server_delegate.cc b/services/ui/ws/window_server_delegate.cc
index 6d1194b..1250bade 100644
--- a/services/ui/ws/window_server_delegate.cc
+++ b/services/ui/ws/window_server_delegate.cc
@@ -21,5 +21,8 @@
   return nullptr;
 }
 
+void WindowServerDelegate::OnWillCreateTreeForWindowManager(
+    bool automatically_create_display_roots) {}
+
 }  // namespace ws
 }  // namespace ui
diff --git a/services/ui/ws/window_server_delegate.h b/services/ui/ws/window_server_delegate.h
index 337cc388..5256e11 100644
--- a/services/ui/ws/window_server_delegate.h
+++ b/services/ui/ws/window_server_delegate.h
@@ -53,6 +53,12 @@
       mojom::WindowTreeRequest* tree_request,
       mojom::WindowTreeClientPtr* client);
 
+  // Called prior to a new WindowTree being created for a
+  // WindowManagerWindowTreeFactory. |automatically_create_display_roots|
+  // mirrors that of CreateWindowTree(). See it for details.
+  virtual void OnWillCreateTreeForWindowManager(
+      bool automatically_create_display_roots);
+
  protected:
   virtual ~WindowServerDelegate() {}
 };
diff --git a/services/ui/ws/window_tree.cc b/services/ui/ws/window_tree.cc
index 85acf92..ecb78b50 100644
--- a/services/ui/ws/window_tree.cc
+++ b/services/ui/ws/window_tree.cc
@@ -12,6 +12,7 @@
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "mojo/public/cpp/bindings/map.h"
+#include "services/ui/display/screen_manager.h"
 #include "services/ui/ws/cursor_location_manager.h"
 #include "services/ui/ws/default_access_policy.h"
 #include "services/ui/ws/display.h"
@@ -29,6 +30,8 @@
 #include "services/ui/ws/window_server.h"
 #include "services/ui/ws/window_tree_binding.h"
 #include "ui/display/display.h"
+#include "ui/display/display_list.h"
+#include "ui/display/screen_base.h"
 #include "ui/display/types/display_constants.h"
 #include "ui/platform_window/mojo/ime_type_converters.h"
 #include "ui/platform_window/text_input_state.h"
@@ -263,20 +266,26 @@
   }
 }
 
-bool WindowTree::ProcessSetDisplayRoot(int64_t display_id,
-                                       const ClientWindowId& client_window_id) {
+ServerWindow* WindowTree::ProcessSetDisplayRoot(
+    const display::Display& display_to_create,
+    const mojom::WmViewportMetrics& transport_viewport_metrics,
+    bool is_primary_display,
+    const ClientWindowId& client_window_id) {
   DCHECK(window_manager_state_);  // Only called for window manager.
+  DVLOG(3) << "SetDisplayRoot client=" << id_
+           << " global window_id=" << client_window_id.id;
   Display* display =
-      window_server_->display_manager()->GetDisplayById(display_id);
-  if (!display) {
-    DVLOG(1) << "SetDisplayRoot called with unknown display " << display_id;
-    return false;
+      window_server_->display_manager()->GetDisplayById(display_to_create.id());
+  if (display) {
+    DVLOG(1) << "SetDisplayRoot called with existing display "
+             << display_to_create.id();
+    return nullptr;
   }
 
   if (automatically_create_display_roots_) {
     DVLOG(1) << "SetDisplayRoot is only applicable when "
              << "automatically_create_display_roots is false";
-    return false;
+    return nullptr;
   }
 
   ServerWindow* window = GetWindowByClientId(client_window_id);
@@ -284,29 +293,44 @@
   if (!window || window->parent()) {
     DVLOG(1) << "SetDisplayRoot called with invalid window id "
              << client_window_id.id;
-    return false;
-  }
-
-  WindowManagerDisplayRoot* display_root =
-      display->GetWindowManagerDisplayRootForUser(
-          window_manager_state_->user_id());
-  DCHECK(display_root);
-  if (!display_root->root()->children().empty()) {
-    DVLOG(1) << "SetDisplayRoot called more than once";
-    return false;
+    return nullptr;
   }
 
   if (base::ContainsValue(roots_, window)) {
     DVLOG(1) << "SetDisplayRoot called with existing root";
-    return false;
+    return nullptr;
   }
 
+  const display::DisplayList::Type display_type =
+      is_primary_display ? display::DisplayList::Type::PRIMARY
+                         : display::DisplayList::Type::NOT_PRIMARY;
+  display::ScreenManager::GetInstance()->GetScreen()->display_list().AddDisplay(
+      display_to_create, display_type);
+  display::ViewportMetrics viewport_metrics;
+  viewport_metrics.bounds_in_pixels =
+      transport_viewport_metrics.bounds_in_pixels;
+  viewport_metrics.device_scale_factor =
+      transport_viewport_metrics.device_scale_factor;
+  viewport_metrics.ui_scale_factor = transport_viewport_metrics.ui_scale_factor;
+  window_server_->display_manager()->AddDisplayForWindowManager(
+      display_to_create, viewport_metrics);
+
+  // OnDisplayAdded() should trigger creation of the Display.
+  display =
+      window_server_->display_manager()->GetDisplayById(display_to_create.id());
+  DCHECK(display);
+  WindowManagerDisplayRoot* display_root =
+      display->GetWindowManagerDisplayRootForUser(
+          window_manager_state_->user_id());
+  DCHECK(display_root);
+  DCHECK(display_root->root()->children().empty());
+
   // NOTE: this doesn't resize the window in anyway. We assume the client takes
   // care of any modifications it needs to do.
   roots_.insert(window);
   Operation op(this, window_server_, OperationType::ADD_WINDOW);
   display_root->root()->Add(window);
-  return true;
+  return window;
 }
 
 bool WindowTree::SetCapture(const ClientWindowId& client_window_id) {
@@ -347,8 +371,12 @@
 bool WindowTree::NewWindow(
     const ClientWindowId& client_window_id,
     const std::map<std::string, std::vector<uint8_t>>& properties) {
-  if (!IsValidIdForNewWindow(client_window_id))
+  DVLOG(3) << "new window client=" << id_
+           << " window_id=" << client_window_id.id;
+  if (!IsValidIdForNewWindow(client_window_id)) {
+    DVLOG(1) << "new window failed, id is not valid for client";
     return false;
+  }
   const WindowId window_id = GenerateNewWindowId();
   DCHECK(!GetWindow(window_id));
   ServerWindow* window =
@@ -497,8 +525,17 @@
 bool WindowTree::SetWindowVisibility(const ClientWindowId& window_id,
                                      bool visible) {
   ServerWindow* window = GetWindowByClientId(window_id);
-  if (!window || !access_policy_->CanChangeWindowVisibility(window))
+  DVLOG(3) << "SetWindowVisibility client=" << id_
+           << " client window_id= " << window_id.id << " global window_id="
+           << (window ? WindowIdToTransportId(window->id()) : 0);
+  if (!window) {
+    DVLOG(1) << "SetWindowVisibility failure, no window";
     return false;
+  }
+  if (!access_policy_->CanChangeWindowVisibility(window)) {
+    DVLOG(1) << "SetWindowVisibility failure, access policy denied change";
+    return false;
+  }
   if (window->visible() == visible)
     return true;
   Operation op(this, window_server_, OperationType::SET_WINDOW_VISIBILITY);
@@ -2105,10 +2142,20 @@
   window->set_extended_hit_test_region(hit_area);
 }
 
-void WindowTree::SetDisplayRoot(int64_t display_id,
+void WindowTree::SetDisplayRoot(const display::Display& display,
+                                mojom::WmViewportMetricsPtr viewport_metrics,
+                                bool is_primary_display,
                                 Id window_id,
                                 const SetDisplayRootCallback& callback) {
-  callback.Run(ProcessSetDisplayRoot(display_id, ClientWindowId(window_id)));
+  ServerWindow* display_root =
+      ProcessSetDisplayRoot(display, *viewport_metrics, is_primary_display,
+                            ClientWindowId(window_id));
+  if (!display_root) {
+    callback.Run(base::Optional<cc::FrameSinkId>());
+    return;
+  }
+  display_root->parent()->SetVisible(true);
+  callback.Run(display_root->frame_sink_id());
 }
 
 void WindowTree::WmResponse(uint32_t change_id, bool response) {
diff --git a/services/ui/ws/window_tree.h b/services/ui/ws/window_tree.h
index fd7da3a..ef9576c 100644
--- a/services/ui/ws/window_tree.h
+++ b/services/ui/ws/window_tree.h
@@ -383,9 +383,13 @@
   // waiting for the last move to be acknowledged.
   void OnWmMoveDragImageAck();
 
-  // Called from SetDisplayRoot(), see mojom for details.
-  bool ProcessSetDisplayRoot(int64_t display_id,
-                             const ClientWindowId& client_window_id);
+  // Called from SetDisplayRoot(), see mojom for details. Returns the root
+  // of the display if successful, otherwise null.
+  ServerWindow* ProcessSetDisplayRoot(
+      const display::Display& display_to_create,
+      const mojom::WmViewportMetrics& transport_viewport_metrics,
+      bool is_primary_display,
+      const ClientWindowId& client_window_id);
 
   // WindowTree:
   void NewWindow(uint32_t change_id,
@@ -496,7 +500,9 @@
   void RemoveActivationParent(Id transport_window_id) override;
   void ActivateNextWindow() override;
   void SetExtendedHitArea(Id window_id, const gfx::Insets& hit_area) override;
-  void SetDisplayRoot(int64_t display_id,
+  void SetDisplayRoot(const display::Display& display,
+                      mojom::WmViewportMetricsPtr viewport_metrics,
+                      bool is_primary_display,
                       Id window_id,
                       const SetDisplayRootCallback& callback) override;
   void WmResponse(uint32_t change_id, bool response) override;
diff --git a/services/ui/ws/window_tree_unittest.cc b/services/ui/ws/window_tree_unittest.cc
index 9bea946..cc10f3d 100644
--- a/services/ui/ws/window_tree_unittest.cc
+++ b/services/ui/ws/window_tree_unittest.cc
@@ -1538,8 +1538,6 @@
   const bool automatically_create_display_roots = false;
   AddWindowManager(window_server(), kTestUserId1,
                    automatically_create_display_roots);
-  const int64_t display_id =
-      screen_manager().AddDisplay(MakeDisplay(0, 0, 1024, 768, 1.0f));
   WindowManagerState* window_manager_state =
       window_server()->GetWindowManagerStateForUser(kTestUserId1);
   ASSERT_TRUE(window_manager_state);
@@ -1550,10 +1548,6 @@
   EXPECT_EQ(1, test_window_manager->connect_count());
   EXPECT_EQ(0, test_window_manager->display_added_count());
 
-  // Add another display and make sure WindowManager is not updated.
-  screen_manager().AddDisplay(MakeDisplay(0, 0, 1024, 768, 1.0f));
-  EXPECT_EQ(0, test_window_manager->display_added_count());
-
   // Create a window for the windowmanager and set it as the root.
   ClientWindowId display_root_id = BuildClientWindowId(window_manager_tree, 10);
   ASSERT_TRUE(window_manager_tree->NewWindow(display_root_id,
@@ -1561,8 +1555,17 @@
   ServerWindow* display_root =
       window_manager_tree->GetWindowByClientId(display_root_id);
   ASSERT_TRUE(display_root);
+  display::Display display1 = MakeDisplay(0, 0, 1024, 768, 1.0f);
+  display1.set_id(101);
+
+  mojom::WmViewportMetrics metrics;
+  metrics.bounds_in_pixels = display1.bounds();
+  metrics.device_scale_factor = 1.5;
+  metrics.ui_scale_factor = 2.5;
+  const bool is_primary_display = true;
   ASSERT_TRUE(WindowTreeTestApi(window_manager_tree)
-                  .ProcessSetDisplayRoot(display_id, display_root_id));
+                  .ProcessSetDisplayRoot(display1, metrics, is_primary_display,
+                                         display_root_id));
   EXPECT_TRUE(display_root->parent());
   EXPECT_TRUE(window_server_delegate()
                   ->last_binding()
@@ -1570,18 +1573,14 @@
                   ->tracker()
                   ->changes()
                   ->empty());
-  ASSERT_EQ(1u, window_manager_tree->roots().size());
-  EXPECT_EQ(display_root, *window_manager_tree->roots().begin());
-  ASSERT_EQ(2u, WindowManagerStateTestApi(window_manager_state)
-                    .window_manager_display_roots()
-                    .size());
+  EXPECT_EQ(1u, window_manager_tree->roots().size());
 
   // Delete the root, which should delete the WindowManagerDisplayRoot.
   EXPECT_TRUE(window_manager_tree->DeleteWindow(display_root_id));
-  ASSERT_TRUE(window_manager_tree->roots().empty());
-  ASSERT_EQ(1u, WindowManagerStateTestApi(window_manager_state)
-                    .window_manager_display_roots()
-                    .size());
+  EXPECT_TRUE(window_manager_tree->roots().empty());
+  EXPECT_TRUE(WindowManagerStateTestApi(window_manager_state)
+                  .window_manager_display_roots()
+                  .empty());
 }
 
 }  // namespace test