Write mojom and StructTraits for DisplaySnapshot.

ui/display/mojo/display_struct_traits_unittest.cc

Index: ui/display/mojo/display_struct_traits_unittest.cc
diff --git a/ui/display/mojo/display_struct_traits_unittest.cc b/ui/display/mojo/display_struct_traits_unittest.cc
index e087e68d63e240e979ba24140c472ba8a62b6b45..1437aab77df4941b742ad17765562afa7f7bb9c1 100644
--- a/ui/display/mojo/display_struct_traits_unittest.cc
+++ b/ui/display/mojo/display_struct_traits_unittest.cc
@@ -2,12 +2,17 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <memory>
+
+#include "base/macros.h"
 #include "base/message_loop/message_loop.h"
 #include "mojo/public/cpp/bindings/binding_set.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/display/display.h"
 #include "ui/display/display_layout.h"
+#include "ui/display/display_snapshot_mojo.h"
 #include "ui/display/mojo/display_struct_traits_test.mojom.h"
+#include "ui/display/types/display_constants.h"
 #include "ui/display/types/display_mode.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/size.h"
@@ -41,6 +46,12 @@ class DisplayStructTraitsTest : public testing::Test,
     callback.Run(std::move(in));
   }
 
+  void EchoDisplaySnapshotMojo(
+      std::unique_ptr<DisplaySnapshotMojo> in,
+      const EchoDisplaySnapshotMojoCallback& callback) override {
+    callback.Run(std::move(in));
+  }
+
   void EchoDisplayPlacement(
       const DisplayPlacement& in,
       const EchoDisplayPlacementCallback& callback) override {
@@ -78,6 +89,43 @@ void CheckDisplayLayoutsEqual(const DisplayLayout& input,
   EXPECT_EQ(input.primary_id, output.primary_id);
 }
 
+void CheckDisplaySnapShotMojoEqual(const DisplaySnapshotMojo& input,
+                                   const DisplaySnapshotMojo& output) {
+  // We want to test each component individually to make sure each data member
+  // was correctly serialized and deserialized.
+  EXPECT_NE(&input, &output);  // Make sure they aren't the same object.
+  EXPECT_EQ(input.display_id(), output.display_id());
+  EXPECT_EQ(input.origin(), output.origin());
+  EXPECT_EQ(input.physical_size(), output.physical_size());
+  EXPECT_EQ(input.type(), output.type());
+  EXPECT_EQ(input.is_aspect_preserving_scaling(),
+            output.is_aspect_preserving_scaling());
+  EXPECT_EQ(input.has_overscan(), output.has_overscan());
+  EXPECT_EQ(input.has_color_correction_matrix(),
+            output.has_color_correction_matrix());
+  EXPECT_EQ(input.display_name(), output.display_name());
+  EXPECT_EQ(input.sys_path(), output.sys_path());
+  EXPECT_EQ(input.product_id(), output.product_id());
+  EXPECT_EQ(input.modes().size(), output.modes().size());
+
+  for (size_t i = 0; i < input.modes().size(); i++)
+    EXPECT_EQ(*input.modes()[i], *output.modes()[i]);
+
+  EXPECT_EQ(input.edid(), output.edid());
+
+  if (!input.current_mode())
+    EXPECT_EQ(nullptr, output.current_mode());
+  else
+    EXPECT_EQ(*input.current_mode(), *output.current_mode());
+
+  if (!input.native_mode())
+    EXPECT_EQ(nullptr, output.native_mode());
+  else
+    EXPECT_EQ(*input.native_mode(), *output.native_mode());
+
+  EXPECT_EQ(input.maximum_cursor_size(), output.maximum_cursor_size());
+}
+
 }  // namespace
 
 TEST_F(DisplayStructTraitsTest, DefaultDisplayValues) {
@@ -219,4 +267,159 @@ TEST_F(DisplayStructTraitsTest, DisplayLayoutTwoMirrored) {
   CheckDisplayLayoutsEqual(*input, *output);
 }
 
+// One display mode, current and native mode nullptr.
+TEST_F(DisplayStructTraitsTest, DisplaySnapshotMojoNullBothIndexPointer) {

[kylechar, 2017/02/15 16:48:45]

DisplaySnapshotCurrentAndNativeModesNull

[thanhph, 2017/02/16 11:48:21]

Done.

+  // prepare sample input with random values
+  const int64_t display_id = 7;
+  const gfx::Point origin(1, 2);
+  const gfx::Size physical_size(5, 9);
+  const gfx::Size maximum_cursor_size(3, 5);
+  const DisplayConnectionType type =
+      display::DISPLAY_CONNECTION_TYPE_DISPLAYPORT;
+  const bool is_aspect_preserving_scaling = true;
+  const bool has_overscan = true;
+  const bool has_color_correction_matrix = true;
+  const std::string display_name("whatever display_name");
+  const base::FilePath sys_path = base::FilePath::FromUTF8Unsafe("a/cb");
+  const int64_t product_id = 19;
+
+  const DisplayMode display_mode(gfx::Size(13, 11), true, 1211.0);

[kylechar, 2017/02/15 16:48:45]

I don't think any displays have refresh rates that high, just from a nit picky
perspective.

[thanhph, 2017/02/16 11:48:21]

I changed to 40.0, 50.0 & 60.0.

+
+  display::DisplaySnapshot::DisplayModeList modes;
+  modes.push_back(display_mode.Clone());

[kylechar, 2017/02/15 16:48:45]

If you think this helps with readability, leave it as is, but you can also do

modes.push_back(base::MakeUnique<DisplayMode>(...));

[thanhph, 2017/02/16 11:48:21]

I'll leave it as it is since it's shorter. Thanks for explicit explanation.

+
+  const DisplayMode* current_mode = nullptr;
+  const DisplayMode* native_mode = nullptr;
+  const std::vector<uint8_t> edid = {1};
+
+  std::unique_ptr<DisplaySnapshotMojo> input =
+      base::MakeUnique<DisplaySnapshotMojo>(
+          display_id, origin, physical_size, type, is_aspect_preserving_scaling,
+          has_overscan, has_color_correction_matrix, display_name, sys_path,
+          product_id, std::move(modes), edid, current_mode, native_mode,
+          maximum_cursor_size);
+
+  std::unique_ptr<DisplaySnapshotMojo> output;
+  GetTraitsTestProxy()->EchoDisplaySnapshotMojo(input->Clone(), &output);
+
+  CheckDisplaySnapShotMojoEqual(*input, *output);
+}
+
+// One display mode that is the native mode and no current mode.
+TEST_F(DisplayStructTraitsTest, DisplaySnapshotMojoNullOneIndexPointer) {

[kylechar, 2017/02/15 16:48:45]

DisplaySnapshotCurrentModeNull

[thanhph, 2017/02/16 11:48:21]

Done.

+  // prepare sample input with random values
+  const int64_t display_id = 6;
+  const gfx::Point origin(11, 32);
+  const gfx::Size physical_size(55, 49);
+  const gfx::Size maximum_cursor_size(13, 95);
+  const DisplayConnectionType type = display::DISPLAY_CONNECTION_TYPE_VGA;
+  const bool is_aspect_preserving_scaling = true;
+  const bool has_overscan = true;
+  const bool has_color_correction_matrix = true;
+  const std::string display_name("whatever display_name");
+  const base::FilePath sys_path = base::FilePath::FromUTF8Unsafe("z/b");
+  const int64_t product_id = 9;
+
+  const DisplayMode display_mode(gfx::Size(13, 11), true, 1211.0);
+
+  display::DisplaySnapshot::DisplayModeList modes;
+  modes.push_back(display_mode.Clone());
+
+  const DisplayMode* current_mode = modes[0].get();

[kylechar, 2017/02/15 16:48:45]

The description says one native mode and no current mode but you're doing the
opposite here.

[thanhph, 2017/02/16 11:48:21]

Done, I swapped current_mode to nullptr.

+  const DisplayMode* native_mode = nullptr;
+  const std::vector<uint8_t> edid = {1};
+
+  std::unique_ptr<DisplaySnapshotMojo> input =
+      base::MakeUnique<DisplaySnapshotMojo>(
+          display_id, origin, physical_size, type, is_aspect_preserving_scaling,
+          has_overscan, has_color_correction_matrix, display_name, sys_path,
+          product_id, std::move(modes), edid, current_mode, native_mode,
+          maximum_cursor_size);
+
+  std::unique_ptr<DisplaySnapshotMojo> output;
+  GetTraitsTestProxy()->EchoDisplaySnapshotMojo(input->Clone(), &output);
+
+  CheckDisplaySnapShotMojoEqual(*input, *output);
+}
+
+// Multiple display modes, both native and current mode set.
+TEST_F(DisplayStructTraitsTest, DisplaySnapshotMojoDefault) {

[kylechar, 2017/02/15 16:48:45]

What does DisplaySnapshotMojoDefault mean exactly?

Maybe DisplaySnapshotExternal

[thanhph, 2017/02/16 11:48:21]

Done.

+  // prepare sample input with random values

[kylechar, 2017/02/15 16:48:45]

Here is some sample data from an external monitor:

id:9834293210466051
origin: 0,1760
physical_size:520x320
has_overscan: false
is_aspect_perserving_scaling: false
has_color_correction_matrix: false
display_name: "HP Z24i"
modes: (1024x768, false, 60.00),
       (1440x900/59.89),
       (1920x1200, false, 59.89f) *this is the native mode
        

[thanhph, 2017/02/16 11:48:21]

Thanks, I added these data in. How do you get these data?

[kylechar, 2017/02/16 14:05:42]

I have this monitor on my desk and I printed the contents of DisplaySnapshot
when I connected it to a Chromebook.

+  const int64_t display_id = 73;
+  const gfx::Point origin(33, 126);
+  const gfx::Size physical_size(152, 294);
+  const gfx::Size maximum_cursor_size(320, 353);
+  const DisplayConnectionType type = display::DISPLAY_CONNECTION_TYPE_HDMI;
+  const bool is_aspect_preserving_scaling = true;
+  const bool has_overscan = true;
+  const bool has_color_correction_matrix = true;
+  const std::string display_name("whatever display_name");
+  const base::FilePath sys_path = base::FilePath::FromUTF8Unsafe("a/cb");
+  const int64_t product_id = 139;
+
+  const DisplayMode display_mode_1(gfx::Size(11, 12), true, 111.0);
+  const DisplayMode display_current_mode(gfx::Size(22, 23), true, 222.0);
+  const DisplayMode display_native_mode(gfx::Size(33, 34), true, 333.0);
+  const DisplayMode display_mode_2(gfx::Size(44, 45), true, 444.0);
+
+  display::DisplaySnapshot::DisplayModeList modes;
+  modes.push_back(display_mode_1.Clone());
+  modes.push_back(display_current_mode.Clone());
+  modes.push_back(display_native_mode.Clone());
+  modes.push_back(display_mode_2.Clone());
+
+  const DisplayMode* current_mode = modes[1].get();
+  const DisplayMode* native_mode = modes[2].get();
+  const std::vector<uint8_t> edid = {2, 3, 4, 5};
+
+  std::unique_ptr<DisplaySnapshotMojo> input =
+      base::MakeUnique<DisplaySnapshotMojo>(
+          display_id, origin, physical_size, type, is_aspect_preserving_scaling,
+          has_overscan, has_color_correction_matrix, display_name, sys_path,
+          product_id, std::move(modes), edid, current_mode, native_mode,
+          maximum_cursor_size);
+
+  std::unique_ptr<DisplaySnapshotMojo> output;
+  GetTraitsTestProxy()->EchoDisplaySnapshotMojo(input->Clone(), &output);
+
+  CheckDisplaySnapShotMojoEqual(*input, *output);
+}
+
+// Tests for both internal display and external display.

[kylechar, 2017/02/15 16:48:45]

This display is an internal display, as per the display type, so this comment
doesn't make sense.

[thanhph, 2017/02/16 11:48:21]

Removed this comment since the test name seems meaningful enough.

+TEST_F(DisplayStructTraitsTest, DisplaySnapshotMojoInternalDisplay) {

[kylechar, 2017/02/15 16:48:45]

DisplaySnapshotInternal maybe?

[thanhph, 2017/02/16 11:48:21]

Done.

+  // prepare sample input with random values

[kylechar, 2017/02/15 16:48:45]

I'll provide you with some sample data from Pixel 1, which is better than random
data.


id:13761487533244416
origin:0,0
physical_size:270x180
has_overscan=false
is_aspect_perserving_scaling=true
has_color_correction_matrix=false
display_name:""
sys_path:
modes=(2560x1700, false, 95.96) this is the native mode
maximum_cursor_size=64x64

[thanhph, 2017/02/16 11:48:21]

nice, thanks!

+  const int64_t display_id = 17;
+  const gfx::Point origin(31, 2);
+  const gfx::Size physical_size(215, 219);
+  const gfx::Size maximum_cursor_size(330, 351);
+  const DisplayConnectionType type = display::DISPLAY_CONNECTION_TYPE_INTERNAL;
+  const bool is_aspect_preserving_scaling = true;
+  const bool has_overscan = true;
+  const bool has_color_correction_matrix = true;
+  const std::string display_name("display_name");
+  const base::FilePath sys_path = base::FilePath::FromUTF8Unsafe("c/a");
+  const int64_t product_id = 139;
+
+  const DisplayMode display_mode(gfx::Size(11, 12), true, 111.0);
+
+  display::DisplaySnapshot::DisplayModeList modes;
+  modes.push_back(display_mode.Clone());
+
+  const DisplayMode* current_mode = modes[0].get();
+  const DisplayMode* native_mode = modes[0].get();
+  const std::vector<uint8_t> edid = {2, 3};
+
+  std::unique_ptr<DisplaySnapshotMojo> input =
+      base::MakeUnique<DisplaySnapshotMojo>(
+          display_id, origin, physical_size, type, is_aspect_preserving_scaling,
+          has_overscan, has_color_correction_matrix, display_name, sys_path,
+          product_id, std::move(modes), edid, current_mode, native_mode,
+          maximum_cursor_size);
+
+  std::unique_ptr<DisplaySnapshotMojo> output;
+  GetTraitsTestProxy()->EchoDisplaySnapshotMojo(input->Clone(), &output);
+
+  CheckDisplaySnapShotMojoEqual(*input, *output);
+}
+
 }  // namespace display