c++ / mojo changes for 'external window mode'

services/ui/ws/display.cc

Index: services/ui/ws/display.cc
diff --git a/services/ui/ws/display.cc b/services/ui/ws/display.cc
index b3981b86032061140d6c25279a890a940e863eab..6c74be3cb476bdc68d23522ed833bb19644b1be6 100644
--- a/services/ui/ws/display.cc
+++ b/services/ui/ws/display.cc
@@ -26,6 +26,7 @@
 #include "services/ui/ws/window_server_delegate.h"
 #include "services/ui/ws/window_tree.h"
 #include "services/ui/ws/window_tree_binding.h"
+#include "services/ui/ws/window_tree_host_factory.h"
 #include "ui/base/cursor/cursor.h"
 
 namespace ui {
@@ -62,6 +63,9 @@ Display::~Display() {
 
 void Display::Init(const PlatformDisplayInitParams& init_params,
                    std::unique_ptr<DisplayBinding> binding) {
+  if (window_server_->IsInExternalWindowMode())
+    DCHECK(window_server_->window_tree_host_factory() && binding);

[kylechar, 2017/03/07 18:19:37]

DCHECKs are compiled out in release builds, that might cause problems with the
if statement.

[tonikitoo, 2017/03/07 20:09:36]

BuildBots (including release bots) seem happy, but if you think it looks odd, I
can try it differently.

+
   binding_ = std::move(binding);
   display_manager()->AddDisplay(this);
 
@@ -204,7 +208,26 @@ void Display::SetTitle(const std::string& title) {
   platform_display_->SetTitle(base::UTF8ToUTF16(title));
 }
 
+void Display::InitDisplayRoot() {
+  DCHECK(window_server_->IsInExternalWindowMode());
+  DCHECK(window_server_->window_tree_host_factory());
+  DCHECK(binding_);
+
+  external_window_mode_display_root_ =
+      base::MakeUnique<WindowManagerDisplayRoot>(this);
+  window_manager_display_root_map_[service_manager::mojom::kRootUserID] =

[kylechar, 2017/03/07 18:19:37]

Once you aren't using WindowManagerDisplayRoot anymore in external mode, you
also wouldn't use this map, right?

[tonikitoo, 2017/03/07 20:09:36]

This is still needed, but should be clean up indeed. Will add a TODO.

When WindowTree::AddWindow is called, this calling out one the methods in
ws::Display that still need this map to be filled up.

+      external_window_mode_display_root_.get();
+
+  ServerWindow* server_window = external_window_mode_display_root_->root();
+  WindowTree* window_tree =
+      window_server_->window_tree_host_factory()->window_tree();
+  window_tree->AddRoot(server_window);
+  window_tree->DoOnEmbed(nullptr /*mojom::WindowTreePtr*/, server_window);
+}
+
 void Display::InitWindowManagerDisplayRoots() {
+  // Tests can create ws::Display instances, directly by-passing

[kylechar, 2017/03/07 18:19:37]

The if (binding_) path is only for tests now?

[tonikitoo, 2017/03/07 20:09:36]

Yes and No.

Yes, because I believe only tests exercise this path today.
No, because one can still call WindowTreeHostFactory::CreateWindowTreeHost
directly, and end up here.
This won't be in 'external window mode' though.

I left this existing code path untouched not to break existing tests, and if we
decide to change/remove it in the future, we should do it altogether.

+  // WindowTreeHostFactory.
   if (binding_) {
     std::unique_ptr<WindowManagerDisplayRoot> display_root_ptr(
         new WindowManagerDisplayRoot(this));
@@ -220,7 +243,6 @@ void Display::InitWindowManagerDisplayRoots() {
   } else {
     CreateWindowManagerDisplayRootsFromFactories();
   }
-  display_manager()->OnDisplayUpdate(this);
 }
 
 void Display::CreateWindowManagerDisplayRootsFromFactories() {
@@ -274,7 +296,13 @@ ServerWindow* Display::GetRootWindow() {
 
 void Display::OnAcceleratedWidgetAvailable() {
   display_manager()->OnDisplayAcceleratedWidgetAvailable(this);
-  InitWindowManagerDisplayRoots();
+
+  if (window_server_->IsInExternalWindowMode())
+    InitDisplayRoot();
+  else
+    InitWindowManagerDisplayRoots();
+
+  display_manager()->OnDisplayUpdate(this);
 }
 
 bool Display::IsInHighContrastMode() {
@@ -283,7 +311,7 @@ bool Display::IsInHighContrastMode() {
 
 void Display::OnEvent(const ui::Event& event) {
   WindowManagerDisplayRoot* display_root = GetActiveWindowManagerDisplayRoot();
-  if (display_root)
+  if (display_root && display_root->window_manager_state())
     display_root->window_manager_state()->ProcessEvent(event, GetId());
   window_server_
       ->GetUserActivityMonitorForUser(
@@ -293,7 +321,7 @@ void Display::OnEvent(const ui::Event& event) {
 
 void Display::OnNativeCaptureLost() {
   WindowManagerDisplayRoot* display_root = GetActiveWindowManagerDisplayRoot();
-  if (display_root)
+  if (display_root && display_root->window_manager_state())
     display_root->window_manager_state()->SetCapture(nullptr, kInvalidClientId);
 }
 
@@ -376,7 +404,7 @@ void Display::OnFocusChanged(FocusControllerChangeSource change_source,
 
   // WindowManagers are always notified of focus changes.
   WindowManagerDisplayRoot* display_root = GetActiveWindowManagerDisplayRoot();
-  if (display_root) {
+  if (display_root && display_root->window_manager_state()) {
     WindowTree* wm_tree = display_root->window_manager_state()->window_tree();
     if (wm_tree != owning_tree_old && wm_tree != embedded_tree_old &&
         wm_tree != owning_tree_new && wm_tree != embedded_tree_new) {