Refactor JSONValueConverter Out of DisplayLayout

Separate the JSON processing code out of DisplayLayout in preparing to move to
ui/gfx since ui/gfx has no JSON dependency.

BUG=595498

Review URL: https://codereview.chromium.org/1808253004

Cr-Commit-Position: refs/heads/master@{#382398}

6 files changed, 289 insertions, 146 deletions

diff --git a/ash/ash.gyp b/ash/ash.gyp
index cd1e630..e6d0ac8 100644
--- a/ash/ash.gyp
+++ b/ash/ash.gyp
@@ -91,6 +91,8 @@
       'display/extended_mouse_warp_controller.h',
       'display/event_transformation_handler.cc',
       'display/event_transformation_handler.h',
+      'display/json_converter.cc',
+      'display/json_converter.h',
       'display/mirror_window_controller.cc',
       'display/mirror_window_controller.h',
       'display/mouse_cursor_event_filter.cc',
@@ -824,11 +826,11 @@
       'display/display_color_manager_chromeos_unittest.cc',
       'display/display_error_observer_chromeos_unittest.cc',
       'display/display_info_unittest.cc',
-      'display/display_layout_unittest.cc',
       'display/display_layout_builder_unittest.cc',
       'display/display_manager_unittest.cc',
       'display/display_util_unittest.cc',
       'display/extended_mouse_warp_controller_unittest.cc',
+      'display/json_converter_unittest.cc',
       'display/mirror_window_controller_unittest.cc',
       'display/mouse_cursor_event_filter_unittest.cc',
       'display/projecting_observer_chromeos_unittest.cc',
diff --git a/ash/display/display_layout.cc b/ash/display/display_layout.cc
index 23a0eeb..1897d76 100644
--- a/ash/display/display_layout.cc
+++ b/ash/display/display_layout.cc
@@ -10,10 +10,8 @@
 #include "ash/ash_switches.h"
 #include "ash/display/display_pref_util.h"
 #include "ash/shell.h"
-#include "base/json/json_value_converter.h"
 #include "base/logging.h"
 #include "base/strings/string_number_conversions.h"
-#include "base/strings/string_piece.h"
 #include "base/strings/stringprintf.h"
 #include "base/values.h"
 #include "ui/gfx/display.h"
@@ -21,50 +19,17 @@
 namespace ash {
 namespace  {
 
+// DisplayPlacement Positions
+const char kTop[] = "top";
+const char kRight[] = "right";
+const char kBottom[] = "bottom";
+const char kLeft[] = "left";
+const char kUnknown[] = "unknown";
+
 // The maximum value for 'offset' in DisplayLayout in case of outliers.  Need
 // to change this value in case to support even larger displays.
 const int kMaxValidOffset = 10000;
 
-// Persistent key names
-const char kMirroredKey[] = "mirrored";
-const char kDefaultUnifiedKey[] = "default_unified";
-const char kPrimaryIdKey[] = "primary-id";
-const char kDisplayPlacementKey[] = "display_placement";
-
-// DisplayPlacement
-const char kPositionKey[] = "position";
-const char kOffsetKey[] = "offset";
-const char kDisplayPlacementDisplayIdKey[] = "display_id";
-const char kDisplayPlacementParentDisplayIdKey[] = "parent_display_id";
-
-using PositionToStringMap = std::map<DisplayPlacement::Position, std::string>;
-using DisplayPlacementMap = std::unordered_map<int64_t, DisplayPlacement>;
-
-const PositionToStringMap* GetPositionToStringMap() {
-  static const PositionToStringMap* map = CreateToStringMap(
-      DisplayPlacement::TOP, "top", DisplayPlacement::BOTTOM, "bottom",
-      DisplayPlacement::RIGHT, "right", DisplayPlacement::LEFT, "left");
-  return map;
-}
-
-std::string ToPositionString(DisplayPlacement::Position position) {
-  const PositionToStringMap* map = GetPositionToStringMap();
-  PositionToStringMap::const_iterator iter = map->find(position);
-  return iter != map->end() ? iter->second : std::string("unknown");
-}
-
-bool GetPositionFromString(const base::StringPiece& position,
-                           DisplayPlacement::Position* field) {
-  if (ReverseFind(GetPositionToStringMap(), position, field))
-    return true;
-  LOG(ERROR) << "Invalid position value:" << position;
-  return false;
-}
-
-bool GetDisplayIdFromString(const base::StringPiece& position, int64_t* field) {
-  return base::StringToInt64(position, field);
-}
-
 bool IsIdInList(int64_t id, const DisplayIdList& list) {
   const auto iter =
       std::find_if(list.begin(), list.end(),
@@ -130,101 +95,64 @@ std::string DisplayPlacement::ToString() const {
     s << "id=" << display_id << ", ";
   if (parent_display_id != gfx::Display::kInvalidDisplayID)
     s << "parent=" << parent_display_id << ", ";
-  s << ToPositionString(position) << ", ";
+  s << PositionToString(position) << ", ";
   s << offset;
   return s.str();
 }
 
 // static
-void DisplayPlacement::RegisterJSONConverter(
-    base::JSONValueConverter<DisplayPlacement>* converter) {
-  converter->RegisterIntField(kOffsetKey, &DisplayPlacement::offset);
-  converter->RegisterCustomField<DisplayPlacement::Position>(
-      kPositionKey, &DisplayPlacement::position, &GetPositionFromString);
-  converter->RegisterCustomField<int64_t>(kDisplayPlacementDisplayIdKey,
-                                          &DisplayPlacement::display_id,
-                                          &GetDisplayIdFromString);
-  converter->RegisterCustomField<int64_t>(kDisplayPlacementParentDisplayIdKey,
-                                          &DisplayPlacement::parent_display_id,
-                                          &GetDisplayIdFromString);
+std::string DisplayPlacement::PositionToString(Position position) {
+  switch (position) {
+    case TOP:
+      return kTop;
+    case RIGHT:
+      return kRight;
+    case BOTTOM:
+      return kBottom;
+    case LEFT:
+      return kLeft;
+  }
+  return kUnknown;
 }
 
-////////////////////////////////////////////////////////////////////////////////
-// DisplayLayout
-
-DisplayLayout::DisplayLayout()
-    : mirrored(false),
-      default_unified(true),
-      primary_id(gfx::Display::kInvalidDisplayID) {}
-
-DisplayLayout::~DisplayLayout() {}
-
 // static
-bool DisplayLayout::ConvertFromValue(const base::Value& value,
-                                     DisplayLayout* layout) {
-  layout->placement_list.clear();
-  base::JSONValueConverter<DisplayLayout> converter;
-  if (!converter.Convert(value, layout))
-    return false;
-  if (layout->placement_list.size() != 0u)
+bool DisplayPlacement::StringToPosition(const base::StringPiece& string,
+                                        Position* position) {
+  if (string == kTop) {
+    *position = TOP;
     return true;
-  // For compatibility with old format.
-  const base::DictionaryValue* dict_value = nullptr;
-  if (!value.GetAsDictionary(&dict_value) || dict_value == nullptr)
-    return false;
-  int offset;
-  if (dict_value->GetInteger(kOffsetKey, &offset)) {
-    DisplayPlacement::Position position;
-    std::string position_str;
-    if (!dict_value->GetString(kPositionKey, &position_str))
-      return false;
-    GetPositionFromString(position_str, &position);
-    layout->placement_list.push_back(new DisplayPlacement(position, offset));
   }
-  return true;
-}
 
-// static
-bool DisplayLayout::ConvertToValue(const DisplayLayout& layout,
-                                   base::Value* value) {
-  base::DictionaryValue* dict_value = nullptr;
-  if (!value->GetAsDictionary(&dict_value) || dict_value == nullptr)
-    return false;
+  if (string == kRight) {
+    *position = RIGHT;
+    return true;
+  }
 
-  dict_value->SetBoolean(kMirroredKey, layout.mirrored);
-  dict_value->SetBoolean(kDefaultUnifiedKey, layout.default_unified);
-  dict_value->SetString(kPrimaryIdKey, base::Int64ToString(layout.primary_id));
+  if (string == kBottom) {
+    *position = BOTTOM;
+    return true;
+  }
 
-  scoped_ptr<base::ListValue> placement_list(new base::ListValue);
-  for (const auto* placement : layout.placement_list) {
-    scoped_ptr<base::DictionaryValue> placement_value(
-        new base::DictionaryValue);
-    placement_value->SetString(kPositionKey,
-                               ToPositionString(placement->position));
-    placement_value->SetInteger(kOffsetKey, placement->offset);
-    placement_value->SetString(kDisplayPlacementDisplayIdKey,
-                               base::Int64ToString(placement->display_id));
-    placement_value->SetString(
-        kDisplayPlacementParentDisplayIdKey,
-        base::Int64ToString(placement->parent_display_id));
-    placement_list->Append(std::move(placement_value));
+  if (string == kLeft) {
+    *position = LEFT;
+    return true;
   }
-  dict_value->Set(kDisplayPlacementKey, std::move(placement_list));
-  return true;
-}
 
-// static
-void DisplayLayout::RegisterJSONConverter(
-    base::JSONValueConverter<DisplayLayout>* converter) {
-  converter->RegisterBoolField(kMirroredKey, &DisplayLayout::mirrored);
-  converter->RegisterBoolField(kDefaultUnifiedKey,
-                               &DisplayLayout::default_unified);
-  converter->RegisterCustomField<int64_t>(
-      kPrimaryIdKey, &DisplayLayout::primary_id, &GetDisplayIdFromString);
-  converter->RegisterRepeatedMessage<DisplayPlacement>(
-      kDisplayPlacementKey, &DisplayLayout::placement_list);
+  LOG(ERROR) << "Invalid position value:" << string;
+
+  return false;
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// DisplayLayout
+
+DisplayLayout::DisplayLayout()
+    : mirrored(false),
+      default_unified(true),
+      primary_id(gfx::Display::kInvalidDisplayID) {}
+
+DisplayLayout::~DisplayLayout() {}
+
 // static
 bool DisplayLayout::Validate(const DisplayIdList& list,
                              const DisplayLayout& layout) {
diff --git a/ash/display/display_layout.h b/ash/display/display_layout.h
index af80b68..136360b 100644
--- a/ash/display/display_layout.h
+++ b/ash/display/display_layout.h
@@ -14,16 +14,17 @@
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_vector.h"
-
-namespace gfx {
-class Display;
-}
+#include "base/strings/string_piece.h"
 
 namespace base {
 class Value;
 template <typename T> class JSONValueConverter;
 }
 
+namespace gfx {
+class Display;
+}
+
 namespace ash {
 
 // An identifier used to manage display layout in DisplayManager /
@@ -68,10 +69,9 @@ struct ASH_EXPORT DisplayPlacement {
 
   std::string ToString() const;
 
-  // Used by JSONValueConverter to generate DisplayPlacement from a
-  // JSON value.  See json_value_converter.h.
-  static void RegisterJSONConverter(
-      base::JSONValueConverter<DisplayPlacement>* converter);
+  static std::string PositionToString(Position position);
+  static bool StringToPosition(const base::StringPiece& string,
+                               Position* position);
 };
 
 class ASH_EXPORT DisplayLayout final {
@@ -79,15 +79,6 @@ class ASH_EXPORT DisplayLayout final {
   DisplayLayout();
   ~DisplayLayout();
 
-  // Converter functions to/from base::Value.
-  static bool ConvertFromValue(const base::Value& value, DisplayLayout* layout);
-  static bool ConvertToValue(const DisplayLayout& layout, base::Value* value);
-
-  // Used by JSONValueConverter to generate DisplayLayout from a
-  // JSON value.  See json_value_converter.h.
-  static void RegisterJSONConverter(
-      base::JSONValueConverter<DisplayLayout>* converter);
-
   // Validates the layout object.
   static bool Validate(const DisplayIdList& list, const DisplayLayout& layout);
 
diff --git a/ash/display/json_converter.cc b/ash/display/json_converter.cc
new file mode 100644
index 0000000..91017b6
--- /dev/null
+++ b/ash/display/json_converter.cc
@@ -0,0 +1,197 @@
+// Copyright 2016 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 "ash/display/json_converter.h"
+
+#include <string>
+
+#include "ash/display/display_layout.h"
+#include "ash/display/display_pref_util.h"
+#include "base/logging.h"
+#include "base/memory/scoped_vector.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/values.h"
+
+namespace ash {
+
+namespace {
+
+// Persistent key names
+const char kMirroredKey[] = "mirrored";
+const char kDefaultUnifiedKey[] = "default_unified";
+const char kPrimaryIdKey[] = "primary-id";
+const char kDisplayPlacementKey[] = "display_placement";
+
+// DisplayPlacement key names
+const char kPositionKey[] = "position";
+const char kOffsetKey[] = "offset";
+const char kDisplayPlacementDisplayIdKey[] = "display_id";
+const char kDisplayPlacementParentDisplayIdKey[] = "parent_display_id";
+
+bool AddLegacyValuesFromValue(const base::Value& value, DisplayLayout* layout) {
+  const base::DictionaryValue* dict_value = nullptr;
+  if (!value.GetAsDictionary(&dict_value))
+    return false;
+  int offset;
+  if (dict_value->GetInteger(kOffsetKey, &offset)) {
+    DisplayPlacement::Position position;
+    std::string position_str;
+    if (!dict_value->GetString(kPositionKey, &position_str))
+      return false;
+    DisplayPlacement::StringToPosition(position_str, &position);
+    layout->placement_list.push_back(new DisplayPlacement(position, offset));
+  }
+  return true;
+}
+
+// Returns true if
+//     The key is missing - output is left unchanged
+//     The key matches the type - output is updated to the value.
+template <typename Getter, typename Output>
+bool UpdateFromDict(const base::DictionaryValue* dict_value,
+                    const std::string& field_name,
+                    Getter getter,
+                    Output* output) {
+  const base::Value* field = nullptr;
+  if (!dict_value->Get(field_name, &field)) {
+    LOG(WARNING) << "Missing field: " << field_name;
+    return true;
+  }
+
+  return (field->*getter)(output);
+}
+
+// No implementation here as specialization is required.
+template <typename Output>
+bool UpdateFromDict(const base::DictionaryValue* dict_value,
+                    const std::string& field_name,
+                    Output* output);
+
+template <>
+bool UpdateFromDict(const base::DictionaryValue* dict_value,
+                    const std::string& field_name,
+                    bool* output) {
+  return UpdateFromDict(dict_value, field_name, &base::Value::GetAsBoolean,
+                        output);
+}
+
+template <>
+bool UpdateFromDict(const base::DictionaryValue* dict_value,
+                    const std::string& field_name,
+                    int* output) {
+  return UpdateFromDict(dict_value, field_name, &base::Value::GetAsInteger,
+                        output);
+}
+
+template <>
+bool UpdateFromDict(const base::DictionaryValue* dict_value,
+                    const std::string& field_name,
+                    DisplayPlacement::Position* output) {
+  bool (base::Value::*getter)(std::string*) const = &base::Value::GetAsString;
+  std::string value;
+  if (!UpdateFromDict(dict_value, field_name, getter, &value))
+    return false;
+
+  return value.empty() ? true
+                       : DisplayPlacement::StringToPosition(value, output);
+}
+
+template <>
+bool UpdateFromDict(const base::DictionaryValue* dict_value,
+                    const std::string& field_name,
+                    int64_t* output) {
+  bool (base::Value::*getter)(std::string*) const = &base::Value::GetAsString;
+  std::string value;
+  if (!UpdateFromDict(dict_value, field_name, getter, &value))
+    return false;
+
+  return value.empty() ? true : base::StringToInt64(value, output);
+}
+
+template <>
+bool UpdateFromDict(const base::DictionaryValue* dict_value,
+                    const std::string& field_name,
+                    ScopedVector<DisplayPlacement>* output) {
+  bool (base::Value::*getter)(const base::ListValue**) const =
+      &base::Value::GetAsList;
+  const base::ListValue* list = nullptr;
+  if (!UpdateFromDict(dict_value, field_name, getter, &list))
+    return false;
+
+  if (list == nullptr)
+    return true;
+
+  output->reserve(list->GetSize());
+  for (const auto& list_item : *list) {
+    const base::DictionaryValue* item_values = nullptr;
+    if (!list_item->GetAsDictionary(&item_values))
+      return false;
+
+    scoped_ptr<DisplayPlacement> item(new DisplayPlacement);
+    if (!UpdateFromDict(item_values, kOffsetKey, &item->offset) ||
+        !UpdateFromDict(item_values, kPositionKey, &item->position) ||
+        !UpdateFromDict(item_values, kDisplayPlacementDisplayIdKey,
+                        &item->display_id) ||
+        !UpdateFromDict(item_values, kDisplayPlacementParentDisplayIdKey,
+                        &item->parent_display_id)) {
+      return false;
+    }
+
+    output->push_back(std::move(item));
+  }
+  return true;
+}
+
+}  // namespace
+
+bool JsonToDisplayLayout(const base::Value& value, DisplayLayout* layout) {
+  layout->placement_list.clear();
+  const base::DictionaryValue* dict_value = nullptr;
+  if (!value.GetAsDictionary(&dict_value))
+    return false;
+
+  if (!UpdateFromDict(dict_value, kMirroredKey, &layout->mirrored) ||
+      !UpdateFromDict(dict_value, kDefaultUnifiedKey,
+                      &layout->default_unified) ||
+      !UpdateFromDict(dict_value, kPrimaryIdKey, &layout->primary_id)) {
+    return false;
+  }
+
+  UpdateFromDict(dict_value, kDisplayPlacementKey, &layout->placement_list);
+
+  if (layout->placement_list.size() != 0u)
+    return true;
+
+  // For compatibility with old format.
+  return AddLegacyValuesFromValue(value, layout);
+}
+
+bool DisplayLayoutToJson(const DisplayLayout& layout, base::Value* value) {
+  base::DictionaryValue* dict_value = nullptr;
+  if (!value->GetAsDictionary(&dict_value))
+    return false;
+
+  dict_value->SetBoolean(kMirroredKey, layout.mirrored);
+  dict_value->SetBoolean(kDefaultUnifiedKey, layout.default_unified);
+  dict_value->SetString(kPrimaryIdKey, base::Int64ToString(layout.primary_id));
+
+  scoped_ptr<base::ListValue> placement_list(new base::ListValue);
+  for (const auto* placement : layout.placement_list) {
+    scoped_ptr<base::DictionaryValue> placement_value(
+        new base::DictionaryValue);
+    placement_value->SetString(
+        kPositionKey, DisplayPlacement::PositionToString(placement->position));
+    placement_value->SetInteger(kOffsetKey, placement->offset);
+    placement_value->SetString(kDisplayPlacementDisplayIdKey,
+                               base::Int64ToString(placement->display_id));
+    placement_value->SetString(
+        kDisplayPlacementParentDisplayIdKey,
+        base::Int64ToString(placement->parent_display_id));
+    placement_list->Append(std::move(placement_value));
+  }
+  dict_value->Set(kDisplayPlacementKey, std::move(placement_list));
+  return true;
+}
+
+}  // namespace ash
diff --git a/ash/display/json_converter.h b/ash/display/json_converter.h
new file mode 100644
index 0000000..4c88eb0
--- /dev/null
+++ b/ash/display/json_converter.h
@@ -0,0 +1,26 @@
+// Copyright 2016 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 ASH_DISPLAY_JSON_CONVERTER_H_
+#define ASH_DISPLAY_JSON_CONVERTER_H_
+
+#include "ash/ash_export.h"
+
+namespace base {
+class Value;
+}
+
+namespace ash {
+
+class DisplayLayout;
+
+ASH_EXPORT bool JsonToDisplayLayout(const base::Value& value,
+                                    DisplayLayout* layout);
+
+ASH_EXPORT bool DisplayLayoutToJson(const DisplayLayout& layout,
+                                    base::Value* value);
+
+}  // namespace ash
+
+#endif  // ASH_DISPLAY_JSON_CONVERTER_H_
diff --git a/ash/display/display_layout_unittest.cc b/ash/display/json_converter_unittest.cc
index 7a614df..3c964d0 100644
--- a/ash/display/display_layout_unittest.cc
+++ b/ash/display/json_converter_unittest.cc
@@ -2,17 +2,16 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "ash/display/display_layout.h"
+#include "ash/display/json_converter.h"
 
+#include "ash/display/display_layout.h"
 #include "base/json/json_reader.h"
 #include "base/values.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace ash {
 
-typedef testing::Test DisplayLayoutTest;
-
-TEST_F(DisplayLayoutTest, ConvertFromToValue) {
+TEST(JsonConverterTest, JsonFromToDisplayLayout) {
   DisplayLayout layout;
   layout.primary_id = 1;
   layout.mirrored = true;
@@ -29,7 +28,7 @@ TEST_F(DisplayLayoutTest, ConvertFromToValue) {
   layout.placement_list[1]->offset = 30;
 
   base::DictionaryValue value;
-  DisplayLayout::ConvertToValue(layout, &value);
+  DisplayLayoutToJson(layout, &value);
 
   const char data[] =
       "{\n"
@@ -57,14 +56,14 @@ TEST_F(DisplayLayoutTest, ConvertFromToValue) {
   EXPECT_TRUE(value.Equals(read_value.get()));
 
   DisplayLayout read_layout;
-  EXPECT_TRUE(DisplayLayout::ConvertFromValue(*read_value, &read_layout));
+  EXPECT_TRUE(JsonToDisplayLayout(*read_value, &read_layout));
   EXPECT_EQ(read_layout.mirrored, layout.mirrored);
   EXPECT_EQ(read_layout.primary_id, layout.primary_id);
   EXPECT_EQ(read_layout.default_unified, layout.default_unified);
   EXPECT_TRUE(read_layout.HasSamePlacementList(layout));
 }
 
-TEST_F(DisplayLayoutTest, ConvertFromOldJSON) {
+TEST(JsonConverterTest, OldJsonToDisplayLayout) {
   const char data[] =
       "{\n"
       "  \"primary-id\": \"1\",\n"
@@ -81,7 +80,7 @@ TEST_F(DisplayLayoutTest, ConvertFromOldJSON) {
                            << error_column;
 
   DisplayLayout read_layout;
-  EXPECT_TRUE(DisplayLayout::ConvertFromValue(*read_value, &read_layout));
+  EXPECT_TRUE(JsonToDisplayLayout(*read_value, &read_layout));
   EXPECT_EQ(true, read_layout.mirrored);
   EXPECT_EQ(1, read_layout.primary_id);
   EXPECT_EQ(false, read_layout.default_unified);