Create mojom and StructTraits for ui/display/types/display_mode.cc

ui/display/types/display_mode.h

Index: ui/display/types/display_mode.h
diff --git a/ui/display/types/display_mode.h b/ui/display/types/display_mode.h
index 0b972d5f497a8288007c0d154c74ca982964f31f..d189d3fb9925c1fea94425fa3249e933264a09e0 100644
--- a/ui/display/types/display_mode.h
+++ b/ui/display/types/display_mode.h
@@ -10,16 +10,25 @@
 #include <string>
 
 #include "base/macros.h"
+#include "mojo/public/cpp/bindings/struct_traits.h"
 #include "ui/display/types/display_types_export.h"
 #include "ui/gfx/geometry/size.h"
 
 namespace display {
 
+namespace mojom {
+class DisplayModeDataView;
+}  // namespace mojom
+
 // This class represents the basic information for a native mode. Platforms will
 // extend this class to add platform specific information about the mode.
 class DISPLAY_TYPES_EXPORT DisplayMode {
  public:
+  DisplayMode();
   DisplayMode(const gfx::Size& size, bool interlaced, float refresh_rate);
+  DisplayMode(DisplayMode&&);

[rjkroege, 2017/01/27 18:54:55]

needs explicit.

Also: without std::move(<display object>) everywhere, does it add much value
given the size of this class?

[thanhph1, 2017/01/27 21:12:23]

Acknowledged. I removed this line.

+  DisplayMode& operator=(DisplayMode&&);

[rjkroege, 2017/01/27 18:54:55]

Do we need this? Can we remove?

[thanhph1, 2017/01/27 21:12:23]

Done.

+
   virtual ~DisplayMode();
   virtual std::unique_ptr<DisplayMode> Clone() const;
 
@@ -30,6 +39,8 @@ class DISPLAY_TYPES_EXPORT DisplayMode {
   virtual std::string ToString() const;
 
  private:
+  friend struct mojo::StructTraits<mojom::DisplayModeDataView, DisplayMode>;
+
   gfx::Size size_;

[rjkroege, 2017/01/27 18:54:55]

I don't like seeing the same structure defined in two places. Someone can add a
field to here and not update the mojom and there would be a odd bug to track
down at some point. 

Could we embed the DisplayModeDataView here instead? I think the struct traits
could be adjusted successfully given the friend declaration above. (I'm not
thrilled about friend but I understand why you need it pending the
de-virtualization of this class.)

[kylechar, 2017/01/27 19:34:18]

Having to update a C++ class, mojom and StructTraits in sync is going to be a
pain point for at least a couple hundred classes :)

You can't embed a DisplayModeDataView here though. DisplayModeDataView doesn't
actually hold any data, it just provides methods to deserialize data from a
serialized blob.

If you wanted to use the same structure you could just use a
display::mojom::DisplayModePtr directly and you don't need a StructTraits.

StructTraits were added to avoid doing that though...

[thanhph1, 2017/01/27 21:12:23]

I chat to Kyle and removed changes in display_mode.(cc|h) are untouched.

   bool is_interlaced_;
   float refresh_rate_;