Revert "[Extension test] Add ability to load event_page extension as SW extension."

This reverts commit 1f54c905.

Reason for revert: Speculative fix for https://crbug.com/968005 (ExtensionApiTest.ChromeRuntimeOpenOptionsPage test failing on MSAN bots)

Original change's description:
> [Extension test] Add ability to load event_page extension as SW extension.
> 
> Add a flag to RunExtensionTest so that an existing event page based
> test extension can be loaded as SW based extension. This can be
> used to run extenision tests with Service Worker based extensions.
> 
> This CL also converts runtime/open_options_page/ test extension to event
> page extension and demonstrates that the extension be loaded
> as SW extension and the corresponding test
> ExtensionApiTest.ChromeRuntimeOpenOptionsPage passes with the flag in
> ServiceWorkerBasedBackgroundTest.ChromeRuntimeOpenOptionsPage.
> 
> Doc: https://docs.google.com/document/d/1PvXZ7VGRGdmd1s99SFByn9NCGj0GAiBhhPmfjj35ZeI/edit#
> 
> Bug: 967899
> Change-Id: I8821d1c9fa22795b8fe0f65f1bd073a034e24431
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1626455
> Commit-Queue: Istiaque Ahmed <lazyboy@chromium.org>
> Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#664062}

TBR=lazyboy@chromium.org,rdevlin.cronin@chromium.org

Change-Id: Ia818e2881a2802e981246c93d205a0ffb9ff9096
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 967899
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1634509Reviewed-by: Adam Rice <ricea@chromium.org>
Commit-Queue: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#664148}

chrome/browser/extensions/extension_apitest.cc

@@ -259,11 +259,6 @@ bool ExtensionApiTest::RunExtensionTestImpl(const std::string& extension_name,
         browser_test_flags |=
             ExtensionBrowserTest::kFlagAllowOldManifestVersions;
       }
-      if (flags & kFlagRunAsServiceWorkerBasedExtension) {
-        browser_test_flags |=
-            ExtensionBrowserTest::kFlagRunAsServiceWorkerBasedExtension;
-      }
-
       extension = LoadExtensionWithFlags(extension_path, browser_test_flags);
     }
     if (!extension) {

chrome/browser/extensions/extension_apitest.h

@@ -61,10 +61,6 @@ class ExtensionApiTest : public ExtensionBrowserTest {
     // Load the extension using //extensions/test/data/ as the root path instead
     // of loading from //chrome/test/data/extensions/api_test/.
     kFlagUseRootExtensionsDir = 1 << 7,
-
-    // Load the event page extension as a Service Worker based background
-    // extension.
-    kFlagRunAsServiceWorkerBasedExtension = 1 << 8,
   };
 
   ExtensionApiTest();

chrome/browser/extensions/extension_browsertest.cc

@@ -16,7 +16,6 @@
 #include "base/files/scoped_temp_dir.h"
 #include "base/path_service.h"
 #include "base/run_loop.h"
-#include "base/strings/strcat.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
@@ -68,12 +67,8 @@
 #include "extensions/browser/notification_types.h"
 #include "extensions/browser/test_extension_registry_observer.h"
 #include "extensions/browser/uninstall_reason.h"
-#include "extensions/common/constants.h"
 #include "extensions/common/extension_set.h"
-#include "extensions/common/file_test_util.h"
-#include "extensions/common/file_util.h"
 #include "extensions/common/switches.h"
-#include "extensions/common/value_builder.h"
 #include "net/url_request/url_request_file_job.h"
 
 #if defined(OS_CHROMEOS)
@@ -242,12 +237,7 @@ const Extension* ExtensionBrowserTest::LoadExtensionIncognito(
 
 const Extension* ExtensionBrowserTest::LoadExtensionWithFlags(
     const base::FilePath& path, int flags) {
-  base::FilePath extension_path = path;
-  if (flags & kFlagRunAsServiceWorkerBasedExtension) {
-    if (!CreateServiceWorkerBasedExtension(path, &extension_path))
-      return nullptr;
-  }
-  return LoadExtensionWithInstallParam(extension_path, flags, std::string());
+  return LoadExtensionWithInstallParam(path, flags, std::string());
 }
 
 const Extension* ExtensionBrowserTest::LoadExtensionWithInstallParam(
@@ -268,123 +258,6 @@ const Extension* ExtensionBrowserTest::LoadExtensionWithInstallParam(
   return extension.get();
 }
 
-bool ExtensionBrowserTest::CreateServiceWorkerBasedExtension(
-    const base::FilePath& path,
-    base::FilePath* out_path) {
-  base::ScopedAllowBlockingForTesting allow_blocking;
-
-  // This dir will contain all files for the Service Worker based extension.
-  base::FilePath temp_extension_container;
-  if (!base::CreateTemporaryDirInDir(temp_dir_.GetPath(),
-                                     base::FilePath::StringType(),
-                                     &temp_extension_container)) {
-    ADD_FAILURE() << "Could not create temporary dir for test under "
-                  << temp_dir_.GetPath();
-    return false;
-  }
-
-  // Copy all files from test dir to temp dir.
-  if (!base::CopyDirectory(path, temp_extension_container,
-                           true /* recursive */)) {
-    ADD_FAILURE() << path.value() << " could not be copied to "
-                  << temp_extension_container.value();
-    return false;
-  }
-
-  const base::FilePath extension_root =
-      temp_extension_container.Append(path.BaseName());
-
-  std::string error;
-  std::unique_ptr<base::DictionaryValue> manifest_dict =
-      file_util::LoadManifest(extension_root, &error);
-  if (!manifest_dict) {
-    ADD_FAILURE() << path.value() << " could not load manifest: " << error;
-    return false;
-  }
-
-  // Retrieve the value of the "background" key and verify that it is
-  // non-persistent and specifies JS files.
-  // Persistent background pages or background pages that specify HTML files
-  // are not supported.
-  base::Value* background_dict =
-      manifest_dict->FindKeyOfType("background", base::Value::Type::DICTIONARY);
-  if (!background_dict) {
-    ADD_FAILURE() << path.value()
-                  << " 'background' key not found in manifest.json";
-    return false;
-  }
-  {
-    base::Value* background_persistent = background_dict->FindKeyOfType(
-        "persistent", base::Value::Type::BOOLEAN);
-    if (!background_persistent || background_persistent->GetBool()) {
-      ADD_FAILURE() << path.value()
-                    << ": Only event pages can be loaded as SW extension.";
-      return false;
-    }
-  }
-  base::Value* background_scripts_list =
-      background_dict->FindKeyOfType("scripts", base::Value::Type::LIST);
-  if (!background_scripts_list) {
-    ADD_FAILURE() << path.value()
-                  << ": Only event pages with JS script(s) can be loaded "
-                     "as SW extension.";
-    return false;
-  }
-
-  // Number of JS scripts must be > 1.
-  base::Value::ListStorage& scripts_list = background_scripts_list->GetList();
-  if (scripts_list.size() < 1) {
-    ADD_FAILURE() << path.value()
-                  << ": Only event pages with JS script(s) can be loaded "
-                     " as SW extension.";
-    return false;
-  }
-
-  // Generate combined script as Service Worker script using importScripts().
-  constexpr const char kGeneratedSWFileName[] = "generated_service_worker__.js";
-
-  std::vector<std::string> script_filenames;
-  for (const base::Value& script : scripts_list)
-    script_filenames.push_back(base::StrCat({"'", script.GetString(), "'"}));
-
-  base::FilePath combined_script_filepath =
-      extension_root.AppendASCII(kGeneratedSWFileName);
-  // Collision with generated script filename.
-  if (base::PathExists(combined_script_filepath)) {
-    ADD_FAILURE() << combined_script_filepath.value()
-                  << " already exists, make sure " << path.value()
-                  << " does not contained file named " << kGeneratedSWFileName;
-    return false;
-  }
-  std::string generated_sw_script_content = base::StringPrintf(
-      "importScripts(%s);", base::JoinString(script_filenames, ",").c_str());
-  if (!file_test_util::WriteFile(combined_script_filepath,
-                                 generated_sw_script_content)) {
-    ADD_FAILURE() << "Could not write combined Service Worker script to: "
-                  << combined_script_filepath.value();
-    return false;
-  }
-
-  // Remove the existing background specification and replace it with a service
-  // worker.
-  background_dict->RemoveKey("persistent");
-  background_dict->RemoveKey("scripts");
-  background_dict->SetStringPath("service_worker", kGeneratedSWFileName);
-
-  // Write out manifest.json.
-  DictionaryBuilder manifest_builder(*manifest_dict);
-  std::string manifest_contents = manifest_builder.ToJSON();
-  base::FilePath manifest_path = extension_root.Append(kManifestFilename);
-  if (!file_test_util::WriteFile(manifest_path, manifest_contents)) {
-    ADD_FAILURE() << "Could not write manifest file to "
-                  << manifest_path.value();
-    return false;
-  }
-
-  *out_path = extension_root;
-  return true;
-}
-
 const Extension* ExtensionBrowserTest::LoadExtensionAsComponentWithManifest(
     const base::FilePath& path,
     const base::FilePath::CharType* manifest_relative_path) {

chrome/browser/extensions/extension_browsertest.h

@@ -62,9 +62,6 @@ class ExtensionBrowserTest : virtual public InProcessBrowserTest {
     // Allow older manifest versions (typically these can't be loaded - we allow
     // them for testing).
     kFlagAllowOldManifestVersions = 1 << 3,
-
-    // Load the provided extension as Service Worker based extension.
-    kFlagRunAsServiceWorkerBasedExtension = 1 << 4,
   };
 
   ExtensionBrowserTest();
@@ -120,16 +117,6 @@ class ExtensionBrowserTest : virtual public InProcessBrowserTest {
       int flags,
       const std::string& install_param);
 
-  // Converts an extension from |path| to a Service Worker based extension and
-  // returns true on success.
-  // If successful, |out_path| contains path of the converted extension.
-  //
-  // NOTE: The conversion works only for extensions with background.scripts and
-  // background.persistent = false; persistent background pages and
-  // background.page are not supported.
-  bool CreateServiceWorkerBasedExtension(const base::FilePath& path,
-                                         base::FilePath* out_path);
-
   // Loads unpacked extension from |path| with manifest |manifest_relative_path|
   // and imitates that it is a component extension.
   // |manifest_relative_path| is relative to |path|.

chrome/browser/extensions/service_worker_apitest.cc

@@ -1777,13 +1777,6 @@ IN_PROC_BROWSER_TEST_F(ServiceWorkerBasedBackgroundTest,
   EXPECT_FALSE(ProcessManager::Get(profile())->HasServiceWorker(*worker_id));
 }
 
-IN_PROC_BROWSER_TEST_F(ServiceWorkerBasedBackgroundTest,
-                       ChromeRuntimeOpenOptionsPage) {
-  ASSERT_TRUE(RunExtensionTestWithFlags("runtime/open_options_page",
-                                        kFlagRunAsServiceWorkerBasedExtension))
-      << message_;
-}
-
 // Tests that console messages logged by extension service workers, both via
 // the typical console.* methods and via our custom bindings console, are
 // passed through the normal ServiceWorker console messaging and are

chrome/test/data/extensions/api_test/runtime/open_options_page/manifest.json

@@ -4,8 +4,7 @@
   "manifest_version": 2,
   "description": "Browser test for chrome.runtime.openOptionsPage, success case.",
   "background": {
-    "scripts": ["test.js"],
-    "persistent": false
+    "scripts": ["test.js"]
   },
   "options_ui": {
     "page": "options.html"

extensions/common/BUILD.gn

@@ -383,8 +383,6 @@ if (enable_extensions) {
     sources = [
       "api/declarative_net_request/test_utils.cc",
       "api/declarative_net_request/test_utils.h",
-      "file_test_util.cc",
-      "file_test_util.h",
     ]
 
     deps = [

extensions/common/file_test_util.cc

-// Copyright 2019 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/common/file_test_util.h"
-#include "base/files/file_util.h"
-
-namespace extensions {
-namespace file_test_util {
-
-bool WriteFile(const base::FilePath& path, base::StringPiece content) {
-  return base::WriteFile(path, content.data(), content.size()) ==
-         static_cast<int>(content.size());
-}
-
-}  // namespace file_test_util
-}  // namespace extensions

extensions/common/file_test_util.h

-// Copyright 2019 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_COMMON_FILE_TEST_UTIL_H_
-#define EXTENSIONS_COMMON_FILE_TEST_UTIL_H_
-
-#include "base/strings/string_piece.h"
-
-namespace base {
-class FilePath;
-}
-
-namespace extensions {
-namespace file_test_util {
-
-// Writes |content| to |path|. Returns true if writing was successful,
-// verifying the number of bytes written equals the size of |content|.
-bool WriteFile(const base::FilePath& path, base::StringPiece content);
-
-}  // namespace file_test_util
-}  // namespace extensions
-
-#endif  // EXTENSIONS_COMMON_FILE_TEST_UTIL_H_