[Extensions] Add unittests for action parsing in the manifest

Add some basic unit tests for parsing all extension action types
("browser_action", "page_action", and "action") in the manifest.

Bug: 893373
Change-Id: I8484377858ca5c837eb649a16c473ed0f0f91140
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1562615
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#650408}

chrome/common/extensions/api/extension_action/action_info.cc

@@ -134,6 +134,12 @@ std::unique_ptr<ActionInfo> ActionInfo::Load(const Extension* extension,
   return result;
 }
 
+// static
+const ActionInfo* ActionInfo::GetExtensionActionInfo(
+    const Extension* extension) {
+  return GetActionInfo(extension, keys::kAction);
+}
+
 // static
 const ActionInfo* ActionInfo::GetBrowserActionInfo(const Extension* extension) {
   return GetActionInfo(extension, keys::kBrowserAction);

chrome/common/extensions/api/extension_action/action_info.h

@@ -43,6 +43,9 @@ struct ActionInfo {
                                           const base::DictionaryValue* dict,
                                           base::string16* error);
 
+  // Returns the action specified under the "action" key, if any.
+  static const ActionInfo* GetExtensionActionInfo(const Extension* extension);
+
   // Returns the extension's browser action, if any.
   static const ActionInfo* GetBrowserActionInfo(const Extension* extension);
 

chrome/common/extensions/manifest_handlers/extension_action_handler_unittest.cc

@@ -6,14 +6,33 @@
 
 #include "base/files/file_path.h"
 #include "base/path_service.h"
+#include "base/strings/strcat.h"
+#include "base/strings/stringprintf.h"
+#include "base/test/values_test_util.h"
+#include "base/values.h"
 #include "chrome/common/chrome_paths.h"
+#include "chrome/common/extensions/api/extension_action/action_info.h"
+#include "components/version_info/channel.h"
+#include "extensions/common/constants.h"
 #include "extensions/common/extension.h"
+#include "extensions/common/extension_icon_set.h"
+#include "extensions/common/features/feature_channel.h"
 #include "extensions/common/file_util.h"
 #include "extensions/common/manifest.h"
+#include "extensions/common/manifest_constants.h"
+#include "extensions/common/manifest_test.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+namespace extensions {
+
 namespace {
 
+enum class TestActionType {
+  kBrowserAction,
+  kPageAction,
+  kAction,
+};
+
 base::FilePath GetTestDataDir() {
   base::FilePath path;
   base::PathService::Get(chrome::DIR_TEST_DATA, &path);
@@ -22,8 +41,6 @@ base::FilePath GetTestDataDir() {
 
 }  // namespace
 
-namespace extensions {
-
 // Tests that an unpacked extension with an invisible browser action
 // default icon fails as expected.
 TEST(ExtensionActionHandlerTest, LoadInvisibleBrowserActionIconUnpacked) {
@@ -56,4 +73,181 @@ TEST(ExtensionActionHandlerTest, LoadInvisiblePageActionIconUnpacked) {
             error);
 }
 
+// A parameterized test suite to test each different extension action key
+// ("page_action", "browser_action", "action").
+class ExtensionActionManifestTest
+    : public ManifestTest,
+      public testing::WithParamInterface<TestActionType> {
+ public:
+  ExtensionActionManifestTest() {}
+  ~ExtensionActionManifestTest() override {}
+
+  // Constructs and returns a ManifestData object with the provided
+  // |action_spec|.
+  ManifestData GetManifestData(const char* action_spec) {
+    constexpr char kManifestStub[] =
+        R"({
+             "name": "Test",
+             "manifest_version": 2,
+             "version": "0.1",
+             "%s": %s
+           })";
+
+    const char* action_key = nullptr;
+    switch (GetParam()) {
+      case TestActionType::kBrowserAction:
+        action_key = manifest_keys::kBrowserAction;
+        break;
+      case TestActionType::kPageAction:
+        action_key = manifest_keys::kPageAction;
+        break;
+      case TestActionType::kAction:
+        action_key = manifest_keys::kAction;
+        break;
+    }
+
+    base::Value manifest_value = base::test::ParseJson(
+        base::StringPrintf(kManifestStub, action_key, action_spec));
+    EXPECT_TRUE(manifest_value.is_dict());
+    EXPECT_FALSE(manifest_value.is_none());
+    return ManifestData(std::move(manifest_value), "test");
+  }
+
+  // Returns the ActionInfo for the given |extension| corresponding with the
+  // action key of the test param.
+  const ActionInfo* GetActionInfo(const Extension& extension) {
+    const ActionInfo* action_info = nullptr;
+    switch (GetParam()) {
+      case TestActionType::kBrowserAction:
+        action_info = ActionInfo::GetBrowserActionInfo(&extension);
+        break;
+      case TestActionType::kPageAction:
+        action_info = ActionInfo::GetPageActionInfo(&extension);
+        break;
+      case TestActionType::kAction:
+        action_info = ActionInfo::GetExtensionActionInfo(&extension);
+        break;
+    }
+
+    return action_info;
+  }
+
+ private:
+  // The "action" key is restricted to trunk.
+  ScopedCurrentChannel scoped_channel_{version_info::Channel::UNKNOWN};
+
+  DISALLOW_COPY_AND_ASSIGN(ExtensionActionManifestTest);
+};
+
+// Tests that parsing an action succeeds and properly populates the given
+// fields.
+TEST_P(ExtensionActionManifestTest, Basic) {
+  constexpr char kValidAllFields[] =
+      R"({
+           "default_popup": "popup.html",
+           "default_title": "Title",
+           "default_icon": "icon.png"
+         })";
+  scoped_refptr<const Extension> extension =
+      LoadAndExpectSuccess(GetManifestData(kValidAllFields));
+  ASSERT_TRUE(extension);
+  const ActionInfo* action_info = GetActionInfo(*extension);
+  ASSERT_TRUE(action_info);
+
+  EXPECT_EQ(extension->GetResourceURL("popup.html"),
+            action_info->default_popup_url);
+  EXPECT_EQ("Title", action_info->default_title);
+  // Make a copy of the map since [] is more readable than find() for comparing
+  // values.
+  ExtensionIconSet::IconMap icons = action_info->default_icon.map();
+  EXPECT_EQ(1u, icons.size());
+  EXPECT_EQ(icons[extension_misc::EXTENSION_ICON_GIGANTOR], "icon.png");
+}
+
+// Tests that specifying an empty action (e.g., "action": {}) works correctly,
+// with empty defaults.
+TEST_P(ExtensionActionManifestTest, TestEmptyAction) {
+  constexpr char kValidNoFields[] = "{}";
+  scoped_refptr<const Extension> extension =
+      LoadAndExpectSuccess(GetManifestData(kValidNoFields));
+  ASSERT_TRUE(extension);
+  const ActionInfo* action_info = GetActionInfo(*extension);
+  ASSERT_TRUE(action_info);
+
+  EXPECT_EQ(GURL(), action_info->default_popup_url);
+  EXPECT_EQ(std::string(), action_info->default_title);
+  const ExtensionIconSet::IconMap& icons = action_info->default_icon.map();
+  EXPECT_TRUE(icons.empty());
+}
+
+// Tests specifying an icon dictionary (with different pixel sizes) in the
+// action.
+TEST_P(ExtensionActionManifestTest, ValidIconDictionary) {
+  constexpr char kValidIconDictionary[] =
+      R"({
+           "default_icon": {
+             "24": "icon24.png",
+             "48": "icon48.png",
+             "79": "icon79.png"
+           }
+         })";
+
+  scoped_refptr<const Extension> extension =
+      LoadAndExpectSuccess(GetManifestData(kValidIconDictionary));
+  ASSERT_TRUE(extension);
+  const ActionInfo* action_info = GetActionInfo(*extension);
+  ASSERT_TRUE(action_info);
+
+  EXPECT_EQ(GURL(), action_info->default_popup_url);
+  EXPECT_EQ(std::string(), action_info->default_title);
+  // Make a copy of the map since [] is more readable than find() for comparing
+  // values.
+  ExtensionIconSet::IconMap icons = action_info->default_icon.map();
+  EXPECT_EQ(3u, icons.size());
+  EXPECT_EQ("icon24.png", icons[24]);
+  EXPECT_EQ("icon48.png", icons[48]);
+  EXPECT_EQ("icon79.png", icons[79]);
+}
+
+// Tests some invalid cases.
+TEST_P(ExtensionActionManifestTest, Invalid) {
+  constexpr char kInvalidTopLevel1[] = "[]";
+  constexpr char kInvalidTopLevel2[] = "\"foo\"";
+  constexpr char kInvalidTopLevel3[] = "17";
+
+  const char* expected_error = nullptr;
+  switch (GetParam()) {
+    case TestActionType::kBrowserAction:
+      expected_error = manifest_errors::kInvalidBrowserAction;
+      break;
+    case TestActionType::kPageAction:
+      expected_error = manifest_errors::kInvalidPageAction;
+      break;
+    case TestActionType::kAction:
+      expected_error = manifest_errors::kInvalidAction;
+      break;
+  }
+
+  for (const char* spec :
+       {kInvalidTopLevel1, kInvalidTopLevel2, kInvalidTopLevel3}) {
+    LoadAndExpectError(GetManifestData(spec), expected_error);
+  }
+
+  constexpr char kInvalidPopup[] = R"({ "default_popup": {} })";
+  LoadAndExpectError(GetManifestData(kInvalidPopup),
+                     manifest_errors::kInvalidActionDefaultPopup);
+  constexpr char kInvalidTitle[] = R"({ "default_title": {} })";
+  LoadAndExpectError(GetManifestData(kInvalidTitle),
+                     manifest_errors::kInvalidActionDefaultTitle);
+  constexpr char kInvalidIcon[] = R"({ "default_icon": [] })";
+  LoadAndExpectError(GetManifestData(kInvalidIcon),
+                     manifest_errors::kInvalidActionDefaultIcon);
+}
+
+INSTANTIATE_TEST_SUITE_P(,
+                         ExtensionActionManifestTest,
+                         testing::Values(TestActionType::kBrowserAction,
+                                         TestActionType::kPageAction,
+                                         TestActionType::kAction));
+
 }  // namespace extensions