Reland "Convert xml_parser.mojom to use mojo/public/mojom/base/values.mojom"

This is a reland of 4f376c3e

Original change's description:
> Convert xml_parser.mojom to use mojo/public/mojom/base/values.mojom
> 
> This change also converts the minimum necessary amount of C++ code to pass
> objects of base::Value by value instead of std::unique_ptr.
> 
> Bug: 646113,799482
> Change-Id: I86dcd6b4cfbb061942cb73f6ffddcf9e712f355f
> Reviewed-on: https://chromium-review.googlesource.com/1012931
> Commit-Queue: Oksana Zhuravlova <oksamyt@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Jay Civelli <jcivelli@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#551470}

Bug: 646113, 799482
Change-Id: I54fdbc1cfd23f7065673a78c2e8e1376717baf62
Reviewed-on: https://chromium-review.googlesource.com/1026471Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Jay Civelli <jcivelli@chromium.org>
Commit-Queue: Oksana Zhuravlova <oksamyt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553800}

services/data_decoder/BUILD.gn

@@ -48,6 +48,7 @@ source_set("tests") {
   deps = [
     ":lib",
     "//base",
+    "//base/test:test_support",
     "//gin",
     "//gin:gin_test",
     "//services/data_decoder/public/cpp",

services/data_decoder/public/cpp/safe_xml_parser.cc

@@ -28,7 +28,7 @@ class SafeXmlParser {
   ~SafeXmlParser();
 
  private:
-  void ReportResults(std::unique_ptr<base::Value> parsed_json,
+  void ReportResults(base::Optional<base::Value> parsed_json,
                      const base::Optional<std::string>& error);
 
   XmlParserCallback callback_;
@@ -56,7 +56,7 @@ SafeXmlParser::SafeXmlParser(service_manager::Connector* connector,
   // Unretained(this) is safe as the xml_parser_ptr_ is owned by this class.
   xml_parser_ptr_.set_connection_error_handler(base::BindOnce(
       &SafeXmlParser::ReportResults, base::Unretained(this),
-      /*parsed_xml=*/nullptr,
+      /*parsed_xml=*/base::nullopt,
       base::make_optional(
           std::string("Connection error with the XML parser process."))));
   xml_parser_ptr_->Parse(
@@ -66,11 +66,14 @@ SafeXmlParser::SafeXmlParser(service_manager::Connector* connector,
 
 SafeXmlParser::~SafeXmlParser() = default;
 
-void SafeXmlParser::ReportResults(std::unique_ptr<base::Value> parsed_xml,
+void SafeXmlParser::ReportResults(base::Optional<base::Value> parsed_xml,
                                   const base::Optional<std::string>& error) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  std::move(callback_).Run(std::move(parsed_xml), error);
+  std::unique_ptr<base::Value> parsed_xml_ptr =
+      parsed_xml ? base::Value::ToUniquePtrValue(std::move(parsed_xml.value()))
+                 : nullptr;
+  std::move(callback_).Run(std::move(parsed_xml_ptr), error);
 
   // This should be the last interaction with this instance, safely delete.
   delete this;

services/data_decoder/public/cpp/safe_xml_parser_unittest.cc

@@ -7,6 +7,7 @@
 #include <memory>
 
 #include "base/bind.h"
+#include "base/test/bind_test_util.h"
 #include "base/values.h"
 #include "build/build_config.h"
 #include "services/data_decoder/xml_parser.h"
@@ -20,13 +21,17 @@ std::unique_ptr<base::Value> ParseXml(const std::string& xml) {
   XmlParser parser_impl(/*service_ref=*/nullptr);
   mojom::XmlParser& parser = parser_impl;
   std::unique_ptr<base::Value> root_node;
-  parser.Parse(xml, base::Bind(
-                        [](std::unique_ptr<base::Value>* node,
-                           std::unique_ptr<base::Value> parsed_root_node,
+
+  parser.Parse(xml,
+               base::BindLambdaForTesting(
+                   [&root_node](base::Optional<base::Value> parsed_root_node,
                                 const base::Optional<std::string>& error) {
-                          *node = std::move(parsed_root_node);
-                        },
-                        &root_node));
+                     root_node = parsed_root_node
+                                     ? base::Value::ToUniquePtrValue(
+                                           std::move(parsed_root_node.value()))
+                                     : nullptr;
+                   }));
+
   return root_node;
 }
 

services/data_decoder/public/mojom/xml_parser.mojom

@@ -10,7 +10,7 @@
 
 module data_decoder.mojom;
 
-import "mojo/common/values.mojom";
+import "mojo/public/mojom/base/values.mojom";
 
 interface XmlParser {
   const string kTypeKey = "type";
@@ -87,5 +87,5 @@ interface XmlParser {
   // Note that the client library provides convenience methods for accessing
   // data from the returned base::Value dictionary structure (see
   // safe_parser_xml.h).
-  Parse(string xml) => (mojo.common.mojom.Value? result, string? error);
+  Parse(string xml) => (mojo_base.mojom.Value? result, string? error);
 };

services/data_decoder/xml_parser.cc

@@ -19,7 +19,7 @@ using NamespaceMap = std::map<std::string, std::string>;
 namespace {
 
 void ReportError(XmlParser::ParseCallback callback, const std::string& error) {
-  std::move(callback).Run(/*result=*/nullptr, base::make_optional(error));
+  std::move(callback).Run(/*result=*/base::nullopt, base::make_optional(error));
 }
 
 enum class TextNodeType { kText, kCData };
@@ -178,8 +178,7 @@ void XmlParser::Parse(const std::string& xml, ParseCallback callback) {
     ReportError(std::move(callback), "Invalid XML: bad content");
     return;
   }
-  std::move(callback).Run(
-      base::Value::ToUniquePtrValue(std::move(root_element)),
+  std::move(callback).Run(base::make_optional(std::move(root_element)),
                           base::Optional<std::string>());
 }
 

services/data_decoder/xml_parser_fuzzer.cc

@@ -16,7 +16,7 @@
 namespace {
 
 void OnParseXml(base::Closure quit_loop,
-                std::unique_ptr<base::Value> value,
+                base::Optional<base::Value> value,
                 const base::Optional<std::string>& error) {
   std::move(quit_loop).Run();
 }

services/data_decoder/xml_parser_unittest.cc

@@ -18,9 +18,11 @@ namespace {
 
 void TestParseXmlCallback(std::unique_ptr<base::Value>* value_out,
                           base::Optional<std::string>* error_out,
-                          std::unique_ptr<base::Value> value,
+                          base::Optional<base::Value> value,
                           const base::Optional<std::string>& error) {
-  *value_out = std::move(value);
+  std::unique_ptr<base::Value> value_ptr =
+      value ? base::Value::ToUniquePtrValue(std::move(value.value())) : nullptr;
+  *value_out = std::move(value_ptr);
   *error_out = error;
 }