Allow parallel creation of windows

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(aura::WindowTreeClient* window_tree_client,
-                         int square_size)
-      : WindowTreeData(square_size) {
-    SetWindowTreeHost(
-        base::MakeUnique<aura::WindowTreeHostMus>(window_tree_client));
-  }
-
-  DISALLOW_COPY_AND_ASSIGN(WindowTreeDataExternal);
-};
-
 int GetSquareSizeForWindow(int window_index) {
   return 50 * window_index + 400;
 };
 
 }  // namespace
 
 MusDemoExternal::MusDemoExternal() {}
 
 MusDemoExternal::~MusDemoExternal() {}
 
@@ skipping 16 matching lines @@
 
   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));
 
   // TODO(tonikitoo,fwang): New windows can be launched without need to wait
   // the respective ::OnEmbed call of the previous instance.
-  OpenNewWindow();
+  for (size_t i = 0; i < number_of_windows_; ++i) {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE,
+        base::Bind(&MusDemoExternal::OpenNewWindow, base::Unretained(this)));
+  }

[fwang, 2017/03/15 14:37:56]

Do you really need this post task stuff? I think you can just append all
WindowTreeData in one go and wait for the OnRooWindowEmbed calls.

Also there are many comments explaining this to update.

[msisov, 2017/03/16 08:17:52]

Done.

 }
 
 void MusDemoExternal::OpenNewWindow() {
-  AppendWindowTreeData(base::MakeUnique<WindowTreeDataExternal>(
-      window_tree_client(),
-      GetSquareSizeForWindow(initialized_windows_count_)));
+  std::unique_ptr<aura::WindowTreeHostMus> mus =
+      base::MakeUnique<aura::WindowTreeHostMus>(window_tree_client());
+  window_tree_host_mus_list_.push_back(std::move(mus));

[fwang, 2017/03/15 14:37:56]

I believe you should keep appending to window_tree_data_list_

[msisov, 2017/03/16 08:17:51]

Done.

 }
 
-void MusDemoExternal::OnEmbed(
-    std::unique_ptr<aura::WindowTreeHostMus> window_tree_host) {
-  DCHECK(!window_tree_host);
+void MusDemoExternal::OnRootWindowEmbed(
+    aura::WindowTreeHostMus* window_tree_host) {
+  DCHECK(window_tree_host);
 
-  // TODO: Clean up WindowTreeClientDelegate::OnEmbed API so that it passes
-  // no ownership of WindowTreeHostMus instance.
-  InitWindowTreeData(nullptr);
-  initialized_windows_count_++;
+  auto it = std::find_if(
+      window_tree_host_mus_list_.begin(), window_tree_host_mus_list_.end(),
+      [window_tree_host](std::unique_ptr<aura::WindowTreeHostMus>& mus) {
+        return mus.get() == window_tree_host;
+      });
 
-  // Open the next window until the requested number of windows is reached.
-  if (initialized_windows_count_ < number_of_windows_)
-    OpenNewWindow();
+  DCHECK(it != window_tree_host_mus_list_.end());

[fwang, 2017/03/15 14:37:56]

This logic exists in RemoveWindowTreeData, so it should probably be shared. This
is possible if you use window_tree_data_list_ instead of your own vector.

[msisov, 2017/03/16 08:17:51]

Ok, I've made FindWindowTreeData, which returns an iterator for a requested
WindowTreeData

+  AppendWindowTreeData(base::MakeUnique<WindowTreeData>(
+      GetSquareSizeForWindow(initialized_windows_count_++)));
+  InitWindowTreeData(std::move(*it));

[tonikitoo, 2017/03/15 15:34:54]

now that you are passing a valid unique_ptr always to WindowTreeData::Init, we
should remove the null-check in it (see line 71):

 69 void WindowTreeData::Init(
 70     std::unique_ptr<aura::WindowTreeHostMus> window_tree_host) {
 71   if (window_tree_host) {
 72     window_tree_host->InitHost();
 73     window_tree_host->Show();
 74     SetWindowTreeHost(std::move(window_tree_host));
 75   }

[msisov, 2017/03/16 08:17:51]

Logic is changed. So it's still applicable.

+
+  window_tree_host_mus_list_.erase(it);

[fwang, 2017/03/15 14:37:56]

No need to do that if your use window_tree_data_list_

[msisov, 2017/03/16 08:17:52]

Done.

 }
 
 void MusDemoExternal::OnEmbedRootDestroyed(
     aura::WindowTreeHostMus* window_tree_host) {
   RemoveWindowTreeData(window_tree_host);
 }
 
 }  // namespace demo
 }  // namespace ui