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

services/ui/demo/mus_demo_external.cc

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "services/ui/demo/mus_demo_external.h"
 
 #include "base/command_line.h"
 #include "base/strings/string_number_conversions.h"
 #include "services/service_manager/public/cpp/service_context.h"
 #include "services/ui/demo/window_tree_data.h"
 #include "services/ui/public/interfaces/constants.mojom.h"
 #include "services/ui/public/interfaces/window_tree_host.mojom.h"
 #include "ui/aura/mus/window_tree_client.h"
 #include "ui/aura/mus/window_tree_host_mus.h"
 #include "ui/display/display.h"
 
 namespace ui {
 namespace demo {
 
 namespace {
 
 class WindowTreeDataExternal : public WindowTreeData {
  public:
   // Creates a new window tree host associated to the WindowTreeData.
-  WindowTreeDataExternal(mojom::WindowTreeHostFactory* factory,
-                         mojom::WindowTreeClientPtr tree_client,
+  WindowTreeDataExternal(aura::WindowTreeClient* window_tree_client,
                          int square_size)
       : WindowTreeData(square_size) {
-    // TODO(tonikitoo,fwang): Extend the API to allow creating WindowTreeHost
-    // using the WindowTreeClient.
-    factory->CreateWindowTreeHost(MakeRequest(&host_), std::move(tree_client));
+    window_tree_host_ =
+        base::MakeUnique<aura::WindowTreeHostMus>(window_tree_client);

[fwang, 2017/03/07 09:21:24]

IIUC, the Chromium style is to use setter for private variables, instead of
making them protected. So this would be

+    SetWindowTreeHost(
+        base::MakeUnique<aura::WindowTreeHostMus>(window_tree_client));

[tonikitoo, 2017/03/07 14:00:49]

Done.

   }
 
  private:
-  // Holds the Mojo pointer to the window tree host.
-  mojom::WindowTreeHostPtr host_;
+  void Init(
+      std::unique_ptr<aura::WindowTreeHostMus> window_tree_host) override {
+    InitImpl(window_tree_host_.get());
+  }

[fwang, 2017/03/07 09:21:25]

Let's postpone too much changes in MusDemo and just handle this case in the
parent class (checking window_tree_host != nullptr).

 
   DISALLOW_COPY_AND_ASSIGN(WindowTreeDataExternal);
 };
 
 int GetSquareSizeForWindow(int window_index) {
   return 50 * window_index + 400;
 };
 
 }  // namespace
 
 MusDemoExternal::MusDemoExternal() {}
 
 MusDemoExternal::~MusDemoExternal() {}
 
 std::unique_ptr<aura::WindowTreeClient>
 MusDemoExternal::CreateWindowTreeClient() {
-  return base::MakeUnique<aura::WindowTreeClient>(
-      context()->connector(), this, nullptr,
-      MakeRequest(&window_tree_client_mojo_));
+  return base::MakeUnique<aura::WindowTreeClient>(context()->connector(), this);
+}
+
+bool MusDemoExternal::HasPendingWindowTreeData() const {
+  return initialized_windows_count_ != window_tree_data_list().size();

[fwang, 2017/03/07 09:21:24]

Again overriding HasPendingWindowTreeData is not needed with the trick above.

[tonikitoo, 2017/03/07 14:00:49]

Done.

 }
 
 void MusDemoExternal::OnStartImpl() {
   const base::CommandLine* command_line =
       base::CommandLine::ForCurrentProcess();
   if (command_line->HasSwitch("external-window-count")) {
     if (!base::StringToSizeT(
             command_line->GetSwitchValueASCII("external-window-count"),
             &number_of_windows_)) {
       LOG(FATAL) << "Invalid value for \'external-window-count\'";
       return;
     }
   }
 
-  // TODO(tonikitoo,fwang): Extend the WindowTreeClient API to allow connection
-  // to the window tree host factory using window_tree_client().
-  context()->connector()->BindInterface(ui::mojom::kServiceName,
-                                        &window_tree_host_factory_);
+  window_tree_client()->ConnectViaWindowTreeHostFactory();
 
   // TODO(tonikitoo,fwang): Implement management of displays in external mode.
   // For now, a fake display is created in order to work around an assertion in
   // aura::GetDeviceScaleFactorFromDisplay().
   AddPrimaryDisplay(display::Display(0));
 
   // The number of windows to open is specified by number_of_windows_. The
   // windows are opened sequentially (the first one here and the others after
   // each call to OnEmbed) to ensure that the WindowTreeHostMus passed to
   // OnEmbed corresponds to the WindowTreeDataExternal::host_ created in
   // OpenNewWindow.
   OpenNewWindow();
 }
 
 void MusDemoExternal::OpenNewWindow() {
-  // TODO(tonikitoo,fwang): Extend the WindowTreeClient API to allow creation
-  // of window tree host. Then pass window_tree_client() here and remove
-  // window_tree_host_factory_ and window_tree_client_mojo_. Currently
-  // window_tree_client_mojo_ is only initialized once so this is incorrect when
-  // kNumberOfWindows > 1.
   AppendWindowTreeData(base::MakeUnique<WindowTreeDataExternal>(
-      window_tree_host_factory_.get(), std::move(window_tree_client_mojo_),
+      window_tree_client(),
       GetSquareSizeForWindow(initialized_windows_count_)));
 }
 
 void MusDemoExternal::OnEmbed(
     std::unique_ptr<aura::WindowTreeHostMus> window_tree_host) {
-  InitWindowTreeData(std::move(window_tree_host));
+  DCHECK(!window_tree_host);
+  InitWindowTreeData(nullptr);

[fwang, 2017/03/07 09:21:24]

This is a bit weird, but I guess it's ok if we don't want to modify MusDemo too
much for now.

[tonikitoo, 2017/03/07 14:00:49]

This is right. As written in the commit message, there are follow up clean ups
planned, which would take mus_demo into account too.

   initialized_windows_count_++;
 
   // Open the next window until the requested number of windows is reached.
   if (initialized_windows_count_ < number_of_windows_)
     OpenNewWindow();
 }
 
 void MusDemoExternal::OnEmbedRootDestroyed(
     aura::WindowTreeHostMus* window_tree_host) {
   RemoveWindowTreeData(window_tree_host);
 }
 
 }  // namespace demo
 }  // namespace ui