Convert json_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.

This change also modifies the union condition of DataView template to add
a check for is_null and pass a nullptr to Deserialize so that optional
values are handled correctly.

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

extensions/browser/json_file_sanitizer.cc

@@ -94,7 +94,7 @@ void JsonFileSanitizer::JsonFileRead(
 
 void JsonFileSanitizer::JsonParsingDone(
     const base::FilePath& file_path,
-    std::unique_ptr<base::Value> json_value,
+    base::Optional<base::Value> json_value,
     const base::Optional<std::string>& error) {
   if (!json_value || !json_value->is_dict()) {
     ReportError(Status::kDecodingError, error ? *error : std::string());

extensions/browser/json_file_sanitizer.h

@@ -77,7 +77,7 @@ class JsonFileSanitizer {
                     std::tuple<std::string, bool, bool> read_and_delete_result);
 
   void JsonParsingDone(const base::FilePath& file_path,
-                       std::unique_ptr<base::Value> json_value,
+                       base::Optional<base::Value> json_value,
                        const base::Optional<std::string>& error);
 
   void JsonFileWritten(const base::FilePath& file_path,

extensions/browser/sandboxed_unpacker.cc

@@ -428,7 +428,7 @@ void SandboxedUnpacker::Unpack(const base::FilePath& directory) {
 }
 
 void SandboxedUnpacker::ReadManifestDone(
-    std::unique_ptr<base::Value> manifest,
+    base::Optional<base::Value> manifest,
     const base::Optional<std::string>& error) {
   DCHECK(unpacker_io_task_runner_->RunsTasksInCurrentSequence());
   if (error) {
@@ -441,7 +441,8 @@ void SandboxedUnpacker::ReadManifestDone(
   }
 
   std::unique_ptr<base::DictionaryValue> manifest_dict =
-      base::DictionaryValue::From(std::move(manifest));
+      base::DictionaryValue::From(
+          base::Value::ToUniquePtrValue(std::move(manifest.value())));
 
   std::string error_msg;
   scoped_refptr<Extension> extension(
@@ -671,7 +672,8 @@ void SandboxedUnpacker::ReadJSONRulesetIfNeeded(
           extension_.get());
   if (!resource) {
     ReadJSONRulesetDone(std::move(manifest),
-                        /*json_ruleset=*/nullptr, /*error=*/base::nullopt);
+                        /*json_ruleset=*/base::nullopt,
+                        /*error=*/base::nullopt);
     return;
   }
 
@@ -682,7 +684,7 @@ void SandboxedUnpacker::ReadJSONRulesetIfNeeded(
 
 void SandboxedUnpacker::ReadJSONRulesetDone(
     std::unique_ptr<base::DictionaryValue> manifest,
-    std::unique_ptr<base::Value> json_ruleset,
+    base::Optional<base::Value> json_ruleset,
     const base::Optional<std::string>& error) {
   DCHECK(unpacker_io_task_runner_->RunsTasksInCurrentSequence());
 
@@ -698,8 +700,13 @@ void SandboxedUnpacker::ReadJSONRulesetDone(
 
   // Index and persist ruleset for the Declarative Net Request API.
   base::Optional<int> dnr_ruleset_checksum;
+  std::unique_ptr<base::Value> json_ruleset_ptr =
+      json_ruleset
+          ? base::Value::ToUniquePtrValue(std::move(json_ruleset.value()))
+          : nullptr;
+
   if (!IndexAndPersistRulesIfNeeded(
-          base::ListValue::From(std::move(json_ruleset)),
+          base::ListValue::From(std::move(json_ruleset_ptr)),
           &dnr_ruleset_checksum)) {
     return;  // Failure was already reported.
   }
@@ -1014,7 +1021,7 @@ void SandboxedUnpacker::ParseJsonFile(
   std::string contents;
   if (!base::ReadFileToString(path, &contents)) {
     std::move(callback).Run(
-        /*value=*/nullptr,
+        /*value=*/base::nullopt,
         /*error=*/base::Optional<std::string>("File doesn't exist."));
     return;
   }

extensions/browser/sandboxed_unpacker.h

@@ -16,6 +16,7 @@
 #include "base/optional.h"
 #include "base/strings/string_piece.h"
 #include "base/time/time.h"
+#include "base/values.h"
 #include "extensions/browser/crx_file_info.h"
 #include "extensions/browser/image_sanitizer.h"
 #include "extensions/browser/install/crx_install_error.h"
@@ -27,8 +28,6 @@
 class SkBitmap;
 
 namespace base {
-class DictionaryValue;
-class ListValue;
 class SequencedTaskRunner;
 }
 
@@ -231,7 +230,7 @@ class SandboxedUnpacker : public base::RefCountedThreadSafe<SandboxedUnpacker> {
 
   // Unpacks the extension in directory and returns the manifest.
   void Unpack(const base::FilePath& directory);
-  void ReadManifestDone(std::unique_ptr<base::Value> manifest,
+  void ReadManifestDone(base::Optional<base::Value> manifest,
                         const base::Optional<std::string>& error);
   void UnpackExtensionSucceeded(
       std::unique_ptr<base::DictionaryValue> manifest);
@@ -256,7 +255,7 @@ class SandboxedUnpacker : public base::RefCountedThreadSafe<SandboxedUnpacker> {
 
   void ReadJSONRulesetIfNeeded(std::unique_ptr<base::DictionaryValue> manifest);
   void ReadJSONRulesetDone(std::unique_ptr<base::DictionaryValue> manifest,
-                           std::unique_ptr<base::Value> json_ruleset,
+                           base::Optional<base::Value> json_ruleset,
                            const base::Optional<std::string>& error);
 
   // Reports unpack success or failure, or unzip failure.

mojo/public/tools/bindings/generators/cpp_templates/struct_data_view_declaration.tmpl

@@ -23,10 +23,10 @@ class {{struct.name}}DataView {
   template <typename UserType>
   WARN_UNUSED_RESULT bool Read{{name|under_to_camel}}(UserType* output) {
 {%-     if pf.min_version != 0 %}
-    auto* pointer = data_->header_.version >= {{pf.min_version}}
+    auto* pointer = data_->header_.version >= {{pf.min_version}} && !data_->{{name}}.is_null()
                     ? &data_->{{name}} : nullptr;
 {%-     else %}
-    auto* pointer = &data_->{{name}};
+    auto* pointer = !data_->{{name}}.is_null() ? &data_->{{name}} : nullptr;
 {%-     endif %}
     return mojo::internal::Deserialize<{{kind|unmapped_type_for_serializer}}>(
         pointer, output, context_);

services/data_decoder/json_parser_impl.cc

@@ -24,9 +24,11 @@ void JsonParserImpl::Parse(const std::string& json, ParseCallback callback) {
   std::unique_ptr<base::Value> value = base::JSONReader::ReadAndReturnError(
       json, base::JSON_PARSE_RFC, &error_code, &error);
   if (value) {
-    std::move(callback).Run(std::move(value), base::nullopt);
+    std::move(callback).Run(base::make_optional(std::move(*value)),
+                            base::nullopt);
   } else {
-    std::move(callback).Run(nullptr, base::make_optional(std::move(error)));
+    std::move(callback).Run(base::nullopt,
+                            base::make_optional(std::move(error)));
   }
 }
 

services/data_decoder/public/cpp/safe_json_parser_impl.cc

@@ -53,14 +53,17 @@ void SafeJsonParserImpl::OnConnectionError() {
   ReportResults(nullptr, "Connection error with the json parser process.");
 }
 
-void SafeJsonParserImpl::OnParseDone(std::unique_ptr<base::Value> result,
+void SafeJsonParserImpl::OnParseDone(base::Optional<base::Value> result,
                                      const base::Optional<std::string>& error) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   // Shut down the utility process.
   json_parser_ptr_.reset();
 
-  ReportResults(std::move(result), error.value_or(""));
+  std::unique_ptr<base::Value> result_ptr =
+      result ? base::Value::ToUniquePtrValue(std::move(result.value()))
+             : nullptr;
+  ReportResults(std::move(result_ptr), error.value_or(""));
 }
 
 void SafeJsonParserImpl::ReportResults(std::unique_ptr<base::Value> parsed_json,

services/data_decoder/public/cpp/safe_json_parser_impl.h

@@ -12,13 +12,10 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/threading/thread_checker.h"
+#include "base/values.h"
 #include "services/data_decoder/public/cpp/safe_json_parser.h"
 #include "services/data_decoder/public/mojom/json_parser.mojom.h"
 
-namespace base {
-class Value;
-}
-
 namespace service_manager {
 class Connector;
 }
@@ -42,7 +39,7 @@ class SafeJsonParserImpl : public SafeJsonParser {
   void OnConnectionError();
 
   // mojom::SafeJsonParser::Parse callback.
-  void OnParseDone(std::unique_ptr<base::Value> result,
+  void OnParseDone(base::Optional<base::Value> result,
                    const base::Optional<std::string>& error);
 
   // Reports the result on the calling task runner via the |success_callback_|

services/data_decoder/public/mojom/json_parser.mojom

@@ -4,8 +4,8 @@
 
 module data_decoder.mojom;
 
-import "mojo/common/values.mojom";
+import "mojo/public/mojom/base/values.mojom";
 
 interface JsonParser {
-  Parse(string json) => (mojo.common.mojom.Value? result, string? error);
+  Parse(string json) => (mojo_base.mojom.Value? result, string? error);
 };