DNR: Show error for incorrect format of "rule_resources" key in manifest

This CL adds an error message to be shown when the DNR extension fails
to load because the "rule_resources" in the manifest is not specified as
a list of resource(s).

This CL also updates PopulateItem util functions by removing the
redundant checks to verify that the input is of dictionary type because
that is verified by the Populate function that they call.

Bug: 1105098

Change-Id: I94efca130f2c1937afd36e08f5efc4308dfbe761
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2297208
Commit-Queue: Utkarsh Patankar <utkpat@microsoft.com>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791325}

extensions/common/api/declarative_net_request/dnr_manifest_handler.cc

@@ -9,6 +9,7 @@
 #include "base/files/file_path.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece.h"
+#include "base/strings/utf_string_conversions.h"
 #include "extensions/common/api/declarative_net_request.h"
 #include "extensions/common/api/declarative_net_request/constants.h"
 #include "extensions/common/api/declarative_net_request/dnr_manifest_data.h"
@@ -63,6 +64,11 @@ bool DNRManifestHandler::Parse(Extension* extension, base::string16* error) {
   std::vector<dnr_api::Ruleset> rulesets;
   if (!json_schema_compiler::util::PopulateArrayFromList(*rules_file_list,
                                                          &rulesets, error)) {
+    DCHECK(!error->empty());
+    error->append(base::ASCIIToUTF16(". "));
+    error->append(ErrorUtils::FormatErrorMessageUTF16(
+        errors::kInvalidDeclarativeNetRequestKey,
+        keys::kDeclarativeRuleResourcesKey));
     return false;
   }
 

extensions/common/api/declarative_net_request/dnr_manifest_unittest.cc

@@ -141,6 +141,24 @@ TEST_F(DNRManifestTest, InvalidRulesFileKey) {
       keys::kDeclarativeRuleResourcesKey));
 }
 
+TEST_F(DNRManifestTest, InvalidRulesFileFormat) {
+  const char* kRulesetFile = "file1.json";
+  std::unique_ptr<base::DictionaryValue> manifest = CreateManifest({});
+  manifest->Set(keys::kDeclarativeNetRequestKey,
+                DictionaryBuilder()
+                    .Set(keys::kDeclarativeRuleResourcesKey,
+                         (ListBuilder().Append(
+                              std::make_unique<base::Value>(kRulesetFile)))
+                             .Build())
+                    .Build());
+
+  WriteManifestAndRuleset(*manifest, {});
+
+  LoadAndExpectError(
+      "expected dictionary, got string. Invalid value for 'rule_resources' "
+      "key");
+}
+
 TEST_F(DNRManifestTest, ZeroRulesets) {
   std::vector<TestRulesetInfo> no_rulesets;
   WriteManifestAndRuleset(*CreateManifest(no_rulesets), no_rulesets);

tools/json_schema_compiler/util.h

@@ -71,11 +71,8 @@ template <class T>
 bool PopulateItem(const base::Value& from,
                   std::unique_ptr<T>* out,
                   base::string16* error) {
-  const base::DictionaryValue* dict = nullptr;
-  if (!from.GetAsDictionary(&dict))
-    return false;
   std::unique_ptr<T> obj(new T());
-  if (!T::Populate(*dict, obj.get(), error))
+  if (!T::Populate(from, obj.get(), error))
     return false;
   *out = std::move(obj);
   return true;
@@ -85,11 +82,8 @@ bool PopulateItem(const base::Value& from,
 // error generation enabled.
 template <class T>
 bool PopulateItem(const base::Value& from, T* out, base::string16* error) {
-  const base::DictionaryValue* dict = nullptr;
-  if (!from.GetAsDictionary(&dict))
-    return false;
   T obj;
-  if (!T::Populate(*dict, &obj, error))
+  if (!T::Populate(from, &obj, error))
     return false;
   *out = std::move(obj);
   return true;