Write mojom and StructTraits for DisplaySnapshot.

ui/display/mojo/display_snapshot_mojo_struct_traits.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.
+
+#ifndef UI_DISPLAY_MOJO_DISPLAY_SNAPSHOT_MOJO_STRUCT_TRAITS_CC_

[kylechar, 2017/02/09 15:05:54]

Remove.

[thanhph1, 2017/02/10 19:54:32]

Done.

+#define UI_DISPLAY_MOJO_DISPLAY_SNAPSHOT_MOJO_STRUCT_TRAITS_CC_
+
+#include "ui/display/mojo/display_snapshot_mojo_struct_traits.h"
+
+#include "ui/display/mojo/display_mode.mojom.h"

[kylechar, 2017/02/09 15:05:54]

Please deduplicate header and source file includes.

[thanhph1, 2017/02/10 19:54:32]

Done. Removed a few.

+#include "ui/display/mojo/display_mode_struct_traits.h"
+#include "ui/display/mojo/display_snapshot_mojo.mojom.h"
+#include "ui/display/types/display_constants.h"
+#include "ui/gfx/geometry/mojo/geometry_struct_traits.h"
+#include "ui/gfx/geometry/size.h"
+
+namespace mojo {
+
+display::mojom::DisplayConnectionType EnumTraits<

[kylechar, 2017/02/09 15:05:54]

Deduplicate.

[thanhph1, 2017/02/10 19:54:32]

Removed.

+    display::mojom::DisplayConnectionType,
+    display::DisplayConnectionType>::ToMojom(display::DisplayConnectionType
+                                                 type) {
+  switch (type) {
+    case display::DisplayConnectionType::DISPLAY_CONNECTION_TYPE_NONE:
+      return display::mojom::DisplayConnectionType::
+          DISPLAY_CONNECTION_TYPE_NONE;
+
+    case display::DisplayConnectionType::DISPLAY_CONNECTION_TYPE_UNKNOWN:
+      return display::mojom::DisplayConnectionType::
+          DISPLAY_CONNECTION_TYPE_UNKNOWN;
+
+    case display::DisplayConnectionType::DISPLAY_CONNECTION_TYPE_INTERNAL:
+      return display::mojom::DisplayConnectionType::
+          DISPLAY_CONNECTION_TYPE_INTERNAL;
+
+    case display::DisplayConnectionType::DISPLAY_CONNECTION_TYPE_VGA:
+      return display::mojom::DisplayConnectionType::DISPLAY_CONNECTION_TYPE_VGA;
+
+    case display::DisplayConnectionType::DISPLAY_CONNECTION_TYPE_HDMI:
+      return display::mojom::DisplayConnectionType::
+          DISPLAY_CONNECTION_TYPE_HDMI;
+
+    case display::DisplayConnectionType::DISPLAY_CONNECTION_TYPE_DVI:
+      return display::mojom::DisplayConnectionType::DISPLAY_CONNECTION_TYPE_DVI;
+
+    case display::DisplayConnectionType::DISPLAY_CONNECTION_TYPE_DISPLAYPORT:
+      return display::mojom::DisplayConnectionType::
+          DISPLAY_CONNECTION_TYPE_DISPLAYPORT;
+
+    case display::DisplayConnectionType::DISPLAY_CONNECTION_TYPE_NETWORK:
+      return display::mojom::DisplayConnectionType::
+          DISPLAY_CONNECTION_TYPE_NETWORK;
+
+    case display::DisplayConnectionType::DISPLAY_CONNECTION_TYPE_VIRTUAL:
+      return display::mojom::DisplayConnectionType::
+          DISPLAY_CONNECTION_TYPE_VIRTUAL;
+  }
+  NOTREACHED();
+  return display::mojom::DisplayConnectionType::DISPLAY_CONNECTION_TYPE_NONE;
+}
+
+// static
+bool EnumTraits<display::mojom::DisplayConnectionType,
+                display::DisplayConnectionType>::
+    FromMojom(display::mojom::DisplayConnectionType type,
+              display::DisplayConnectionType* out) {
+  switch (type) {
+    case display::mojom::DisplayConnectionType::DISPLAY_CONNECTION_TYPE_NONE:
+      *out = display::DisplayConnectionType::DISPLAY_CONNECTION_TYPE_NONE;
+      return true;
+
+    case display::mojom::DisplayConnectionType::DISPLAY_CONNECTION_TYPE_UNKNOWN:
+      *out = display::DisplayConnectionType::DISPLAY_CONNECTION_TYPE_UNKNOWN;
+      return true;
+
+    case display::mojom::DisplayConnectionType::
+        DISPLAY_CONNECTION_TYPE_INTERNAL:
+      *out = display::DisplayConnectionType::DISPLAY_CONNECTION_TYPE_INTERNAL;
+      return true;
+
+    case display::mojom::DisplayConnectionType::DISPLAY_CONNECTION_TYPE_VGA:
+      *out = display::DisplayConnectionType::DISPLAY_CONNECTION_TYPE_VGA;
+      return true;
+
+    case display::mojom::DisplayConnectionType::DISPLAY_CONNECTION_TYPE_HDMI:
+      *out = display::DisplayConnectionType::DISPLAY_CONNECTION_TYPE_HDMI;
+      return true;
+
+    case display::mojom::DisplayConnectionType::DISPLAY_CONNECTION_TYPE_DVI:
+      *out = display::DisplayConnectionType::DISPLAY_CONNECTION_TYPE_DVI;
+      return true;
+
+    case display::mojom::DisplayConnectionType::
+        DISPLAY_CONNECTION_TYPE_DISPLAYPORT:
+      *out =
+          display::DisplayConnectionType::DISPLAY_CONNECTION_TYPE_DISPLAYPORT;
+      return true;
+
+    case display::mojom::DisplayConnectionType::DISPLAY_CONNECTION_TYPE_NETWORK:
+      *out = display::DisplayConnectionType::DISPLAY_CONNECTION_TYPE_NETWORK;
+      return true;
+
+    case display::mojom::DisplayConnectionType::DISPLAY_CONNECTION_TYPE_VIRTUAL:
+      *out = display::DisplayConnectionType::DISPLAY_CONNECTION_TYPE_VIRTUAL;
+      return true;
+  }
+  NOTREACHED();
+  return false;
+}
+
+// static
+bool StructTraits<display::mojom::DisplaySnapshotMojoDataView,
+                  std::unique_ptr<display::DisplaySnapshotMojo>>::
+    Read(display::mojom::DisplaySnapshotMojoDataView data,
+         std::unique_ptr<display::DisplaySnapshotMojo>* out) {
+  int64_t display_id = data.display_id();

[kylechar, 2017/02/09 15:05:54]

You don't really need a temporary variable if there is a normal accessor method,
right?

[thanhph1, 2017/02/10 19:54:32]

Done.

+
+  gfx::Point origin;
+  if (!data.ReadOrigin(&origin))
+    return false;
+
+  gfx::Size physical_size;
+  if (!data.ReadPhysicalSize(&physical_size))
+    return false;
+
+  display::DisplayConnectionType type;
+  if (!data.ReadType(&type))
+    return false;
+
+  bool is_aspect_preserving_scaling = data.is_aspect_preserving_scaling();
+  bool has_overscan = data.has_overscan();
+  bool has_color_correction_matrix = data.has_color_correction_matrix();
+
+  std::string display_name;
+  if (!data.ReadDisplayName(&display_name))
+    return false;
+
+  base::FilePath::StringType string_path;
+  if (!data.ReadSysPath(&string_path))
+    return false;
+  const base::FilePath::StringPieceType sys_path(string_path);
+  const base::FilePath filePath(sys_path);
+
+  std::vector<std::unique_ptr<display::DisplayMode>> modes1;
+  if (!data.ReadModes(&modes1))
+    return false;
+
+  display::DisplaySnapshot::DisplayModeList modes;
+  for (int i = 0; i < (int)modes1.size(); i++) {

[kylechar, 2017/02/09 15:05:54]

This would be clearer as a for each style loop. Also C style casts aren't
allowed anyways.

[thanhph1, 2017/02/10 19:54:32]

Cool, done!

[kylechar, 2017/02/10 21:43:30]

Oh I see where you got the for_each thing for. Sorry, this is what "for each"
loop refers to. The std::for_each function is a bit different.

http://en.cppreference.com/w/cpp/language/range-for

+    std::unique_ptr<const display::DisplayMode> temp(modes1[i]->Clone());
+    modes.push_back(std::move(temp));

[kylechar, 2017/02/09 15:05:54]

Why clone then move? Why not just clone?

[thanhph1, 2017/02/10 19:54:32]

Done.

+  }
+
+  int current_mode_index = data.current_mode_index();
+  if ((current_mode_index < 0) || (current_mode_index >= (int)modes1.size()))

[kylechar, 2017/02/09 15:05:54]

What happens if current_mode == null when you are serializing? Also C style
casts are banned.

[thanhph1, 2017/02/10 19:54:32]

Refactored. current_mode will be null if it doesn't find a match in the vector.

+    return false;
+  const display::DisplayMode* current_mode = modes[current_mode_index].get();
+
+  int native_mode_index = data.native_mode_index();
+  if ((native_mode_index < 0) || (native_mode_index >= (int)modes1.size()))
+    return false;
+  const display::DisplayMode* native_mode = modes[native_mode_index].get();
+
+  std::vector<uint8_t> edid;
+  if (!data.ReadEdid(&edid))
+    return false;
+  int64_t product_id = data.product_id();
+
+  *out = base::MakeUnique<display::DisplaySnapshotMojo>(
+      display_id, origin, physical_size, type, is_aspect_preserving_scaling,
+      has_overscan, has_color_correction_matrix, display_name, filePath,
+      product_id, std::move(modes), edid, current_mode, native_mode);
+  return true;
+}
+
+}  // namespace mojo
+
+#endif  // UI_DISPLAY_MOJO_DISPLAY_SNAPSHOT_MOJO_STRUCT_TRAITS_CC_