Move DOM distiller protoc plugin to new base::Value style.

Presently it uses the old deprecated style, which is also slightly less
efficient.

Change-Id: I09ea546e2254856d19ee7f0043b10575b7356260
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2176657
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#838767}

components/dom_distiller/core/distiller_page.cc

@@ -41,10 +41,10 @@ std::string GetDistillerScriptWithOptions(
     return "";
   }
 
-  std::unique_ptr<base::Value> options_value(
-      dom_distiller::proto::json::DomDistillerOptions::WriteToValue(options));
+  base::Value options_value =
+      dom_distiller::proto::json::DomDistillerOptions::WriteToValue(options);
   std::string options_json;
-  if (!base::JSONWriter::Write(*options_value, &options_json)) {
+  if (!base::JSONWriter::Write(options_value, &options_json)) {
     NOTREACHED();
   }
   size_t options_offset = script.find(kOptionsPlaceholder);
@@ -100,7 +100,7 @@ void DistillerPage::OnDistillationDone(const GURL& page_url,
   } else {
     found_content =
         dom_distiller::proto::json::DomDistillerResult::ReadFromValue(
-            value, distiller_result.get());
+            *value, distiller_result.get());
     if (!found_content) {
       DVLOG(1) << "Unable to parse DomDistillerResult.";
     } else {

third_party/dom_distiller_js/protoc_plugins/json_values_converter.py

@@ -65,9 +65,8 @@ class CppConverterWriter(writer.CodeWriter):
       # Nothing to write for enums.
 
       self.Output(
-          'static bool ReadFromValue(const base::Value* json, {generated_class_name}* message) {{\n'
-          '  const base::DictionaryValue* dict;\n'
-          '  if (!json->GetAsDictionary(&dict)) goto error;\n'
+          'static bool ReadFromValue(const base::Value& dict, {generated_class_name}* message) {{\n'
+          '  if (!dict.is_dict()) goto error;\n'
           '',
           generated_class_name=generated_class_name)
 
@@ -82,8 +81,8 @@ class CppConverterWriter(writer.CodeWriter):
           '  return false;\n'
           '}}\n'
           '\n'
-          'static std::unique_ptr<base::DictionaryValue> WriteToValue(const {generated_class_name}& message) {{\n'
-          '  std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue());\n'
+          'static base::Value WriteToValue(const {generated_class_name}& message) {{\n'
+          '  base::Value dict(base::Value::Type::DICTIONARY);\n'
           '',
           generated_class_name=generated_class_name)
 
@@ -115,56 +114,52 @@ class CppConverterWriter(writer.CodeWriter):
     self.Output('}}')
 
   def RepeatedMemberFieldWriteToValue(self, field):
-    prologue = (
-        'auto field_list = std::make_unique<base::ListValue>();\n'
-        'for (int i = 0; i < message.{field_name}_size(); ++i) {{\n'
-    )
-
     if field.IsClassType():
-      middle = (
-          'std::unique_ptr<base::Value> inner_message_value = \n'
-          '    {inner_class_converter}::WriteToValue(message.{field_name}(i));\n'
-          'field_list->Append(std::move(inner_message_value));\n'
+      self.Output(
+          'const auto& repeated_field = message.{field_name}();\n'
+          'base::Value::ListStorage field_list;\n'
+          'field_list.reserve(repeated_field.size());\n'
+          'for (const auto& element : repeated_field) {{\n'
+          '  field_list.push_back(\n'
+          '      {inner_class_converter}::WriteToValue(element));\n'
+          '}}\n'
+          'dict.SetKey("{field_number}",\n'
+          '            base::Value(std::move(field_list)));\n',
+          field_number=field.JavascriptIndex(),
+          field_name=field.name,
+          inner_class_converter=field.CppConverterType()
       )
     else:
-      middle = (
-          'field_list->Append{value_type}(message.{field_name}(i));\n'
+      self.Output(
+          'const auto& repeated_field = message.{field_name}();\n'
+          'base::Value::ListStorage field_list(\n'
+          '    repeated_field.begin(), repeated_field.end());\n'
+          'dict.SetKey("{field_number}",\n'
+          '            base::Value(std::move(field_list)));\n',
+          field_number=field.JavascriptIndex(),
+          field_name=field.name
       )
 
-    epilogue = (
-        '\n}}\n'
-        'dict->Set("{field_number}", std::move(field_list));'
-    )
-    self.Output(
-        prologue + Indented(middle) + epilogue,
-        field_number=field.JavascriptIndex(),
-        field_name=field.name,
-        value_type=field.CppValueType() if not field.IsClassType() else None,
-        inner_class_converter=field.CppConverterType()
-    )
-
   def OptionalMemberFieldWriteToValue(self, field):
     if field.IsClassType():
-      body = (
-          'std::unique_ptr<base::Value> inner_message_value = \n'
-          '    {inner_class_converter}::WriteToValue(message.{field_name}());\n'
-          'dict->Set("{field_number}", std::move(inner_message_value));\n'
+      self.Output(
+          'dict.SetKey("{field_number}",\n'
+          '            {inner_class_converter}::WriteToValue(\n'
+          '                message.{field_name}()));\n',
+          field_number=field.JavascriptIndex(),
+          field_name=field.name,
+          inner_class_converter=field.CppConverterType()
       )
     else:
-      body = (
-          'dict->Set{value_type}("{field_number}", message.{field_name}());\n'
+      self.Output(
+          'dict.Set{value_type}Key("{field_number}", message.{field_name}());\n',
+          field_number=field.JavascriptIndex(),
+          field_name=field.name,
+          value_type=field.CppValueType()
       )
 
-    self.Output(
-        body,
-        field_number=field.JavascriptIndex(),
-        field_name=field.name,
-        value_type=field.CppValueType() if not field.IsClassType() else None,
-        inner_class_converter=field.CppConverterType(),
-    )
-
   def WriteFieldRead(self, field):
-    self.Output('if (dict->HasKey("{field_number}")) {{',
+    self.Output('if (const auto* value = dict.FindKey("{field_number}")) {{',
                 field_number=field.JavascriptIndex())
 
     with self.AddIndent():
@@ -176,69 +171,54 @@ class CppConverterWriter(writer.CodeWriter):
     self.Output('}}')
 
   def RepeatedMemberFieldRead(self, field):
-    prologue = (
-        'const base::ListValue* field_list;\n'
-        'if (!dict->GetList("{field_number}", &field_list)) {{\n'
+    self.Output(
+        'if (!value->is_list()) {{\n'
         '  goto error;\n'
         '}}\n'
-        'for (size_t i = 0; i < field_list->GetSize(); ++i) {{\n'
+        'for (const auto& element : value->GetList()) {{\n'
     )
 
-    if field.IsClassType():
-      middle = (
-          'const base::Value* inner_message_value;\n'
-          'if (!field_list->Get(i, &inner_message_value)) {{\n'
-          '  goto error;\n'
-          '}}\n'
-          'if (!{inner_class_parser}::ReadFromValue(inner_message_value, message->add_{field_name}())) {{\n'
-          '  goto error;\n'
-          '}}\n'
-      )
-    else:
-      middle = (
-          '{cpp_type} field_value;\n'
-          'if (!field_list->Get{value_type}(i, &field_value)) {{\n'
-          '  goto error;\n'
-          '}}\n'
-          'message->add_{field_name}(field_value);\n'
-      )
-
-    self.Output(
-        prologue + Indented(middle) + '\n}}',
-        field_number=field.JavascriptIndex(),
-        field_name=field.name,
-        cpp_type=field.CppPrimitiveType() if not field.IsClassType() else None,
-        value_type=field.CppValueType() if not field.IsClassType() else None,
-        inner_class_parser=field.CppConverterType()
-    )
+    with self.AddIndent():
+      if field.IsClassType():
+        self.Output(
+            'if (!{inner_class_parser}::ReadFromValue(element, message->add_{field_name}())) {{\n'
+            '  goto error;\n'
+            '}}\n',
+            field_name=field.name,
+            inner_class_parser=field.CppConverterType()
+        )
+      else:
+        self.Output(
+            'if (!{predicate}) {{\n'
+            '  goto error;\n'
+            '}}\n'
+            'message->add_{field_name}(element.Get{value_type}());\n',
+            field_name=field.name,
+            value_type=field.CppValueType(),
+            predicate=field.CppValuePredicate('element')
+        )
+
+    self.Output('}}\n')
 
   def OptionalMemberFieldRead(self, field):
     if field.IsClassType():
       self.Output(
-          'const base::Value* inner_message_value;\n'
-          'if (!dict->Get("{field_number}", &inner_message_value)) {{\n'
-          '  goto error;\n'
-          '}}\n'
-          'if (!{inner_class_parser}::ReadFromValue(inner_message_value, message->mutable_{field_name}())) {{\n'
+          'if (!{inner_class_parser}::ReadFromValue(*value, message->mutable_{field_name}())) {{\n'
           '  goto error;\n'
-          '}}\n'
-          '',
+          '}}\n',
           field_number=field.JavascriptIndex(),
           field_name=field.name,
           inner_class_parser=field.CppConverterType()
       )
     else:
       self.Output(
-          '{cpp_type} field_value;\n'
-          'if (!dict->Get{value_type}("{field_number}", &field_value)) {{\n'
+          'if (!{predicate}) {{\n'
           '  goto error;\n'
           '}}\n'
-          'message->set_{field_name}(field_value);\n'
-          '',
-          field_number=field.JavascriptIndex(),
+          'message->set_{field_name}(value->Get{value_type}());\n',
           field_name=field.name,
-          cpp_type=field.CppPrimitiveType(),
-          value_type=field.CppValueType()
+          value_type=field.CppValueType(),
+          predicate=field.CppValuePredicate('(*value)')
       )
 
 

third_party/dom_distiller_js/protoc_plugins/util/plugin_protos.py

@@ -230,6 +230,9 @@ class ProtoField(object):
   def CppValueType(self):
     return types.GetCppValueType(self.CppPrimitiveType())
 
+  def CppValuePredicate(self, variable_name):
+    return types.GetCppValuePredicate(self.CppPrimitiveType(), variable_name)
+
   def CheckSupported(self):
     if self.Extendee():
       return 'Unsupported field extension: ' + self.DebugString()

third_party/dom_distiller_js/protoc_plugins/util/types.py

@@ -31,8 +31,8 @@ _cpp_base_type_map = {
 _cpp_type_to_value_type_map = {
     'double': 'Double',
     'float': 'Double',
-    'int': 'Integer',
-    'bool': 'Boolean',
+    'int': 'Int',
+    'bool': 'Bool',
     'std::string': 'String',
 }
 
@@ -59,6 +59,14 @@ def GetCppValueType(primitive_type):
   return _cpp_type_to_value_type_map[primitive_type]
 
 
+def GetCppValuePredicate(primitive_type, variable_name):
+  if primitive_type == 'double' or primitive_type == 'float':
+    return '({var}.is_int() || {var}.is_double())'.format(var=variable_name)
+  else:
+    return '{var}.is_{value_type}()'.format(
+        var=variable_name, value_type=GetCppValueType(primitive_type).lower())
+
+
 # TYPE_ENUM and TYPE_MESSAGE are supported, but their types depend on type_name.
 _java_base_type_map = {
     FieldDescriptorProto.TYPE_DOUBLE: 'double',