Changing ZipFileInstaller to use the Unzip service.

As part of the effort to deprecate UtilityProcessHost, changing
extensions::UnzipInstaller to use the new Unzip service instead of
UtilityProcessMojoClient.

This remove the last use of a utility process for extensions, and as a
result the extensions/utility directory can be removed.

Tbr: tsepez@chromium.org,finnur@chromium.org
Bug: 799220
Change-Id: Ibe4c7f0c16909c99ed572822718ac56a6bcb57fa
Reviewed-on: https://chromium-review.googlesource.com/937902
Reviewed-by: Jay Civelli <jcivelli@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Jay Civelli <jcivelli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542303}

.gn

@@ -113,7 +113,6 @@ check_targets = [
   "//extensions/renderer:unit_tests",
   "//extensions/shell:browser_tests",
   "//extensions/shell:unit_tests",
-  "//extensions/utility:unit_tests",
   "//gin/*",
   "//google_apis/*",
   "//google_update/*",

chrome/browser/extensions/BUILD.gn

@@ -853,6 +853,7 @@ static_library("extensions") {
     "//components/sync_sessions",
     "//components/translate/core/browser",
     "//components/undo",
+    "//components/unzip_service/public/cpp",
     "//components/update_client",
     "//components/url_matcher",
     "//components/user_prefs",

chrome/browser/extensions/DEPS

@@ -37,6 +37,9 @@ specific_include_rules = {
     # TODO(mash): Remove. http://crbug.com/678705
     "+ash/shell.h",
   ],
+  "zipfile_installer_unittest.cc": [
+    "+services/data_decoder",
+  ],
   "test_extension_system.cc": [
     "+services/data_decoder",
   ],

chrome/browser/extensions/zipfile_installer_unittest.cc

@@ -2,8 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <vector>
+
 #include "base/bind.h"
 #include "base/command_line.h"
+#include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/message_loop/message_loop.h"
@@ -19,12 +22,16 @@
 #include "chrome/browser/extensions/test_extension_system.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/test/base/testing_profile.h"
+#include "components/unzip_service/unzip_service.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "content/public/test/test_utils.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_registry_observer.h"
 #include "extensions/common/constants.h"
 #include "extensions/common/extension.h"
+#include "services/data_decoder/data_decoder_service.h"
+#include "services/service_manager/public/cpp/connector.h"
+#include "services/service_manager/public/cpp/test/test_connector_factory.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 #if defined(OS_CHROMEOS)
@@ -81,12 +88,27 @@ struct MockExtensionRegistryObserver : public ExtensionRegistryObserver {
   base::Closure quit_closure;
 };
 
+struct UnzipFileFilterTestCase {
+  const base::FilePath::CharType* input;
+  const bool should_unzip;
+};
+
 }  // namespace
 
 class ZipFileInstallerTest : public testing::Test {
  public:
   ZipFileInstallerTest()
-      : browser_threads_(content::TestBrowserThreadBundle::IO_MAINLOOP) {}
+      : browser_threads_(content::TestBrowserThreadBundle::IO_MAINLOOP) {
+    service_manager::TestConnectorFactory::NameToServiceMap services;
+    services.insert(std::make_pair("data_decoder",
+                                   data_decoder::DataDecoderService::Create()));
+    services.insert(
+        std::make_pair("unzip_service", unzip::UnzipService::CreateService()));
+    test_connector_factory_ =
+        service_manager::TestConnectorFactory::CreateForServices(
+            std::move(services));
+    connector_ = test_connector_factory_->CreateConnector();
+  }
 
   void SetUp() override {
     extensions::LoadErrorReporter::Init(/*enable_noisy_errors=*/false);
@@ -121,8 +143,8 @@ class ZipFileInstallerTest : public testing::Test {
                         .AppendASCII("zipfile_installer")
                         .AppendASCII(zip_name);
     ASSERT_TRUE(base::PathExists(original_path)) << original_path.value();
-
     zipfile_installer_ = ZipFileInstaller::Create(
+        connector_.get(),
         MakeRegisterInExtensionServiceCallback(extension_service_));
 
     base::ThreadTaskRunnerHandle::Get()->PostTask(
@@ -131,6 +153,17 @@ class ZipFileInstallerTest : public testing::Test {
     observer_.WaitForInstall(expect_error);
   }
 
+  void RunZipFileFilterTest(
+      const std::vector<UnzipFileFilterTestCase>& cases,
+      base::RepeatingCallback<bool(const base::FilePath&)>& filter) {
+    for (size_t i = 0; i < cases.size(); ++i) {
+      base::FilePath input(cases[i].input);
+      bool observed = filter.Run(input);
+      EXPECT_EQ(cases[i].should_unzip, observed)
+          << "i: " << i << ", input: " << input.value();
+    }
+  }
+
  protected:
   scoped_refptr<ZipFileInstaller> zipfile_installer_;
 
@@ -148,6 +181,11 @@ class ZipFileInstallerTest : public testing::Test {
   // ChromeOS needs a user manager to instantiate an extension service.
   chromeos::ScopedTestUserManager test_user_manager_;
 #endif
+
+ private:
+  std::unique_ptr<service_manager::TestConnectorFactory>
+      test_connector_factory_;
+  std::unique_ptr<service_manager::Connector> connector_;
 };
 
 TEST_F(ZipFileInstallerTest, GoodZip) {
@@ -165,4 +203,49 @@ TEST_F(ZipFileInstallerTest, ZipWithPublicKey) {
   EXPECT_EQ(observer_.last_extension_installed, kIdForPublicKey);
 }
 
+TEST_F(ZipFileInstallerTest, NonTheme_FileExtractionFilter) {
+  const std::vector<UnzipFileFilterTestCase> cases = {
+      {FILE_PATH_LITERAL("foo"), true},
+      {FILE_PATH_LITERAL("foo.nexe"), true},
+      {FILE_PATH_LITERAL("foo.dll"), true},
+      {FILE_PATH_LITERAL("foo.jpg.exe"), false},
+      {FILE_PATH_LITERAL("foo.exe"), false},
+      {FILE_PATH_LITERAL("foo.EXE"), false},
+      {FILE_PATH_LITERAL("file_without_extension"), true},
+  };
+  base::RepeatingCallback<bool(const base::FilePath&)> filter =
+      base::BindRepeating(&ZipFileInstaller::ShouldExtractFile, false);
+  RunZipFileFilterTest(cases, filter);
+}
+
+TEST_F(ZipFileInstallerTest, Theme_FileExtractionFilter) {
+  const std::vector<UnzipFileFilterTestCase> cases = {
+      {FILE_PATH_LITERAL("image.jpg"), true},
+      {FILE_PATH_LITERAL("IMAGE.JPEG"), true},
+      {FILE_PATH_LITERAL("test/image.bmp"), true},
+      {FILE_PATH_LITERAL("test/IMAGE.gif"), true},
+      {FILE_PATH_LITERAL("test/image.WEBP"), true},
+      {FILE_PATH_LITERAL("test/dir/file.image.png"), true},
+      {FILE_PATH_LITERAL("manifest.json"), true},
+      {FILE_PATH_LITERAL("other.html"), false},
+      {FILE_PATH_LITERAL("file_without_extension"), true},
+  };
+  base::RepeatingCallback<bool(const base::FilePath&)> filter =
+      base::BindRepeating(&ZipFileInstaller::ShouldExtractFile, true);
+  RunZipFileFilterTest(cases, filter);
+}
+
+TEST_F(ZipFileInstallerTest, ManifestExtractionFilter) {
+  const std::vector<UnzipFileFilterTestCase> cases = {
+      {FILE_PATH_LITERAL("manifest.json"), true},
+      {FILE_PATH_LITERAL("MANIFEST.JSON"), true},
+      {FILE_PATH_LITERAL("test/manifest.json"), false},
+      {FILE_PATH_LITERAL("manifest.json/test"), false},
+      {FILE_PATH_LITERAL("other.file"), false},
+  };
+  base::RepeatingCallback<bool(const base::FilePath&)> filter =
+      base::BindRepeating(&ZipFileInstaller::IsManifestFile);
+  RunZipFileFilterTest(cases, filter);
+}
+
 }  // namespace extensions

chrome/browser/ui/webui/extensions/install_extension_handler.cc

@@ -20,6 +20,7 @@
 #include "content/public/browser/web_ui.h"
 #include "content/public/browser/web_ui_data_source.h"
 #include "content/public/common/drop_data.h"
+#include "content/public/common/service_manager_connection.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/common/feature_switch.h"
 #include "net/base/filename_util.h"
@@ -100,6 +101,7 @@ void InstallExtensionHandler::HandleInstallMessage(
 
   if (file_display_name_.MatchesExtension(FILE_PATH_LITERAL(".zip"))) {
     ZipFileInstaller::Create(
+        content::ServiceManagerConnection::GetForProcess()->GetConnector(),
         MakeRegisterInExtensionServiceCallback(
             ExtensionSystem::Get(profile)->extension_service()))
         ->LoadFromZipFile(file_to_install_);

chrome/utility/BUILD.gn

@@ -116,7 +116,6 @@ static_library("utility") {
       "//chrome/common/extensions/api",
       "//chrome/services/media_gallery_util:lib",
       "//chrome/services/removable_storage_writer:lib",
-      "//extensions/utility",
     ]
 
     public_deps += [ "//chrome/common/extensions/api" ]

chrome/utility/DEPS

@@ -29,7 +29,6 @@ include_rules = [
   "+content/public/utility",
   "+extensions/common",
   "+extensions/buildflags",
-  "+extensions/utility",
   "+media",
   "+services/network/public",
   "+services/network/url_request_context_builder_mojo.h",

chrome/utility/chrome_content_utility_client.cc

@@ -51,7 +51,6 @@
 #include "chrome/services/removable_storage_writer/public/mojom/constants.mojom.h"
 #include "chrome/services/removable_storage_writer/removable_storage_writer_service.h"
 #include "chrome/utility/extensions/extensions_handler.h"
-#include "extensions/utility/utility_handler.h"
 #if defined(OS_WIN)
 #include "chrome/services/wifi_util_win/public/mojom/constants.mojom.h"
 #include "chrome/services/wifi_util_win/wifi_util_win_service.h"
@@ -118,10 +117,6 @@ ChromeContentUtilityClient::ChromeContentUtilityClient()
 ChromeContentUtilityClient::~ChromeContentUtilityClient() = default;
 
 void ChromeContentUtilityClient::UtilityThreadStarted() {
-#if BUILDFLAG(ENABLE_EXTENSIONS)
-  extensions::utility_handler::UtilityThreadStarted();
-#endif
-
 #if defined(OS_WIN)
   base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
   utility_process_running_elevated_ = command_line->HasSwitch(
@@ -137,10 +132,6 @@ void ChromeContentUtilityClient::UtilityThreadStarted() {
     return;
 
   auto registry = std::make_unique<service_manager::BinderRegistry>();
-#if BUILDFLAG(ENABLE_EXTENSIONS)
-  extensions::utility_handler::ExposeInterfacesToBrowser(
-      registry.get(), utility_process_running_elevated_);
-#endif
   // If our process runs with elevated privileges, only add elevated Mojo
   // interfaces to the interface registry.
   if (!utility_process_running_elevated_) {

chrome/utility/extensions/DEPS

@@ -1,5 +1,5 @@
 include_rules = [
   "+extensions/buildflags",
   "+extensions/common",
-  "+extensions/utility",
+  "+extensions/features",
 ]

extensions/BUILD.gn

@@ -223,7 +223,6 @@ test("extensions_unittests") {
     "//extensions/common:unit_tests",
     "//extensions/renderer:unit_tests",
     "//extensions/shell:unit_tests",
-    "//extensions/utility:unit_tests",
     "//services/data_decoder:lib",
     "//services/service_manager/public/cpp/test:test_support",
     "//ui/gl:test_support",

extensions/browser/BUILD.gn

@@ -615,6 +615,8 @@ source_set("unit_tests") {
     "//components/pref_registry:pref_registry",
     "//components/prefs:test_support",
     "//components/sync_preferences:test_support",
+    "//components/unzip_service:lib",
+    "//components/unzip_service/public/cpp:test_support",
     "//components/update_client",
     "//components/url_matcher",
     "//components/user_prefs",
@@ -629,8 +631,10 @@ source_set("unit_tests") {
     "//extensions/strings",
     "//ipc:test_support",
     "//net:test_support",
+    "//services/data_decoder:lib",
     "//services/data_decoder/public/cpp:test_support",
     "//services/device/public/mojom",
+    "//services/service_manager/public/cpp/test:test_support",
     "//storage/browser:test_support",
     "//third_party/leveldatabase",
     "//third_party/zlib/google:zip",

extensions/browser/DEPS

@@ -8,6 +8,7 @@ include_rules = [
   "+components/sync",
   "+components/sync_preferences",
   "+components/update_client",
+  "+components/unzip_service/public",
   "+components/variations",
   "+components/version_info",
   "+components/web_cache",
@@ -56,4 +57,8 @@ specific_include_rules = {
     "+chrome/test/base/testing_profile.h",
     "+chrome/test/base/ui_test_utils.h",
   ],
+  "sandboxed_unpacker_unittest.cc": [
+    "+components/unzip_service",
+    "+services/data_decoder",
+  ],
 }