ws: Add property support to WindowServiceClient::NewWindow

Make a shared helper for getting the window type from properties.
Apply properties to the window like the old WindowTreeClient.

Bug: 837695
Change-Id: Icfe1efcddb52b2c3ef9d3ed218e02c105885e230
Reviewed-on: https://chromium-review.googlesource.com/1056349
Commit-Queue: Michael Wasserman <msw@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558209}

ash/ws/window_service_delegate_impl.cc

@@ -7,9 +7,9 @@
 #include "ash/wm/container_finder.h"
 #include "ash/wm/top_level_window_factory.h"
 #include "mojo/public/cpp/bindings/map.h"
-#include "services/ui/public/cpp/property_type_converters.h"
 #include "services/ui/public/interfaces/window_manager.mojom.h"
 #include "services/ui/public/interfaces/window_manager_constants.mojom.h"
+#include "ui/aura/mus/property_utils.h"
 #include "ui/aura/window.h"
 
 namespace ash {
@@ -23,13 +23,8 @@ std::unique_ptr<aura::Window> WindowServiceDelegateImpl::NewTopLevel(
     const base::flat_map<std::string, std::vector<uint8_t>>& properties) {
   std::map<std::string, std::vector<uint8_t>> property_map =
       mojo::FlatMapToMap(properties);
-  ui::mojom::WindowType window_type = ui::mojom::WindowType::UNKNOWN;
-  auto type_iter =
-      property_map.find(ui::mojom::WindowManager::kWindowType_InitProperty);
-  if (type_iter != property_map.end()) {
-    window_type = static_cast<ui::mojom::WindowType>(
-        mojo::ConvertTo<int32_t>(type_iter->second));
-  }
+  ui::mojom::WindowType window_type =
+      aura::GetWindowTypeFromProperties(property_map);
 
   auto* window =
       CreateAndParentTopLevelWindow(nullptr /* window_manager */, window_type,

services/ui/ws2/window_service_client.cc

@@ -20,6 +20,7 @@
 #include "ui/aura/client/transient_window_client.h"
 #include "ui/aura/env.h"
 #include "ui/aura/mus/property_converter.h"
+#include "ui/aura/mus/property_utils.h"
 #include "ui/aura/window.h"
 #include "ui/aura/window_observer.h"
 #include "ui/aura/window_tree_host.h"
@@ -375,10 +376,15 @@ bool WindowServiceClient::NewWindowImpl(
   }
   aura::Window* window = AddClientCreatedWindow(
       client_window_id, std::make_unique<aura::Window>(nullptr));
+
+  SetWindowType(window, aura::GetWindowTypeFromProperties(properties));
+  for (auto& pair : properties) {
+    window_service_->property_converter()->SetPropertyFromTransportValue(
+        window, pair.first, &pair.second);
+  }
   window->Init(LAYER_NOT_DRAWN);
   // Windows created by the client should only be destroyed by the client.
   window->set_owned_by_parent(false);
-  // TODO(crbug.com/837695): Apply |properties|.
   return true;
 }
 
@@ -668,9 +674,9 @@ void WindowServiceClient::NewWindow(
     Id transport_window_id,
     const base::Optional<base::flat_map<std::string, std::vector<uint8_t>>>&
         transport_properties) {
-  // TODO(crbug.com/837695): Map and validate |transport_properties|.
   std::map<std::string, std::vector<uint8_t>> properties;
-
+  if (transport_properties.has_value())
+    properties = mojo::FlatMapToMap(transport_properties.value());
   window_tree_client_->OnChangeCompleted(
       change_id,
       NewWindowImpl(MakeClientWindowId(transport_window_id), properties));

services/ui/ws2/window_service_client_test_helper.cc

@@ -19,6 +19,15 @@ mojom::WindowTree* WindowServiceClientTestHelper::window_tree() {
   return static_cast<mojom::WindowTree*>(window_service_client_);
 }
 
+aura::Window* WindowServiceClientTestHelper::NewWindow(
+    Id transport_window_id,
+    base::flat_map<std::string, std::vector<uint8_t>> properties) {
+  const uint32_t change_id = 1u;
+  window_service_client_->NewWindow(change_id, transport_window_id, properties);
+  return window_service_client_->GetWindowByClientId(
+      window_service_client_->MakeClientWindowId(transport_window_id));
+}
+
 aura::Window* WindowServiceClientTestHelper::NewTopLevelWindow(
     Id transport_window_id,
     base::flat_map<std::string, std::vector<uint8_t>> properties) {

services/ui/ws2/window_service_client_test_helper.h

@@ -41,6 +41,9 @@ class WindowServiceClientTestHelper {
 
   mojom::WindowTree* window_tree();
 
+  aura::Window* NewWindow(
+      Id transport_window_id,
+      base::flat_map<std::string, std::vector<uint8_t>> properties = {});
   aura::Window* NewTopLevelWindow(
       Id transport_window_id,
       base::flat_map<std::string, std::vector<uint8_t>> properties = {});

services/ui/ws2/window_service_client_unittest.cc

@@ -115,7 +115,31 @@ class TestLayoutManager : public aura::LayoutManager {
   DISALLOW_COPY_AND_ASSIGN(TestLayoutManager);
 };
 
-TEST(WindowServiceClientTest, CreateTopLevel) {
+TEST(WindowServiceClientTest, NewWindow) {
+  WindowServiceTestHelper helper;
+  EXPECT_TRUE(helper.changes()->empty());
+  aura::Window* top_level = helper.helper()->NewWindow(1);
+  ASSERT_TRUE(top_level);
+  EXPECT_EQ("ChangeCompleted id=1 sucess=true",
+            SingleChangeToDescription(*helper.changes()));
+  helper.changes()->clear();
+}
+
+TEST(WindowServiceClientTest, NewWindowWithProperties) {
+  WindowServiceTestHelper helper;
+  EXPECT_TRUE(helper.changes()->empty());
+  aura::PropertyConverter::PrimitiveType value = true;
+  std::vector<uint8_t> transport = mojo::ConvertTo<std::vector<uint8_t>>(value);
+  aura::Window* top_level = helper.helper()->NewWindow(
+      1, {{ui::mojom::WindowManager::kAlwaysOnTop_Property, transport}});
+  ASSERT_TRUE(top_level);
+  EXPECT_EQ("ChangeCompleted id=1 sucess=true",
+            SingleChangeToDescription(*helper.changes()));
+  EXPECT_TRUE(top_level->GetProperty(aura::client::kAlwaysOnTopKey));
+  helper.changes()->clear();
+}
+
+TEST(WindowServiceClientTest, NewTopLevelWindow) {
   WindowServiceTestHelper helper;
   EXPECT_TRUE(helper.changes()->empty());
   aura::Window* top_level = helper.helper()->NewTopLevelWindow(1);
@@ -125,7 +149,7 @@ TEST(WindowServiceClientTest, CreateTopLevel) {
   helper.changes()->clear();
 }
 
-TEST(WindowServiceClientTest, CreateTopLevelWithProperties) {
+TEST(WindowServiceClientTest, NewTopLevelWindowWithProperties) {
   WindowServiceTestHelper helper;
   EXPECT_TRUE(helper.changes()->empty());
   aura::PropertyConverter::PrimitiveType value = true;
@@ -139,7 +163,7 @@ TEST(WindowServiceClientTest, CreateTopLevelWithProperties) {
   helper.changes()->clear();
 }
 
-TEST(WindowServiceClientTest, SetBounds) {
+TEST(WindowServiceClientTest, SetWindowBounds) {
   WindowServiceTestHelper helper;
   aura::Window* top_level = helper.helper()->NewTopLevelWindow(1);
   helper.changes()->clear();
@@ -179,7 +203,7 @@ TEST(WindowServiceClientTest, SetBounds) {
   helper.changes()->clear();
 }
 
-TEST(WindowServiceClientTest, SetProperty) {
+TEST(WindowServiceClientTest, SetWindowProperty) {
   WindowServiceTestHelper helper;
   aura::Window* top_level = helper.helper()->NewTopLevelWindow(1);
   helper.changes()->clear();

ui/aura/mus/property_utils.cc

@@ -4,6 +4,8 @@
 
 #include "ui/aura/mus/property_utils.h"
 
+#include "services/ui/public/cpp/property_type_converters.h"
+#include "services/ui/public/interfaces/window_manager.mojom.h"
 #include "services/ui/public/interfaces/window_manager_constants.mojom.h"
 #include "ui/aura/client/aura_constants.h"
 #include "ui/aura/client/window_types.h"
@@ -39,8 +41,20 @@ client::WindowType UiWindowTypeToWindowType(ui::mojom::WindowType type) {
 }  // namespace
 
 void SetWindowType(Window* window, ui::mojom::WindowType window_type) {
+  if (window_type == ui::mojom::WindowType::UNKNOWN)
+    return;
   window->SetProperty(client::kWindowTypeKey, window_type);
   window->SetType(UiWindowTypeToWindowType(window_type));
 }
 
+ui::mojom::WindowType GetWindowTypeFromProperties(
+    const std::map<std::string, std::vector<uint8_t>>& properties) {
+  auto iter =
+      properties.find(ui::mojom::WindowManager::kWindowType_InitProperty);
+  if (iter == properties.end())
+    return ui::mojom::WindowType::UNKNOWN;
+  return static_cast<ui::mojom::WindowType>(
+      mojo::ConvertTo<int32_t>(iter->second));
+}
+
 }  // namespace aura

ui/aura/mus/property_utils.h

@@ -6,6 +6,9 @@
 #define UI_AURA_MUS_PROPERTY_UTILS_H_
 
 #include <stdint.h>
+#include <map>
+#include <string>
+#include <vector>
 
 #include "ui/aura/aura_export.h"
 
@@ -21,10 +24,14 @@ class Window;
 
 // Configures the two window type properties on |window|. Specifically this
 // sets the property client::kWindowTypeKey as well as calling SetType().
-// This *must* be called before Init().
+// This *must* be called before Init(). No-op for WindowType::UNKNOWN.
 AURA_EXPORT void SetWindowType(Window* window,
                                ui::mojom::WindowType window_type);
 
+// Returns the window type specified in |properties|, or WindowType::UNKNOWN.
+AURA_EXPORT ui::mojom::WindowType GetWindowTypeFromProperties(
+    const std::map<std::string, std::vector<uint8_t>>& properties);
+
 }  // namespace aura
 
 #endif  // UI_AURA_MUS_PROPERTY_UTILS_H_

ui/aura/mus/window_tree_client.cc

@@ -153,20 +153,6 @@ bool IsInternalProperty(const void* key) {
   return key == client::kModalKey || key == client::kChildModalParentKey;
 }
 
-void SetWindowTypeFromProperties(
-    Window* window,
-    const base::flat_map<std::string, std::vector<uint8_t>>& properties) {
-  auto type_iter =
-      properties.find(ui::mojom::WindowManager::kWindowType_InitProperty);
-  if (type_iter == properties.end())
-    return;
-
-  // TODO: need to validate type! http://crbug.com/654924.
-  ui::mojom::WindowType window_type = static_cast<ui::mojom::WindowType>(
-      mojo::ConvertTo<int32_t>(type_iter->second));
-  SetWindowType(window, window_type);
-}
-
 // Create and return a MouseEvent or TouchEvent from |event| if |event| is a
 // PointerEvent, otherwise return the copy of |event|.
 std::unique_ptr<ui::Event> MapEvent(const ui::Event& event) {
@@ -752,7 +738,9 @@ WindowMus* WindowTreeClient::NewWindowFromWindowData(
   window_port_mus_ptr->should_restack_transient_children_ = false;
   Window* window = new Window(nullptr, std::move(window_port_mus));
   WindowMus* window_mus = window_port_mus_ptr;
-  SetWindowTypeFromProperties(window, window_data.properties);
+  std::map<std::string, std::vector<uint8_t>> properties =
+      mojo::FlatMapToMap(window_data.properties);
+  SetWindowType(window, GetWindowTypeFromProperties(properties));
   window->Init(ui::LAYER_NOT_DRAWN);
   SetLocalPropertiesFromServerProperties(window_mus, window_data);
   window_mus->SetBoundsFromServer(
@@ -2011,14 +1999,7 @@ void WindowTreeClient::WmCreateTopLevelWindow(
   DCHECK(frame_sink_id.is_valid());
   std::map<std::string, std::vector<uint8_t>> properties =
       mojo::FlatMapToMap(transport_properties);
-  ui::mojom::WindowType window_type = ui::mojom::WindowType::UNKNOWN;
-  auto type_iter =
-      properties.find(ui::mojom::WindowManager::kWindowType_InitProperty);
-  if (type_iter != properties.end()) {
-    // TODO: validation! http://crbug.com/654924.
-    window_type = static_cast<ui::mojom::WindowType>(
-        mojo::ConvertTo<int32_t>(type_iter->second));
-  }
+  ui::mojom::WindowType window_type = GetWindowTypeFromProperties(properties);
   Window* window = window_manager_delegate_->OnWmCreateTopLevelWindow(
       window_type, &properties);
   if (!window) {