Fix app_shell shutdown crash due to BrowserContextKeyedServices

apps/shell/shell_browser_main_parts.cc

 // Copyright 2013 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 "apps/shell/shell_browser_main_parts.h"
 
 #include "apps/shell/shell_browser_context.h"
 #include "apps/shell/shell_extension_system.h"
 #include "apps/shell/shell_extensions_browser_client.h"
 #include "apps/shell/shell_extensions_client.h"
 #include "apps/shell/web_view_window.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/path_service.h"
 #include "base/run_loop.h"
-#include "chrome/browser/extensions/extension_system_factory.h"
+#include "chrome/browser/extensions/extension_prefs_factory.h"
+#include "chrome/browser/extensions/extension_system.h"
 #include "chrome/common/chrome_paths.h"
 #include "chromeos/chromeos_paths.h"
+#include "components/browser_context_keyed_service/browser_context_dependency_manager.h"
 #include "content/public/common/result_codes.h"
 #include "extensions/common/extension_paths.h"
 #include "ui/aura/env.h"
 #include "ui/aura/root_window.h"
 #include "ui/aura/test/test_screen.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/screen.h"
 #include "ui/wm/test/wm_test_helper.h"
 
 using content::BrowserContext;
 using extensions::Extension;
+using extensions::ExtensionSystem;
+using extensions::ShellExtensionSystem;
 
 namespace apps {
+namespace {
+
+// TODO(jamescook): Register additional BrowserContextKeyedService factories
+// here. See ChromeBrowserMainExtraPartsProfiles for details.
+void EnsureBrowserContextKeyedServiceFactoriesBuilt() {
+  extensions::ExtensionPrefsFactory::GetInstance();

[Yoyo Zhou, 2014/01/08 02:29:42]

Is there some reason this can't be declared as a DependsOn instead?

[James Cook, 2014/01/08 17:37:02]

Yeah, it should be done there. Chrome does it in both places, but in this case
DependsOn() is sufficient. Good catch.

+}
+
+}  // namespace
 
 ShellBrowserMainParts::ShellBrowserMainParts(
     const content::MainFunctionParams& parameters)
     : extension_system_(NULL) {
 }
 
 ShellBrowserMainParts::~ShellBrowserMainParts() {
 }
 
 void ShellBrowserMainParts::PreMainMessageLoopStart() {
@@ skipping 31 matching lines @@
   // Initialize our "profile" equivalent.
   browser_context_.reset(new ShellBrowserContext);
 
   extensions_client_.reset(new ShellExtensionsClient());
   extensions::ExtensionsClient::Set(extensions_client_.get());
 
   extensions_browser_client_.reset(
       new ShellExtensionsBrowserClient(browser_context_.get()));
   extensions::ExtensionsBrowserClient::Set(extensions_browser_client_.get());
 
-  // TODO(jamescook): Initialize policy::ProfilePolicyConnector.
-
+  // Create our custom ExtensionSystem first because other
+  // BrowserContextKeyedServices depend on it.
   CreateExtensionSystem();
 
-  // TODO(jamescook): Create the rest of the services using
-  // BrowserContextDependencyManager::CreateBrowserContextServices.
+  EnsureBrowserContextKeyedServiceFactoriesBuilt();
+  BrowserContextDependencyManager::GetInstance()->CreateBrowserContextServices(
+      browser_context_.get());
 
   CreateRootWindow();
 
   const std::string kAppSwitch = "app";
   CommandLine* command_line = CommandLine::ForCurrentProcess();
   if (command_line->HasSwitch(kAppSwitch)) {
     base::FilePath app_dir(command_line->GetSwitchValueNative(kAppSwitch));
     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.
     ShowWebViewWindow(browser_context_.get(),
                       wm_test_helper_->root_window()->window());
   }
 }
 
 bool ShellBrowserMainParts::MainMessageLoopRun(int* result_code)  {
   base::RunLoop run_loop;
   run_loop.Run();
   *result_code = content::RESULT_CODE_NORMAL_EXIT;
   return true;
 }
 
 void ShellBrowserMainParts::PostMainMessageLoopRun() {
   DestroyRootWindow();
-  // TODO(jamescook): Destroy the rest of the services with
-  // BrowserContextDependencyManager::DestroyBrowserContextServices.
+  BrowserContextDependencyManager::GetInstance()->DestroyBrowserContextServices(
+      browser_context_.get());
+  extension_system_ = NULL;
   extensions::ExtensionsBrowserClient::Set(NULL);
   extensions_browser_client_.reset();
   browser_context_.reset();
   aura::Env::DeleteInstance();
-
-  LOG(WARNING) << "-----------------------------------";
-  LOG(WARNING) << "app_shell is expected to crash now.";
-  LOG(WARNING) << "-----------------------------------";
 }
 
 void ShellBrowserMainParts::OnRootWindowHostCloseRequested(
     const aura::RootWindow* root) {
   base::MessageLoop::current()->PostTask(FROM_HERE,
                                          base::MessageLoop::QuitClosure());
 }
 
 void ShellBrowserMainParts::CreateRootWindow() {
   test_screen_.reset(aura::TestScreen::Create());
   // TODO(jamescook): Replace this with a real Screen implementation.
   gfx::Screen::SetScreenInstance(gfx::SCREEN_TYPE_NATIVE, test_screen_.get());
   // Set up basic pieces of views::corewm.
   wm_test_helper_.reset(new wm::WMTestHelper(gfx::Size(800, 600)));
   // Ensure the X window gets mapped.
   wm_test_helper_->root_window()->host()->Show();
   // Watch for the user clicking the close box.
   wm_test_helper_->root_window()->AddRootWindowObserver(this);
 }
 
 void ShellBrowserMainParts::DestroyRootWindow() {
   wm_test_helper_->root_window()->RemoveRootWindowObserver(this);
   wm_test_helper_->root_window()->PrepareForShutdown();
   wm_test_helper_.reset();
 }
 
 void ShellBrowserMainParts::CreateExtensionSystem() {
   DCHECK(browser_context_);
-  extension_system_ =
-      new extensions::ShellExtensionSystem(browser_context_.get());
-  extensions::ExtensionSystemFactory::GetInstance()->SetCustomInstance(
-      extension_system_);
-  // Must occur after setting the instance above, as it will end up calling
-  // ExtensionSystem::Get().
+  extension_system_ = static_cast<ShellExtensionSystem*>(
+      ExtensionSystem::GetForBrowserContext(browser_context_.get()));
   extension_system_->InitForRegularProfile(true);
 }
 
 }  // namespace apps