Load extensions in AppShell

Loading an extension is just like loading a platform app (but without
sending a launch event). So we can support --load-extension in
AppShell.

Also call extension_system->Init() unconditionally.

NOPRESUBMIT=true # existing ScopedAllowIO test usage is allowed

Bug: 742646
Change-Id: Id3c2adbebddc62d22953399f98e781229bda6d2b
Reviewed-on: https://chromium-review.googlesource.com/570809
Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487394}

extensions/shell/browser/default_shell_browser_main_delegate.cc

@@ -24,19 +24,33 @@
 
 namespace extensions {
 
-DefaultShellBrowserMainDelegate::DefaultShellBrowserMainDelegate() {
-}
+namespace {
 
-DefaultShellBrowserMainDelegate::~DefaultShellBrowserMainDelegate() {
+void LoadExtensionsFromCommandLine(ShellExtensionSystem* extension_system) {
+  base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
+  if (!command_line->HasSwitch(switches::kLoadExtension))
+    return;
+
+  base::CommandLine::StringType path_list =
+      command_line->GetSwitchValueNative(switches::kLoadExtension);
+
+  base::StringTokenizerT<base::CommandLine::StringType,
+                         base::CommandLine::StringType::const_iterator>
+      tokenizer(path_list, FILE_PATH_LITERAL(","));
+  while (tokenizer.GetNext()) {
+    extension_system->LoadExtension(
+        base::MakeAbsoluteFilePath(base::FilePath(tokenizer.token())));
+  }
 }
 
-void DefaultShellBrowserMainDelegate::Start(
+void LoadAppsFromCommandLine(ShellExtensionSystem* extension_system,
                              content::BrowserContext* browser_context) {
   base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
-  if (command_line->HasSwitch(switches::kLoadApps)) {
-    ShellExtensionSystem* extension_system = static_cast<ShellExtensionSystem*>(
-        ExtensionSystem::Get(browser_context));
-    extension_system->Init();
+  if (!command_line->HasSwitch(switches::kLoadApps)) {
+    LOG(ERROR) << "No app specified. Use --" << switches::kLoadApps
+               << " to load and launch an app.";
+    return;
+  }
 
   base::CommandLine::StringType path_list =
       command_line->GetSwitchValueNative(switches::kLoadApps);
@@ -64,10 +78,24 @@ void DefaultShellBrowserMainDelegate::Start(
   } else {
     LOG(ERROR) << "Could not load any apps.";
   }
-  } else {
-    LOG(ERROR) << "--" << switches::kLoadApps
-               << " unset; boredom is in your future";
-  }
+}
+
+}  // namespace
+
+DefaultShellBrowserMainDelegate::DefaultShellBrowserMainDelegate() {
+}
+
+DefaultShellBrowserMainDelegate::~DefaultShellBrowserMainDelegate() {
+}
+
+void DefaultShellBrowserMainDelegate::Start(
+    content::BrowserContext* browser_context) {
+  ShellExtensionSystem* extension_system =
+      static_cast<ShellExtensionSystem*>(ExtensionSystem::Get(browser_context));
+  extension_system->Init();
+
+  LoadExtensionsFromCommandLine(extension_system);
+  LoadAppsFromCommandLine(extension_system, browser_context);
 }
 
 void DefaultShellBrowserMainDelegate::Shutdown() {

extensions/shell/browser/shell_browser_main_parts.cc

@@ -23,7 +23,6 @@
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/updater/update_service.h"
 #include "extensions/common/constants.h"
-#include "extensions/common/switches.h"
 #include "extensions/shell/browser/shell_browser_context.h"
 #include "extensions/shell/browser/shell_browser_context_keyed_service_factories.h"
 #include "extensions/shell/browser/shell_browser_main_delegate.h"

extensions/shell/browser/shell_browsertest.cc

@@ -17,8 +17,13 @@
 
 namespace extensions {
 
+// Test that we can load an extension.
+IN_PROC_BROWSER_TEST_F(ShellApiTest, LoadExtension) {
+  ASSERT_TRUE(RunExtensionTest("extension")) << message_;
+}
+
 // Test that we can open an app window and wait for it to load.
-IN_PROC_BROWSER_TEST_F(ShellApiTest, Basic) {
+IN_PROC_BROWSER_TEST_F(ShellApiTest, LoadApp) {
   ASSERT_TRUE(RunAppTest("platform_app")) << message_;
 
   // A window was created.

extensions/shell/browser/shell_extension_system.cc

@@ -42,24 +42,26 @@ ShellExtensionSystem::ShellExtensionSystem(BrowserContext* browser_context)
 ShellExtensionSystem::~ShellExtensionSystem() {
 }
 
-const Extension* ShellExtensionSystem::LoadApp(const base::FilePath& app_dir) {
+const Extension* ShellExtensionSystem::LoadExtension(
+    const base::FilePath& extension_dir) {
   // app_shell only supports unpacked extensions.
   // NOTE: If you add packed extension support consider removing the flag
   // FOLLOW_SYMLINKS_ANYWHERE below. Packed extensions should not have symlinks.
-  CHECK(base::DirectoryExists(app_dir)) << app_dir.AsUTF8Unsafe();
+  CHECK(base::DirectoryExists(extension_dir)) << extension_dir.AsUTF8Unsafe();
   int load_flags = Extension::FOLLOW_SYMLINKS_ANYWHERE;
   std::string load_error;
   scoped_refptr<Extension> extension = file_util::LoadExtension(
-      app_dir, Manifest::COMMAND_LINE, load_flags, &load_error);
+      extension_dir, Manifest::COMMAND_LINE, load_flags, &load_error);
   if (!extension.get()) {
-    LOG(ERROR) << "Loading extension at " << app_dir.value()
+    LOG(ERROR) << "Loading extension at " << extension_dir.value()
                << " failed with: " << load_error;
     return nullptr;
   }
 
   // Log warnings.
   if (extension->install_warnings().size()) {
-    LOG(WARNING) << "Warnings loading extension at " << app_dir.value() << ":";
+    LOG(WARNING) << "Warnings loading extension at " << extension_dir.value()
+                 << ":";
     for (const auto& warning : extension->install_warnings())
       LOG(WARNING) << warning.message;
   }
@@ -82,9 +84,15 @@ const Extension* ShellExtensionSystem::LoadApp(const base::FilePath& app_dir) {
   RendererStartupHelperFactory::GetForBrowserContext(browser_context_)
       ->OnExtensionLoaded(*extension);
 
+  ExtensionRegistry::Get(browser_context_)->TriggerOnLoaded(extension.get());
+
   return extension.get();
 }
 
+const Extension* ShellExtensionSystem::LoadApp(const base::FilePath& app_dir) {
+  return LoadExtension(app_dir);
+}
+
 void ShellExtensionSystem::Init() {
   // Inform the rest of the extensions system to start.
   ready_.Signal();

extensions/shell/browser/shell_extension_system.h

@@ -32,8 +32,14 @@ class ShellExtensionSystem : public ExtensionSystem {
   explicit ShellExtensionSystem(content::BrowserContext* browser_context);
   ~ShellExtensionSystem() override;
 
-  // Loads an unpacked application from a directory. Returns the extension on
-  // success, or null otherwise.
+  // Loads an unpacked extension from a directory. Returns the extension on
+  // success, or nullptr otherwise.
+  const Extension* LoadExtension(const base::FilePath& extension_dir);
+
+  // Loads an unpacked platform app from a directory. Returns the extension on
+  // success, or nullptr otherwise.
+  // Currently this just calls LoadExtension, as apps are not loaded differently
+  // than other extensions. Use LaunchApp() to actually launch the loaded app.
   const Extension* LoadApp(const base::FilePath& app_dir);
 
   // Initializes the extension system.

extensions/shell/test/shell_apitest.cc

@@ -22,30 +22,43 @@ ShellApiTest::ShellApiTest() {
 ShellApiTest::~ShellApiTest() {
 }
 
-const Extension* ShellApiTest::LoadApp(const std::string& app_dir) {
+const Extension* ShellApiTest::LoadExtension(const std::string& extension_dir) {
   base::ThreadRestrictions::ScopedAllowIO allow_io;
   base::FilePath test_data_dir;
   PathService::Get(extensions::DIR_TEST_DATA, &test_data_dir);
-  test_data_dir = test_data_dir.AppendASCII(app_dir);
+  base::FilePath extension_path = test_data_dir.AppendASCII(extension_dir);
 
-  const Extension* extension = extension_system_->LoadApp(test_data_dir);
-  if (!extension)
-    return NULL;
+  return extension_system_->LoadExtension(extension_path);
+}
 
-  extension_system_->LaunchApp(extension->id());
+const Extension* ShellApiTest::LoadApp(const std::string& app_dir) {
+  base::ThreadRestrictions::ScopedAllowIO allow_io;
+  base::FilePath test_data_dir;
+  PathService::Get(extensions::DIR_TEST_DATA, &test_data_dir);
+  base::FilePath app_path = test_data_dir.AppendASCII(app_dir);
 
+  const Extension* extension = extension_system_->LoadApp(app_path);
+  if (extension)
+    extension_system_->LaunchApp(extension->id());
   return extension;
 }
 
+bool ShellApiTest::RunExtensionTest(const std::string& extension_dir) {
+  ResultCatcher catcher;
+  return RunTest(LoadExtension(extension_dir), &catcher);
+}
+
 bool ShellApiTest::RunAppTest(const std::string& app_dir) {
   ResultCatcher catcher;
+  return RunTest(LoadApp(app_dir), &catcher);
+}
 
-  const Extension* extension = LoadApp(app_dir);
+bool ShellApiTest::RunTest(const Extension* extension, ResultCatcher* catcher) {
   if (!extension)
     return false;
 
-  if (!catcher.GetNextResult()) {
-    message_ = catcher.message();
+  if (!catcher->GetNextResult()) {
+    message_ = catcher->message();
     return false;
   }
 

extensions/shell/test/shell_apitest.h

@@ -13,6 +13,7 @@
 namespace extensions {
 
 class Extension;
+class ResultCatcher;
 
 // Base class for app shell browser API tests that load an app/extension
 // and wait for a success message from the chrome.test API.
@@ -21,15 +22,22 @@ class ShellApiTest : public AppShellTest {
   ShellApiTest();
   ~ShellApiTest() override;
 
-  // Load and run an unpacked platform app from the |app_dir| directory. Unlike
-  // |RunAppTest|, it won't wait automatically for any kind of notification.
-  // Returns an instance of the extension that was just loaded.
+  // Loads an unpacked extension. Returns an instance of the extension that was
+  // just loaded.
+  // |extension_dir| should be a subpath under extensions/test/data.
+  const Extension* LoadExtension(const std::string& extension_dir);
+
+  // Loads and launches an unpacked platform app. Returns an instance of the
+  // extension that was just loaded.
+  // |app_dir| should be a subpath under extensions/test/data.
   const Extension* LoadApp(const std::string& app_dir);
 
-  // Loads an unpacked platform app from a directory using the current
-  // ExtensionSystem, launches it, and waits for a chrome.test success
-  // notification. Returns true if the test succeeds. |app_dir| is a
-  // subpath under extensions/test/data.
+  // Loads an unpacked extension and waits for a chrome.test success
+  // notification. Returns true if the test succeeds.
+  bool RunExtensionTest(const std::string& extension_dir);
+
+  // Loads and launches an unpacked platform app and waits for a chrome.test
+  // success notification. Returns true if the test succeeds.
   bool RunAppTest(const std::string& app_dir);
 
   // Removes the |app| from the ExtensionRegistry and dispatches
@@ -41,6 +49,8 @@ class ShellApiTest : public AppShellTest {
   std::string message_;
 
  private:
+  bool RunTest(const Extension* extension, ResultCatcher* catcher);
+
   DISALLOW_COPY_AND_ASSIGN(ShellApiTest);
 };
 

extensions/test/data/extension/background.js

+// Copyright 2017 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.addEventListener('load', function() {
+  chrome.test.notifyPass();
+});

extensions/test/data/extension/manifest.json

+{
+  "name": "Test Extension",
+  "description": "Simple testing extension. Succeeds when its background page runs.",
+  "manifest_version": 2,
+  "version": "1",
+  "background": {
+    "scripts": ["background.js"]
+  }
+}