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

ui/aura/mus/window_tree_client.cc

Index: ui/aura/mus/window_tree_client.cc
diff --git a/ui/aura/mus/window_tree_client.cc b/ui/aura/mus/window_tree_client.cc
index 0f7351c738917443789d004d657cba9e57e89b8b..08925d5ffd3e5aec2e45078d5f1f4fe79b2b7d47 100644
--- a/ui/aura/mus/window_tree_client.cc
+++ b/ui/aura/mus/window_tree_client.cc
@@ -226,6 +226,20 @@ void WindowTreeClient::ConnectViaWindowTreeFactory() {
   SetWindowTree(std::move(window_tree));
 }
 
+void WindowTreeClient::ConnectViaWindowTreeHostFactory() {
+  // The client id doesn't really matter, we use 101 purely for debugging.
+  client_id_ = 101;
+
+  ui::mojom::WindowTreeHostFactoryRegistrarPtr host_factory_registrar;
+  connector_->BindInterface(ui::mojom::kServiceName, &host_factory_registrar);
+
+  ui::mojom::WindowTreePtr window_tree;
+  host_factory_registrar->Register(MakeRequest(&window_tree_host_factory_ptr_),
+                                   MakeRequest(&window_tree),
+                                   binding_.CreateInterfacePtrAndBind());
+  SetWindowTree(std::move(window_tree));
+}
+
 void WindowTreeClient::ConnectAsWindowManager() {
   DCHECK(window_manager_delegate_);
 
@@ -237,6 +251,10 @@ void WindowTreeClient::ConnectAsWindowManager() {
   SetWindowTree(std::move(window_tree));
 }
 
+void WindowTreeClient::CreateHost(ui::mojom::WindowTreeHostRequest host) {
+  window_tree_host_factory_ptr_->CreateWindowTreeHost(std::move(host));

[sky, 2017/02/28 05:02:22]

I suspect that this function shouldn't be called directly. That is, when client
code creates an aura::WindowTreeHostMus the code ends up here. That's assuming
the WindowTreeclient was configured using ConnectViaWindowTreeHostFactory. If
the WindowTreeClient wasn't configured in that way then the code needs to do
what it is doing today.

[fwang, 2017/02/28 08:23:53]

Mmmh, see the changes in MusDemo for how it is used. Currently, it is the other
way around. After a call to WindowTreeClient::ConnectViaWindowTreeHostFactory(),
each WindowTreeClient::CreateHost() will create a ws::Display on the server
which will call back OnEmbed on the client (with a nullptr) and creates an
aura::WindowTreeHostMus.

[sky, 2017/02/28 17:34:09]

Sure, but doing that is awkward and inconvenient, by which I mean having to wait
for a callback before you can do anything is error prone. It would be much more
natural to create the WindowTreeHostMus and have
WindowTreeHostMus/WindowTreeClient take care of everything for you. Doing this
will work better with how views works.

[tonikitoo, 2017/03/01 05:05:44]

Hum, I believe I understand sky's point. It might require a bit of change on the
current host creation flow.

sky: Would it be possible to change it as per your request in a follow up CL, so
that we expedite moving this one forward?

[sky, 2017/03/01 17:08:38]

I would prefer we go in the right direction, which I believe is what I outlined.

+}
+
 void WindowTreeClient::SetCanFocus(Window* window, bool can_focus) {
   DCHECK(tree_);
   DCHECK(window);
@@ -505,7 +523,6 @@ void WindowTreeClient::OnEmbedImpl(ui::mojom::WindowTree* window_tree,
   client_id_ = client_id;
   WindowTreeConnectionEstablished(window_tree);
 
-  DCHECK(roots_.empty());

[fwang, 2017/02/28 08:23:53]

Should this be kept when we do OnEmbed without configuring with
ConnectViaWindowManagerHostFactory (if that's ever possible)?

[tonikitoo, 2017/03/01 05:05:44]

From OnEmbedImpl it is a bit hard. I have moved this DCHECK to OnEmbed.

   std::unique_ptr<WindowTreeHostMus> window_tree_host =
       CreateWindowTreeHost(WindowMusType::EMBED, *root_data, display_id);
 
@@ -839,8 +856,10 @@ void WindowTreeClient::OnEmbed(ClientSpecificId client_id,
                                int64_t display_id,
                                Id focused_window_id,
                                bool drawn) {
-  DCHECK(!tree_ptr_);
-  tree_ptr_ = std::move(tree);
+  DCHECK(tree_ptr_);
+  DCHECK(!tree);
+  // No need to set 'tree_ptr_' here because it was already set during
+  // ConnectViaWindowManagerHostFactory.

[fwang, 2017/02/28 08:23:53]

What happens when we do OnEmbed without configuring with
ConnectViaWindowManagerHostFactory?

[tonikitoo, 2017/03/01 05:05:44]

Done.

 
   is_from_embed_ = true;