Moving the extension JSONRuleset unpacking to the browser process.

The JSON ruleset parsing step when unpacking an extension now happens
in the browser process using the safe JSON parser.

Also several changes from base::Bind to base::BindOnce.

Bug: 804379
Change-Id: Ie133b429a0ae101c368568b4ae73b940b07d1744
Reviewed-on: https://chromium-review.googlesource.com/879352
Commit-Queue: Jay Civelli <jcivelli@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531754}

extensions/browser/sandboxed_unpacker.h

@@ -14,6 +14,7 @@
 #include "base/memory/ref_counted_delete_on_sequence.h"
 #include "base/memory/weak_ptr.h"
 #include "base/optional.h"
+#include "base/strings/string_piece.h"
 #include "base/time/time.h"
 #include "content/public/browser/utility_process_mojo_client.h"
 #include "extensions/browser/crx_file_info.h"
@@ -240,31 +241,35 @@ class SandboxedUnpacker : public base::RefCountedThreadSafe<SandboxedUnpacker> {
   // Unpacks the extension in directory and returns the manifest.
   void Unpack(const base::FilePath& directory);
   void UnpackDone(const base::string16& error,
-                  std::unique_ptr<base::DictionaryValue> manifest,
-                  std::unique_ptr<base::ListValue> json_ruleset);
-  void UnpackExtensionSucceeded(std::unique_ptr<base::DictionaryValue> manifest,
-                                std::unique_ptr<base::ListValue> json_ruleset);
+                  std::unique_ptr<base::DictionaryValue> manifest);
+  void UnpackExtensionSucceeded(
+      std::unique_ptr<base::DictionaryValue> manifest);
   void UnpackExtensionFailed(const base::string16& error);
 
+  void ReportUnpackingError(base::StringPiece error);
+
   void ImageSanitizationDone(std::unique_ptr<base::DictionaryValue> manifest,
-                             std::unique_ptr<base::ListValue> json_ruleset,
                              ImageSanitizer::Status status,
                              const base::FilePath& path);
   void ImageSanitizerDecodedImage(const base::FilePath& path, SkBitmap image);
 
-  void ReadMessageCatalogs(std::unique_ptr<base::DictionaryValue> manifest,
-                           std::unique_ptr<base::ListValue> json_ruleset);
+  void ReadMessageCatalogs(std::unique_ptr<base::DictionaryValue> manifest);
 
   void SanitizeMessageCatalogs(
       std::unique_ptr<base::DictionaryValue> manifest,
-      std::unique_ptr<base::ListValue> json_ruleset,
       const std::set<base::FilePath>& message_catalog_paths);
 
   void MessageCatalogsSanitized(std::unique_ptr<base::DictionaryValue> manifest,
-                                std::unique_ptr<base::ListValue> json_ruleset,
                                 JsonFileSanitizer::Status status,
                                 const std::string& error_msg);
 
+  void ReadJSONRulesetIfNeeded(std::unique_ptr<base::DictionaryValue> manifest);
+  void ReadJSONRulesetDone(
+      std::unique_ptr<base::DictionaryValue> manifest,
+      data_decoder::mojom::JsonParserPtr json_parser_ptr_keep_alive,
+      std::unique_ptr<base::Value> json_ruleset,
+      const base::Optional<std::string>& error);
+
   // Reports unpack success or failure, or unzip failure.
   void ReportSuccess(std::unique_ptr<base::DictionaryValue> original_manifest,
                      const base::Optional<int>& dnr_ruleset_checksum);

extensions/common/extension_unpacker.mojom

@@ -34,8 +34,7 @@ interface ExtensionUnpacker {
          ManifestLocation location,
          int32 creation_flags)
       => (mojo.common.mojom.String16 error,
-          mojo.common.mojom.DictionaryValue? manifest,
-          mojo.common.mojom.ListValue? json_ruleset);
+          mojo.common.mojom.DictionaryValue? manifest);
 };
 
 [Native]

extensions/utility/unpacker.cc

@@ -7,7 +7,6 @@
 #include <stddef.h>
 
 #include <algorithm>
-#include <tuple>
 #include <utility>
 
 #include "base/files/file_util.h"
@@ -16,21 +15,16 @@
 #include "base/numerics/safe_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
-#include "base/threading/thread.h"
 #include "base/values.h"
-#include "extensions/common/api/declarative_net_request/dnr_manifest_data.h"
 #include "extensions/common/constants.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_l10n_util.h"
 #include "extensions/common/extension_utility_types.h"
-#include "extensions/common/extensions_client.h"
 #include "extensions/common/file_util.h"
 #include "extensions/common/manifest.h"
 #include "extensions/common/manifest_constants.h"
 #include "extensions/strings/grit/extensions_strings.h"
-#include "net/base/file_stream.h"
 #include "ui/base/l10n/l10n_util.h"
-#include "ui/gfx/geometry/size.h"
 
 namespace extensions {
 
@@ -135,39 +129,11 @@ bool Unpacker::Run() {
   }
   extension->AddInstallWarnings(warnings);
 
-  return ReadJSONRulesetIfNeeded(extension.get());
-}
-
-bool Unpacker::ReadJSONRulesetIfNeeded(const Extension* extension) {
-  const ExtensionResource* resource =
-      declarative_net_request::DNRManifestData::GetRulesetResource(extension);
-  // The extension did not provide a ruleset.
-  if (!resource)
-    return true;
-
-  std::string error;
-  JSONFileValueDeserializer deserializer(resource->GetFilePath());
-  std::unique_ptr<base::Value> root = deserializer.Deserialize(nullptr, &error);
-  if (!root) {
-    SetError(error);
-    return false;
-  }
-
-  if (!root->is_list()) {
-    SetError(errors::kDeclarativeNetRequestListNotPassed);
-    return false;
-  }
-
-  parsed_json_ruleset_ = base::ListValue::From(std::move(root));
   return true;
 }
 
 void Unpacker::SetError(const std::string& error) {
-  SetUTF16Error(base::UTF8ToUTF16(error));
-}
-
-void Unpacker::SetUTF16Error(const base::string16& error) {
-  error_message_ = error;
+  error_message_ = base::UTF8ToUTF16(error);
 }
 
 }  // namespace extensions

extensions/utility/unpacker.h

@@ -15,11 +15,9 @@
 
 namespace base {
 class DictionaryValue;
-class ListValue;
 }
 
 namespace extensions {
-class Extension;
 
 // This class unpacks an extension.  It is designed to be used in a sandboxed
 // child process.  We parse various bits of the extension, then report back to
@@ -57,16 +55,7 @@ class Unpacker {
     return std::move(parsed_manifest_);
   }
 
-  std::unique_ptr<base::ListValue> TakeParsedJSONRuleset() {
-    return std::move(parsed_json_ruleset_);
-  }
-
  private:
-  // Reads the Declarative Net Request API JSON ruleset for |extension| if it
-  // provided one. Returns false and populates |error_message_| in case of an
-  // error.
-  bool ReadJSONRulesetIfNeeded(const Extension* extension);
-
   // Write the decoded messages to kDecodedMessageCatalogsFilename.  We do this
   // instead of sending them over IPC, since they are so large.  Returns true on
   // success.
@@ -85,7 +74,6 @@ class Unpacker {
 
   // Set the error message.
   void SetError(const std::string& error);
-  void SetUTF16Error(const base::string16& error);
 
   // The directory to do work in.
   base::FilePath working_dir_;
@@ -105,10 +93,6 @@ class Unpacker {
   // The parsed version of the manifest JSON contained in the extension.
   std::unique_ptr<base::DictionaryValue> parsed_manifest_;
 
-  // The parsed version of the Declarative Net Request API ruleset. Null if the
-  // extension did not provide one.
-  std::unique_ptr<base::ListValue> parsed_json_ruleset_;
-
   // The last error message that was set.  Empty if there were no errors.
   base::string16 error_message_;
 

extensions/utility/utility_handler.cc

@@ -32,7 +32,6 @@ namespace {
 
 struct UnpackResult {
   std::unique_ptr<base::DictionaryValue> parsed_manifest;
-  std::unique_ptr<base::ListValue> parsed_json_ruleset;
   base::string16 error;
 };
 
@@ -51,7 +50,6 @@ UnpackResult UnpackOnBackgroundTaskRunner(const base::FilePath& path,
   UnpackResult result;
   if (unpacker.Run()) {
     result.parsed_manifest = unpacker.TakeParsedManifest();
-    result.parsed_json_ruleset = unpacker.TakeParsedJSONRuleset();
   } else {
     result.error = unpacker.error_message();
   }
@@ -117,8 +115,7 @@ class ExtensionUnpackerImpl : public extensions::mojom::ExtensionUnpacker {
         base::BindOnce(
             [](UnpackCallback callback, UnpackResult result) {
               std::move(callback).Run(result.error,
-                                      std::move(result.parsed_manifest),
-                                      std::move(result.parsed_json_ruleset));
+                                      std::move(result.parsed_manifest));
             },
             std::move(callback)));
   }