Allow app_shell to run past extension manifest parsing

* Extracted non-Chrome-specific manifest handler registration into
  src/extensions/common/common_manfiest_handlers.cc
* Moved app_shell app load/launch code into ShellExtensionSystem so it can
  signal that the system is ready
* Refactored more of runtime API to use browser context instead of Profile
* Cleaned up or removed some uses of ExtensionService

This allows app_shell to get into the background page loading code before it crashes.

BUG=162530,288226,309909
TEST=existing browser_tests of extension system

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@243343 0039d316-1c4b-4281-b951-d872f2087c98

apps/shell/shell_browser_main_parts.cc

@@ -15,7 +15,6 @@
 #include "base/run_loop.h"
 #include "chrome/browser/extensions/extension_system_factory.h"
 #include "chrome/common/chrome_paths.h"
-#include "chrome/common/extensions/extension_file_util.h"
 #include "chromeos/chromeos_paths.h"
 #include "content/public/common/result_codes.h"
 #include "extensions/common/extension_paths.h"
@@ -94,7 +93,7 @@ void ShellBrowserMainParts::PreMainMessageLoopRun() {
   CommandLine* command_line = CommandLine::ForCurrentProcess();
   if (command_line->HasSwitch(kAppSwitch)) {
     base::FilePath app_dir(command_line->GetSwitchValueNative(kAppSwitch));
-    LoadAndLaunchApp(app_dir);
+    extension_system_->LoadAndLaunchApp(app_dir);
   } else {
     // TODO(jamescook): For demo purposes create a window with a WebView just
     // to ensure that the content module is properly initialized.
@@ -159,24 +158,4 @@ void ShellBrowserMainParts::CreateExtensionSystem() {
   extension_system_->InitForRegularProfile(true);
 }
 
-bool ShellBrowserMainParts::LoadAndLaunchApp(const base::FilePath& app_dir) {
-  DCHECK(extension_system_);
-  std::string load_error;
-  scoped_refptr<Extension> extension =
-      extension_file_util::LoadExtension(app_dir,
-                                         extensions::Manifest::COMMAND_LINE,
-                                         Extension::NO_FLAGS,
-                                         &load_error);
-  if (!extension) {
-    LOG(ERROR) << "Loading extension at " << app_dir.value()
-        << " failed with: " << load_error;
-    return false;
-  }
-
-  // TODO(jamescook): Add to ExtensionRegistry.
-  // TODO(jamescook): Set ExtensionSystem ready.
-  // TODO(jamescook): Send NOTIFICATION_EXTENSION_LOADED.
-  return true;
-}
-
 }  // namespace apps

apps/shell/shell_browser_main_parts.h

@@ -15,10 +15,6 @@ namespace aura {
 class TestScreen;
 }
 
-namespace base {
-class FilePath;
-}
-
 namespace content {
 class ShellBrowserContext;
 struct MainFunctionParams;
@@ -73,10 +69,6 @@ class ShellBrowserMainParts : public content::BrowserMainParts,
   // Creates and initializes the ExtensionSystem.
   void CreateExtensionSystem();
 
-  // Loads an unpacked application from a directory and attempts to launch it.
-  // Returns true on success.
-  bool LoadAndLaunchApp(const base::FilePath& app_dir);
-
   scoped_ptr<ShellBrowserContext> browser_context_;
   scoped_ptr<ShellExtensionsClient> extensions_client_;
   scoped_ptr<ShellExtensionsBrowserClient> extensions_browser_client_;

apps/shell/shell_extension_system.cc

@@ -4,10 +4,19 @@
 
 #include "apps/shell/shell_extension_system.h"
 
+#include <string>
+
 #include "base/command_line.h"
+#include "base/files/file_path.h"
+#include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/extension_prefs.h"
+#include "chrome/common/extensions/extension_file_util.h"
 #include "content/public/browser/browser_context.h"
+#include "content/public/browser/notification_details.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/browser/notification_source.h"
 #include "extensions/browser/event_router.h"
+#include "extensions/browser/extension_registry.h"
 #include "extensions/browser/lazy_background_task_queue.h"
 #include "extensions/browser/process_manager.h"
 
@@ -22,6 +31,42 @@ ShellExtensionSystem::ShellExtensionSystem(BrowserContext* browser_context)
 ShellExtensionSystem::~ShellExtensionSystem() {
 }
 
+bool ShellExtensionSystem::LoadAndLaunchApp(const base::FilePath& app_dir) {
+  std::string load_error;
+  scoped_refptr<Extension> extension =
+      extension_file_util::LoadExtension(app_dir,
+                                         extensions::Manifest::COMMAND_LINE,
+                                         Extension::NO_FLAGS,
+                                         &load_error);
+  if (!extension) {
+    LOG(ERROR) << "Loading extension at " << app_dir.value()
+        << " failed with: " << load_error;
+    return false;
+  }
+
+  ExtensionRegistry::Get(browser_context_)->AddEnabled(extension);
+
+  // TODO(jamescook): If RegisterExtensionWithRequestContexts() did something,
+  // this would be the place to call it.
+
+  content::NotificationService::current()->Notify(
+      chrome::NOTIFICATION_EXTENSION_LOADED,
+      content::Source<BrowserContext>(browser_context_),
+      content::Details<const Extension>(extension));
+
+  // Inform the rest of the extensions system to start.
+  ready_.Signal();
+  LOG(WARNING) << "-----------------------------------";
+  LOG(WARNING) << "app_shell is expected to crash now.";
+  LOG(WARNING) << "-----------------------------------";
+  content::NotificationService::current()->Notify(
+      chrome::NOTIFICATION_EXTENSIONS_READY,
+      content::Source<BrowserContext>(browser_context_),
+      content::NotificationService::NoDetails());
+
+  return true;
+}
+
 void ShellExtensionSystem::Shutdown() {
 }
 
@@ -34,7 +79,7 @@ void ShellExtensionSystem::InitForRegularProfile(bool extensions_enabled) {
 }
 
 ExtensionService* ShellExtensionSystem::extension_service() {
-  // This class only has an ExtensionServiceInterface.
+  NOTREACHED();
   return NULL;
 }
 

apps/shell/shell_extension_system.h

@@ -9,6 +9,10 @@
 #include "chrome/browser/extensions/extension_system.h"
 #include "extensions/common/one_shot_event.h"
 
+namespace base {
+class FilePath;
+}
+
 namespace content {
 class BrowserContext;
 }
@@ -26,6 +30,10 @@ class ShellExtensionSystem : public ExtensionSystem {
   explicit ShellExtensionSystem(content::BrowserContext* browser_context);
   virtual ~ShellExtensionSystem();
 
+  // Loads an unpacked application from a directory and attempts to launch it.
+  // Returns true on success.
+  bool LoadAndLaunchApp(const base::FilePath& app_dir);
+
   // BrowserContextKeyedService implementation:
   virtual void Shutdown() OVERRIDE;
 

apps/shell/shell_extensions_client.cc

@@ -6,6 +6,8 @@
 
 #include "base/logging.h"
 #include "chrome/common/extensions/features/base_feature_provider.h"
+#include "extensions/common/common_manifest_handlers.h"
+#include "extensions/common/manifest_handler.h"
 #include "extensions/common/permissions/permission_message_provider.h"
 #include "extensions/common/permissions/permissions_provider.h"
 #include "extensions/common/url_pattern_set.h"
@@ -91,6 +93,9 @@ ShellExtensionsClient::~ShellExtensionsClient() {
 }
 
 void ShellExtensionsClient::Initialize() {
+  extensions::RegisterCommonManifestHandlers();
+  extensions::ManifestHandler::FinalizeRegistration();
+
   // TODO(jamescook): Do we need to whitelist any extensions?
 }
 

chrome/browser/extensions/api/runtime/runtime_api.cc

@@ -27,6 +27,7 @@
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "extensions/browser/event_router.h"
+#include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extensions_browser_client.h"
 #include "extensions/browser/lazy_background_task_queue.h"
 #include "extensions/browser/process_manager.h"
@@ -73,40 +74,41 @@ const char kUninstallUrl[] = "uninstall_url";
 // with the equivalent Pepper API.
 const char kPackageDirectoryPath[] = "crxfs";
 
-static void DispatchOnStartupEventImpl(
-    Profile* profile,
-    const std::string& extension_id,
-    bool first_call,
-    ExtensionHost* host) {
+void DispatchOnStartupEventImpl(BrowserContext* browser_context,
+                                const std::string& extension_id,
+                                bool first_call,
+                                ExtensionHost* host) {
   // A NULL host from the LazyBackgroundTaskQueue means the page failed to
   // load. Give up.
   if (!host && !first_call)
     return;
 
-  // Don't send onStartup events to incognito profiles.
-  if (profile->IsOffTheRecord())
+  // Don't send onStartup events to incognito browser contexts.
+  if (browser_context->IsOffTheRecord())
     return;
 
-  if (g_browser_process->IsShuttingDown() ||
-      !g_browser_process->profile_manager()->IsValidProfile(profile))
+  if (ExtensionsBrowserClient::Get()->IsShuttingDown() ||
+      !ExtensionsBrowserClient::Get()->IsValidContext(browser_context))
     return;
-  ExtensionSystem* system = ExtensionSystem::Get(profile);
+  ExtensionSystem* system =
+      ExtensionSystem::GetForBrowserContext(browser_context);
   if (!system)
     return;
 
   // If this is a persistent background page, we want to wait for it to load
   // (it might not be ready, since this is startup). But only enqueue once.
   // If it fails to load the first time, don't bother trying again.
   const Extension* extension =
-      system->extension_service()->extensions()->GetByID(extension_id);
+      ExtensionRegistry::Get(browser_context)->enabled_extensions().GetByID(
+          extension_id);
   if (extension && BackgroundInfo::HasPersistentBackgroundPage(extension) &&
       first_call &&
       system->lazy_background_task_queue()->
-          ShouldEnqueueTask(profile, extension)) {
+          ShouldEnqueueTask(browser_context, extension)) {
     system->lazy_background_task_queue()->AddPendingTask(
-        profile, extension_id,
+        browser_context, extension_id,
         base::Bind(&DispatchOnStartupEventImpl,
-                   profile, extension_id, false));
+                   browser_context, extension_id, false));
     return;
   }
 
@@ -260,9 +262,7 @@ void RuntimeAPI::OnChromeUpdateAvailable() {
 // static
 void RuntimeEventRouter::DispatchOnStartupEvent(
     content::BrowserContext* context, const std::string& extension_id) {
-  // TODO(jamescook): Convert to BrowserContext all the way down.
-  Profile* profile = static_cast<Profile*>(context);
-  DispatchOnStartupEventImpl(profile, extension_id, true, NULL);
+  DispatchOnStartupEventImpl(context, extension_id, true, NULL);
 }
 
 // static

chrome/browser/extensions/extension_web_contents_observer.cc

@@ -16,6 +16,7 @@
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/site_instance.h"
 #include "content/public/browser/web_contents.h"
+#include "extensions/browser/extension_registry.h"
 #include "extensions/browser/view_type_utils.h"
 #include "extensions/common/constants.h"
 
@@ -59,8 +60,7 @@ void ExtensionWebContentsObserver::RenderViewCreated(
   // Some extensions use file:// URLs.
   if (type == Manifest::TYPE_EXTENSION ||
       type == Manifest::TYPE_LEGACY_PACKAGED_APP) {
-    if (ExtensionSystem::Get(profile_)->extension_service()->
-            extension_prefs()->AllowFileAccess(extension->id())) {
+    if (ExtensionPrefs::Get(profile_)->AllowFileAccess(extension->id())) {
       content::ChildProcessSecurityPolicy::GetInstance()->GrantScheme(
           process->GetID(), content::kFileScheme);
     }
@@ -119,7 +119,8 @@ const Extension* ExtensionWebContentsObserver::GetExtension(
   if (!site.SchemeIs(kExtensionScheme))
     return NULL;
 
-  ExtensionService* service = profile_->GetExtensionService();
+  ExtensionService* service =
+      ExtensionSystem::Get(profile_)->extension_service();
   if (!service)
     return NULL;
 
@@ -132,7 +133,8 @@ const Extension* ExtensionWebContentsObserver::GetExtension(
 
   // May be null if the extension doesn't exist, for example if somebody typos
   // a chrome-extension:// URL.
-  return service->extensions()->GetByID(site.host());
+  return ExtensionRegistry::Get(profile_)->enabled_extensions().GetByID(
+      site.host());
 }
 
 }  // namespace extensions

chrome/common/extensions/chrome_extensions_client.cc

@@ -10,8 +10,10 @@
 #include "chrome/common/extensions/features/base_feature_provider.h"
 #include "chrome/common/url_constants.h"
 #include "content/public/common/url_constants.h"
+#include "extensions/common/common_manifest_handlers.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/manifest_constants.h"
+#include "extensions/common/manifest_handler.h"
 #include "extensions/common/permissions/api_permission_set.h"
 #include "extensions/common/permissions/permission_message.h"
 #include "extensions/common/switches.h"
@@ -38,7 +40,13 @@ ChromeExtensionsClient::~ChromeExtensionsClient() {
 }
 
 void ChromeExtensionsClient::Initialize() {
-  RegisterChromeManifestHandlers();
+  // Registration could already be finalized in unit tests, where the utility
+  // thread runs in-process.
+  if (!ManifestHandler::IsRegistrationFinalized()) {
+    RegisterCommonManifestHandlers();
+    RegisterChromeManifestHandlers();
+    ManifestHandler::FinalizeRegistration();
+  }
 
   // Set up the scripting whitelist.
   // Whitelist ChromeVox, an accessibility extension from Google that needs

chrome/common/extensions/chrome_manifest_handlers.cc

@@ -38,58 +38,41 @@
 #include "chrome/common/extensions/mime_types_handler.h"
 #include "chrome/common/extensions/web_accessible_resources_handler.h"
 #include "chrome/common/extensions/webview_handler.h"
-#include "extensions/common/manifest_handlers/background_info.h"
-#include "extensions/common/manifest_handlers/csp_info.h"
-#include "extensions/common/manifest_handlers/incognito_info.h"
-#include "extensions/common/manifest_handlers/kiosk_mode_info.h"
-#include "extensions/common/manifest_handlers/offline_enabled_info.h"
 #include "extensions/common/manifest_handlers/requirements_info.h"
-#include "extensions/common/manifest_handlers/sandboxed_page_info.h"
-#include "extensions/common/manifest_handlers/shared_module_info.h"
 
 namespace extensions {
 
 void RegisterChromeManifestHandlers() {
-  // This can happen in unit tests, where the utility thread runs in-process.
-  if (ManifestHandler::IsRegistrationFinalized())
-    return;
+  DCHECK(!ManifestHandler::IsRegistrationFinalized());
 #if defined(ENABLE_EXTENSIONS)
   (new AppIsolationHandler)->Register();
   (new AppLaunchManifestHandler)->Register();
-  (new BackgroundManifestHandler)->Register();
   (new BrowserActionHandler)->Register();
   (new CommandsHandler)->Register();
   (new ContentScriptsHandler)->Register();
-  (new CSPHandler(false))->Register();
-  (new CSPHandler(true))->Register();
   (new DefaultLocaleHandler)->Register();
   (new DevToolsPageHandler)->Register();
   (new ExternallyConnectableHandler)->Register();
   (new FileBrowserHandlerParser)->Register();
   (new FileHandlersParser)->Register();
   (new HomepageURLHandler)->Register();
   (new IconsHandler)->Register();
-  (new IncognitoHandler)->Register();
 #if defined(OS_CHROMEOS)
   (new InputComponentsHandler)->Register();
 #endif
-  (new KioskModeHandler)->Register();
   (new ManagedModeHandler)->Register();
   (new MediaGalleriesHandlerParser)->Register();
   (new MimeTypesHandlerParser)->Register();
   (new MinimumChromeVersionChecker)->Register();
   (new NaClModulesHandler)->Register();
   (new OAuth2ManifestHandler)->Register();
-  (new OfflineEnabledHandler)->Register();
   (new OmniboxHandler)->Register();
   (new OptionsPageHandler)->Register();
   (new PageActionHandler)->Register();
   (new PluginsHandler)->Register();
-  (new RequirementsHandler)->Register();
-  (new SandboxedPageHandler)->Register();
+  (new RequirementsHandler)->Register();  // Depends on plugins.
   (new SettingsOverridesHandler)->Register();
   (new ScriptBadgeHandler)->Register();
-  (new SharedModuleHandler)->Register();
   (new SocketsManifestHandler)->Register();
   (new SpellcheckHandler)->Register();
   (new StorageSchemaManifestHandler)->Register();
@@ -101,7 +84,6 @@ void RegisterChromeManifestHandlers() {
   (new URLOverridesHandler)->Register();
   (new WebAccessibleResourcesHandler)->Register();
   (new WebviewHandler)->Register();
-  ManifestHandler::FinalizeRegistration();
 #endif
 }
 

chrome/common/extensions/chrome_manifest_handlers.h

@@ -8,7 +8,7 @@
 namespace extensions {
 
 // Registers all manifest handlers used in Chrome. Should be called
-// once in each process.
+// once in each process. See also extensions/common/common_manifest_handlers.h.
 void RegisterChromeManifestHandlers();
 
 }  // namespace extensions