Move the last extensions/common unit tests to extensions_unittests

They don't need to run in Chrome's unit_tests suite any more.

* Move ExtensionL10UtilTest
* Move FileUtilTest
* Keep browser action and page action tests in Chrome, since those are
  Chrome-specific concepts

This CL depends on https://codereview.chromium.org/565423003/ for cleanup
of _manifest_features.json

BUG=397165
TEST=extensions_unittests, unit_tests

Review URL: https://codereview.chromium.org/562273006

Cr-Commit-Position: refs/heads/master@{#295605}

chrome/chrome_tests_unit.gypi

@@ -11,12 +11,6 @@
       '../components/autofill/content/renderer/test_password_autofill_agent.h',
       '../components/autofill/content/renderer/test_password_generation_agent.cc',
       '../components/autofill/content/renderer/test_password_generation_agent.h',
-      # TODO(tfarina): Move these entries over to extensions_unittests target.
-      # http://crbug.com/348066. They are duplicated here because we haven't
-      # extensions_unittests running in the bots yet. Until that happens,
-      # they should be kept here.
-      '../extensions/common/extension_l10n_util_unittest.cc',
-      '../extensions/common/file_util_unittest.cc',
       # histograms.xml is analyzed by AboutFlagsHistogramTest, so this
       # dependency is needed to make commit bots run unit_tests on
       # histograms.xml changes.

chrome/common/extensions/extension_file_util_unittest.cc

@@ -5,6 +5,7 @@
 #include "chrome/common/extensions/extension_file_util.h"
 
 #include <set>
+#include <string>
 
 #include "base/path_service.h"
 #include "chrome/common/chrome_paths.h"
@@ -38,4 +39,35 @@ TEST_F(ExtensionFileUtilTest, GetBrowserImagePaths) {
   EXPECT_EQ("icon.png", paths.begin()->BaseName().AsUTF8Unsafe());
 }
 
+// Test that extensions with zero-length action icons will not load.
+TEST_F(ExtensionFileUtilTest, CheckZeroLengthActionIconFiles) {
+  base::FilePath install_dir;
+  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &install_dir));
+
+  // Try to install an extension with a zero-length browser action icon file.
+  base::FilePath ext_dir = install_dir.AppendASCII("extensions")
+                               .AppendASCII("bad")
+                               .AppendASCII("Extensions")
+                               .AppendASCII("gggggggggggggggggggggggggggggggg");
+
+  std::string error;
+  scoped_refptr<Extension> extension2(file_util::LoadExtension(
+      ext_dir, Manifest::UNPACKED, Extension::NO_FLAGS, &error));
+  EXPECT_FALSE(extension2.get());
+  EXPECT_STREQ("Could not load icon 'icon.png' for browser action.",
+               error.c_str());
+
+  // Try to install an extension with a zero-length page action icon file.
+  ext_dir = install_dir.AppendASCII("extensions")
+                .AppendASCII("bad")
+                .AppendASCII("Extensions")
+                .AppendASCII("hhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhh");
+
+  scoped_refptr<Extension> extension3(file_util::LoadExtension(
+      ext_dir, Manifest::UNPACKED, Extension::NO_FLAGS, &error));
+  EXPECT_FALSE(extension3.get());
+  EXPECT_STREQ("Could not load icon 'icon.png' for page action.",
+               error.c_str());
+}
+
 }  // namespace extensions

chrome/common/extensions/manifest_handlers/content_scripts_manifest_unittest.cc

@@ -3,11 +3,15 @@
 // found in the LICENSE file.
 
 #include "base/command_line.h"
+#include "base/files/file_path.h"
+#include "base/path_service.h"
 #include "base/strings/string_number_conversions.h"
+#include "chrome/common/chrome_paths.h"
 #include "chrome/common/extensions/manifest_handlers/content_scripts_handler.h"
 #include "chrome/common/extensions/manifest_tests/chrome_manifest_test.h"
 #include "extensions/common/error_utils.h"
 #include "extensions/common/extension.h"
+#include "extensions/common/file_util.h"
 #include "extensions/common/manifest_constants.h"
 #include "extensions/common/switches.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -83,4 +87,21 @@ TEST_F(ContentScriptsManifestTest, ContentScriptIds) {
   EXPECT_EQ(id + 1, user_scripts2[0].id());
 }
 
+TEST_F(ContentScriptsManifestTest, FailLoadingNonUTF8Scripts) {
+  base::FilePath install_dir;
+  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &install_dir));
+  install_dir = install_dir.AppendASCII("extensions")
+                    .AppendASCII("bad")
+                    .AppendASCII("bad_encoding");
+
+  std::string error;
+  scoped_refptr<Extension> extension(file_util::LoadExtension(
+      install_dir, Manifest::UNPACKED, Extension::NO_FLAGS, &error));
+  ASSERT_TRUE(extension.get() == NULL);
+  ASSERT_STREQ(
+      "Could not load file 'bad_encoding.js' for content script. "
+      "It isn't UTF-8 encoded.",
+      error.c_str());
+}
+
 }  // namespace extensions

chrome/test/data/extensions/bad/Extensions/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee/1.0/manifest.json

-{
-  "version": ""1.0",
-  "name": "My extension 3",
-  "content_scripts": [
-    {
-      "matches": ["http://*/*"]
-    }
-  ]
-}

chrome/test/data/extensions/bad/Extensions/ffffffffffffffffffffffffffffffff/manifest.json

-{
-  "name": "My extension with a zero-length icon",
-  "version": "1.0",
-  "icons": {
-    "16": "icon.png"
-  },
-  "manifest_version": 2
-}

extensions/BUILD.gn

@@ -87,8 +87,6 @@ source_set("test_support") {
     "test/test_extensions_client.h",
     "test/test_permission_message_provider.cc",
     "test/test_permission_message_provider.h",
-    "test/test_permissions_provider.cc",
-    "test/test_permissions_provider.h",
    ]
 
   deps = [
@@ -156,10 +154,6 @@ repack("shell_and_test_pak") {
 # does not have a reference to CreateNativeWebModalManager but it still links.
 # The GN build fails with this symbol being undefined.
 if (false) {
-# TODO(tfarina): Many extension unit tests run as part of Chrome"s
-# unit_tests target. They should be moved here, which may require some
-# refactoring (ExtensionsBrowserClient, TestingProfile, etc.).
-# http://crbug.com/348066
 test("unittests") {
   output_name = "extensions_unittests"
 
@@ -205,8 +199,10 @@ test("unittests") {
     "common/api/sockets/sockets_manifest_permission_unittest.cc",
     "common/csp_validator_unittest.cc",
     "common/event_filter_unittest.cc",
+    "common/extension_l10n_util_unittest.cc",
     "common/extension_resource_unittest.cc",
     "common/extension_set_unittest.cc",
+    "common/file_util_unittest.cc",
     "common/manifest_handler_unittest.cc",
     "common/manifest_handlers/shared_module_manifest_unittest.cc",
     "common/message_bundle_unittest.cc",

extensions/common/extension_l10n_util_unittest.cc

@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "extensions/common/extension_l10n_util.h"
+
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
@@ -10,22 +12,19 @@
 #include "base/path_service.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
-#include "chrome/common/chrome_paths.h"
 #include "extensions/common/constants.h"
 #include "extensions/common/error_utils.h"
-#include "extensions/common/extension_l10n_util.h"
+#include "extensions/common/extension_paths.h"
 #include "extensions/common/manifest_constants.h"
 #include "extensions/common/message_bundle.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/l10n/l10n_util.h"
 
-using extensions::kLocaleFolder;
-using extensions::kMessagesFilename;
-using extensions::MessageBundle;
+namespace extensions {
 
-namespace errors = extensions::manifest_errors;
-namespace keys = extensions::manifest_keys;
+namespace errors = manifest_errors;
+namespace keys = manifest_keys;
 
 namespace {
 
@@ -108,13 +107,9 @@ TEST(ExtensionL10nUtil, GetValidLocalesWithUnsupportedLocale) {
 
 TEST(ExtensionL10nUtil, GetValidLocalesWithValidLocalesAndMessagesFile) {
   base::FilePath install_dir;
-  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &install_dir));
-  install_dir = install_dir.AppendASCII("extensions")
-                    .AppendASCII("good")
-                    .AppendASCII("Extensions")
-                    .AppendASCII("behllobkkfkfnphdnhnkndlbkcpglgmj")
-                    .AppendASCII("1.0.0.0")
-                    .Append(kLocaleFolder);
+  ASSERT_TRUE(PathService::Get(DIR_TEST_DATA, &install_dir));
+  install_dir =
+      install_dir.AppendASCII("extension_with_locales").Append(kLocaleFolder);
 
   std::string error;
   std::set<std::string> locales;
@@ -128,13 +123,9 @@ TEST(ExtensionL10nUtil, GetValidLocalesWithValidLocalesAndMessagesFile) {
 
 TEST(ExtensionL10nUtil, LoadMessageCatalogsValidFallback) {
   base::FilePath install_dir;
-  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &install_dir));
-  install_dir = install_dir.AppendASCII("extensions")
-                    .AppendASCII("good")
-                    .AppendASCII("Extensions")
-                    .AppendASCII("behllobkkfkfnphdnhnkndlbkcpglgmj")
-                    .AppendASCII("1.0.0.0")
-                    .Append(kLocaleFolder);
+  ASSERT_TRUE(PathService::Get(DIR_TEST_DATA, &install_dir));
+  install_dir =
+      install_dir.AppendASCII("extension_with_locales").Append(kLocaleFolder);
 
   std::string error;
   std::set<std::string> locales;
@@ -185,7 +176,7 @@ TEST(ExtensionL10nUtil, LoadMessageCatalogsBadJSONFormat) {
   std::string error;
   EXPECT_TRUE(NULL == extension_l10n_util::LoadMessageCatalogs(
                           src_path, "en_US", "sr", valid_locales, &error));
-  EXPECT_EQ(extensions::ErrorUtils::FormatErrorMessage(
+  EXPECT_EQ(ErrorUtils::FormatErrorMessage(
                 errors::kLocalesInvalidLocale,
                 base::UTF16ToUTF8(messages_file.LossyDisplayName()),
                 "Line: 1, column: 10, Unexpected token."),
@@ -660,3 +651,4 @@ TEST(ExtensionL10nUtil, GetAllFallbackLocales) {
 }
 
 }  // namespace
+}  // namespace extensions

extensions/common/file_util_unittest.cc

@@ -11,9 +11,9 @@
 #include "base/path_service.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
-#include "chrome/common/chrome_paths.h"
 #include "extensions/common/constants.h"
 #include "extensions/common/extension.h"
+#include "extensions/common/extension_paths.h"
 #include "extensions/common/manifest.h"
 #include "extensions/common/manifest_constants.h"
 #include "grit/extensions_strings.h"
@@ -116,12 +116,8 @@ TEST_F(FileUtilTest, InstallUninstallGarbageCollect) {
 
 TEST_F(FileUtilTest, LoadExtensionWithValidLocales) {
   base::FilePath install_dir;
-  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &install_dir));
-  install_dir = install_dir.AppendASCII("extensions")
-                    .AppendASCII("good")
-                    .AppendASCII("Extensions")
-                    .AppendASCII("behllobkkfkfnphdnhnkndlbkcpglgmj")
-                    .AppendASCII("1.0.0.0");
+  ASSERT_TRUE(PathService::Get(DIR_TEST_DATA, &install_dir));
+  install_dir = install_dir.AppendASCII("extension_with_locales");
 
   std::string error;
   scoped_refptr<Extension> extension(file_util::LoadExtension(
@@ -132,12 +128,8 @@ TEST_F(FileUtilTest, LoadExtensionWithValidLocales) {
 
 TEST_F(FileUtilTest, LoadExtensionWithoutLocalesFolder) {
   base::FilePath install_dir;
-  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &install_dir));
-  install_dir = install_dir.AppendASCII("extensions")
-                    .AppendASCII("good")
-                    .AppendASCII("Extensions")
-                    .AppendASCII("bjafgdebaacbbbecmhlhpofkepfkgcpa")
-                    .AppendASCII("1.0");
+  ASSERT_TRUE(PathService::Get(DIR_TEST_DATA, &install_dir));
+  install_dir = install_dir.AppendASCII("extension_without_locales");
 
   std::string error;
   scoped_refptr<Extension> extension(file_util::LoadExtension(
@@ -192,12 +184,9 @@ TEST_F(FileUtilTest, CheckIllegalFilenamesReservedAndIllegal) {
 
 TEST_F(FileUtilTest, LoadExtensionGivesHelpfullErrorOnMissingManifest) {
   base::FilePath install_dir;
-  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &install_dir));
-  install_dir = install_dir.AppendASCII("extensions")
-                    .AppendASCII("bad")
-                    .AppendASCII("Extensions")
-                    .AppendASCII("dddddddddddddddddddddddddddddddd")
-                    .AppendASCII("1.0");
+  ASSERT_TRUE(PathService::Get(DIR_TEST_DATA, &install_dir));
+  install_dir =
+      install_dir.AppendASCII("file_util").AppendASCII("missing_manifest");
 
   std::string error;
   scoped_refptr<Extension> extension(file_util::LoadExtension(
@@ -209,12 +198,9 @@ TEST_F(FileUtilTest, LoadExtensionGivesHelpfullErrorOnMissingManifest) {
 
 TEST_F(FileUtilTest, LoadExtensionGivesHelpfullErrorOnBadManifest) {
   base::FilePath install_dir;
-  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &install_dir));
-  install_dir = install_dir.AppendASCII("extensions")
-                    .AppendASCII("bad")
-                    .AppendASCII("Extensions")
-                    .AppendASCII("eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee")
-                    .AppendASCII("1.0");
+  ASSERT_TRUE(PathService::Get(DIR_TEST_DATA, &install_dir));
+  install_dir =
+      install_dir.AppendASCII("file_util").AppendASCII("bad_manifest");
 
   std::string error;
   scoped_refptr<Extension> extension(file_util::LoadExtension(
@@ -227,23 +213,6 @@ TEST_F(FileUtilTest, LoadExtensionGivesHelpfullErrorOnBadManifest) {
       error.c_str());
 }
 
-TEST_F(FileUtilTest, FailLoadingNonUTF8Scripts) {
-  base::FilePath install_dir;
-  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &install_dir));
-  install_dir = install_dir.AppendASCII("extensions")
-                    .AppendASCII("bad")
-                    .AppendASCII("bad_encoding");
-
-  std::string error;
-  scoped_refptr<Extension> extension(file_util::LoadExtension(
-      install_dir, Manifest::UNPACKED, Extension::NO_FLAGS, &error));
-  ASSERT_TRUE(extension.get() == NULL);
-  ASSERT_STREQ(
-      "Could not load file 'bad_encoding.js' for content script. "
-      "It isn't UTF-8 encoded.",
-      error.c_str());
-}
-
 TEST_F(FileUtilTest, ValidateThemeUTF8) {
   base::ScopedTempDir temp;
   ASSERT_TRUE(temp.CreateUniqueTempDir());
@@ -409,45 +378,19 @@ TEST_F(FileUtilTest, WarnOnPrivateKey) {
                   "extension includes the key file.*ext_root.a_key.pem"));
 }
 
-TEST_F(FileUtilTest, CheckZeroLengthImageFile) {
+TEST_F(FileUtilTest, CheckZeroLengthIconFile) {
   base::FilePath install_dir;
-  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &install_dir));
+  ASSERT_TRUE(PathService::Get(DIR_TEST_DATA, &install_dir));
 
   // Try to install an extension with a zero-length icon file.
-  base::FilePath ext_dir = install_dir.AppendASCII("extensions")
-                               .AppendASCII("bad")
-                               .AppendASCII("Extensions")
-                               .AppendASCII("ffffffffffffffffffffffffffffffff");
+  base::FilePath ext_dir =
+      install_dir.AppendASCII("file_util").AppendASCII("bad_icon");
 
   std::string error;
   scoped_refptr<Extension> extension(file_util::LoadExtension(
       ext_dir, Manifest::UNPACKED, Extension::NO_FLAGS, &error));
   EXPECT_TRUE(extension.get() == NULL);
   EXPECT_STREQ("Could not load extension icon 'icon.png'.", error.c_str());
-
-  // Try to install an extension with a zero-length browser action icon file.
-  ext_dir = install_dir.AppendASCII("extensions")
-                .AppendASCII("bad")
-                .AppendASCII("Extensions")
-                .AppendASCII("gggggggggggggggggggggggggggggggg");
-
-  scoped_refptr<Extension> extension2(file_util::LoadExtension(
-      ext_dir, Manifest::UNPACKED, Extension::NO_FLAGS, &error));
-  EXPECT_TRUE(extension2.get() == NULL);
-  EXPECT_STREQ("Could not load icon 'icon.png' for browser action.",
-               error.c_str());
-
-  // Try to install an extension with a zero-length page action icon file.
-  ext_dir = install_dir.AppendASCII("extensions")
-                .AppendASCII("bad")
-                .AppendASCII("Extensions")
-                .AppendASCII("hhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhh");
-
-  scoped_refptr<Extension> extension3(file_util::LoadExtension(
-      ext_dir, Manifest::UNPACKED, Extension::NO_FLAGS, &error));
-  EXPECT_TRUE(extension3.get() == NULL);
-  EXPECT_STREQ("Could not load icon 'icon.png' for page action.",
-               error.c_str());
 }
 
 TEST_F(FileUtilTest, ExtensionURLToRelativeFilePath) {

extensions/extensions.gyp

@@ -947,8 +947,6 @@
         'test/test_extensions_client.h',
         'test/test_permission_message_provider.cc',
         'test/test_permission_message_provider.h',
-        'test/test_permissions_provider.cc',
-        'test/test_permissions_provider.h',
       ],
       # Disable c4267 warnings until we fix size_t to int truncations.
       'msvs_disabled_warnings': [ 4267, ],
@@ -1004,10 +1002,6 @@
       ],
     },
     {
-      # TODO(tfarina): Many extension unit tests run as part of Chrome's
-      # unit_tests target. They should be moved here, which may require some
-      # refactoring (ExtensionsBrowserClient, TestingProfile, etc.).
-      # http://crbug.com/348066
       'target_name': 'extensions_unittests',
       'type': 'executable',
       'dependencies': [
@@ -1089,11 +1083,13 @@
         'common/api/sockets/sockets_manifest_permission_unittest.cc',
         'common/csp_validator_unittest.cc',
         'common/event_filter_unittest.cc',
+        'common/extension_l10n_util_unittest.cc',
         'common/extension_resource_unittest.cc',
         'common/extension_set_unittest.cc',
         'common/features/base_feature_provider_unittest.cc',
         'common/features/complex_feature_unittest.cc',
         'common/features/simple_feature_unittest.cc',
+        'common/file_util_unittest.cc',
         'common/manifest_handler_unittest.cc',
         'common/manifest_handlers/externally_connectable_unittest.cc',
         'common/manifest_handlers/file_handler_manifest_unittest.cc',

extensions/test/data/extension_with_locales/_locales/en/messages.json

+{
+  "chrome_extension_name": {
+    "message": "My extension 1"
+  },
+  "chrome_extension_description": {
+    "message": "The first extension that I made."
+  },
+  "color": {
+    "message": "Colour"
+  },
+  "not_in_US_or_GB": {
+    "message": "Not in the US or GB."
+  }
+}

extensions/test/data/extension_with_locales/_locales/en_US/messages.json

+{
+  "chrome_extension_name": {
+    "message": "My extension 1"
+  },
+  "chrome_extension_description": {
+    "message": "The first extension that I made."
+  },
+  "color": {
+    "message": "Color"
+  }
+}

extensions/test/data/extension_with_locales/_locales/sr/messages.json

+{
+  "chrome_extension_name": {
+    "message": "Моја порука 1"
+  },
+  "chrome_extension_description": {
+    "message": "Прва екстензија коју сам написао."
+  }
+}

extensions/test/data/extension_with_locales/manifest.json

+{
+  "version": "1.0.0.0",
+  "manifest_version": 2,
+  "name": "__MSG_chrome_extension_name__",
+  "description": "__MSG_chrome_extension_description__",
+  "default_locale": "en_US"
+}

extensions/test/data/extension_without_locales/manifest.json

+{
+  "version": "1.0.0.0",
+  "manifest_version": 2,
+  "name": "Simple extension without locales",
+  "description": "extensions_unittests --gtest_filter=FileUtilTest.LoadExtensionWithoutLocalesFolder"
+}

extensions/test/data/file_util/bad_manifest/manifest.json

+{
+  "version": """,
+  "name": "extra quote above"
+}

extensions/test/data/file_util/missing_manifest/README

+This directory simulates an extension with a missing manifest.json file.

extensions/test/test_permissions_provider.cc

-// Copyright 2014 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "extensions/test/test_permissions_provider.h"
-
-namespace extensions {
-
-TestPermissionsProvider::TestPermissionsProvider() {}
-
-TestPermissionsProvider::~TestPermissionsProvider() {}
-
-std::vector<APIPermissionInfo*> TestPermissionsProvider::GetAllPermissions()
-    const {
-  // TODO(tfarina): This needs to have a "real" implementation, otherwise
-  // some tests under extensions_unittests will fail. http://crbug.com/348066
-  std::vector<APIPermissionInfo*> permissions;
-  return permissions;
-}
-
-std::vector<PermissionsProvider::AliasInfo>
-TestPermissionsProvider::GetAllAliases() const {
-  std::vector<PermissionsProvider::AliasInfo> aliases;
-  return aliases;
-}
-
-}  // namespace extensions

extensions/test/test_permissions_provider.h

-// Copyright 2014 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef EXTENSIONS_TEST_TEST_PERMISSIONS_PROVIDER_H_
-#define EXTENSIONS_TEST_TEST_PERMISSIONS_PROVIDER_H_
-
-#include "base/macros.h"
-#include "extensions/common/permissions/permissions_provider.h"
-
-namespace extensions {
-
-class TestPermissionsProvider : public PermissionsProvider {
- public:
-  TestPermissionsProvider();
-  virtual ~TestPermissionsProvider();
-
- private:
-  // PermissionsProvider:
-  virtual std::vector<APIPermissionInfo*> GetAllPermissions() const OVERRIDE;
-  virtual std::vector<AliasInfo> GetAllAliases() const OVERRIDE;
-
-  DISALLOW_COPY_AND_ASSIGN(TestPermissionsProvider);
-};
-
-}  // namespace extensions
-
-#endif  // EXTENSIONS_TEST_TEST_PERMISSIONS_PROVIDER_H_