Move unpacker_unittests.cc and data files from chrome/ to extensions/

Most of the changes here are just moves, with only a few more notable
changes:

Add "manifest_version : 2" and change from theme to extension
-extensions/test/data/unpacker/bad_zip/manifest.json
-extensions/test/data/unpacker/bad_image/manifest.json
-extensions/test/data/unpacker/good_package/manifest.json

Changed BadPathError test and misc cleanup
-extensions/utility/unpacker_unittest.cc

Support for changes to the above mentioned BadPathError test
-extensions/test/test_extensions_client.h
-extensions/test/test_extensions_client.cc

Entire directory just deleted, not moved
-chrome/test/data/extensions/unpacker/bad_path/

BUG=447014

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

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

chrome/browser/extensions/sandboxed_unpacker_unittest.cc

@@ -16,6 +16,7 @@
 #include "content/public/test/test_utils.h"
 #include "extensions/common/constants.h"
 #include "extensions/common/extension.h"
+#include "extensions/common/extension_paths.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 
@@ -75,10 +76,8 @@ class SandboxedUnpackerTest : public testing::Test {
 
   void SetupUnpacker(const std::string& crx_name) {
     base::FilePath original_path;
-    ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &original_path));
-    original_path = original_path.AppendASCII("extensions")
-        .AppendASCII("unpacker")
-        .AppendASCII(crx_name);
+    ASSERT_TRUE(PathService::Get(extensions::DIR_TEST_DATA, &original_path));
+    original_path = original_path.AppendASCII("unpacker").AppendASCII(crx_name);
     ASSERT_TRUE(base::PathExists(original_path)) << original_path.value();
 
     sandboxed_unpacker_ = new SandboxedUnpacker(

chrome/chrome_tests_unit.gypi

@@ -926,7 +926,6 @@
       'renderer/extensions/extension_localization_peer_unittest.cc',
       'renderer/extensions/renderer_permissions_policy_delegate_unittest.cc',
       'renderer/media/cast_ipc_dispatcher_unittest.cc',
-      'utility/extensions/unpacker_unittest.cc',
       'utility/image_writer/image_writer_unittest.cc',
       'utility/media_galleries/image_metadata_extractor_unittest.cc',
     ],

extensions/BUILD.gn

@@ -255,6 +255,7 @@ if (false) {
       "renderer/script_context_unittest.cc",
       "renderer/utils_unittest.cc",
       "test/extensions_unittests_main.cc",
+      "utility/unpacker_unittest.cc",
     ]
 
     deps = [

extensions/extensions.gyp

@@ -1304,6 +1304,7 @@
         'renderer/script_context_unittest.cc',
         'renderer/utils_unittest.cc',
         'test/extensions_unittests_main.cc',
+        'utility/unpacker_unittest.cc',
       ],
       # Disable c4267 warnings until we fix size_t to int truncations.
       'msvs_disabled_warnings': [ 4267, ],

chrome/test/data/extensions/unpacker/bad_image/manifest.json → extensions/test/data/unpacker/bad_image/manifest.json

@@ -1,6 +1,7 @@
 {
   "name": "Bad Image",
   "version": "1.0",
+  "manifest_version": 2,
   "description": "Package with a bad image to be used for unit testing.",
-  "theme": {"images" : {"bad" : "not_really_an_image.png"}}
+  "icons": { "128":  "not_really_an_image.png" }
 }

chrome/test/data/extensions/unpacker/bad_zip/manifest.json → extensions/test/data/unpacker/bad_zip/manifest.json

@@ -1,6 +1,9 @@
 {
   "name": "Bad ZIP",
+  "manifest_version": 2,
   "version": "1.0",
   "description": "Good package to be mangled by bad_zip.sh for unit testing.",
-  "theme": {"images" : {"logo" : "product_logo_128.png"}}
+  "icons": {
+    "128" : "product_logo_128.png"
+  }
 }

chrome/test/data/extensions/unpacker/good_package/manifest.json → extensions/test/data/unpacker/good_package/manifest.json

@@ -1,6 +1,9 @@
 {
   "name": "Good Package",
+  "manifest_version": 2,
   "version": "1.0",
   "description": "Good (valid) package to be used for unit testing.",
-  "theme": {"images" : {"logo" : "product_logo_128.png"}}
+  "icons": {
+    "128": "product_logo_128.png"
+  }
 }

extensions/test/test_extensions_client.cc

@@ -4,6 +4,7 @@
 
 #include "extensions/test/test_extensions_client.h"
 
+#include "base/stl_util.h"
 #include "extensions/common/api/generated_schemas.h"
 #include "extensions/common/common_manifest_handlers.h"
 #include "extensions/common/extension_urls.h"
@@ -38,6 +39,16 @@ TestExtensionsClient::TestExtensionsClient() {
 TestExtensionsClient::~TestExtensionsClient() {
 }
 
+void TestExtensionsClient::AddBrowserImagePathsFilter(
+    BrowserImagePathsFilter* filter) {
+  browser_image_filters_.insert(filter);
+}
+
+void TestExtensionsClient::RemoveBrowserImagePathsFilter(
+    BrowserImagePathsFilter* filter) {
+  browser_image_filters_.erase(filter);
+}
+
 void TestExtensionsClient::Initialize() {
   // Registration could already be finalized in unit tests, where the utility
   // thread runs in-process.
@@ -167,4 +178,13 @@ bool TestExtensionsClient::IsBlacklistUpdateURL(const GURL& url) const {
   return true;
 }
 
+std::set<base::FilePath> TestExtensionsClient::GetBrowserImagePaths(
+    const Extension* extension) {
+  std::set<base::FilePath> result =
+      ExtensionsClient::GetBrowserImagePaths(extension);
+  for (auto filter : browser_image_filters_)
+    filter->Filter(extension, &result);
+  return result;
+}
+
 }  // namespace extensions

extensions/test/test_extensions_client.h

@@ -12,9 +12,20 @@ namespace extensions {
 
 class TestExtensionsClient : public ExtensionsClient {
  public:
+  // An interface that lets tests change the set of image paths before they are
+  // returned by TestExtensionClient::GetBrowserImagePaths.
+  class BrowserImagePathsFilter {
+   public:
+    virtual void Filter(const Extension* extension,
+                        std::set<base::FilePath>* paths) = 0;
+  };
+
   TestExtensionsClient();
   ~TestExtensionsClient() override;
 
+  void AddBrowserImagePathsFilter(BrowserImagePathsFilter* filter);
+  void RemoveBrowserImagePathsFilter(BrowserImagePathsFilter* filter);
+
  private:
   void Initialize() override;
   const PermissionMessageProvider& GetPermissionMessageProvider()
@@ -44,13 +55,17 @@ class TestExtensionsClient : public ExtensionsClient {
   std::string GetWebstoreBaseURL() const override;
   std::string GetWebstoreUpdateURL() const override;
   bool IsBlacklistUpdateURL(const GURL& url) const override;
+  std::set<base::FilePath> GetBrowserImagePaths(
+      const Extension* extension) override;
 
   // A whitelist of extensions that can script anywhere. Do not add to this
   // list (except in tests) without consulting the Extensions team first.
   // Note: Component extensions have this right implicitly and do not need to be
   // added to this list.
   ScriptingWhitelist scripting_whitelist_;
 
+  std::set<BrowserImagePathsFilter*> browser_image_filters_;
+
   DISALLOW_COPY_AND_ASSIGN(TestExtensionsClient);
 };
 

chrome/utility/extensions/unpacker_unittest.cc → extensions/utility/unpacker_unittest.cc

@@ -8,10 +8,11 @@
 #include "base/strings/string_util.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/extension.h"
+#include "extensions/common/extension_paths.h"
 #include "extensions/common/manifest_constants.h"
+#include "extensions/test/test_extensions_client.h"
 #include "extensions/utility/unpacker.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/skia/include/core/SkBitmap.h"
@@ -26,17 +27,14 @@ namespace keys = manifest_keys;
 class UnpackerTest : public testing::Test {
  public:
   ~UnpackerTest() override {
-    LOG(WARNING) << "Deleting temp dir: "
-                 << temp_dir_.path().LossyDisplayName();
-    LOG(WARNING) << temp_dir_.Delete();
+    VLOG(1) << "Deleting temp dir: " << temp_dir_.path().LossyDisplayName();
+    VLOG(1) << temp_dir_.Delete();
   }
 
   void SetupUnpacker(const std::string& crx_name) {
     base::FilePath original_path;
-    ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &original_path));
-    original_path = original_path.AppendASCII("extensions")
-                        .AppendASCII("unpacker")
-                        .AppendASCII(crx_name);
+    ASSERT_TRUE(PathService::Get(DIR_TEST_DATA, &original_path));
+    original_path = original_path.AppendASCII("unpacker").AppendASCII(crx_name);
     ASSERT_TRUE(base::PathExists(original_path)) << original_path.value();
 
     // Try bots won't let us write into DIR_TEST_DATA, so we have to create
@@ -151,9 +149,42 @@ TEST_F(UnpackerTest, UnzipError) {
   EXPECT_EQ(ASCIIToUTF16(kExpected), unpacker_->error_message());
 }
 
+namespace {
+
+// Inserts an illegal path into the browser images returned by
+// TestExtensionsClient for any extension.
+class IllegalImagePathInserter
+    : public TestExtensionsClient::BrowserImagePathsFilter {
+ public:
+  IllegalImagePathInserter(TestExtensionsClient* client) : client_(client) {
+    client_->AddBrowserImagePathsFilter(this);
+  }
+
+  virtual ~IllegalImagePathInserter() {
+    client_->RemoveBrowserImagePathsFilter(this);
+  }
+
+  void Filter(const Extension* extension,
+              std::set<base::FilePath>* paths) override {
+    base::FilePath illegal_path =
+        base::FilePath(base::FilePath::kParentDirectory)
+            .AppendASCII(kTempExtensionName)
+            .AppendASCII("product_logo_128.png");
+    paths->insert(illegal_path);
+  }
+
+ private:
+  TestExtensionsClient* client_;
+};
+
+}  // namespace
+
 TEST_F(UnpackerTest, BadPathError) {
   const char kExpected[] = "Illegal path (absolute or relative with '..'): ";
-  SetupUnpacker("bad_path.crx");
+  SetupUnpacker("good_package.crx");
+  IllegalImagePathInserter inserter(
+      static_cast<TestExtensionsClient*>(ExtensionsClient::Get()));
+
   EXPECT_FALSE(unpacker_->Run());
   EXPECT_TRUE(
       StartsWith(unpacker_->error_message(), ASCIIToUTF16(kExpected), false))