Have content/ create and destroy its own threads.

chrome/browser/chrome_browser_main.cc

 // Copyright (c) 2011 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 "chrome/browser/chrome_browser_main.h"
 
 #include <algorithm>
 #include <string>
 #include <vector>
 
@@ skipping 56 matching lines @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/search_engines/search_engine_type.h"
 #include "chrome/browser/search_engines/template_url.h"
 #include "chrome/browser/search_engines/template_url_service.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
 #include "chrome/browser/service/service_process_control.h"
 #include "chrome/browser/shell_integration.h"
 #include "chrome/browser/translate/translate_manager.h"
 #include "chrome/browser/ui/browser.h"
-#include "chrome/browser/ui/browser_init.h"
 #include "chrome/browser/ui/webui/chrome_url_data_manager_backend.h"
 #include "chrome/browser/web_resource/gpu_blacklist_updater.h"
 #include "chrome/common/child_process_logging.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_result_codes.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/chrome_version_info.h"
 #include "chrome/common/env_vars.h"
 #include "chrome/common/json_pref_store.h"
 #include "chrome/common/jstemplate_builder.h"
 #include "chrome/common/logging_chrome.h"
 #include "chrome/common/net/net_resource_provider.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/profiling.h"
 #include "chrome/installer/util/google_update_settings.h"
 #include "content/browser/renderer_host/resource_dispatcher_host.h"
 #include "content/common/child_process.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/io_thread_delegate.h"
 #include "content/public/common/content_client.h"
 #include "content/public/common/main_function_params.h"
 #include "grit/app_locale_settings.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
 #include "grit/platform_locale_settings.h"
 #include "net/base/cookie_monster.h"
 #include "net/base/net_module.h"
 #include "net/base/sdch_manager.h"
 #include "net/http/http_basic_stream.h"
@@ skipping 194 matching lines @@
   net::URLRequestThrottlerManager::GetInstance()->set_enable_thread_checks(
       true);
 
   // TODO(joi): Passing the NetLog here is temporary; once I switch the
   // URLRequestThrottlerManager to be part of the URLRequestContext it will
   // come from there. Doing it this way for now (2011/5/12) to try to fail
   // fast in case A/B experiment gives unexpected results.
   net::URLRequestThrottlerManager::GetInstance()->set_net_log(net_log);
 }
 
-// Creates key child threads. We need to do this explicitly since
-// BrowserThread::PostTask silently deletes a posted task if the target message
-// loop isn't created.
-void CreateChildThreads(BrowserProcessImpl* process) {
-  process->db_thread();
-  process->file_thread();
-  process->process_launcher_thread();
-  process->cache_thread();
-  process->io_thread();
-#if defined(OS_CHROMEOS)
-  process->web_socket_proxy_thread();
-#endif
-  // Create watchdog thread after creating all other threads because it will
-  // watch the other threads and they must be running.
-  process->watchdog_thread();
-}
-
 // Returns the new local state object, guaranteed non-NULL.
 PrefService* InitializeLocalState(const CommandLine& parsed_command_line,
                                   bool is_first_run) {
   FilePath local_state_path;
   PathService::Get(chrome::FILE_LOCAL_STATE, &local_state_path);
   bool local_state_file_exists = file_util::PathExists(local_state_path);
 
   // Load local state.  This includes the application locale so we know which
   // locale dll to load.
   PrefService* local_state = g_browser_process->local_state();
@@ skipping 330 matching lines @@
 ChromeBrowserMainParts::ChromeBrowserMainParts(
     const content::MainFunctionParams& parameters)
     : parameters_(parameters),
       parsed_command_line_(parameters.command_line),
       result_code_(content::RESULT_CODE_NORMAL_EXIT),
       shutdown_watcher_(new ShutdownWatcherHelper()),
       record_search_engine_(false),
       translate_manager_(NULL),
       profile_(NULL),
       run_message_loop_(true),
-      notify_result_(ProcessSingleton::PROCESS_NONE) {
+      notify_result_(ProcessSingleton::PROCESS_NONE),
+      is_first_run_(false),
+      first_run_ui_bypass_(false),
+      metrics_(NULL),
+      local_state_(NULL),
+      restart_last_session_(false) {
   // If we're running tests (ui_task is non-null).
   if (parameters.ui_task)
     browser_defaults::enable_help_app = false;
 }
 
 ChromeBrowserMainParts::~ChromeBrowserMainParts() {
 }
 
 // This will be called after the command-line has been mutated by about:flags
 MetricsService* ChromeBrowserMainParts::SetupMetricsAndFieldTrials(
@@ skipping 482 matching lines @@
 
 void ChromeBrowserMainParts::ToolkitInitialized() {
 }
 
 void ChromeBrowserMainParts::PreMainMessageLoopStart() {
 }
 
 void ChromeBrowserMainParts::PostMainMessageLoopStart() {
 }
 
-void ChromeBrowserMainParts::PreMainMessageLoopRun() {
+content::IOThreadDelegate* ChromeBrowserMainParts::PreMainMessageLoopRun() {
   result_code_ = PreMainMessageLoopRunImpl();
+  return browser_process_->io_thread();
+}
+
+void ChromeBrowserMainParts::PreMainMessageLoopRunThreadsCreated() {
+  result_code_ = PreMainMessageLoopRunThreadsCreatedImpl();
 }
 
 int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() {
   run_message_loop_ = false;
-  FilePath user_data_dir;
 #if defined(OS_WIN)
-  PathService::Get(chrome::DIR_USER_DATA, &user_data_dir);
+  PathService::Get(chrome::DIR_USER_DATA, &user_data_dir_);
 #else
   // Getting the user data dir can fail if the directory isn't
   // creatable, for example; on Windows in code below we bring up a
   // dialog prompting the user to pick a different directory.
   // However, ProcessSingleton needs a real user_data_dir on Mac/Linux,
   // so it's better to fail here than fail mysteriously elsewhere.
-  CHECK(PathService::Get(chrome::DIR_USER_DATA, &user_data_dir))
+  CHECK(PathService::Get(chrome::DIR_USER_DATA, &user_data_dir_))
       << "Must be able to get user data directory!";
 #endif
 
-  process_singleton_.reset(new ProcessSingleton(user_data_dir));
+  process_singleton_.reset(new ProcessSingleton(user_data_dir_));
 
-  bool is_first_run = FirstRun::IsChromeFirstRun() ||
+  is_first_run_ = FirstRun::IsChromeFirstRun() ||
       parsed_command_line().HasSwitch(switches::kFirstRun);
 
 #if defined(USE_AURA)
   // No first run on Aura. See crbug.com/99439.
-  is_first_run = false;
+  is_first_run_ = false;
 #endif
 
   if (parsed_command_line().HasSwitch(switches::kImport) ||
       parsed_command_line().HasSwitch(switches::kImportFromFile)) {
     // We use different BrowserProcess when importing so no GoogleURLTracker is
     // instantiated (as it makes a net::URLRequest and we don't have an IO
     // thread, see bug #1292702).
     browser_process_.reset(new FirstRunBrowserProcess(parsed_command_line()));
-    is_first_run = false;
+    is_first_run_ = false;
   } else {
     browser_process_.reset(new BrowserProcessImpl(parsed_command_line()));
   }
 
   if (parsed_command_line().HasSwitch(switches::kEnableTracking)) {
     // User wants to override default tracking status.
     std::string flag =
       parsed_command_line().GetSwitchValueASCII(switches::kEnableTracking);
     bool enabled = flag.compare("0") != 0;
     tracked_objects::ThreadData::InitializeAndSetTrackingStatus(enabled);
   }
 
   // This forces the TabCloseableStateWatcher to be created and, on chromeos,
   // register for the notifications it needs to track the closeable state of
   // tabs.
   g_browser_process->tab_closeable_state_watcher();
 
-  PrefService* local_state = InitializeLocalState(parsed_command_line(),
-                                                  is_first_run);
+  local_state_ = InitializeLocalState(parsed_command_line(),
+                                      is_first_run_);

[robertshield, 2011/11/14 04:15:55]

It looks like local_state_ and metrics_ will be dangling pointers once
g_browser_process is destroyed, which happens in FinalCleanup() iiuc.

Should these two be explicitly NULLed out at some point?

 
 #if defined(USE_LINUX_BREAKPAD)
   // Needs to be called after we have chrome::DIR_USER_DATA and
   // g_browser_process.
   g_browser_process->file_thread()->message_loop()->PostTask(FROM_HERE,
       new GetLinuxDistroTask());
 
-  if (IsCrashReportingEnabled(local_state))
+  if (IsCrashReportingEnabled(local_state_))
     InitCrashReporter();
 #endif
 
   // If we're running tests (ui_task is non-null), then the ResourceBundle
   // has already been initialized.
   if (parameters().ui_task) {
     g_browser_process->SetApplicationLocale("en-US");
   } else {
     // Mac starts it earlier in |PreMainMessageLoopStart()| (because it is
     // needed when loading the MainMenu.nib and the language doesn't depend on
     // anything since it comes from Cocoa.
 #if defined(OS_MACOSX)
     g_browser_process->SetApplicationLocale(l10n_util::GetLocaleOverride());
 #else
     const std::string locale =
-        local_state->GetString(prefs::kApplicationLocale);
+        local_state_->GetString(prefs::kApplicationLocale);
     // On a POSIX OS other than ChromeOS, the parameter that is passed to the
     // method InitSharedInstance is ignored.
     const std::string loaded_locale =
         ResourceBundle::InitSharedInstance(locale);
     if (loaded_locale.empty() &&
         !parsed_command_line().HasSwitch(switches::kNoErrorDialogs)) {
       ShowMissingLocaleMessageBox();
       return chrome::RESULT_CODE_MISSING_DATA;
     }
     CHECK(!loaded_locale.empty()) << "Locale could not be found for " << locale;
@@ skipping 39 matching lines @@
   // This needs to be called after the locale has been set.
   RegisterTranslateableItems();
 #endif
 
   browser_init_.reset(new BrowserInit);
 
   // On first run, we need to process the predictor preferences before the
   // browser's profile_manager object is created, but after ResourceBundle
   // is initialized.
   master_prefs_.reset(new FirstRun::MasterPrefs);
-  bool first_run_ui_bypass = false;  // True to skip first run UI.
-  if (is_first_run) {
-    first_run_ui_bypass =
-        !FirstRun::ProcessMasterPreferences(user_data_dir, master_prefs_.get());
+  first_run_ui_bypass_ = false;  // True to skip first run UI.
+  if (is_first_run_) {
+    first_run_ui_bypass_ = !FirstRun::ProcessMasterPreferences(
+        user_data_dir_, master_prefs_.get());
     AddFirstRunNewTabs(browser_init_.get(), master_prefs_->new_tabs);
 
     // If we are running in App mode, we do not want to show the importer
     // (first run) UI.
-    if (!first_run_ui_bypass &&
+    if (!first_run_ui_bypass_ &&
         (parsed_command_line().HasSwitch(switches::kApp) ||
          parsed_command_line().HasSwitch(switches::kAppId) ||
          parsed_command_line().HasSwitch(switches::kNoFirstRun)))
-      first_run_ui_bypass = true;
+      first_run_ui_bypass_ = true;
   }
 
   // TODO(viettrungluu): why don't we run this earlier?
   if (!parsed_command_line().HasSwitch(switches::kNoErrorDialogs))
     WarnAboutMinimumSystemRequirements();
 
   // Enable print preview once for supported platforms.
 #if defined(GOOGLE_CHROME_BUILD)
-  local_state->RegisterBooleanPref(prefs::kPrintingPrintPreviewEnabledOnce,
+  local_state_->RegisterBooleanPref(prefs::kPrintingPrintPreviewEnabledOnce,
                                    false,
                                    PrefService::UNSYNCABLE_PREF);
-  if (!local_state->GetBoolean(prefs::kPrintingPrintPreviewEnabledOnce)) {
-    local_state->SetBoolean(prefs::kPrintingPrintPreviewEnabledOnce, true);
-    about_flags::SetExperimentEnabled(local_state, "print-preview", true);
+  if (!local_state_->GetBoolean(prefs::kPrintingPrintPreviewEnabledOnce)) {
+    local_state_->SetBoolean(prefs::kPrintingPrintPreviewEnabledOnce, true);
+    about_flags::SetExperimentEnabled(local_state_, "print-preview", true);
   }
 #endif
 
 #if defined(TOUCH_UI)
   // Always add the --views-desktop flag, if not already set.
   // If the user disabled the flag from about:flags, it will get unset by the
   // call to ConvertFlagsToSwitches in the following line.
   if (!parsed_command_line().HasSwitch(switches::kViewsDesktop))
     CommandLine::ForCurrentProcess()->AppendSwitchASCII(switches::kViewsDesktop,
                                                         "other");
 #endif
 
   // Convert active labs into switches. Modifies the current command line.
-  about_flags::ConvertFlagsToSwitches(local_state,
+  about_flags::ConvertFlagsToSwitches(local_state_,
                                       CommandLine::ForCurrentProcess());
 
   // Reset the command line in the crash report details, since we may have
   // just changed it to include experiments.
   child_process_logging::SetCommandLine(CommandLine::ForCurrentProcess());
 
   InitializeNetworkOptions(parsed_command_line());
   InitializeURLRequestThrottlerManager(browser_process_->net_log());
 
   // Initialize histogram synchronizer system. This is a singleton and is used
   // for posting tasks via NewRunnableMethod. Its deleted when it goes out of
   // scope. Even though NewRunnableMethod does AddRef and Release, the object
   // will not be deleted after the Task is executed.
   histogram_synchronizer_ = new HistogramSynchronizer();
   tracking_synchronizer_ = new chrome_browser_metrics::TrackingSynchronizer();
 
   // Now the command line has been mutated based on about:flags, we can
   // set up metrics and initialize field trials.
-  MetricsService* metrics = SetupMetricsAndFieldTrials(local_state);
+  metrics_ = SetupMetricsAndFieldTrials(local_state_);
 
 #if defined(USE_WEBKIT_COMPOSITOR)
   // We need to ensure WebKit has been initialized before we start the WebKit
   // compositor. This is done by the ResourceDispatcherHost on creation.
   g_browser_process->resource_dispatcher_host();
 #endif
 
   // Now that all preferences have been registered, set the install date
   // for the uninstall metrics if this is our first run. This only actually
   // gets used if the user has metrics reporting enabled at uninstall time.
   int64 install_date =
-      local_state->GetInt64(prefs::kUninstallMetricsInstallDate);
+      local_state_->GetInt64(prefs::kUninstallMetricsInstallDate);
   if (install_date == 0) {
-    local_state->SetInt64(prefs::kUninstallMetricsInstallDate,
+    local_state_->SetInt64(prefs::kUninstallMetricsInstallDate,
                           base::Time::Now().ToTimeT());
   }
 
 #if defined(OS_MACOSX)
   // Get the Keychain API to register for distributed notifications on the main
   // thread, which has a proper CFRunloop, instead of later on the I/O thread,
   // which doesn't. This ensures those notifications will get delivered
   // properly. See issue 37766.
   // (Note that the callback mask here is empty. I don't want to register for
   // any callbacks, I just want to initialize the mechanism.)
   SecKeychainAddCallback(&KeychainCallback, 0, NULL);
 #endif
 
-  CreateChildThreads(browser_process_.get());
+  // This initializes the IOThread object.  This needs to happen
+  // before the thread itself is created.
+  browser_process_->CreateIOThreadState();
+
+  return content::RESULT_CODE_NORMAL_EXIT;
+}
+
+int ChromeBrowserMainParts::PreMainMessageLoopRunThreadsCreatedImpl() {
+  // Create watchdog thread after creating all other threads because it will
+  // watch the other threads and they must be running.
+  browser_process_->watchdog_thread();
 
 #if defined(OS_CHROMEOS)
   // Now that the file thread exists we can record our stats.
   chromeos::BootTimesLoader::Get()->RecordChromeMainStats();
 
 #if defined(TOOLKIT_USES_GTK)
   // Read locale-specific GTK resource information.
   std::string gtkrc = l10n_util::GetStringUTF8(IDS_LOCALE_GTKRC);
   if (!gtkrc.empty())
     gtk_rc_parse_string(gtkrc.c_str());
@@ skipping 121 matching lines @@
     // Redirects Chrome logging to the user data dir.
     logging::RedirectChromeLogging(parsed_command_line());
 
     // Initialize user policy before creating the profile so the profile
     // initialization code sees policy settings.
     g_browser_process->browser_policy_connector()->InitializeUserPolicy(
         username);
   }
 #endif
 
-  if (is_first_run) {
+  if (is_first_run_) {
     // Warn the ProfileManager that an import process will run, possibly
     // locking the WebDataService directory of the next Profile created.
     g_browser_process->profile_manager()->SetWillImport();
   }
 
-  profile_ = CreateProfile(parameters(), user_data_dir, parsed_command_line());
+  profile_ = CreateProfile(parameters(), user_data_dir_, parsed_command_line());
   if (!profile_)
     return content::RESULT_CODE_NORMAL_EXIT;
 
   // Autoload any profiles which are running background apps.
   // TODO(rlp): Do this on a separate thread. See http://crbug.com/99075.
   if (!BackgroundModeManager::IsBackgroundModePermanentlyDisabled(
       &parsed_command_line())) {
     g_browser_process->profile_manager()->AutoloadProfiles();
   }
   // Post-profile init ---------------------------------------------------------
@@ skipping 58 matching lines @@
       return content::RESULT_CODE_NORMAL_EXIT;
     }
   }
 #endif
 
   // Show the First Run UI if this is the first time Chrome has been run on
   // this computer, or we're being compelled to do so by a command line flag.
   // Note that this be done _after_ the PrefService is initialized and all
   // preferences are registered, since some of the code that the importer
   // touches reads preferences.
-  if (is_first_run) {
-    if (!first_run_ui_bypass) {
+  if (is_first_run_) {
+    if (!first_run_ui_bypass_) {
       FirstRun::AutoImport(profile_,
                            master_prefs_->homepage_defined,
                            master_prefs_->do_import_items,
                            master_prefs_->dont_import_items,
                            master_prefs_->run_search_engine_experiment,
                            master_prefs_->randomize_search_engine_experiment,
                            master_prefs_->make_chrome_default,
                            process_singleton_.get());
 #if defined(OS_POSIX)
       // On Windows, the download is tagged with enable/disable stats so there
       // is no need for this code.
 
       // If stats reporting was turned on by the first run dialog then toggle
       // the pref.
       if (GoogleUpdateSettings::GetCollectStatsConsent())
-        local_state->SetBoolean(prefs::kMetricsReportingEnabled, true);
+        local_state_->SetBoolean(prefs::kMetricsReportingEnabled, true);
 #endif  // OS_POSIX
-    }  // if (!first_run_ui_bypass)
+    }  // if (!first_run_ui_bypass_)
 
     Browser::SetNewHomePagePrefs(profile_->GetPrefs());
     g_browser_process->profile_manager()->OnImportFinished(profile_);
   }  // if (is_first_run)
 
 #if defined(OS_WIN)
   // Sets things up so that if we crash from this point on, a dialog will
   // popup asking the user to restart chrome. It is done this late to avoid
   // testing against a bunch of special cases that are taken care early on.
   ChromeBrowserMainPartsWin::PrepareRestartOnCrashEnviroment(
@@ skipping 98 matching lines @@
 
 #if defined(OS_WIN) && !defined(GOOGLE_CHROME_BUILD)
   if (parsed_command_line().HasSwitch(switches::kDebugPrint)) {
     FilePath path =
         parsed_command_line().GetSwitchValuePath(switches::kDebugPrint);
     printing::PrintedDocument::set_debug_dump_path(path);
   }
 #endif
 
   HandleTestParameters(parsed_command_line());
-  RecordBreakpadStatusUMA(metrics);
-  about_flags::RecordUMAStatistics(local_state);
+  RecordBreakpadStatusUMA(metrics_);
+  about_flags::RecordUMAStatistics(local_state_);
   LanguageUsageMetrics::RecordAcceptLanguages(
       profile_->GetPrefs()->GetString(prefs::kAcceptLanguages));
   LanguageUsageMetrics::RecordApplicationLanguage(
       g_browser_process->GetApplicationLocale());
 
 #if defined(OS_WIN)
   fragmentation_checker::RecordFragmentationMetricForCurrentModule();
 #endif
 
 #if defined(OS_CHROMEOS)
-  metrics->StartExternalMetrics();
+  metrics_->StartExternalMetrics();
 
   // Initialize the audio handler on ChromeOS.
   chromeos::AudioHandler::Initialize();
 
   // Listen for system key events so that the user will be able to adjust the
   // volume on the login screen, if Chrome is running on Chrome OS
   // (i.e. not Linux desktop), and in non-test mode.
   if (chromeos::system::runtime_environment::IsRunningOnChromeOS() &&
       !parameters().ui_task) {  // ui_task is non-NULL when running tests.
     chromeos::SystemKeyEventListener::Initialize();
@@ skipping 12 matching lines @@
       return content::RESULT_CODE_NORMAL_EXIT;
     return chrome::RESULT_CODE_UNINSTALL_EXTENSION_ERROR;
   }
 
   // Start watching for a hang.
   MetricsService::LogNeedForCleanShutdown();
 
 #if defined(OS_WIN)
   // We check this here because if the profile is OTR (chromeos possibility)
   // it won't still be accessible after browser is destroyed.
-  record_search_engine_ = is_first_run && !profile_->IsOffTheRecord();
+  record_search_engine_ = is_first_run_ && !profile_->IsOffTheRecord();
 #endif
 
   // ChildProcess:: is a misnomer unless you consider context.  Use
   // of --wait-for-debugger only makes sense when Chrome itself is a
   // child process (e.g. when launched by PyAuto).
   if (parsed_command_line().HasSwitch(switches::kWaitForDebugger)) {
     ChildProcess::WaitForDebugger("Browser");
   }
 
 #if defined(OS_CHROMEOS)
@@ skipping 42 matching lines @@
     parameters().ui_task->Run();
     delete parameters().ui_task;
     run_message_loop_ = false;
   } else {
     // Most general initialization is behind us, but opening a
     // tab and/or session restore and such is still to be done.
     base::TimeTicks browser_open_start = base::TimeTicks::Now();
 
     // We are in regular browser boot sequence. Open initial tabs and enter the
     // main message loop.
+    int result_code;
     if (browser_init_->Start(parsed_command_line(), FilePath(), profile_,
-                             &result_code_)) {
-#if defined(OS_WIN) || (defined(OS_LINUX) && !defined(OS_CHROMEOS))
+                             &result_code)) {
+#if (defined(OS_WIN) || defined(OS_LINUX)) && !defined(OS_CHROMEOS)
       // Initialize autoupdate timer. Timer callback costs basically nothing
       // when browser is not in persistent mode, so it's OK to let it ride on
       // the main thread. This needs to be done here because we don't want
       // to start the timer when Chrome is run inside a test harness.
       g_browser_process->StartAutoupdateTimer();
 #endif
 
 #if defined(OS_LINUX) && !defined(OS_CHROMEOS)
       // On Linux, the running exe will be updated if an upgrade becomes
       // available while the browser is running.  We need to save the last
@@ skipping 150 matching lines @@
   // The XInput2 event listener needs to be shut down earlier than when
   // Singletons are finally destroyed in AtExitManager.
   chromeos::XInputHierarchyChangedEventListener::GetInstance()->Stop();
 
   // chromeos::SystemKeyEventListener::Shutdown() is always safe to call,
   // even if Initialize() wasn't called.
   chromeos::SystemKeyEventListener::Shutdown();
   chromeos::AudioHandler::Shutdown();
 #endif
 
+  restart_last_session_ = browser_shutdown::ShutdownPreThreadsStop();
+  browser_process_->StartTearDown();
+}
+
+void ChromeBrowserMainParts::PreStopThread(BrowserThread::ID identifier) {
+  browser_process_->PreStopThread(identifier);
+}
+
+void ChromeBrowserMainParts::PostStopThread(BrowserThread::ID identifier) {
+  browser_process_->PostStopThread(identifier);
+}
+
+void ChromeBrowserMainParts::FinalCleanup() {
   // browser_shutdown takes care of deleting browser_process, so we need to
   // release it.
   ignore_result(browser_process_.release());

[robertshield, 2011/11/14 04:15:55]

This isn't new to this CL, but why does ChromeBrowserMainParts bother having a
scoped_ptr on the BrowserProcessImpl instance?

Afaict, ChromeBrowserMainParts is never responsible for deleting the
BrowserProcessImpl instance, making the ownership implied by the scoped_ptr
misleading at best.

In fact, it looks like it would be an error for browser_process_ to be destroyed
as a result of this ChromeBrowserMainParts instance being destroyed without
FinalCleanup() having been called. (Maybe a CHECK in ~ChromeBrowserMainParts()
to ensure that FinalCleanup HAS been called?)

Perhaps browser_process_ should be removed and existing usages replaced with 
g_browser_process?

-  browser_shutdown::Shutdown();
+  browser_shutdown::ShutdownPostThreadsStop(restart_last_session_);
   master_prefs_.reset();
   process_singleton_.reset();
 
   // We need to do this check as late as possible, but due to modularity, this
   // may be the last point in Chrome.  This would be more effective if done at
   // a higher level on the stack, so that it is impossible for an early return
   // to bypass this code.  Perhaps we need a *final* hook that is called on all
   // paths from content/browser/browser_main.
   CHECK(MetricsService::UmaMetricsProperlyShutdown());
 }
@@ skipping 18 matching lines @@
   if (env->GetVar(chrome::kPreReadEnvironmentVariable, &pre_read) &&
       (pre_read == "0" || pre_read == "1")) {
     std::string uma_name(name);
     uma_name += "_PreRead";
     uma_name += pre_read == "1" ? "Enabled" : "Disabled";
     AddPreReadHistogramTime(uma_name.c_str(), time);
   }
 #endif
 #endif
 }