diff options
37 files changed, 955 insertions, 508 deletions
diff --git a/chrome/browser/browser_process_impl.cc b/chrome/browser/browser_process_impl.cc index 27ccef7..e84a219 100644 --- a/chrome/browser/browser_process_impl.cc +++ b/chrome/browser/browser_process_impl.cc @@ -68,7 +68,7 @@ #include "chrome/common/switch_utils.h" #include "chrome/common/url_constants.h" #include "chrome/installer/util/google_update_constants.h" -#include "content/browser/browser_process_sub_thread.h" +#include "content/browser/browser_child_process_host.h" #include "content/browser/child_process_security_policy.h" #include "content/browser/download/download_file_manager.h" #include "content/browser/download/download_status_updater.h" @@ -124,15 +124,7 @@ using content::BrowserThread; BrowserProcessImpl::BrowserProcessImpl(const CommandLine& command_line) : created_resource_dispatcher_host_(false), created_metrics_service_(false), - created_io_thread_(false), - created_file_thread_(false), - created_db_thread_(false), - created_process_launcher_thread_(false), - created_cache_thread_(false), created_watchdog_thread_(false), -#if defined(OS_CHROMEOS) - created_web_socket_proxy_thread_(false), -#endif created_profile_manager_(false), created_local_state_(false), created_icon_manager_(false), @@ -165,12 +157,20 @@ BrowserProcessImpl::BrowserProcessImpl(const CommandLine& command_line) } BrowserProcessImpl::~BrowserProcessImpl() { -#if defined(OS_CHROMEOS) - if (web_socket_proxy_thread_.get()) - chromeos::WebSocketProxyController::Shutdown(); - web_socket_proxy_thread_.reset(); -#endif + // See StartTearDown and PreStopThread functions below, this is where + // most destruction happens so that it can be interleaved with threads + // going away. + + // Wait for the pending print jobs to finish. + print_job_manager_->OnQuit(); + print_job_manager_.reset(); + tracked_objects::ThreadData::EnsureCleanupWasCalled(4); + + g_browser_process = NULL; +} + +void BrowserProcessImpl::StartTearDown() { // Delete the AutomationProviderList before NotificationService, // since it may try to unregister notifications // Both NotificationService and AutomationProvider are singleton instances in @@ -239,49 +239,65 @@ BrowserProcessImpl::~BrowserProcessImpl() { // Stop the watchdog thread before stopping other threads. watchdog_thread_.reset(); +} - // Need to stop io_thread_ before resource_dispatcher_host_, since - // io_thread_ may still deref ResourceDispatcherHost and handle resource - // request before going away. - io_thread_.reset(); - - // The IO thread was the only user of this thread. - cache_thread_.reset(); - - // Stop the process launcher thread after the IO thread, in case the IO thread - // posted a task to terminate a process on the process launcher thread. - process_launcher_thread_.reset(); +void BrowserProcessImpl::PreStartThread(BrowserThread::ID thread_id) { + switch (thread_id) { + case BrowserThread::IO: + CreateIOThreadState(); + break; - // Clean up state that lives on the file_thread_ before it goes away. - if (resource_dispatcher_host_.get()) { - resource_dispatcher_host()->download_file_manager()->Shutdown(); - resource_dispatcher_host()->save_file_manager()->Shutdown(); + default: + break; } +} - // Need to stop the file_thread_ here to force it to process messages in its - // message loop from the previous call to shutdown the DownloadFileManager, - // SaveFileManager and SessionService. - file_thread_.reset(); - - // With the file_thread_ flushed, we can release any icon resources. - icon_manager_.reset(); - - // Need to destroy ResourceDispatcherHost before PluginService and - // SafeBrowsingService, since it caches a pointer to it. This also - // causes the webkit thread to terminate. - resource_dispatcher_host_.reset(); - - // Wait for the pending print jobs to finish. - print_job_manager_->OnQuit(); - print_job_manager_.reset(); - - // Destroy TabCloseableStateWatcher before NotificationService since the - // former registers for notifications. - tab_closeable_state_watcher_.reset(); +void BrowserProcessImpl::PostStartThread(BrowserThread::ID thread_id) { +} - tracked_objects::ThreadData::EnsureCleanupWasCalled(4); +void BrowserProcessImpl::PreStopThread(BrowserThread::ID thread_id) { + switch (thread_id) { +#if defined(OS_CHROMEOS) + case BrowserThread::WEB_SOCKET_PROXY: + chromeos::WebSocketProxyController::Shutdown(); + break; +#endif + case BrowserThread::FILE: + // Clean up state that lives on or uses the file_thread_ before + // it goes away. + if (resource_dispatcher_host_.get()) { + resource_dispatcher_host()->download_file_manager()->Shutdown(); + resource_dispatcher_host()->save_file_manager()->Shutdown(); + } + break; + case BrowserThread::WEBKIT: + // Need to destroy ResourceDispatcherHost before PluginService + // and SafeBrowsingService, since it caches a pointer to + // it. This also causes the webkit thread to terminate (which is + // still the responsibility of the embedder, not of the content + // framework). + resource_dispatcher_host_.reset(); + break; + default: + break; + } +} - g_browser_process = NULL; +void BrowserProcessImpl::PostStopThread(BrowserThread::ID thread_id) { + switch (thread_id) { + case BrowserThread::FILE: + // With the file_thread_ flushed, we can release any icon resources. + icon_manager_.reset(); + break; + case BrowserThread::IO: + // Reset associated state right after actual thread is stopped, + // as io_thread_.global_ cleanup happens in CleanUp on the IO + // thread, i.e. as the thread exits its message loop. + io_thread_.reset(); + break; + default: + break; + } } #if defined(OS_WIN) @@ -395,37 +411,28 @@ MetricsService* BrowserProcessImpl::metrics_service() { IOThread* BrowserProcessImpl::io_thread() { DCHECK(CalledOnValidThread()); - if (!created_io_thread_) - CreateIOThread(); + DCHECK(io_thread_.get()); return io_thread_.get(); } base::Thread* BrowserProcessImpl::file_thread() { DCHECK(CalledOnValidThread()); - if (!created_file_thread_) - CreateFileThread(); - return file_thread_.get(); + return BrowserThread::UnsafeGetBrowserThread(BrowserThread::FILE); } base::Thread* BrowserProcessImpl::db_thread() { DCHECK(CalledOnValidThread()); - if (!created_db_thread_) - CreateDBThread(); - return db_thread_.get(); + return BrowserThread::UnsafeGetBrowserThread(BrowserThread::DB); } base::Thread* BrowserProcessImpl::process_launcher_thread() { DCHECK(CalledOnValidThread()); - if (!created_process_launcher_thread_) - CreateProcessLauncherThread(); - return process_launcher_thread_.get(); + return BrowserThread::UnsafeGetBrowserThread(BrowserThread::PROCESS_LAUNCHER); } base::Thread* BrowserProcessImpl::cache_thread() { DCHECK(CalledOnValidThread()); - if (!created_cache_thread_) - CreateCacheThread(); - return cache_thread_.get(); + return BrowserThread::UnsafeGetBrowserThread(BrowserThread::CACHE); } WatchDogThread* BrowserProcessImpl::watchdog_thread() { @@ -439,10 +446,7 @@ WatchDogThread* BrowserProcessImpl::watchdog_thread() { #if defined(OS_CHROMEOS) base::Thread* BrowserProcessImpl::web_socket_proxy_thread() { DCHECK(CalledOnValidThread()); - if (!created_web_socket_proxy_thread_) - CreateWebSocketProxyThread(); - DCHECK(web_socket_proxy_thread_.get() != NULL); - return web_socket_proxy_thread_.get(); + return BrowserThread::UnsafeGetBrowserThread(BrowserThread::WEB_SOCKET_PROXY); } #endif @@ -642,7 +646,10 @@ safe_browsing::ClientSideDetectionService* } bool BrowserProcessImpl::plugin_finder_disabled() const { - return *plugin_finder_disabled_pref_; + if (plugin_finder_disabled_pref_.get()) + return plugin_finder_disabled_pref_->GetValue(); + else + return false; } void BrowserProcessImpl::Observe(int type, @@ -757,14 +764,11 @@ void BrowserProcessImpl::CreateMetricsService() { metrics_service_.reset(new MetricsService); } -void BrowserProcessImpl::CreateIOThread() { - DCHECK(!created_io_thread_ && io_thread_.get() == NULL); - created_io_thread_ = true; - - // Prior to starting the io thread, we create the plugin service as - // it is predominantly used from the io thread, but must be created - // on the main thread. The service ctor is inexpensive and does not - // invoke the io_thread() accessor. +void BrowserProcessImpl::CreateIOThreadState() { + // Prior to any processing happening on the io thread, we create the + // plugin service as it is predominantly used from the io thread, + // but must be created on the main thread. The service ctor is + // inexpensive and does not invoke the io_thread() accessor. PluginService* plugin_service = PluginService::GetInstance(); plugin_service->Init(); plugin_service->set_filter(ChromePluginServiceFilter::GetInstance()); @@ -789,83 +793,9 @@ void BrowserProcessImpl::CreateIOThread() { scoped_ptr<IOThread> thread(new IOThread( local_state(), net_log_.get(), extension_event_router_forwarder_.get())); - base::Thread::Options options; - options.message_loop_type = MessageLoop::TYPE_IO; - if (!thread->StartWithOptions(options)) - return; io_thread_.swap(thread); } -void BrowserProcessImpl::CreateFileThread() { - DCHECK(!created_file_thread_ && file_thread_.get() == NULL); - created_file_thread_ = true; - - scoped_ptr<base::Thread> thread( - new content::BrowserProcessSubThread(BrowserThread::FILE)); - base::Thread::Options options; -#if defined(OS_WIN) - // On Windows, the FILE thread needs to be have a UI message loop which pumps - // messages in such a way that Google Update can communicate back to us. - options.message_loop_type = MessageLoop::TYPE_UI; -#else - options.message_loop_type = MessageLoop::TYPE_IO; -#endif - if (!thread->StartWithOptions(options)) - return; - file_thread_.swap(thread); -} - -#if defined(OS_CHROMEOS) -void BrowserProcessImpl::CreateWebSocketProxyThread() { - DCHECK(!created_web_socket_proxy_thread_); - DCHECK(web_socket_proxy_thread_.get() == NULL); - created_web_socket_proxy_thread_ = true; - - scoped_ptr<base::Thread> thread( - new content::BrowserProcessSubThread(BrowserThread::WEB_SOCKET_PROXY)); - base::Thread::Options options; - options.message_loop_type = MessageLoop::TYPE_IO; - if (!thread->StartWithOptions(options)) - return; - web_socket_proxy_thread_.swap(thread); -} -#endif - -void BrowserProcessImpl::CreateDBThread() { - DCHECK(!created_db_thread_ && db_thread_.get() == NULL); - created_db_thread_ = true; - - scoped_ptr<base::Thread> thread( - new content::BrowserProcessSubThread(BrowserThread::DB)); - if (!thread->Start()) - return; - db_thread_.swap(thread); -} - -void BrowserProcessImpl::CreateProcessLauncherThread() { - DCHECK(!created_process_launcher_thread_ && !process_launcher_thread_.get()); - created_process_launcher_thread_ = true; - - scoped_ptr<base::Thread> thread( - new content::BrowserProcessSubThread(BrowserThread::PROCESS_LAUNCHER)); - if (!thread->Start()) - return; - process_launcher_thread_.swap(thread); -} - -void BrowserProcessImpl::CreateCacheThread() { - DCHECK(!created_cache_thread_ && !cache_thread_.get()); - created_cache_thread_ = true; - - scoped_ptr<base::Thread> thread( - new content::DeprecatedBrowserThread(BrowserThread::CACHE)); - base::Thread::Options options; - options.message_loop_type = MessageLoop::TYPE_IO; - if (!thread->StartWithOptions(options)) - return; - cache_thread_.swap(thread); -} - void BrowserProcessImpl::CreateWatchdogThread() { DCHECK(!created_watchdog_thread_ && watchdog_thread_.get() == NULL); created_watchdog_thread_ = true; @@ -911,9 +841,10 @@ void BrowserProcessImpl::CreateLocalState() { // Initialize the preference for the plugin finder policy. // This preference is only needed on the IO thread so make it available there. local_state_->RegisterBooleanPref(prefs::kDisablePluginFinder, false); - plugin_finder_disabled_pref_.Init(prefs::kDisablePluginFinder, + plugin_finder_disabled_pref_.reset(new BooleanPrefMember); + plugin_finder_disabled_pref_->Init(prefs::kDisablePluginFinder, local_state_.get(), NULL); - plugin_finder_disabled_pref_.MoveToThread(BrowserThread::IO); + plugin_finder_disabled_pref_->MoveToThread(BrowserThread::IO); // Another policy that needs to be defined before the net subsystem is // initialized is MaxConnectionsPerProxy so we do it here. diff --git a/chrome/browser/browser_process_impl.h b/chrome/browser/browser_process_impl.h index 46d4d52..2227ab0 100644 --- a/chrome/browser/browser_process_impl.h +++ b/chrome/browser/browser_process_impl.h @@ -21,6 +21,7 @@ #include "chrome/browser/browser_process.h" #include "chrome/browser/prefs/pref_change_registrar.h" #include "chrome/browser/prefs/pref_member.h" +#include "content/public/browser/browser_thread.h" #include "content/public/browser/notification_observer.h" #include "content/public/browser/notification_registrar.h" #include "ipc/ipc_message.h" @@ -29,6 +30,7 @@ class BrowserOnlineStateObserver; class ChromeNetLog; class ChromeResourceDispatcherHostDelegate; class CommandLine; +class ChromeFrameFriendOfBrowserProcessImpl; // TODO(joi): Remove class RemoteDebuggingServer; class TabCloseableStateWatcher; @@ -44,6 +46,19 @@ class BrowserProcessImpl : public BrowserProcess, explicit BrowserProcessImpl(const CommandLine& command_line); virtual ~BrowserProcessImpl(); + // Some of our startup is interleaved with thread creation, driven + // by these functions. + void PreStartThread(content::BrowserThread::ID identifier); + void PostStartThread(content::BrowserThread::ID identifier); + + // Most cleanup is done by these functions, driven from + // ChromeBrowserMain based on notifications from the content + // framework, rather than in the destructor, so that we can + // interleave cleanup with threads being stopped. + void StartTearDown(); + void PreStopThread(content::BrowserThread::ID identifier); + void PostStopThread(content::BrowserThread::ID identifier); + base::Thread* process_launcher_thread(); // BrowserProcess methods @@ -117,20 +132,18 @@ class BrowserProcessImpl : public BrowserProcess, virtual CRLSetFetcher* crl_set_fetcher() OVERRIDE; private: + // TODO(joi): Remove. Temporary hack to get at CreateIOThreadState. + friend class ChromeFrameFriendOfBrowserProcessImpl; + + // Must be called right before the IO thread is started. + void CreateIOThreadState(); + void CreateResourceDispatcherHost(); void CreateMetricsService(); - void CreateIOThread(); - static void CleanupOnIOThread(); - - void CreateFileThread(); - void CreateDBThread(); - void CreateProcessLauncherThread(); - void CreateCacheThread(); - void CreateGpuThread(); void CreateWatchdogThread(); #if defined(OS_CHROMEOS) - void CreateWebSocketProxyThread(); + void InitializeWebSocketProxyThread(); #endif void CreateTemplateURLService(); void CreateProfileManager(); @@ -161,29 +174,11 @@ class BrowserProcessImpl : public BrowserProcess, bool created_metrics_service_; scoped_ptr<MetricsService> metrics_service_; - bool created_io_thread_; scoped_ptr<IOThread> io_thread_; - bool created_file_thread_; - scoped_ptr<base::Thread> file_thread_; - - bool created_db_thread_; - scoped_ptr<base::Thread> db_thread_; - - bool created_process_launcher_thread_; - scoped_ptr<base::Thread> process_launcher_thread_; - - bool created_cache_thread_; - scoped_ptr<base::Thread> cache_thread_; - bool created_watchdog_thread_; scoped_ptr<WatchDogThread> watchdog_thread_; -#if defined(OS_CHROMEOS) - bool created_web_socket_proxy_thread_; - scoped_ptr<base::Thread> web_socket_proxy_thread_; -#endif - bool created_profile_manager_; scoped_ptr<ProfileManager> profile_manager_; @@ -270,7 +265,7 @@ class BrowserProcessImpl : public BrowserProcess, scoped_refptr<MHTMLGenerationManager> mhtml_generation_manager_; // Monitors the state of the 'DisablePluginFinder' policy. - BooleanPrefMember plugin_finder_disabled_pref_; + scoped_ptr<BooleanPrefMember> plugin_finder_disabled_pref_; #if (defined(OS_WIN) || defined(OS_LINUX)) && !defined(OS_CHROMEOS) base::RepeatingTimer<BrowserProcessImpl> autoupdate_timer_; diff --git a/chrome/browser/browser_shutdown.cc b/chrome/browser/browser_shutdown.cc index 5aa4b66..044b505 100644 --- a/chrome/browser/browser_shutdown.cc +++ b/chrome/browser/browser_shutdown.cc @@ -114,7 +114,7 @@ FilePath GetShutdownMsPath() { return shutdown_ms_file.AppendASCII(kShutdownMsFile); } -void Shutdown() { +bool ShutdownPreThreadsStop() { #if defined(OS_CHROMEOS) chromeos::BootTimesLoader::Get()->AddLogoutTimeMarker( "BrowserShutdownStarted", false); @@ -162,6 +162,10 @@ void Shutdown() { RLZTracker::CleanupRlz(); #endif + return restart_last_session; +} + +void ShutdownPostThreadsStop(bool restart_last_session) { // The jank'o'meter requires that the browser process has been destroyed // before calling UninstallJankometer(). delete g_browser_process; diff --git a/chrome/browser/browser_shutdown.h b/chrome/browser/browser_shutdown.h index b567b5d..78481b8 100644 --- a/chrome/browser/browser_shutdown.h +++ b/chrome/browser/browser_shutdown.h @@ -34,13 +34,18 @@ void OnShutdownStarting(ShutdownType type); // Get the current shutdown type. ShutdownType GetShutdownType(); -// Invoked in two ways: -// . When the last browser has been deleted and the message loop has finished -// running. -// . When ChromeFrame::EndSession is invoked and we need to do cleanup. -// NOTE: in this case the message loop is still running, but will die soon -// after this returns. -void Shutdown(); +// Performs the shutdown tasks that need to be done before +// BrowserProcess and the various threads go away. +// +// Returns true if the session should be restarted. +bool ShutdownPreThreadsStop(); + +// Performs the remaining shutdown tasks after all threads but the +// main thread have been stopped. This includes deleting g_browser_process. +// +// The provided parameter indicates whether a preference to restart +// the session was present. +void ShutdownPostThreadsStop(bool restart_last_session); // Called at startup to create a histogram from our previous shutdown time. void ReadLastShutdownInfo(); diff --git a/chrome/browser/chrome_browser_main.cc b/chrome/browser/chrome_browser_main.cc index 614da15..c0cf773 100644 --- a/chrome/browser/chrome_browser_main.cc +++ b/chrome/browser/chrome_browser_main.cc @@ -76,7 +76,6 @@ #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" @@ -311,23 +310,6 @@ void InitializeURLRequestThrottlerManager(net::NetLog* net_log) { 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) { @@ -687,7 +669,12 @@ ChromeBrowserMainParts::ChromeBrowserMainParts( 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; @@ -1207,31 +1194,55 @@ void ChromeBrowserMainParts::PostMainMessageLoopStart() { chrome_extra_parts_[i]->PostMainMessageLoopStart(); } -void ChromeBrowserMainParts::PreMainMessageLoopRun() { - result_code_ = PreMainMessageLoopRunImpl(); +void ChromeBrowserMainParts::PreCreateThreads() { + result_code_ = PreCreateThreadsImpl(); for (size_t i = 0; i < chrome_extra_parts_.size(); ++i) chrome_extra_parts_[i]->PreMainMessageLoopRun(); } -int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() { +void ChromeBrowserMainParts::PreStartThread( + content::BrowserThread::ID thread_id) { + browser_process_->PreStartThread(thread_id); +} + +void ChromeBrowserMainParts::PostStartThread( + content::BrowserThread::ID thread_id) { + browser_process_->PostStartThread(thread_id); + switch (thread_id) { + case BrowserThread::FILE: + // Now the command line has been mutated based on about:flags, + // and the file thread has been started, we can set up metrics + // and initialize field trials. + metrics_ = SetupMetricsAndFieldTrials(local_state_); + break; + + default: + break; + } +} + +void ChromeBrowserMainParts::PreMainMessageLoopRun() { + result_code_ = PreMainMessageLoopRunImpl(); +} + +int ChromeBrowserMainParts::PreCreateThreadsImpl() { 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 (parsed_command_line().HasSwitch(switches::kImport) || @@ -1240,7 +1251,7 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() { // 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())); } @@ -1258,8 +1269,8 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() { // 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_); #if defined(USE_LINUX_BREAKPAD) // Needs to be called after we have chrome::DIR_USER_DATA and @@ -1267,7 +1278,7 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() { g_browser_process->file_thread()->message_loop()->PostTask(FROM_HERE, new GetLinuxDistroTask()); - if (IsCrashReportingEnabled(local_state)) + if (IsCrashReportingEnabled(local_state_)) InitCrashReporter(); #endif @@ -1283,7 +1294,7 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() { 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 = @@ -1343,19 +1354,19 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() { // 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? @@ -1364,17 +1375,17 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() { // 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 // 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 @@ -1391,10 +1402,6 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() { 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); - #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. @@ -1405,9 +1412,9 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() { // 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()); } @@ -1421,7 +1428,13 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() { SecKeychainAddCallback(&KeychainCallback, 0, NULL); #endif - CreateChildThreads(browser_process_.get()); + return content::RESULT_CODE_NORMAL_EXIT; +} + +int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() { + // 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. @@ -1571,13 +1584,13 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() { } #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; @@ -1661,8 +1674,8 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() { // 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, @@ -1678,9 +1691,9 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() { // 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_); @@ -1799,8 +1812,8 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() { #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( @@ -1811,7 +1824,7 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() { #endif #if defined(OS_CHROMEOS) - metrics->StartExternalMetrics(); + metrics_->StartExternalMetrics(); // Initialize the audio handler on ChromeOS. chromeos::AudioHandler::Initialize(); @@ -1844,7 +1857,7 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() { #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 @@ -1900,8 +1913,9 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() { // 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_)) { + &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 @@ -2068,10 +2082,23 @@ void ChromeBrowserMainParts::PostMainMessageLoopRun() { 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::PostDestroyThreads() { // browser_shutdown takes care of deleting browser_process, so we need to // release it. ignore_result(browser_process_.release()); - browser_shutdown::Shutdown(); + browser_shutdown::ShutdownPostThreadsStop(restart_last_session_); master_prefs_.reset(); process_singleton_.reset(); diff --git a/chrome/browser/chrome_browser_main.h b/chrome/browser/chrome_browser_main.h index 7f40f35..5ef3eb9 100644 --- a/chrome/browser/chrome_browser_main.h +++ b/chrome/browser/chrome_browser_main.h @@ -14,7 +14,9 @@ #include "base/tracked_objects.h" #include "chrome/browser/first_run/first_run.h" #include "chrome/browser/process_singleton.h" +#include "chrome/browser/ui/browser_init.h" #include "content/public/browser/browser_main_parts.h" +#include "content/public/browser/browser_thread.h" class BrowserInit; class BrowserProcessImpl; @@ -58,9 +60,15 @@ class ChromeBrowserMainParts : public content::BrowserMainParts { virtual void ToolkitInitialized() OVERRIDE; virtual void PreMainMessageLoopStart() OVERRIDE; virtual void PostMainMessageLoopStart() OVERRIDE; + virtual void PreCreateThreads() OVERRIDE; + virtual void PreStartThread(content::BrowserThread::ID identifier) OVERRIDE; + virtual void PostStartThread(content::BrowserThread::ID identifier) OVERRIDE; virtual void PreMainMessageLoopRun() OVERRIDE; virtual bool MainMessageLoopRun(int* result_code) OVERRIDE; virtual void PostMainMessageLoopRun() OVERRIDE; + virtual void PreStopThread(content::BrowserThread::ID identifier) OVERRIDE; + virtual void PostStopThread(content::BrowserThread::ID identifier) OVERRIDE; + virtual void PostDestroyThreads() OVERRIDE; // Displays a warning message that we can't find any locale data files. virtual void ShowMissingLocaleMessageBox() = 0; @@ -121,6 +129,7 @@ class ChromeBrowserMainParts : public content::BrowserMainParts { // Methods for Main Message Loop ------------------------------------------- + int PreCreateThreadsImpl(); int PreMainMessageLoopRunImpl(); // Members initialized on construction --------------------------------------- @@ -165,6 +174,17 @@ class ChromeBrowserMainParts : public content::BrowserMainParts { // Initialized in SetupMetricsAndFieldTrials. scoped_refptr<FieldTrialSynchronizer> field_trial_synchronizer_; + // Members initialized in PreMainMessageLoopRun, needed in + // PreMainMessageLoopRunThreadsCreated. + bool is_first_run_; + bool first_run_ui_bypass_; + MetricsService* metrics_; + PrefService* local_state_; + FilePath user_data_dir_; + + // Members needed across shutdown methods. + bool restart_last_session_; + FRIEND_TEST_ALL_PREFIXES(BrowserMainTest, WarmConnectionFieldTrial_WarmestSocket); FRIEND_TEST_ALL_PREFIXES(BrowserMainTest, WarmConnectionFieldTrial_Random); diff --git a/chrome/browser/chromeos/input_method/xkeyboard.cc b/chrome/browser/chromeos/input_method/xkeyboard.cc index e3379b2..c69c211 100644 --- a/chrome/browser/chromeos/input_method/xkeyboard.cc +++ b/chrome/browser/chromeos/input_method/xkeyboard.cc @@ -9,9 +9,6 @@ #include <string> #include <utility> -#include <X11/XKBlib.h> -#include <X11/Xlib.h> -#include <glib.h> #include <stdlib.h> #include <string.h> @@ -25,6 +22,11 @@ #include "content/public/browser/browser_thread.h" #include "ui/base/x/x11_util.h" +// These includes conflict with base/tracked_objects.h so must come last. +#include <X11/XKBlib.h> +#include <X11/Xlib.h> +#include <glib.h> + using content::BrowserThread; namespace chromeos { diff --git a/chrome/browser/chromeos/login/login_utils_browsertest.cc b/chrome/browser/chromeos/login/login_utils_browsertest.cc index 85b15a4..72257ed 100644 --- a/chrome/browser/chromeos/login/login_utils_browsertest.cc +++ b/chrome/browser/chromeos/login/login_utils_browsertest.cc @@ -30,6 +30,7 @@ #include "chrome/common/net/gaia/gaia_auth_consumer.h" #include "chrome/test/base/testing_browser_process.h" #include "chrome/test/base/testing_pref_service.h" +#include "content/public/browser/browser_thread.h" #include "content/public/common/url_fetcher_delegate.h" #include "content/test/test_browser_thread.h" #include "content/test/test_url_fetcher_factory.h" @@ -48,6 +49,7 @@ using ::testing::DoAll; using ::testing::Return; using ::testing::SetArgPointee; using ::testing::_; +using content::BrowserThread; const char kTrue[] = "true"; const char kDomain[] = "domain.com"; @@ -74,20 +76,19 @@ ACTION_P(MockSessionManagerClientPolicyCallback, policy) { arg0.Run(policy); } -// Subclass IOThread to expose set_message_loop. -class TestIOThread : public IOThread { - public: - explicit TestIOThread(PrefService* local_state) - : IOThread(local_state, NULL, NULL) {} - - using IOThread::set_message_loop; -}; - template<typename TESTBASE> class LoginUtilsTestBase : public TESTBASE, public LoginUtils::Delegate, public LoginStatusConsumer { public: + // Initialization here is important. The UI thread gets the test's + // message loop, as does the file thread (which never actually gets + // started - so this is a way to fake multiple threads on a single + // test thread). The IO thread does not get the message loop set, + // and is never started. This is necessary so that we skip various + // bits of initialization that get posted to the IO thread. We do + // however, at one point in the test, temporarily set the message + // loop for the IO thread. LoginUtilsTestBase() : loop_(MessageLoop::TYPE_IO), browser_process_( @@ -95,7 +96,8 @@ class LoginUtilsTestBase : public TESTBASE, local_state_(browser_process_), ui_thread_(content::BrowserThread::UI, &loop_), file_thread_(content::BrowserThread::FILE, &loop_), - io_thread_(local_state_.Get()), + io_thread_(content::BrowserThread::IO), + io_thread_state_(local_state_.Get(), NULL, NULL), prepared_profile_(NULL) {} virtual void SetUp() OVERRIDE { @@ -111,7 +113,7 @@ class LoginUtilsTestBase : public TESTBASE, local_state_.Get()->RegisterStringPref(prefs::kApplicationLocale, ""); - browser_process_->SetIOThread(&io_thread_); + browser_process_->SetIOThread(&io_thread_state_); DBusThreadManager::InitializeForTesting(&dbus_thread_manager_); @@ -169,12 +171,16 @@ class LoginUtilsTestBase : public TESTBASE, // g_browser_process->profile_manager() is valid during initialization. // Run a task on a temporary BrowserThread::IO that allows skipping // these routines. - io_thread_.set_message_loop(&loop_); + // + // It is important to not have a fake message loop on the IO + // thread for the whole test, see comment on LoginUtilsTestBase + // constructor for details. + io_thread_.DeprecatedSetMessageLoop(&loop_); loop_.PostTask(FROM_HERE, base::Bind(&LoginUtilsTestBase::TearDownOnIO, base::Unretained(this))); loop_.RunAllPending(); - io_thread_.set_message_loop(NULL); + io_thread_.DeprecatedSetMessageLoop(NULL); } // These trigger some tasks that have to run while BrowserThread::UI @@ -305,7 +311,8 @@ class LoginUtilsTestBase : public TESTBASE, content::TestBrowserThread ui_thread_; content::TestBrowserThread file_thread_; - TestIOThread io_thread_; + content::TestBrowserThread io_thread_; + IOThread io_thread_state_; MockDBusThreadManager dbus_thread_manager_; TestURLFetcherFactory test_url_fetcher_factory_; diff --git a/chrome/browser/chromeos/login/webui_screen_locker.cc b/chrome/browser/chromeos/login/webui_screen_locker.cc index 4294eeb..a6798c7 100644 --- a/chrome/browser/chromeos/login/webui_screen_locker.cc +++ b/chrome/browser/chromeos/login/webui_screen_locker.cc @@ -4,15 +4,6 @@ #include "chrome/browser/chromeos/login/webui_screen_locker.h" -#include <X11/extensions/XTest.h> -#include <X11/keysym.h> -#include <gdk/gdkkeysyms.h> -#include <gdk/gdkx.h> - -// Evil hack to undo X11 evil #define. -#undef None -#undef Status - #include "base/command_line.h" #include "base/utf_string_conversions.h" #include "base/values.h" @@ -35,6 +26,12 @@ #include "ui/gfx/screen.h" #include "ui/views/widget/native_widget_gtk.h" +// These conflict with some of the Chrome headers, so must be included last. +#include <X11/extensions/XTest.h> +#include <X11/keysym.h> +#include <gdk/gdkkeysyms.h> +#include <gdk/gdkx.h> + namespace { // URL which corresponds to the login WebUI. diff --git a/chrome/browser/io_thread.cc b/chrome/browser/io_thread.cc index 18d1d2b..17b17a4 100644 --- a/chrome/browser/io_thread.cc +++ b/chrome/browser/io_thread.cc @@ -316,7 +316,8 @@ class SystemURLRequestContextGetter : public net::URLRequestContextGetter { SystemURLRequestContextGetter::SystemURLRequestContextGetter( IOThread* io_thread) : io_thread_(io_thread), - io_message_loop_proxy_(io_thread->message_loop_proxy()) { + io_message_loop_proxy_( + BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO)) { } SystemURLRequestContextGetter::~SystemURLRequestContextGetter() {} @@ -347,8 +348,7 @@ IOThread::IOThread( PrefService* local_state, ChromeNetLog* net_log, ExtensionEventRouterForwarder* extension_event_router_forwarder) - : content::BrowserProcessSubThread(BrowserThread::IO), - net_log_(net_log), + : net_log_(net_log), extension_event_router_forwarder_(extension_event_router_forwarder), globals_(NULL), sdch_manager_(NULL), @@ -371,17 +371,17 @@ IOThread::IOThread( local_state); ssl_config_service_manager_.reset( SSLConfigServiceManager::CreateDefaultManager(local_state)); - MessageLoop::current()->PostTask( - FROM_HERE, base::Bind(&IOThread::InitSystemRequestContext, - weak_factory_.GetWeakPtr())); + + BrowserThread::SetDelegate(BrowserThread::IO, this); } IOThread::~IOThread() { + // This isn't needed for production code, but in tests, IOThread may + // be multiply constructed. + BrowserThread::SetDelegate(BrowserThread::IO, NULL); + if (pref_proxy_config_tracker_.get()) pref_proxy_config_tracker_->DetachFromPrefService(); - // We cannot rely on our base class to stop the thread since we want our - // CleanUp function to run. - Stop(); DCHECK(!globals_); } @@ -407,9 +407,7 @@ void IOThread::Init() { // messages around; it shouldn't be allowed to perform any blocking disk I/O. base::ThreadRestrictions::SetIOAllowed(false); - content::BrowserProcessSubThread::Init(); - - DCHECK_EQ(MessageLoop::TYPE_IO, message_loop()->type()); + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); #if defined(USE_NSS) net::SetMessageLoopForOCSP(); @@ -480,6 +478,26 @@ void IOThread::Init() { sdch_manager_ = new net::SdchManager(); sdch_manager_->set_sdch_fetcher(new SdchDictionaryFetcher); + + // InitSystemRequestContext turns right around and posts a task back + // to the IO thread, so we can't let it run until we know the IO + // thread has started. + // + // Note that since we are at BrowserThread::Init time, the UI thread + // is blocked waiting for the thread to start. Therefore, posting + // this task to the main thread's message loop here is guaranteed to + // get it onto the message loop while the IOThread object still + // exists. However, the message might not be processed on the UI + // thread until after IOThread is gone, so use a weak pointer. + BrowserThread::PostTask(BrowserThread::UI, + FROM_HERE, + base::Bind(&IOThread::InitSystemRequestContext, + weak_factory_.GetWeakPtr())); + + // We constructed the weak pointer on the IO thread but it will be + // used on the UI thread. Call this to avoid a thread checker + // error. + weak_factory_.DetachFromThread(); } void IOThread::CleanUp() { @@ -520,10 +538,6 @@ void IOThread::CleanUp() { base::debug::LeakTracker<net::URLRequest>::CheckForLeaks(); base::debug::LeakTracker<SystemURLRequestContextGetter>::CheckForLeaks(); - - // This will delete the |notification_service_|. Make sure it's done after - // anything else can reference it. - content::BrowserProcessSubThread::CleanUp(); } // static @@ -574,6 +588,11 @@ void IOThread::ClearHostCache() { host_cache->clear(); } +MessageLoop* IOThread::message_loop() const { + return BrowserThread::UnsafeGetBrowserThread( + BrowserThread::IO)->message_loop(); +} + net::SSLConfigService* IOThread::GetSSLConfigService() { return ssl_config_service_manager_->Get(); } @@ -593,6 +612,8 @@ void IOThread::InitSystemRequestContext() { } system_url_request_context_getter_ = new SystemURLRequestContextGetter(this); + // Safe to post an unretained this pointer, since IOThread is + // guaranteed to outlive the IO BrowserThread. message_loop()->PostTask( FROM_HERE, base::Bind(&IOThread::InitSystemRequestContextOnIOThread, base::Unretained(this))); diff --git a/chrome/browser/io_thread.h b/chrome/browser/io_thread.h index b8be74b..41be0c9 100644 --- a/chrome/browser/io_thread.h +++ b/chrome/browser/io_thread.h @@ -9,13 +9,26 @@ #include <string> #include "base/basictypes.h" +#include "base/compiler_specific.h" +#include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" #include "base/memory/weak_ptr.h" #include "chrome/browser/net/ssl_config_service_manager.h" #include "chrome/browser/prefs/pref_member.h" -#include "content/browser/browser_process_sub_thread.h" +#include "content/public/browser/browser_thread.h" +#include "content/public/browser/browser_thread_delegate.h" #include "net/base/network_change_notifier.h" +// TODO(joi): Remove these in a follow-up change and IWYU in files +// that were getting them directly or indirectly from here. +#include "base/memory/ref_counted.h" +#include "base/memory/scoped_ptr.h" +#include "base/message_loop.h" +#include "base/message_loop_proxy.h" +#include "base/synchronization/lock.h" +#include "base/threading/thread.h" + +class BrowserProcessImpl; class ChromeNetLog; class ExtensionEventRouterForwarder; class MediaInternals; @@ -43,7 +56,10 @@ class URLRequestContextGetter; class URLSecurityManager; } // namespace net -class IOThread : public content::BrowserProcessSubThread { +// Contains state associated with, initialized and cleaned up on, and +// primarily used on, the IO thread. Also acts as a convenience +// accessor to the Thread object for the IO thread. +class IOThread : public content::BrowserThreadDelegate { public: struct Globals { Globals(); @@ -109,26 +125,37 @@ class IOThread : public content::BrowserProcessSubThread { // called on the IO thread. void ClearHostCache(); - protected: + // Convenience method similar to base::Thread, giving access to the + // actual IO thread. + // TODO(joi): Remove this in follow-up changes. + MessageLoop* message_loop() const; + + private: + // BrowserThreadDelegate implementation, runs on the IO thread. + // This handles initialization and destruction of state that must + // live on the IO thread. virtual void Init() OVERRIDE; virtual void CleanUp() OVERRIDE; - private: // Provide SystemURLRequestContextGetter with access to // InitSystemRequestContext(). friend class SystemURLRequestContextGetter; - static void RegisterPrefs(PrefService* local_state); - - net::HttpAuthHandlerFactory* CreateDefaultAuthHandlerFactory( - net::HostResolver* resolver); - + // Global state must be initialized on the IO thread, then this + // method must be invoked on the UI thread. void InitSystemRequestContext(); // Lazy initialization of system request context for - // SystemURLRequestContextGetter. To be called on IO thread. + // SystemURLRequestContextGetter. To be called on IO thread only + // after global state has been initialized on the IO thread, and + // SystemRequestContext state has been initialized on the UI thread. void InitSystemRequestContextOnIOThread(); + static void RegisterPrefs(PrefService* local_state); + + net::HttpAuthHandlerFactory* CreateDefaultAuthHandlerFactory( + net::HostResolver* resolver); + // Returns an SSLConfigService instance. net::SSLConfigService* GetSSLConfigService(); diff --git a/chrome/browser/sync/tools/DEPS b/chrome/browser/sync/tools/DEPS index 877f421..5fa3c59 100644 --- a/chrome/browser/sync/tools/DEPS +++ b/chrome/browser/sync/tools/DEPS @@ -1,4 +1,6 @@ include_rules = [ + # sync_tools needs to manage its own BrowserThreads. + "+content/browser", # sync_tools depends on the google cache invalidation API. "+google/cacheinvalidation", # sync_tools depends on libjingle. diff --git a/chrome/browser/sync/tools/sync_listen_notifications.cc b/chrome/browser/sync/tools/sync_listen_notifications.cc index 91d1fed..b380daa 100644 --- a/chrome/browser/sync/tools/sync_listen_notifications.cc +++ b/chrome/browser/sync/tools/sync_listen_notifications.cc @@ -14,6 +14,7 @@ #include "base/memory/scoped_ptr.h" #include "base/memory/weak_ptr.h" #include "base/message_loop.h" +#include "base/threading/thread.h" #include "chrome/browser/sync/notifier/invalidation_version_tracker.h" #include "chrome/browser/sync/notifier/sync_notifier.h" #include "chrome/browser/sync/notifier/sync_notifier_factory.h" @@ -22,6 +23,7 @@ #include "chrome/browser/sync/syncable/model_type_payload_map.h" #include "chrome/test/base/test_url_request_context_getter.h" #include "content/public/browser/browser_thread.h" +#include "content/browser/browser_thread_impl.h" using content::BrowserThread; @@ -95,9 +97,8 @@ int main(int argc, char* argv[]) { logging::DISABLE_DCHECK_FOR_NON_OFFICIAL_RELEASE_BUILDS); MessageLoop ui_loop; - content::DeprecatedBrowserThread ui_thread(BrowserThread::UI, &ui_loop); - - content::DeprecatedBrowserThread io_thread(BrowserThread::IO); + content::BrowserThreadImpl ui_thread(BrowserThread::UI, &ui_loop); + content::BrowserThreadImpl io_thread(BrowserThread::IO); base::Thread::Options options; options.message_loop_type = MessageLoop::TYPE_IO; io_thread.StartWithOptions(options); diff --git a/chrome/browser/ui/browser_list.cc b/chrome/browser/ui/browser_list.cc index daaead6..6827139 100644 --- a/chrome/browser/ui/browser_list.cc +++ b/chrome/browser/ui/browser_list.cc @@ -20,6 +20,7 @@ #include "chrome/common/chrome_notification_types.h" #include "chrome/common/pref_names.h" #include "content/browser/tab_contents/navigation_details.h" +#include "content/public/browser/browser_shutdown.h" #include "content/public/browser/notification_registrar.h" #include "content/public/browser/notification_service.h" #include "content/public/browser/render_process_host.h" @@ -548,7 +549,12 @@ void BrowserList::ExitCleanly() { } #endif -static void TimeLimitedSessionEnding() { +// static +void BrowserList::SessionEnding() { + // This is a time-limited shutdown where we need to write as much to + // disk as we can as soon as we can, and where we must kill the + // process within a hang timeout to avoid user prompts. + // Start watching for hang during shutdown, and crash it if takes too long. // We disarm when |shutdown_watcher| object is destroyed, which is when we // exit this function. @@ -582,23 +588,8 @@ static void TimeLimitedSessionEnding() { content::NotificationService::AllSources(), content::NotificationService::NoDetails()); - // And shutdown. - browser_shutdown::Shutdown(); -} - -// static -void BrowserList::SessionEnding() { - TimeLimitedSessionEnding(); - -#if defined(OS_WIN) - // At this point the message loop is still running yet we've shut everything - // down. If any messages are processed we'll likely crash. Exit now. - ExitProcess(content::RESULT_CODE_NORMAL_EXIT); -#elif defined(OS_POSIX) && !defined(OS_MACOSX) - _exit(content::RESULT_CODE_NORMAL_EXIT); -#else - NOTIMPLEMENTED(); -#endif + // This will end by terminating the process. + content::ImmediateShutdownAndExitProcess(); } // static diff --git a/chrome/browser/ui/gtk/gtk_util.cc b/chrome/browser/ui/gtk/gtk_util.cc index b00c062..7c1c9e36 100644 --- a/chrome/browser/ui/gtk/gtk_util.cc +++ b/chrome/browser/ui/gtk/gtk_util.cc @@ -5,10 +5,9 @@ #include "chrome/browser/ui/gtk/gtk_util.h" #include <cairo/cairo.h> -#include <gdk/gdkx.h> -#include <gtk/gtk.h> #include <cstdarg> + #include <map> #include "base/environment.h" @@ -53,6 +52,10 @@ #include "chrome/browser/ui/gtk/browser_window_gtk.h" #endif +// These conflict with base/tracked_objects.h, so need to come last. +#include <gdk/gdkx.h> +#include <gtk/gtk.h> + namespace { #if defined(GOOGLE_CHROME_BUILD) diff --git a/chrome/browser/ui/gtk/select_file_dialog_impl_kde.cc b/chrome/browser/ui/gtk/select_file_dialog_impl_kde.cc index dac604b7..ca0e2cd 100644 --- a/chrome/browser/ui/gtk/select_file_dialog_impl_kde.cc +++ b/chrome/browser/ui/gtk/select_file_dialog_impl_kde.cc @@ -2,8 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include <gdk/gdkx.h> -#include <gtk/gtk.h> +#include "content/public/browser/browser_thread.h" #include <set> @@ -22,6 +21,10 @@ #include "grit/generated_resources.h" #include "ui/base/l10n/l10n_util.h" +// These conflict with base/tracked_objects.h, so need to come last. +#include <gdk/gdkx.h> +#include <gtk/gtk.h> + using content::BrowserThread; namespace { diff --git a/chrome/tools/profiles/generate_profile.cc b/chrome/tools/profiles/generate_profile.cc index adebcd9..51c0f9c 100644 --- a/chrome/tools/profiles/generate_profile.cc +++ b/chrome/tools/profiles/generate_profile.cc @@ -23,6 +23,7 @@ #include "chrome/common/chrome_paths.h" #include "chrome/common/thumbnail_score.h" #include "chrome/test/base/testing_profile.h" +#include "content/browser/browser_thread_impl.h" #include "content/browser/notification_service_impl.h" #include "content/public/browser/browser_thread.h" #include "third_party/skia/include/core/SkBitmap.h" @@ -236,8 +237,8 @@ int main(int argc, const char* argv[]) { ResourceBundle::InitSharedInstance("en-US"); NotificationServiceImpl notification_service; MessageLoopForUI message_loop; - content::DeprecatedBrowserThread ui_thread(BrowserThread::UI, &message_loop); - content::DeprecatedBrowserThread db_thread(BrowserThread::DB, &message_loop); + content::BrowserThreadImpl ui_thread(BrowserThread::UI, &message_loop); + content::BrowserThreadImpl db_thread(BrowserThread::DB, &message_loop); TestingProfile profile; profile.CreateHistoryService(false, false); if (types & TOP_SITES) { diff --git a/chrome_frame/chrome_frame.gyp b/chrome_frame/chrome_frame.gyp index f1eb15f..c14440b 100644 --- a/chrome_frame/chrome_frame.gyp +++ b/chrome_frame/chrome_frame.gyp @@ -380,6 +380,7 @@ '../chrome/chrome.gyp:syncapi_core', '../chrome/chrome_resources.gyp:chrome_resources', '../content/content.gyp:content_gpu', + '../content/content.gyp:test_support_content', '../net/net.gyp:net', '../net/net.gyp:net_test_support', '../skia/skia.gyp:skia', diff --git a/chrome_frame/test/net/fake_external_tab.cc b/chrome_frame/test/net/fake_external_tab.cc index b33ad1e..5a7d371 100644 --- a/chrome_frame/test/net/fake_external_tab.cc +++ b/chrome_frame/test/net/fake_external_tab.cc @@ -23,10 +23,12 @@ #include "base/system_monitor/system_monitor.h" #include "base/test/test_timeouts.h" #include "base/threading/platform_thread.h" +#include "base/threading/thread.h" #include "base/win/scoped_com_initializer.h" #include "base/win/scoped_comptr.h" #include "base/win/scoped_handle.h" #include "chrome/browser/automation/automation_provider_list.h" +#include "chrome/browser/browser_process_impl.h" // TODO(joi): Remove #include "chrome/browser/chrome_content_browser_client.h" #include "chrome/browser/prefs/browser_prefs.h" #include "chrome/browser/prefs/pref_service.h" @@ -52,6 +54,7 @@ #include "content/public/browser/render_process_host.h" #include "content/public/common/content_client.h" #include "content/public/common/content_paths.h" +#include "content/test/test_browser_thread.h" // TODO(joi): Remove #include "testing/gtest/include/gtest/gtest.h" #include "ui/base/resource/resource_bundle.h" #include "ui/base/ui_base_paths.h" @@ -273,8 +276,6 @@ void FakeExternalTab::Initialize() { browser_process_->local_state()->RegisterBooleanPref( prefs::kMetricsReportingEnabled, false); - FilePath profile_path(ProfileManager::GetDefaultProfileDir(user_data())); - // Initialize the content client which that code uses to talk to Chrome. content::SetContentClient(&g_chrome_content_client.Get()); @@ -283,14 +284,12 @@ void FakeExternalTab::Initialize() { content::GetContentClient()->set_browser(&g_browser_client.Get()); content::GetContentClient()->set_renderer(&g_renderer_client.Get()); +} +void FakeExternalTab::InitializePostThreadsCreated() { + FilePath profile_path(ProfileManager::GetDefaultProfileDir(user_data())); Profile* profile = g_browser_process->profile_manager()->GetProfile(profile_path); - - // Create the child threads. - g_browser_process->db_thread(); - g_browser_process->file_thread(); - g_browser_process->io_thread(); } void FakeExternalTab::Shutdown() { @@ -301,16 +300,39 @@ void FakeExternalTab::Shutdown() { ResourceBundle::CleanupSharedInstance(); } +// TODO(joi): Remove! +class ChromeFrameFriendOfBrowserProcessImpl { + public: + static void CreateIOThreadState() { + reinterpret_cast<BrowserProcessImpl*>( + g_browser_process)->CreateIOThreadState(); + } +}; + CFUrlRequestUnittestRunner::CFUrlRequestUnittestRunner(int argc, char** argv) : NetTestSuite(argc, argv), chrome_frame_html_("/chrome_frame", kChromeFrameHtml), registrar_(chrome_frame_test::GetTestBedType()), test_result_(0) { // Register the main thread by instantiating it, but don't call any methods. - main_thread_.reset(new content::DeprecatedBrowserThread( + main_thread_.reset(new content::TestBrowserThread( BrowserThread::UI, MessageLoop::current())); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); fake_chrome_.Initialize(); + + db_thread_.reset(new content::TestBrowserThread(BrowserThread::DB)); + db_thread_->Start(); + + file_thread_.reset(new content::TestBrowserThread(BrowserThread::FILE)); + file_thread_->Start(); + + ChromeFrameFriendOfBrowserProcessImpl::CreateIOThreadState(); + + io_thread_.reset(new content::TestBrowserThread(BrowserThread::IO)); + io_thread_->StartIOThread(); + + fake_chrome_.InitializePostThreadsCreated(); + pss_subclass_.reset(new ProcessSingletonSubclass(this)); EXPECT_TRUE(pss_subclass_->Subclass(fake_chrome_.user_data())); StartChromeFrameInHostBrowser(); @@ -587,13 +609,15 @@ const char* IEVersionToString(IEVersion version) { } int main(int argc, char** argv) { - if (chrome_frame_test::GetInstalledIEVersion() >= IE_9) { + // TODO(joi): Remove the "true" part here and fix the log statement below. + if (true || chrome_frame_test::GetInstalledIEVersion() >= IE_9) { // Adding this here as the command line and the logging stuff gets // initialized in the NetTestSuite constructor. Did not want to break that. base::AtExitManager at_exit_manager; CommandLine::Init(argc, argv); CFUrlRequestUnittestRunner::InitializeLogging(); - LOG(INFO) << "Not running ChromeFrame net tests on IE9+"; + LOG(INFO) << "Temporarily not running any ChromeFrame " + << "net tests (http://crbug.com/105435)"; return 0; } diff --git a/chrome_frame/test/net/fake_external_tab.h b/chrome_frame/test/net/fake_external_tab.h index 756e8f8..e3eb6b1 100644 --- a/chrome_frame/test/net/fake_external_tab.h +++ b/chrome_frame/test/net/fake_external_tab.h @@ -17,7 +17,7 @@ #include "chrome_frame/test/net/test_automation_provider.h" #include "chrome_frame/test/test_server.h" #include "chrome_frame/test_utils.h" -#include "content/public/browser/browser_thread.h" +#include "content/test/test_browser_thread.h" #include "net/base/net_test_suite.h" class ProcessSingleton; @@ -32,6 +32,7 @@ class FakeExternalTab { virtual ~FakeExternalTab(); virtual void Initialize(); + virtual void InitializePostThreadsCreated(); virtual void Shutdown(); const FilePath& user_data() const { @@ -105,9 +106,15 @@ class CFUrlRequestUnittestRunner // on the main thread. FakeExternalTab fake_chrome_; scoped_ptr<ProcessSingletonSubclass> pss_subclass_; - scoped_ptr<content::DeprecatedBrowserThread> main_thread_; + scoped_ptr<content::TestBrowserThread> main_thread_; ScopedChromeFrameRegistrar registrar_; int test_result_; + + // TODO(joi): This should be fixed so that this test executable uses + // content::BrowserMainParts. As it stands it is a horrible hack. + scoped_ptr<content::TestBrowserThread> db_thread_; + scoped_ptr<content::TestBrowserThread> file_thread_; + scoped_ptr<content::TestBrowserThread> io_thread_; }; #endif // CHROME_FRAME_TEST_NET_FAKE_EXTERNAL_TAB_H_ diff --git a/content/browser/browser_main_loop.cc b/content/browser/browser_main_loop.cc index dd93e58..c6d448a 100644 --- a/content/browser/browser_main_loop.cc +++ b/content/browser/browser_main_loop.cc @@ -16,6 +16,7 @@ #include "content/common/hi_res_timer_manager.h" #include "content/common/sandbox_policy.h" #include "content/public/browser/browser_main_parts.h" +#include "content/public/browser/browser_shutdown.h" #include "content/public/browser/content_browser_client.h" #include "content/public/common/content_switches.h" #include "content/public/common/main_function_params.h" @@ -145,6 +146,33 @@ static void SetUpGLibLogHandler() { namespace content { +// The currently-running BrowserMainLoop. There can be one or zero. +// This is stored to enable immediate shutdown when needed. +BrowserMainLoop* current_browser_main_loop = NULL; + +// This is just to be able to keep ShutdownThreadsAndCleanUp out of +// the public interface of BrowserMainLoop. +class BrowserShutdownImpl { + public: + static void ImmediateShutdownAndExitProcess() { + DCHECK(current_browser_main_loop); + current_browser_main_loop->ShutdownThreadsAndCleanUp(); + +#if defined(OS_WIN) + // At this point the message loop is still running yet we've shut everything + // down. If any messages are processed we'll likely crash. Exit now. + ExitProcess(content::RESULT_CODE_NORMAL_EXIT); +#elif defined(OS_POSIX) && !defined(OS_MACOSX) + _exit(content::RESULT_CODE_NORMAL_EXIT); +#else + NOTIMPLEMENTED(); +#endif + } +}; + +void ImmediateShutdownAndExitProcess() { + BrowserShutdownImpl::ImmediateShutdownAndExitProcess(); +} // BrowserMainLoop construction / destructione ============================= @@ -152,12 +180,16 @@ BrowserMainLoop::BrowserMainLoop(const content::MainFunctionParams& parameters) : parameters_(parameters), parsed_command_line_(parameters.command_line), result_code_(content::RESULT_CODE_NORMAL_EXIT) { + DCHECK(!current_browser_main_loop); + current_browser_main_loop = this; #if defined(OS_WIN) OleInitialize(NULL); #endif } BrowserMainLoop::~BrowserMainLoop() { + DCHECK_EQ(this, current_browser_main_loop); + current_browser_main_loop = NULL; #if defined(OS_WIN) OleUninitialize(); #endif @@ -262,6 +294,85 @@ void BrowserMainLoop::MainMessageLoopStart() { void BrowserMainLoop::RunMainMessageLoopParts( bool* completed_main_message_loop) { if (parts_.get()) + parts_->PreCreateThreads(); + + base::Thread::Options default_options; + base::Thread::Options io_message_loop_options; + io_message_loop_options.message_loop_type = MessageLoop::TYPE_IO; + base::Thread::Options ui_message_loop_options; + ui_message_loop_options.message_loop_type = MessageLoop::TYPE_UI; + + // Start threads in the order they occur in the BrowserThread::ID + // enumeration, except for BrowserThread::UI which is the main + // thread. + // + // Must be size_t so we can increment it. + for (size_t thread_id = BrowserThread::UI + 1; + thread_id < BrowserThread::ID_COUNT; + ++thread_id) { + scoped_ptr<BrowserProcessSubThread>* thread_to_start = NULL; + base::Thread::Options* options = &default_options; + + switch (thread_id) { + case BrowserThread::DB: + thread_to_start = &db_thread_; + break; + case BrowserThread::WEBKIT: + // For now, the WebKit thread in the browser is owned by + // ResourceDispatcherHost, not by the content framework. Until + // this is fixed, we don't start the thread but still call + // Pre/PostStartThread for the ID. + break; + case BrowserThread::FILE: + thread_to_start = &file_thread_; +#if defined(OS_WIN) + // On Windows, the FILE thread needs to be have a UI message loop + // which pumps messages in such a way that Google Update can + // communicate back to us. + options = &ui_message_loop_options; +#else + options = &io_message_loop_options; +#endif + break; + case BrowserThread::PROCESS_LAUNCHER: + thread_to_start = &process_launcher_thread_; + break; + case BrowserThread::CACHE: + thread_to_start = &cache_thread_; + options = &io_message_loop_options; + break; + case BrowserThread::IO: + thread_to_start = &io_thread_; + options = &io_message_loop_options; + break; +#if defined(OS_CHROMEOS) + case BrowserThread::WEB_SOCKET_PROXY: + thread_to_start = &web_socket_proxy_thread_; + options = &io_message_loop_options; + break; +#endif + case BrowserThread::UI: + case BrowserThread::ID_COUNT: + default: + NOTREACHED(); + break; + } + + BrowserThread::ID id = static_cast<BrowserThread::ID>(thread_id); + + if (parts_.get()) + parts_->PreStartThread(id); + + if (thread_to_start) { + (*thread_to_start).reset(new BrowserProcessSubThread(id)); + (*thread_to_start)->StartWithOptions(*options); + } + + if (parts_.get()) + parts_->PostStartThread(id); + } + + if (parts_.get()) parts_->PreMainMessageLoopRun(); TRACE_EVENT_BEGIN_ETW("BrowserMain:MESSAGE_LOOP", 0, ""); @@ -281,8 +392,96 @@ void BrowserMainLoop::RunMainMessageLoopParts( if (completed_main_message_loop) *completed_main_message_loop = true; + ShutdownThreadsAndCleanUp(); +} + +void BrowserMainLoop::ShutdownThreadsAndCleanUp() { + // Teardown may start in PostMainMessageLoopRun, and during teardown we + // need to be able to perform IO. + base::ThreadRestrictions::SetIOAllowed(true); + BrowserThread::PostTask( + BrowserThread::IO, + FROM_HERE, + NewRunnableFunction(&base::ThreadRestrictions::SetIOAllowed, true)); + if (parts_.get()) parts_->PostMainMessageLoopRun(); + + // Must be size_t so we can subtract from it. + for (size_t thread_id = BrowserThread::ID_COUNT - 1; + thread_id >= (BrowserThread::UI + 1); + --thread_id) { + // Find the thread object we want to stop. Looping over all valid + // BrowserThread IDs and DCHECKing on a missing case in the switch + // statement helps avoid a mismatch between this code and the + // BrowserThread::ID enumeration. + // + // The destruction order is the reverse order of occurrence in the + // BrowserThread::ID list. The rationale for the order is as + // follows (need to be filled in a bit): + // + // - (Not sure why the WEB_SOCKET_PROXY thread is stopped first.) + // + // - The IO thread is the only user of the CACHE thread. + // + // - The PROCESS_LAUNCHER thread must be stopped after IO in case + // the IO thread posted a task to terminate a process on the + // process launcher thread. + // + // - (Not sure why FILE needs to stop before WEBKIT.) + // + // - The WEBKIT thread (which currently is the responsibility of + // the embedder to stop, by destroying ResourceDispatcherHost + // before the DB thread is stopped) + // + // - (Not sure why DB stops last.) + scoped_ptr<BrowserProcessSubThread>* thread_to_stop = NULL; + switch (thread_id) { + case BrowserThread::DB: + thread_to_stop = &db_thread_; + break; + case BrowserThread::WEBKIT: + // For now, the WebKit thread in the browser is owned by + // ResourceDispatcherHost, not by the content framework. Until + // this is fixed, we don't stop the thread but still call + // Pre/PostStopThread for the ID. + break; + case BrowserThread::FILE: + thread_to_stop = &file_thread_; + break; + case BrowserThread::PROCESS_LAUNCHER: + thread_to_stop = &process_launcher_thread_; + break; + case BrowserThread::CACHE: + thread_to_stop = &cache_thread_; + break; + case BrowserThread::IO: + thread_to_stop = &io_thread_; + break; +#if defined(OS_CHROMEOS) + case BrowserThread::WEB_SOCKET_PROXY: + thread_to_stop = &web_socket_proxy_thread_; + break; +#endif + case BrowserThread::UI: + case BrowserThread::ID_COUNT: + default: + NOTREACHED(); + break; + } + + BrowserThread::ID id = static_cast<BrowserThread::ID>(thread_id); + + if (parts_.get()) + parts_->PreStopThread(id); + if (thread_to_stop) + thread_to_stop->reset(); + if (parts_.get()) + parts_->PostStopThread(id); + } + + if (parts_.get()) + parts_->PostDestroyThreads(); } void BrowserMainLoop::InitializeMainThread() { diff --git a/content/browser/browser_main_loop.h b/content/browser/browser_main_loop.h index d02d578..3038332 100644 --- a/content/browser/browser_main_loop.h +++ b/content/browser/browser_main_loop.h @@ -8,6 +8,7 @@ #include "base/basictypes.h" #include "base/memory/scoped_ptr.h" +#include "content/browser/browser_process_sub_thread.h" class CommandLine; class HighResolutionTimerManager; @@ -25,6 +26,7 @@ class NetworkChangeNotifier; namespace content { class BrowserMainParts; +class BrowserShutdownImpl; class BrowserThreadImpl; struct MainFunctionParams; @@ -46,6 +48,13 @@ class BrowserMainLoop { int GetResultCode() const { return result_code_; } private: + // For ShutdownThreadsAndCleanUp. + friend class BrowserShutdownImpl; + + // Performs the shutdown sequence, starting with PostMainMessageLoopRun + // through stopping threads to PostDestroyThreads. + void ShutdownThreadsAndCleanUp(); + void InitializeMainThread(); // Members initialized on construction --------------------------------------- @@ -69,6 +78,14 @@ class BrowserMainLoop { // Members initialized in |InitializeMainThread()| --------------------------- // This must get destroyed before other threads that are created in parts_. scoped_ptr<BrowserThreadImpl> main_thread_; + scoped_ptr<BrowserProcessSubThread> io_thread_; + scoped_ptr<BrowserProcessSubThread> file_thread_; + scoped_ptr<BrowserProcessSubThread> db_thread_; + scoped_ptr<BrowserProcessSubThread> process_launcher_thread_; + scoped_ptr<BrowserProcessSubThread> cache_thread_; +#if defined(OS_CHROMEOS) + scoped_ptr<BrowserProcessSubThread> web_socket_proxy_thread_; +#endif DISALLOW_COPY_AND_ASSIGN(BrowserMainLoop); }; diff --git a/content/browser/browser_process_sub_thread.cc b/content/browser/browser_process_sub_thread.cc index 31c129e..7c77d7e 100644 --- a/content/browser/browser_process_sub_thread.cc +++ b/content/browser/browser_process_sub_thread.cc @@ -17,8 +17,6 @@ BrowserProcessSubThread::BrowserProcessSubThread(BrowserThread::ID identifier) : BrowserThreadImpl(identifier) {} BrowserProcessSubThread::~BrowserProcessSubThread() { - // We cannot rely on our base class to stop the thread since we want our - // CleanUp function to run. Stop(); } @@ -29,9 +27,13 @@ void BrowserProcessSubThread::Init() { #endif notification_service_ = new NotificationServiceImpl; + + BrowserThreadImpl::Init(); } void BrowserProcessSubThread::CleanUp() { + BrowserThreadImpl::CleanUp(); + delete notification_service_; notification_service_ = NULL; diff --git a/content/browser/browser_thread_impl.cc b/content/browser/browser_thread_impl.cc index 023259b..0025f77 100644 --- a/content/browser/browser_thread_impl.cc +++ b/content/browser/browser_thread_impl.cc @@ -4,16 +4,19 @@ #include "content/browser/browser_thread_impl.h" +#include "base/atomicops.h" #include "base/bind.h" #include "base/lazy_instance.h" #include "base/message_loop.h" #include "base/message_loop_proxy.h" #include "base/threading/thread_restrictions.h" +namespace content { + namespace { // Friendly names for the well-known threads. -static const char* browser_thread_names[content::BrowserThread::ID_COUNT] = { +static const char* g_browser_thread_names[BrowserThread::ID_COUNT] = { "", // UI (name assembled in browser_main.cc). "Chrome_DBThread", // DB "Chrome_WebKitThread", // WEBKIT @@ -26,38 +29,86 @@ static const char* browser_thread_names[content::BrowserThread::ID_COUNT] = { #endif }; -} // namespace - -namespace content { - -namespace { - -// This lock protects |g_browser_threads|. Do not read or modify that array -// without holding this lock. Do not block while holding this lock. +// This lock protects |g_browser_threads|. Do not read or modify that +// array without holding this lock. Do not block while holding this +// lock. base::LazyInstance<base::Lock, base::LeakyLazyInstanceTraits<base::Lock> > g_lock = LAZY_INSTANCE_INITIALIZER; - -// An array of the BrowserThread objects. This array is protected by |g_lock|. -// The threads are not owned by this array. Typically, the threads are owned -// on the UI thread by the g_browser_process object. BrowserThreads remove +// This array is protected by |g_lock|. The threads are not owned by this +// array. Typically, the threads are owned on the UI thread by +// content::BrowserMainLoop. BrowserThreadImpl objects remove // themselves from this array upon destruction. -BrowserThread* g_browser_threads[BrowserThread::ID_COUNT]; +static BrowserThreadImpl* g_browser_threads[BrowserThread::ID_COUNT]; + +// Only atomic operations are used on this array. The delegates are +// not owned by this array, rather by whoever calls +// BrowserThread::SetDelegate. +static BrowserThreadDelegate* g_browser_thread_delegates[ + BrowserThread::ID_COUNT]; } // namespace -BrowserThreadImpl::BrowserThreadImpl(BrowserThread::ID identifier) - : BrowserThread(identifier) { +BrowserThreadImpl::BrowserThreadImpl(ID identifier) + : Thread(g_browser_thread_names[identifier]), + identifier_(identifier) { + Initialize(); } -BrowserThreadImpl::BrowserThreadImpl(BrowserThread::ID identifier, +BrowserThreadImpl::BrowserThreadImpl(ID identifier, MessageLoop* message_loop) - : BrowserThread(identifier, message_loop) { + : Thread(message_loop->thread_name().c_str()), + identifier_(identifier) { + set_message_loop(message_loop); + Initialize(); +} + +void BrowserThreadImpl::Init() { + using base::subtle::AtomicWord; + AtomicWord* storage = + reinterpret_cast<AtomicWord*>(&g_browser_thread_delegates[identifier_]); + AtomicWord stored_pointer = base::subtle::NoBarrier_Load(storage); + BrowserThreadDelegate* delegate = + reinterpret_cast<BrowserThreadDelegate*>(stored_pointer); + if (delegate) + delegate->Init(); +} + +void BrowserThreadImpl::CleanUp() { + using base::subtle::AtomicWord; + AtomicWord* storage = + reinterpret_cast<AtomicWord*>(&g_browser_thread_delegates[identifier_]); + AtomicWord stored_pointer = base::subtle::NoBarrier_Load(storage); + BrowserThreadDelegate* delegate = + reinterpret_cast<BrowserThreadDelegate*>(stored_pointer); + + if (delegate) + delegate->CleanUp(); +} + +void BrowserThreadImpl::Initialize() { + base::AutoLock lock(g_lock.Get()); + DCHECK(identifier_ >= 0 && identifier_ < ID_COUNT); + DCHECK(g_browser_threads[identifier_] == NULL); + g_browser_threads[identifier_] = this; } BrowserThreadImpl::~BrowserThreadImpl() { + // All Thread subclasses must call Stop() in the destructor. This is + // doubly important here as various bits of code check they are on + // the right BrowserThread. Stop(); + + base::AutoLock lock(g_lock.Get()); + g_browser_threads[identifier_] = NULL; +#ifndef NDEBUG + // Double check that the threads are ordered correctly in the enumeration. + for (int i = identifier_ + 1; i < ID_COUNT; ++i) { + DCHECK(!g_browser_threads[i]) << + "Threads must be listed in the reverse order that they die"; + } +#endif } // static @@ -77,7 +128,7 @@ bool BrowserThreadImpl::PostTaskHelper( BrowserThread::ID current_thread; bool guaranteed_to_outlive_target_thread = GetCurrentThreadIdentifier(¤t_thread) && - current_thread >= identifier; + current_thread <= identifier; if (!guaranteed_to_outlive_target_thread) g_lock.Get().Acquire(); @@ -118,7 +169,7 @@ bool BrowserThreadImpl::PostTaskHelper( BrowserThread::ID current_thread; bool guaranteed_to_outlive_target_thread = GetCurrentThreadIdentifier(¤t_thread) && - current_thread >= identifier; + current_thread <= identifier; if (!guaranteed_to_outlive_target_thread) g_lock.Get().Acquire(); @@ -139,18 +190,6 @@ bool BrowserThreadImpl::PostTaskHelper( return !!message_loop; } -// TODO(joi): Remove -DeprecatedBrowserThread::DeprecatedBrowserThread(BrowserThread::ID identifier) - : BrowserThread(identifier) { -} -DeprecatedBrowserThread::DeprecatedBrowserThread(BrowserThread::ID identifier, - MessageLoop* message_loop) - : BrowserThread(identifier, message_loop) { -} -DeprecatedBrowserThread::~DeprecatedBrowserThread() { - Stop(); -} - // An implementation of MessageLoopProxy to be used in conjunction // with BrowserThread. class BrowserThreadMessageLoopProxy : public base::MessageLoopProxy { @@ -215,44 +254,6 @@ class BrowserThreadMessageLoopProxy : public base::MessageLoopProxy { DISALLOW_COPY_AND_ASSIGN(BrowserThreadMessageLoopProxy); }; -BrowserThread::BrowserThread(ID identifier) - : Thread(browser_thread_names[identifier]), - identifier_(identifier) { - Initialize(); -} - -BrowserThread::BrowserThread(ID identifier, - MessageLoop* message_loop) - : Thread(message_loop->thread_name().c_str()), - identifier_(identifier) { - set_message_loop(message_loop); - Initialize(); -} - -void BrowserThread::Initialize() { - base::AutoLock lock(g_lock.Get()); - DCHECK(identifier_ >= 0 && identifier_ < ID_COUNT); - DCHECK(g_browser_threads[identifier_] == NULL); - g_browser_threads[identifier_] = this; -} - -BrowserThread::~BrowserThread() { - // Stop the thread here, instead of the parent's class destructor. This is so - // that if there are pending tasks that run, code that checks that it's on the - // correct BrowserThread succeeds. - Stop(); - - base::AutoLock lock(g_lock.Get()); - g_browser_threads[identifier_] = NULL; -#ifndef NDEBUG - // Double check that the threads are ordered correctly in the enumeration. - for (int i = identifier_ + 1; i < ID_COUNT; ++i) { - DCHECK(!g_browser_threads[i]) << - "Threads must be listed in the reverse order that they die"; - } -#endif -} - // static bool BrowserThread::IsWellKnownThread(ID identifier) { base::AutoLock lock(g_lock.Get()); @@ -393,4 +394,23 @@ BrowserThread::GetMessageLoopProxyForThread( return proxy; } +base::Thread* BrowserThread::UnsafeGetBrowserThread(ID identifier) { + base::AutoLock lock(g_lock.Get()); + base::Thread* thread = g_browser_threads[identifier]; + DCHECK(thread); + return thread; +} + +void BrowserThread::SetDelegate(ID identifier, + BrowserThreadDelegate* delegate) { + using base::subtle::AtomicWord; + AtomicWord* storage = reinterpret_cast<AtomicWord*>( + &g_browser_thread_delegates[identifier]); + AtomicWord old_pointer = base::subtle::NoBarrier_AtomicExchange( + storage, reinterpret_cast<AtomicWord>(delegate)); + + // This catches registration when previously registered. + DCHECK(!delegate || !old_pointer); +} + } // namespace content diff --git a/content/browser/browser_thread_impl.h b/content/browser/browser_thread_impl.h index e24b385..d00ff5b 100644 --- a/content/browser/browser_thread_impl.h +++ b/content/browser/browser_thread_impl.h @@ -6,22 +6,33 @@ #define CONTENT_BROWSER_BROWSER_THREAD_IMPL_H_ #pragma once +#include "base/synchronization/lock.h" +#include "base/threading/thread.h" #include "content/common/content_export.h" #include "content/public/browser/browser_thread.h" namespace content { -class CONTENT_EXPORT BrowserThreadImpl : public BrowserThread { +class CONTENT_EXPORT BrowserThreadImpl + : public BrowserThread, public base::Thread { public: + // Construct a BrowserThreadImpl with the supplied identifier. It is an error + // to construct a BrowserThreadImpl that already exists. explicit BrowserThreadImpl(BrowserThread::ID identifier); + + // Special constructor for the main (UI) thread and unittests. We use a dummy + // thread here since the main thread already exists. BrowserThreadImpl(BrowserThread::ID identifier, MessageLoop* message_loop); virtual ~BrowserThreadImpl(); + protected: + virtual void Init() OVERRIDE; + virtual void CleanUp() OVERRIDE; + private: - // We implement most functionality on the public set of - // BrowserThread functions, but state is stored in the - // BrowserThreadImpl to keep the public API cleaner. Therefore make - // BrowserThread a friend class. + // We implement all the functionality of the public BrowserThread + // functions, but state is stored in the BrowserThreadImpl to keep + // the API cleaner. Therefore make BrowserThread a friend class. friend class BrowserThread; // TODO(brettw) remove this variant when Task->Closure migration is complete. @@ -37,6 +48,13 @@ class CONTENT_EXPORT BrowserThreadImpl : public BrowserThread { const base::Closure& task, int64 delay_ms, bool nestable); + + // Common initialization code for the constructors. + void Initialize(); + + // The identifier of this thread. Only one thread can exist with a given + // identifier at a given time. + ID identifier_; }; } // namespace content diff --git a/content/content_browser.gypi b/content/content_browser.gypi index 0bfe9da..d55e9f5 100644 --- a/content/content_browser.gypi +++ b/content/content_browser.gypi @@ -27,7 +27,9 @@ ], 'sources': [ 'public/browser/browser_main_parts.h', + 'public/browser/browser_shutdown.h', 'public/browser/browser_thread.h', + 'public/browser/browser_thread_delegate.h', 'public/browser/content_browser_client.h', 'public/browser/content_ipc_logging.h', 'public/browser/download_manager_delegate.h', diff --git a/content/public/browser/browser_main_parts.h b/content/public/browser/browser_main_parts.h index 860541c..4bce3cf 100644 --- a/content/public/browser/browser_main_parts.h +++ b/content/public/browser/browser_main_parts.h @@ -8,6 +8,7 @@ #include "base/basictypes.h" #include "content/common/content_export.h" +#include "content/public/browser/browser_thread.h" namespace content { @@ -64,6 +65,24 @@ class CONTENT_EXPORT BrowserMainParts { // Allows an embedder to do any extra toolkit initialization. virtual void ToolkitInitialized() = 0; + // Called just before any child threads owned by the content + // framework are created. + // + // The main message loop has been started at this point (but has not + // been run), and the toolkit has been initialized. + virtual void PreCreateThreads() = 0; + + // Called once for each thread owned by the content framework just + // before and just after the thread object is created and started. + // This happens in the order of the threads' appearence in the + // BrowserThread::ID enumeration. Note that there will be no such + // call for BrowserThread::UI, since it is the main thread of the + // application. + virtual void PreStartThread(BrowserThread::ID identifier) = 0; + virtual void PostStartThread(BrowserThread::ID identifier) = 0; + + // This is called just before the main message loop is run. The + // various browser threads have all been created at this point virtual void PreMainMessageLoopRun() = 0; // Returns true if the message loop was run, false otherwise. @@ -71,8 +90,22 @@ class CONTENT_EXPORT BrowserMainParts { // May set |result_code|, which will be returned by |BrowserMain()|. virtual bool MainMessageLoopRun(int* result_code) = 0; + // This happens after the main message loop has stopped, but before + // threads are stopped. virtual void PostMainMessageLoopRun() = 0; + // Called once for each thread owned by the content framework just + // before and just after it is torn down. This is in reverse order + // of the threads' appearance in the BrowserThread::ID enumeration. + // Note that you will not receive such a call for BrowserThread::UI, + // since it is the main thread of the application. + virtual void PreStopThread(BrowserThread::ID identifier) = 0; + virtual void PostStopThread(BrowserThread::ID identifier) = 0; + + // Called as the very last part of shutdown, after threads have been + // stopped and destroyed. + virtual void PostDestroyThreads() = 0; + private: DISALLOW_COPY_AND_ASSIGN(BrowserMainParts); }; diff --git a/content/public/browser/browser_shutdown.h b/content/public/browser/browser_shutdown.h new file mode 100644 index 0000000..72a4ce0 --- /dev/null +++ b/content/public/browser/browser_shutdown.h @@ -0,0 +1,32 @@ +// 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. + +#ifndef CONTENT_PUBLIC_BROWSER_BROWSER_SHUTDOWN_H_ +#define CONTENT_PUBLIC_BROWSER_BROWSER_SHUTDOWN_H_ +#pragma once + +#include "content/common/content_export.h" + +namespace content { + +// This can be used for as-fast-as-possible shutdown, in cases where +// time for shutdown is limited and we just need to write out as much +// data as possible before our time runs out. +// +// This causes the shutdown sequence embodied by +// BrowserMainParts::PostMainMessageLoopRun through +// BrowserMainParts::PostDestroyThreads to occur, i.e. we pretend the +// message loop finished, all threads are stopped in sequence and +// PreStopThread/PostStopThread gets called, and at least, +// PostDestroyThreads is called. +// +// As this violates the normal order of shutdown, likely leaving the +// process in a bad state, the last thing this function does is +// terminate the process (right after calling +// BrowserMainParts::PostDestroyThreads). +CONTENT_EXPORT void ImmediateShutdownAndExitProcess(); + +} // namespace content + +#endif // CONTENT_PUBLIC_BROWSER_BROWSER_SHUTDOWN_H_ diff --git a/content/public/browser/browser_thread.h b/content/public/browser/browser_thread.h index 66dc009..4478558 100644 --- a/content/public/browser/browser_thread.h +++ b/content/public/browser/browser_thread.h @@ -6,11 +6,21 @@ #define CONTENT_PUBLIC_BROWSER_BROWSER_THREAD_H_ #pragma once +#include "base/basictypes.h" #include "base/callback.h" -#include "base/synchronization/lock.h" #include "base/task.h" -#include "base/threading/thread.h" +#include "base/tracked_objects.h" #include "content/common/content_export.h" +#include "content/public/browser/browser_thread_delegate.h" + +// TODO(joi): Remove these in a follow-up change and IWYU in files +// that were getting them directly or indirectly from here. +#include "base/memory/ref_counted.h" +#include "base/memory/scoped_ptr.h" +#include "base/message_loop.h" +#include "base/message_loop_proxy.h" +#include "base/synchronization/lock.h" +#include "base/threading/thread.h" #if defined(UNIT_TEST) #include "base/logging.h" @@ -18,12 +28,12 @@ namespace base { class MessageLoopProxy; +class Thread; } namespace content { class BrowserThreadImpl; -class DeprecatedBrowserThread; /////////////////////////////////////////////////////////////////////////////// // BrowserThread @@ -47,7 +57,7 @@ class DeprecatedBrowserThread; // task is posted to is guaranteed to outlive the current thread, then no locks // are used. You should never need to cache pointers to MessageLoops, since // they're not thread safe. -class CONTENT_EXPORT BrowserThread : public base::Thread { +class CONTENT_EXPORT BrowserThread { public: // An enumeration of the well-known threads. // NOTE: threads must be listed in the order of their life-time, with each @@ -170,6 +180,33 @@ class CONTENT_EXPORT BrowserThread : public base::Thread { static scoped_refptr<base::MessageLoopProxy> GetMessageLoopProxyForThread( ID identifier); + // Gets the Thread object for the specified thread, or NULL if the + // thread has not been created (or has been destroyed during + // shutdown). + // + // Before calling this, you must have called content::ContentMain + // with a command-line that would specify a browser process (e.g. an + // empty command line). + // + // This is unsafe as your pointer may become invalid close to + // shutdown. + // + // TODO(joi): Remove this once clients such as BrowserProcessImpl + // (and classes that call things like + // g_browser_process->file_thread()) are switched to using + // MessageLoopProxy. + static base::Thread* UnsafeGetBrowserThread(ID identifier); + + // Sets the delegate for the specified BrowserThread. + // + // Only one delegate may be registered at a time. Delegates may be + // unregistered by providing a NULL pointer. + // + // If the caller unregisters a delegate before CleanUp has been + // called, it must perform its own locking to ensure the delegate is + // not deleted while unregistering. + static void SetDelegate(ID identifier, BrowserThreadDelegate* delegate); + // Use these templates in conjuction with RefCountedThreadSafe when you want // to ensure that an object is deleted on a specific thread. This is needed // when an object can hop between threads (i.e. IO -> FILE -> IO), and thread @@ -213,40 +250,10 @@ class CONTENT_EXPORT BrowserThread : public base::Thread { struct DeleteOnWebKitThread : public DeleteOnThread<WEBKIT> { }; private: - // Construct a BrowserThread with the supplied identifier. It is an error - // to construct a BrowserThread that already exists. - explicit BrowserThread(ID identifier); - - // Special constructor for the main (UI) thread and unittests. We use a dummy - // thread here since the main thread already exists. - BrowserThread(ID identifier, MessageLoop* message_loop); - - virtual ~BrowserThread(); - - // Common initialization code for the constructors. - void Initialize(); - - // Constructors are only available through this subclass. - friend class content::BrowserThreadImpl; + friend class BrowserThreadImpl; - // TODO(joi): Remove. - friend class DeprecatedBrowserThread; - - // The identifier of this thread. Only one thread can exist with a given - // identifier at a given time. - // TODO(joi): Move to BrowserThreadImpl, and make constructors here - // do-nothing. - ID identifier_; -}; - -// Temporary escape hatch for chrome/ to construct BrowserThread, -// until we make content/ construct its own threads. -class CONTENT_EXPORT DeprecatedBrowserThread : public BrowserThread { - public: - explicit DeprecatedBrowserThread(BrowserThread::ID identifier); - DeprecatedBrowserThread(BrowserThread::ID identifier, - MessageLoop* message_loop); - virtual ~DeprecatedBrowserThread(); + BrowserThread() {} + DISALLOW_COPY_AND_ASSIGN(BrowserThread); }; } // namespace content diff --git a/content/public/browser/browser_thread_delegate.h b/content/public/browser/browser_thread_delegate.h new file mode 100644 index 0000000..ae9fc7b --- /dev/null +++ b/content/public/browser/browser_thread_delegate.h @@ -0,0 +1,32 @@ +// 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. + +#ifndef CONTENT_PUBLIC_BROWSER_BROWSER_THREAD_DELEGATE_H_ +#define CONTENT_PUBLIC_BROWSER_BROWSER_THREAD_DELEGATE_H_ +#pragma once + +namespace content { + +// A class with this type may be registered via +// BrowserThread::SetDelegate. +// +// If registered as such, it will receive an Init() call right before +// the BrowserThread in question starts its message loop (and right +// after the BrowserThread has done its own initialization), and a +// CleanUp call right after the message loop ends (and before the +// BrowserThread has done its own clean-up). +class BrowserThreadDelegate { + public: + virtual ~BrowserThreadDelegate() {} + + // Called just prior to starting the message loop. + virtual void Init() = 0; + + // Called just after the message loop ends. + virtual void CleanUp() = 0; +}; + +} // namespace content + +#endif // CONTENT_PUBLIC_BROWSER_BROWSER_THREAD_DELEGATE_H_ diff --git a/content/shell/shell_browser_context.cc b/content/shell/shell_browser_context.cc index 0f75d89..f011b32 100644 --- a/content/shell/shell_browser_context.cc +++ b/content/shell/shell_browser_context.cc @@ -8,6 +8,7 @@ #include "base/file_util.h" #include "base/logging.h" #include "base/path_service.h" +#include "base/threading/thread.h" #include "content/browser/appcache/chrome_appcache_service.h" #include "content/browser/chrome_blob_storage_context.h" #include "content/browser/download/download_id_factory.h" @@ -140,8 +141,10 @@ net::URLRequestContextGetter* ShellBrowserContext::GetRequestContext() { if (!url_request_getter_) { url_request_getter_ = new ShellURLRequestContextGetter( GetPath(), - shell_main_parts_->io_thread()->message_loop(), - shell_main_parts_->file_thread()->message_loop()); + BrowserThread::UnsafeGetBrowserThread( + BrowserThread::IO)->message_loop(), + BrowserThread::UnsafeGetBrowserThread( + BrowserThread::FILE)->message_loop()); } return url_request_getter_; } diff --git a/content/shell/shell_browser_main.cc b/content/shell/shell_browser_main.cc index a5f2fbe..324139e 100644 --- a/content/shell/shell_browser_main.cc +++ b/content/shell/shell_browser_main.cc @@ -40,41 +40,9 @@ ShellBrowserMainParts::ShellBrowserMainParts( } ShellBrowserMainParts::~ShellBrowserMainParts() { - base::ThreadRestrictions::SetIOAllowed(true); - io_thread()->message_loop()->PostTask( - FROM_HERE, base::IgnoreReturn<bool>( - base::Bind(&base::ThreadRestrictions::SetIOAllowed, true))); - - browser_context_.reset(); - - resource_dispatcher_host_->download_file_manager()->Shutdown(); - resource_dispatcher_host_->save_file_manager()->Shutdown(); - resource_dispatcher_host_->Shutdown(); - io_thread_.reset(); - cache_thread_.reset(); - process_launcher_thread_.reset(); - file_thread_.reset(); - resource_dispatcher_host_.reset(); // Kills WebKit thread. - db_thread_.reset(); } -void ShellBrowserMainParts::PreMainMessageLoopRun() { - db_thread_.reset(new BrowserProcessSubThread(BrowserThread::DB)); - db_thread_->Start(); - file_thread_.reset(new BrowserProcessSubThread(BrowserThread::FILE)); - file_thread_->Start(); - process_launcher_thread_.reset( - new BrowserProcessSubThread(BrowserThread::PROCESS_LAUNCHER)); - process_launcher_thread_->Start(); - - base::Thread::Options options; - options.message_loop_type = MessageLoop::TYPE_IO; - - cache_thread_.reset(new BrowserProcessSubThread(BrowserThread::CACHE)); - cache_thread_->StartWithOptions(options); - io_thread_.reset(new BrowserProcessSubThread(BrowserThread::IO)); - io_thread_->StartWithOptions(options); - +void ShellBrowserMainParts::PreCreateThreads() { browser_context_.reset(new ShellBrowserContext(this)); Shell::PlatformInitialize(); @@ -85,7 +53,22 @@ void ShellBrowserMainParts::PreMainMessageLoopRun() { NULL, MSG_ROUTING_NONE, NULL); +} + +void ShellBrowserMainParts::PostMainMessageLoopRun() { + browser_context_.reset(); + resource_dispatcher_host_->download_file_manager()->Shutdown(); + resource_dispatcher_host_->save_file_manager()->Shutdown(); + resource_dispatcher_host_->Shutdown(); +} + +void ShellBrowserMainParts::PreStopThread(BrowserThread::ID id) { + if (id == BrowserThread::WEBKIT) { + // It remains the embedder's responsibility to kill the WebKit + // thread. This happens when RDH is destroyed. + resource_dispatcher_host_.reset(); + } } bool ShellBrowserMainParts::MainMessageLoopRun(int* result_code) { diff --git a/content/shell/shell_browser_main.h b/content/shell/shell_browser_main.h index 0e617ca..ffa44e6 100644 --- a/content/shell/shell_browser_main.h +++ b/content/shell/shell_browser_main.h @@ -35,28 +35,25 @@ class ShellBrowserMainParts : public BrowserMainParts { virtual void PreMainMessageLoopStart() OVERRIDE {} virtual void ToolkitInitialized() OVERRIDE {} virtual void PostMainMessageLoopStart() OVERRIDE {} - virtual void PreMainMessageLoopRun() OVERRIDE; + virtual void PreCreateThreads() OVERRIDE; + virtual void PreStartThread(BrowserThread::ID id) OVERRIDE {} + virtual void PostStartThread(BrowserThread::ID id) OVERRIDE {} + virtual void PreMainMessageLoopRun() OVERRIDE {} virtual bool MainMessageLoopRun(int* result_code) OVERRIDE; - virtual void PostMainMessageLoopRun() OVERRIDE {} + virtual void PostMainMessageLoopRun() OVERRIDE; + virtual void PreStopThread(BrowserThread::ID id) OVERRIDE; + virtual void PostStopThread(BrowserThread::ID) OVERRIDE {} + virtual void PostDestroyThreads() OVERRIDE {} ResourceDispatcherHost* GetResourceDispatcherHost(); ui::Clipboard* GetClipboard(); - base::Thread* io_thread() { return io_thread_.get(); } - base::Thread* file_thread() { return file_thread_.get(); } - private: scoped_ptr<ShellBrowserContext> browser_context_; scoped_ptr<ResourceDispatcherHost> resource_dispatcher_host_; scoped_ptr<ui::Clipboard> clipboard_; - scoped_ptr<base::Thread> io_thread_; - scoped_ptr<base::Thread> file_thread_; - scoped_ptr<base::Thread> db_thread_; - scoped_ptr<base::Thread> process_launcher_thread_; - scoped_ptr<base::Thread> cache_thread_; - DISALLOW_COPY_AND_ASSIGN(ShellBrowserMainParts); }; diff --git a/content/test/test_browser_thread.cc b/content/test/test_browser_thread.cc index bd31919..9224726 100644 --- a/content/test/test_browser_thread.cc +++ b/content/test/test_browser_thread.cc @@ -10,13 +10,37 @@ namespace content { +// This gives access to set_message_loop(). +class TestBrowserThreadImpl : public BrowserThreadImpl { + public: + explicit TestBrowserThreadImpl(BrowserThread::ID identifier) + : BrowserThreadImpl(identifier) { + } + + TestBrowserThreadImpl(BrowserThread::ID identifier, + MessageLoop* message_loop) + : BrowserThreadImpl(identifier, message_loop) { + } + + virtual ~TestBrowserThreadImpl() { + Stop(); + } + + void set_message_loop(MessageLoop* loop) { + Thread::set_message_loop(loop); + } + + private: + DISALLOW_COPY_AND_ASSIGN(TestBrowserThreadImpl); +}; + TestBrowserThread::TestBrowserThread(BrowserThread::ID identifier) - : impl_(new BrowserThreadImpl(identifier)) { + : impl_(new TestBrowserThreadImpl(identifier)) { } TestBrowserThread::TestBrowserThread(BrowserThread::ID identifier, MessageLoop* message_loop) - : impl_(new BrowserThreadImpl(identifier, message_loop)) { + : impl_(new TestBrowserThreadImpl(identifier, message_loop)) { } TestBrowserThread::~TestBrowserThread() { @@ -45,4 +69,8 @@ base::Thread* TestBrowserThread::DeprecatedGetThreadObject() { return impl_.get(); } +void TestBrowserThread::DeprecatedSetMessageLoop(MessageLoop* loop) { + impl_->set_message_loop(loop); +} + } // namespace content diff --git a/content/test/test_browser_thread.h b/content/test/test_browser_thread.h index 5d3439e..9466ae2 100644 --- a/content/test/test_browser_thread.h +++ b/content/test/test_browser_thread.h @@ -18,7 +18,7 @@ class Thread; namespace content { -class BrowserThreadImpl; +class TestBrowserThreadImpl; // A BrowserThread for unit tests; this lets unit tests in chrome/ // create BrowserThread instances. @@ -49,8 +49,12 @@ class TestBrowserThread { // in new tests. base::Thread* DeprecatedGetThreadObject(); + // Sets the message loop to use for the thread. This should not be + // used in new tests. + void DeprecatedSetMessageLoop(MessageLoop* loop); + private: - scoped_ptr<BrowserThreadImpl> impl_; + scoped_ptr<TestBrowserThreadImpl> impl_; DISALLOW_COPY_AND_ASSIGN(TestBrowserThread); }; diff --git a/tools/valgrind/memcheck/suppressions.txt b/tools/valgrind/memcheck/suppressions.txt index 489d45e..5e04522 100644 --- a/tools/valgrind/memcheck/suppressions.txt +++ b/tools/valgrind/memcheck/suppressions.txt @@ -5183,7 +5183,7 @@ Memcheck:Leak fun:_Znw* fun:_ZN16ProcessSingletonC1ERK8FilePath - fun:_ZN22ChromeBrowserMainParts25PreMainMessageLoopRunImplEv + fun:_ZN22ChromeBrowserMainParts20PreCreateThreadsImplEv } { bug_104690 diff --git a/views/accessible_pane_view.cc b/views/accessible_pane_view.cc index 1b1387f..e0cce04 100644 --- a/views/accessible_pane_view.cc +++ b/views/accessible_pane_view.cc @@ -4,6 +4,7 @@ #include "views/accessible_pane_view.h" +#include "base/message_loop.h" #include "ui/base/accessibility/accessible_view_state.h" #include "ui/views/focus/focus_search.h" #include "ui/views/focus/view_storage.h" |