[Extensions] Beef up _test_resources handling

ExtensionBrowserTests look for paths beginning with "_test_resources" in
extension resources being loaded, and redirect them. The original
motivation for this was to allow us to re-use common utility files
between loaded test extensions.

We want to use this in the PDF extension tests. For this, we want to
load resources that aren't part of the extension itself as if they
were - specifically, for the sake of exercising module code.

Beef up the _test_resources handling by:
- adding support for subclasses to specify their own parent directory
  through ExtensionBrowserTest::GetTestResourcesParentDir(). This allows
  PDF tests to load resources from chrome/test/data/pdf instead of
  chrome/test/data/extensions.
- adding browser test coverage, including the basic case, a component
  extension case (which the PDF extension is), and the customized
  resource parent.

This is needed to unblock the PDF extension's use of modules, which in
turn is needed for the Polymer 2 -> Polymer 3 migration.

Bug: 1005029
Change-Id: I8a998eebf38bb680ffde72f512b962d83fe75989
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1850779
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Kelvin Jiang <kelvinjiang@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706055}

chrome/browser/extensions/extension_browsertest.cc

@@ -155,6 +155,15 @@ bool ExtensionBrowserTest::ShouldEnableInstallVerification() {
   return false;
 }
 
+base::FilePath ExtensionBrowserTest::GetTestResourcesParentDir() {
+  // Don't use |test_data_dir_| here (even though it points to
+  // chrome/test/data/extensions by default) because subclasses have the ability
+  // to alter it by overriding the SetUpCommandLine() method.
+  base::FilePath test_root_path;
+  base::PathService::Get(chrome::DIR_TEST_DATA, &test_root_path);
+  return test_root_path.AppendASCII("extensions");
+}
+
 // static
 const Extension* ExtensionBrowserTest::GetExtensionByPath(
     const ExtensionSet& extensions,
@@ -212,14 +221,8 @@ void ExtensionBrowserTest::SetUpOnMainThread() {
         test_extension_cache_.get());
   }
 
-  // We don't use test_data_dir_ here because we want this to point to
-  // chrome/test/data/extensions, and subclasses have a nasty habit of altering
-  // the data dir in SetUpCommandLine().
-  base::FilePath test_root_path;
-  base::PathService::Get(chrome::DIR_TEST_DATA, &test_root_path);
-  test_root_path = test_root_path.AppendASCII("extensions");
-  test_protocol_handler_ =
-      base::Bind(&ExtensionProtocolTestResourcesHandler, test_root_path);
+  test_protocol_handler_ = base::Bind(&ExtensionProtocolTestResourcesHandler,
+                                      GetTestResourcesParentDir());
   SetExtensionProtocolTestHandler(&test_protocol_handler_);
 }
 

chrome/browser/extensions/extension_browsertest.h

@@ -114,6 +114,11 @@ class ExtensionBrowserTest : virtual public InProcessBrowserTest {
   // about install verification.
   virtual bool ShouldEnableInstallVerification();
 
+  // Returns the path of the directory from which to serve resources when they
+  // are prefixed with "_test_resources/".
+  // The default is chrome/test/data/extensions/.
+  virtual base::FilePath GetTestResourcesParentDir();
+
   static const Extension* GetExtensionByPath(const ExtensionSet& extensions,
                                              const base::FilePath& path);
 

chrome/browser/extensions/test_resources_browsertest.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 "base/optional.h"
+#include "base/path_service.h"
+#include "base/strings/stringprintf.h"
+#include "chrome/browser/extensions/extension_browsertest.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/common/chrome_paths.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "content/public/test/browser_test_utils.h"
+#include "extensions/common/extension.h"
+#include "extensions/common/manifest.h"
+#include "extensions/test/test_extension_dir.h"
+
+namespace extensions {
+
+namespace {
+
+// The value set by the script
+// in chrome/test/data/extensions/test_resources_test/script.js.
+constexpr int kSentinelValue = 42;
+
+// Returns the value of window.injectedSentinel from the active web contents of
+// |browser|.
+base::Optional<int> RetrieveSentinelValue(Browser* browser) {
+  int result = 0;
+  content::WebContents* web_contents =
+      browser->tab_strip_model()->GetActiveWebContents();
+  if (!content::ExecuteScriptAndExtractInt(
+          web_contents,
+          "domAutomationController.send(window.injectedSentinel);", &result)) {
+    ADD_FAILURE() << "Failed to execute script.";
+    return base::nullopt;
+  }
+
+  return result;
+}
+
+class ExtensionBrowserTestWithCustomTestResourcesLocation
+    : public ExtensionBrowserTest {
+ public:
+  ExtensionBrowserTestWithCustomTestResourcesLocation() = default;
+  ~ExtensionBrowserTestWithCustomTestResourcesLocation() override = default;
+
+ private:
+  // Instead of serving _test_resources/ paths from chrome/test/data/extensions,
+  // serve them from chrome/test/data/extensions/test_resources_test.
+  base::FilePath GetTestResourcesParentDir() override {
+    base::FilePath test_root_path;
+    base::PathService::Get(chrome::DIR_TEST_DATA, &test_root_path);
+    return test_root_path.AppendASCII("extensions/test_resources_test");
+  }
+
+  DISALLOW_COPY_AND_ASSIGN(ExtensionBrowserTestWithCustomTestResourcesLocation);
+};
+
+}  // namespace
+
+// A simple test to ensure resources can be served from _test_resources/, and
+// properly load.
+IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, TestResourcesLoad) {
+  TestExtensionDir test_dir;
+  test_dir.WriteManifest(
+      R"({
+           "name": "Test Extension",
+           "version": "0.1",
+           "manifest_version": 2
+         })");
+  constexpr char kPageHtml[] =
+      R"(<html>
+           <script src="_test_resources/test_resources_test/test_script.js">
+           </script>
+         </html>)";
+  test_dir.WriteFile(FILE_PATH_LITERAL("page.html"), kPageHtml);
+
+  const Extension* extension = LoadExtension(test_dir.UnpackedPath());
+  ASSERT_TRUE(extension);
+
+  ui_test_utils::NavigateToURL(browser(),
+                               extension->GetResourceURL("page.html"));
+
+  base::Optional<int> sentinel = RetrieveSentinelValue(browser());
+  ASSERT_TRUE(sentinel);
+  EXPECT_EQ(kSentinelValue, *sentinel);
+}
+
+// Tests that resources from _test_resources work in component extensions
+// (which have a slightly different load path).
+IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest,
+                       TestResourcesLoadInComponentExtension) {
+  TestExtensionDir test_dir;
+  constexpr char kKey[] =
+      "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQC+uU63MD6T82Ldq5wjrDFn5mGmPnnnj"
+      "WZBWxYXfpG4kVf0s+p24VkXwTXsxeI12bRm8/ft9sOq0XiLfgQEh5JrVUZqvFlaZYoS+g"
+      "iZfUqzKFGMLa4uiSMDnvv+byxrqAepKz5G8XX/q5Wm5cvpdjwgiu9z9iM768xJy+Ca/G5"
+      "qQwIDAQAB";
+  constexpr char kManifestTemplate[] =
+      R"({
+           "name": "Test Extension",
+           "version": "0.1",
+           "manifest_version": 2,
+           "key": "%s"
+         })";
+  test_dir.WriteManifest(base::StringPrintf(kManifestTemplate, kKey));
+
+  constexpr char kPageHtml[] =
+      R"(<html>
+           <script src="_test_resources/test_resources_test/test_script.js">
+           </script>
+         </html>)";
+  test_dir.WriteFile(FILE_PATH_LITERAL("page.html"), kPageHtml);
+
+  const Extension* extension =
+      LoadExtensionAsComponent(test_dir.UnpackedPath());
+  ASSERT_TRUE(extension);
+  EXPECT_EQ(Manifest::COMPONENT, extension->location());
+
+  ui_test_utils::NavigateToURL(browser(),
+                               extension->GetResourceURL("page.html"));
+
+  base::Optional<int> sentinel = RetrieveSentinelValue(browser());
+  ASSERT_TRUE(sentinel);
+  EXPECT_EQ(kSentinelValue, *sentinel);
+}
+
+// Tests that resources from _test_resources can be loaded from different
+// directories. Though the default is chrome/test/data/extensions, a test class
+// can specify its own.
+IN_PROC_BROWSER_TEST_F(ExtensionBrowserTestWithCustomTestResourcesLocation,
+                       TestResourcesLoadFromCustomPath) {
+  TestExtensionDir test_dir;
+  test_dir.WriteManifest(
+      R"({
+           "name": "Test Extension",
+           "version": "0.1",
+           "manifest_version": 2
+         })");
+  // Note: Since this class serves _test_resources from
+  // chrome/test/data/extensions/test_resources_test, the
+  // path is just _test_resources/test_script.js.
+  constexpr char kPageHtml[] =
+      R"(<html>
+           <script src="_test_resources/test_script.js"></script>
+         </html>)";
+  test_dir.WriteFile(FILE_PATH_LITERAL("page.html"), kPageHtml);
+
+  const Extension* extension = LoadExtension(test_dir.UnpackedPath());
+  ASSERT_TRUE(extension);
+
+  ui_test_utils::NavigateToURL(browser(),
+                               extension->GetResourceURL("page.html"));
+
+  base::Optional<int> sentinel = RetrieveSentinelValue(browser());
+  ASSERT_TRUE(sentinel);
+  EXPECT_EQ(kSentinelValue, *sentinel);
+}
+
+}  // namespace extensions

chrome/test/BUILD.gn

@@ -1752,6 +1752,7 @@ if (!is_android) {
         "../browser/extensions/startup_helper_browsertest.cc",
         "../browser/extensions/stubs_apitest.cc",
         "../browser/extensions/subscribe_page_action_browsertest.cc",
+        "../browser/extensions/test_resources_browsertest.cc",
         "../browser/extensions/updater/extension_cache_fake.cc",
         "../browser/extensions/updater/extension_cache_fake.h",
         "../browser/extensions/updater/extension_update_client_base_browsertest.cc",

chrome/test/data/extensions/test_resources_test/test_script.js

+// 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.
+
+window.injectedSentinel = 42;