Makes WindowTreeHostMus supply properties directly to window creation

ui/aura/mus/window_tree_client_unittest.cc

 // Copyright 2015 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 "ui/aura/mus/window_tree_client.h"
 
 #include <stdint.h>
 
 #include "base/logging.h"
 #include "base/macros.h"
@@ skipping 128 matching lines @@
 TEST_F(WindowTreeClientWmTest, OnWindowHierarchyChangedWithProperties) {
   RegisterTestProperties(GetPropertyConverter());
   window_tree()->AckAllChanges();
   const Id child_window_id = server_id(root_window()) + 11;
   ui::mojom::WindowDataPtr data = ui::mojom::WindowData::New();
   const uint8_t server_test_property1_value = 91;
   data->properties[kTestPropertyServerKey1] =
       ConvertToPropertyTransportValue(server_test_property1_value);
   data->properties[ui::mojom::WindowManager::kWindowType_Property] =
       mojo::ConvertTo<std::vector<uint8_t>>(
           static_cast<int32_t>(ui::mojom::WindowType::BUBBLE));

[msw, 2016/12/02 00:31:08]

Should this use ConvertToPropertyTransportValue?

[sky, 2016/12/02 01:08:43]

As the window type can't change after the window is created it is supplied in
the initial set of properties and not mapped to a property on the aura::Window.
Because of this the type doesn't need to be int64.

   data->parent_id = server_id(root_window());
   data->window_id = child_window_id;
   data->bounds = gfx::Rect(1, 2, 3, 4);
   data->visible = false;
   std::vector<ui::mojom::WindowDataPtr> data_array(1);
   data_array[0] = std::move(data);
   ASSERT_TRUE(root_window()->children().empty());
   window_tree_client()->OnWindowHierarchyChanged(
       child_window_id, 0, server_id(root_window()), std::move(data_array));
   ASSERT_FALSE(window_tree()->has_change());
@@ skipping 885 matching lines @@
                                           display_id, true);
   EXPECT_EQ(initial_root_count, window_tree_client_impl()->GetRoots().size());
 }
 
 TEST_F(WindowTreeClientClientTest, NewTopLevelWindowGetsProperties) {
   RegisterTestProperties(GetPropertyConverter());
   const uint8_t property_value = 11;
   std::map<std::string, std::vector<uint8_t>> properties;
   properties[kTestPropertyServerKey1] =
       ConvertToPropertyTransportValue(property_value);
+  const char kUnknownPropertyKey[] = "unknown-property";
+  using UnknownPropertyType = int32_t;
+  const UnknownPropertyType kUnknownPropertyValue = 101;
+  properties[kUnknownPropertyKey] =
+      mojo::ConvertTo<std::vector<uint8_t>>(kUnknownPropertyValue);

[msw, 2016/12/02 00:31:08]

I guess PropertyConverter *never* converts this value, so it's okay to use a
smaller primitive type? (PropertyConverter ensures primitive property arrays are
8u for int64_t, which is a decent guard against users setting incompatible array
values) I wonder if some other ConvertToPropertyTransportValue calls in this
file should also use direct conversion instead? I wonder how else we can
guarantee that this property is never converted to an aura::WindowProperty, and
if we should comment here or somewhere about the difference between
PropertyConverter primitive transport values and other (not sure what to call
them? mus-only?) primitive transport values?

[sky, 2016/12/02 01:08:43]

That's right.
I think window_manager.mojom is the right place to document what the transport
types are. We are a bit inconsistent right now, I'll follow up on this in a
future patch when I make it consistent.

   std::unique_ptr<WindowTreeHostMus> window_tree_host =
       base::MakeUnique<WindowTreeHostMus>(window_tree_client_impl(),
                                           &properties);
   // Verify the property made it to the window.
   EXPECT_EQ(property_value,
             window_tree_host->window()->GetProperty(kTestPropertyKey1));
 
   // Get the id of the in flight change for creating the new top level window.
   uint32_t change_id;
   ASSERT_TRUE(window_tree()->GetAndRemoveFirstChangeOfType(
       WindowTreeChangeType::NEW_TOP_LEVEL, &change_id));
 
-  // Verify the property was sent to the server.
+  // Verify the properties were sent to the server.
   base::Optional<std::unordered_map<std::string, std::vector<uint8_t>>>
       transport_properties = window_tree()->GetLastNewWindowProperties();
   ASSERT_TRUE(transport_properties.has_value());
   std::map<std::string, std::vector<uint8_t>> properties2 =
       mojo::UnorderedMapToMap(*transport_properties);
   ASSERT_EQ(1u, properties2.count(kTestPropertyServerKey1));
   // PropertyConverter uses int64_t values, even for smaller types like uint8_t.
   ASSERT_EQ(8u, properties2[kTestPropertyServerKey1].size());
   EXPECT_EQ(static_cast<int64_t>(property_value),
             mojo::ConvertTo<int64_t>(properties2[kTestPropertyServerKey1]));
+
+  ASSERT_EQ(1u, properties2.count(kUnknownPropertyKey));
+  ASSERT_EQ(sizeof(UnknownPropertyType),
+            properties2[kUnknownPropertyKey].size());
+  EXPECT_EQ(kUnknownPropertyValue, mojo::ConvertTo<UnknownPropertyType>(
+                                       properties2[kUnknownPropertyKey]));
 }
 
 // Tests both SetCapture and ReleaseCapture, to ensure that Window is properly
 // updated on failures.
 TEST_F(WindowTreeClientWmTest, ExplicitCapture) {
   root_window()->SetCapture();
   EXPECT_TRUE(root_window()->HasCapture());
   ASSERT_TRUE(window_tree()->AckSingleChangeOfType(
       WindowTreeChangeType::CAPTURE, false));
   EXPECT_FALSE(root_window()->HasCapture());
@@ skipping 280 matching lines @@
           .CallWmNewDisplayAdded(display, std::move(root_data), parent_drawn);
   EXPECT_EQ(display.bounds(), window_tree_host->GetBoundsInPixels());
   // The root window of the WindowTreeHost always has an origin of 0,0.
   EXPECT_EQ(gfx::Rect(display.bounds().size()),
             window_tree_host->window()->bounds());
   EXPECT_TRUE(window_tree_host->window()->IsVisible());
   EXPECT_EQ(display.id(), window_tree_host->display_id());
 }
 
 }  // namespace aura