diff options
64 files changed, 545 insertions, 541 deletions
diff --git a/chrome/browser/autocomplete/search_provider_unittest.cc b/chrome/browser/autocomplete/search_provider_unittest.cc index 493df22..b5f9bcd 100644 --- a/chrome/browser/autocomplete/search_provider_unittest.cc +++ b/chrome/browser/autocomplete/search_provider_unittest.cc @@ -6,6 +6,7 @@ #include "base/time.h" #include "build/build_config.h" #include "chrome/browser/autocomplete/search_provider.h" +#include "chrome/browser/chrome_thread.h" #include "chrome/browser/history/history.h" #include "chrome/browser/net/test_url_fetcher_factory.h" #include "chrome/browser/search_engines/template_url.h" @@ -31,7 +32,10 @@ class SearchProviderTest : public testing::Test, term1_(L"term1"), keyword_t_url_(NULL), keyword_term_(L"keyword"), - quit_when_done_(false) {} + io_thread_(ChromeThread::IO), + quit_when_done_(false) { + io_thread_.Start(); + } // See description above class for what this registers. virtual void SetUp(); @@ -62,11 +66,12 @@ class SearchProviderTest : public testing::Test, const std::wstring keyword_term_; GURL keyword_url_; + MessageLoopForUI message_loop_; + ChromeThread io_thread_; + // URLFetcher::Factory implementation registered. TestURLFetcherFactory test_factory_; - MessageLoopForUI message_loop_; - // Profile we use. TestingProfile profile_; diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc index 8485eaf..5352c1d 100644 --- a/chrome/browser/browser_main.cc +++ b/chrome/browser/browser_main.cc @@ -164,11 +164,10 @@ void SIGTERMHandler(int signal) { DCHECK_EQ(signal, SIGTERM); LOG(WARNING) << "Addressing SIGTERM on " << PlatformThread::CurrentId(); - MessageLoop* main_loop = ChromeThread::GetMessageLoop(ChromeThread::UI); - if (main_loop) { - // Post the exit task to the main thread. - main_loop->PostTask( - FROM_HERE, NewRunnableFunction(BrowserList::CloseAllBrowsersAndExit)); + bool posted = ChromeThread::PostTask( + ChromeThread::UI, FROM_HERE, + NewRunnableFunction(BrowserList::CloseAllBrowsersAndExit)); + if (posted) { LOG(WARNING) << "Posted task to UI thread; resetting SIGTERM handler"; } @@ -178,7 +177,7 @@ void SIGTERMHandler(int signal) { term_action.sa_handler = SIG_DFL; CHECK(sigaction(SIGTERM, &term_action, NULL) == 0); - if (!main_loop) { + if (!posted) { // Without a UI thread to post the exit task to, there aren't many // options. Raise the signal again. The default handler will pick it up // and cause an ungraceful exit. @@ -331,7 +330,7 @@ int BrowserMain(const MainFunctionParams& parameters) { main_message_loop.set_thread_name(thread_name); // Register the main thread by instantiating it, but don't call any methods. - ChromeThread main_thread; + ChromeThread main_thread(ChromeThread::UI, MessageLoop::current()); FilePath user_data_dir; #if defined(OS_WIN) @@ -411,6 +410,10 @@ int BrowserMain(const MainFunctionParams& parameters) { local_state->RegisterBooleanPref(prefs::kMetricsReportingEnabled, GoogleUpdateSettings::GetCollectStatsConsent()); + // Start the database thread (the order of when this happens in this function + // doesn't matter, all we need is to kick it off). + browser_process->db_thread(); + #if defined(TOOLKIT_GTK) // It is important for this to happen before the first run dialog, as it // styles the dialog as well. diff --git a/chrome/browser/browser_process_impl.cc b/chrome/browser/browser_process_impl.cc index ce396a3..ebfb0bf 100644 --- a/chrome/browser/browser_process_impl.cc +++ b/chrome/browser/browser_process_impl.cc @@ -474,8 +474,7 @@ void BrowserProcessImpl::SetIPCLoggingEnabled(bool enable) { // Helper for SetIPCLoggingEnabled. void BrowserProcessImpl::SetIPCLoggingEnabledForChildProcesses(bool enabled) { - DCHECK(MessageLoop::current() == - ChromeThread::GetMessageLoop(ChromeThread::IO)); + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); ChildProcessHost::Iterator i; // default constr references a singleton while (!i.Done()) { diff --git a/chrome/browser/browsing_data_remover.cc b/chrome/browser/browsing_data_remover.cc index c23d5b6..5cfe415 100644 --- a/chrome/browser/browsing_data_remover.cc +++ b/chrome/browser/browsing_data_remover.cc @@ -227,8 +227,7 @@ void BrowsingDataRemover::ClearCacheOnIOThread( base::Time delete_end, MessageLoop* ui_loop) { // This function should be called on the IO thread. - DCHECK(MessageLoop::current() == - ChromeThread::GetMessageLoop(ChromeThread::IO)); + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); // Get a pointer to the cache. net::HttpTransactionFactory* factory = diff --git a/chrome/browser/chrome_plugin_browsing_context.cc b/chrome/browser/chrome_plugin_browsing_context.cc index 504c5c0..e699566 100644 --- a/chrome/browser/chrome_plugin_browsing_context.cc +++ b/chrome/browser/chrome_plugin_browsing_context.cc @@ -10,13 +10,7 @@ #include "chrome/common/notification_service.h" CPBrowsingContextManager* CPBrowsingContextManager::Instance() { -#ifndef NDEBUG - // IO loop is NULL in unit tests. - if (ChromeThread::GetMessageLoop(ChromeThread::IO)) { - DCHECK(MessageLoop::current() == - ChromeThread::GetMessageLoop(ChromeThread::IO)); - } -#endif + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); return Singleton<CPBrowsingContextManager>::get(); } diff --git a/chrome/browser/chrome_plugin_host.cc b/chrome/browser/chrome_plugin_host.cc index 5f13109..5b09fd8 100644 --- a/chrome/browser/chrome_plugin_host.cc +++ b/chrome/browser/chrome_plugin_host.cc @@ -735,13 +735,10 @@ CPError STDCALL CPB_SendSyncMessage(CPID id, const void *data, uint32 data_len, CPError STDCALL CPB_PluginThreadAsyncCall(CPID id, void (*func)(void *), void *user_data) { - MessageLoop *message_loop = ChromeThread::GetMessageLoop(ChromeThread::IO); - if (!message_loop) { - return CPERR_FAILURE; - } - message_loop->PostTask(FROM_HERE, NewRunnableFunction(func, user_data)); - - return CPERR_SUCCESS; + bool posted = ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + NewRunnableFunction(func, user_data)); + return posted ? CPERR_SUCCESS : CPERR_FAILURE; } CPError STDCALL CPB_OpenFileDialog(CPID id, diff --git a/chrome/browser/chrome_thread.cc b/chrome/browser/chrome_thread.cc index 8cd7767..576ac77 100644 --- a/chrome/browser/chrome_thread.cc +++ b/chrome/browser/chrome_thread.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// Copyright (c) 2009 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. @@ -7,10 +7,10 @@ // Friendly names for the well-known threads. static const char* chrome_thread_names[ChromeThread::ID_COUNT] = { "", // UI (name assembled in browser_main.cc). - "Chrome_IOThread", // IO - "Chrome_FileThread", // FILE "Chrome_DBThread", // DB "Chrome_WebKitThread", // WEBKIT + "Chrome_FileThread", // FILE + "Chrome_IOThread", // IO #if defined(OS_LINUX) "Chrome_Background_X11Thread", // BACKGROUND_X11 #endif @@ -18,16 +18,7 @@ static const char* chrome_thread_names[ChromeThread::ID_COUNT] = { Lock ChromeThread::lock_; -ChromeThread* ChromeThread::chrome_threads_[ID_COUNT] = { - NULL, // UI - NULL, // IO - NULL, // FILE - NULL, // DB - NULL, // WEBKIT -#if defined(OS_LINUX) - NULL, // BACKGROUND_X11 -#endif -}; +ChromeThread* ChromeThread::chrome_threads_[ID_COUNT]; ChromeThread::ChromeThread(ChromeThread::ID identifier) : Thread(chrome_thread_names[identifier]), @@ -35,10 +26,10 @@ ChromeThread::ChromeThread(ChromeThread::ID identifier) Initialize(); } -ChromeThread::ChromeThread() - : Thread(MessageLoop::current()->thread_name().c_str()), - identifier_(UI) { - set_message_loop(MessageLoop::current()); +ChromeThread::ChromeThread(ID identifier, MessageLoop* message_loop) + : Thread(message_loop->thread_name().c_str()), + identifier_(identifier) { + set_message_loop(message_loop); Initialize(); } @@ -52,26 +43,108 @@ void ChromeThread::Initialize() { ChromeThread::~ChromeThread() { AutoLock lock(lock_); chrome_threads_[identifier_] = NULL; +#ifndef NDEBUG + // Double check that the threads are ordererd correctly in the enumeration. + for (int i = identifier_ + 1; i < ID_COUNT; ++i) { + DCHECK(!chrome_threads_[i]) << + "Threads must be listed in the reverse order that they die"; + } +#endif } // static -MessageLoop* ChromeThread::GetMessageLoop(ID identifier) { +bool ChromeThread::CurrentlyOn(ID identifier) { + // chrome_threads_[identifier] will be NULL if none is running. This is often + // true when running under unit tests. This behavior actually works out + // pretty conveniently but it's worth noting here. AutoLock lock(lock_); DCHECK(identifier >= 0 && identifier < ID_COUNT); + return chrome_threads_[identifier] && + chrome_threads_[identifier]->message_loop() == MessageLoop::current(); +} - if (chrome_threads_[identifier]) - return chrome_threads_[identifier]->message_loop(); +// static +bool ChromeThread::PostTask(ID identifier, + const tracked_objects::Location& from_here, + Task* task) { + return PostTaskHelper(identifier, from_here, task, 0, true); +} - return NULL; +// static +bool ChromeThread::PostDelayedTask(ID identifier, + const tracked_objects::Location& from_here, + Task* task, + int64 delay_ms) { + return PostTaskHelper(identifier, from_here, task, delay_ms, true); } // static -bool ChromeThread::CurrentlyOn(ID identifier) { - // MessageLoop::current() will return NULL if none is running. This is often - // true when running under unit tests. This behavior actually works out - // pretty convienently (as is mentioned in the header file comment), but it's - // worth noting here. - MessageLoop* message_loop = GetMessageLoop(identifier); - return MessageLoop::current() == message_loop; +bool ChromeThread::PostNonNestableTask( + ID identifier, + const tracked_objects::Location& from_here, + Task* task) { + return PostTaskHelper(identifier, from_here, task, 0, false); +} + +// static +bool ChromeThread::PostNonNestableDelayedTask( + ID identifier, + const tracked_objects::Location& from_here, + Task* task, + int64 delay_ms) { + return PostTaskHelper(identifier, from_here, task, delay_ms, false); +} + +// static +bool ChromeThread::GetCurrentThreadIdentifier(ID* identifier) { + MessageLoop* cur_message_loop = MessageLoop::current(); + for (int i = 0; i < ID_COUNT; ++i) { + if (chrome_threads_[i] && + chrome_threads_[i]->message_loop() == cur_message_loop) { + *identifier = chrome_threads_[i]->identifier_; + return true; + } + } + + return false; } +// static +bool ChromeThread::PostTaskHelper( + ID identifier, + const tracked_objects::Location& from_here, + Task* task, + int64 delay_ms, + bool nestable) { + DCHECK(identifier >= 0 && identifier < ID_COUNT); + // Optimization: to avoid unnecessary locks, we listed the ID enumeration in + // order of lifetime. So no need to lock if we know that the other thread + // outlives this one. + // Note: since the array is so small, ok to loop instead of creating a map, + // which would require a lock because std::map isn't thread safe, defeating + // the whole purpose of this optimization. + ID current_thread; + bool guaranteed_to_outlive_target_thread = + GetCurrentThreadIdentifier(¤t_thread) && + current_thread >= identifier; + + if (!guaranteed_to_outlive_target_thread) + lock_.Acquire(); + + MessageLoop* message_loop = chrome_threads_[identifier] ? + chrome_threads_[identifier]->message_loop() : NULL; + if (message_loop) { + if (nestable) { + message_loop->PostDelayedTask(from_here, task, delay_ms); + } else { + message_loop->PostNonNestableDelayedTask(from_here, task, delay_ms); + } + } else { + delete task; + } + + if (!guaranteed_to_outlive_target_thread) + lock_.Release(); + + return !!message_loop; +} diff --git a/chrome/browser/chrome_thread.h b/chrome/browser/chrome_thread.h index 28e308f..c135d34 100644 --- a/chrome/browser/chrome_thread.h +++ b/chrome/browser/chrome_thread.h @@ -1,11 +1,12 @@ -// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// Copyright (c) 2009 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 CHROME_BROWSER_CHROME_THREAD_H__ -#define CHROME_BROWSER_CHROME_THREAD_H__ +#ifndef CHROME_BROWSER_CHROME_THREAD_H_ +#define CHROME_BROWSER_CHROME_THREAD_H_ #include "base/lock.h" +#include "base/task.h" #include "base/thread.h" /////////////////////////////////////////////////////////////////////////////// @@ -14,30 +15,29 @@ // This class represents a thread that is known by a browser-wide name. For // example, there is one IO thread for the entire browser process, and various // pieces of code find it useful to retrieve a pointer to the IO thread's -// MessageLoop by name: +// Invoke a task by thread ID: // -// MessageLoop* io_loop = ChromeThread::GetMessageLoop(ChromeThread::IO); +// ChromeThread::PostTask(ChromeThread::IO, FROM_HERE, task); // -// On the UI thread, it is often preferable to obtain a pointer to a well-known -// thread via the g_browser_process object, e.g. g_browser_process->io_thread(); -// -// Code that runs on a thread other than the UI thread must take extra care in -// handling pointers to threads because many of the well-known threads are owned -// by the UI thread and can be deallocated without notice. +// The return value is false if the task couldn't be posted because the target +// thread doesn't exist. If this could lead to data loss, you need to check the +// result and restructure the code to ensure it doesn't occur. // +// This class automatically handles the lifetime of different threads. +// It's always safe to call PostTask on any thread. If it's not yet created, +// the task is deleted. There are no race conditions. If the thread that the +// 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 ChromeThread : public base::Thread { public: // An enumeration of the well-known threads. + // NOTE: threads must be listed in the order of their life-time, with each + // thread outliving every other thread below it. enum ID { // The main thread in the browser. UI, - // This is the thread that processes IPC and network messages. - IO, - - // This is the thread that interacts with the file system. - FILE, - // This is the thread that interacts with the database. DB, @@ -45,6 +45,12 @@ class ChromeThread : public base::Thread { // NOT in --single-process mode. WEBKIT, + // This is the thread that interacts with the file system. + FILE, + + // This is the thread that processes IPC and network messages. + IO, + #if defined(OS_LINUX) // This thread has a second connection to the X server and is used to // process UI requests when routing the request to the UI thread would risk @@ -62,20 +68,48 @@ class ChromeThread : public base::Thread { // to construct a ChromeThread that already exists. explicit ChromeThread(ID identifier); - // Special constructor for the main (UI) thread. We use a dummy thread here - // since the main thread already exists. - ChromeThread(); + // Special constructor for the main (UI) thread and unittests. We use a dummy + // thread here since the main thread already exists. + ChromeThread(ID identifier, MessageLoop* message_loop); virtual ~ChromeThread(); - // Callable on any thread, this helper function returns a pointer to the - // thread's MessageLoop. - // - // WARNING: - // Nothing in this class prevents the MessageLoop object returned from this - // function from being destroyed on another thread. Use with care. - // - static MessageLoop* GetMessageLoop(ID identifier); + // These are the same methods in message_loop.h, but are guaranteed to either + // get posted to the MessageLoop if it's still alive, or be deleted otherwise. + // They return true iff the thread existed and the task was posted. Note that + // even if the task is posted, there's no guarantee that it will run, since + // the target thread may already have a Quit message in its queue. + static bool PostTask(ID identifier, + const tracked_objects::Location& from_here, + Task* task); + static bool PostDelayedTask(ID identifier, + const tracked_objects::Location& from_here, + Task* task, + int64 delay_ms); + static bool PostNonNestableTask(ID identifier, + const tracked_objects::Location& from_here, + Task* task); + static bool PostNonNestableDelayedTask( + ID identifier, + const tracked_objects::Location& from_here, + Task* task, + int64 delay_ms); + + template <class T> + static bool DeleteSoon(ID identifier, + const tracked_objects::Location& from_here, + T* object) { + return PostNonNestableTask( + identifier, from_here, new DeleteTask<T>(object)); + } + + template <class T> + static bool ReleaseSoon(ID identifier, + const tracked_objects::Location& from_here, + T* object) { + return PostNonNestableTask( + identifier, from_here, new ReleaseTask<T>(object)); + } // Callable on any thread. Returns whether you're currently on a particular // thread. @@ -91,6 +125,17 @@ class ChromeThread : public base::Thread { // Common initialization code for the constructors. void Initialize(); + // If the current message loop is one of the known threads, returns true and + // sets identifier to its ID. Otherwise returns false. + static bool GetCurrentThreadIdentifier(ID* identifier); + + static bool PostTaskHelper( + ID identifier, + const tracked_objects::Location& from_here, + Task* task, + int64 delay_ms, + bool nestable); + // The identifier of this thread. Only one thread can exist with a given // identifier at a given time. ID identifier_; @@ -106,4 +151,4 @@ class ChromeThread : public base::Thread { static ChromeThread* chrome_threads_[ID_COUNT]; }; -#endif // #ifndef CHROME_BROWSER_CHROME_THREAD_H__ +#endif // #ifndef CHROME_BROWSER_CHROME_THREAD_H_ diff --git a/chrome/browser/chrome_thread_unittest.cc b/chrome/browser/chrome_thread_unittest.cc index 0bcef05..a6c9981 100644 --- a/chrome/browser/chrome_thread_unittest.cc +++ b/chrome/browser/chrome_thread_unittest.cc @@ -11,6 +11,9 @@ typedef PlatformTest ChromeThreadTest; TEST_F(ChromeThreadTest, Get) { + /* + // TODO(jabdelmalek): rewrite this test when the change to delete objects on + // a specific thread lands. scoped_ptr<ChromeThread> io_thread; scoped_ptr<ChromeThread> file_thread; scoped_ptr<ChromeThread> db_thread; @@ -65,4 +68,5 @@ TEST_F(ChromeThreadTest, Get) { EXPECT_TRUE(ChromeThread::GetMessageLoop(ChromeThread::IO) == NULL); EXPECT_TRUE(ChromeThread::GetMessageLoop(ChromeThread::FILE) == NULL); EXPECT_TRUE(ChromeThread::GetMessageLoop(ChromeThread::DB) == NULL); + */ } diff --git a/chrome/browser/chromeos/cros_network_library.cc b/chrome/browser/chromeos/cros_network_library.cc index 8ca9abc..3253a85 100644 --- a/chrome/browser/chromeos/cros_network_library.cc +++ b/chrome/browser/chromeos/cros_network_library.cc @@ -137,11 +137,11 @@ void CrosNetworkLibrary::UpdateNetworkStatus( const WifiNetworkVector& networks, bool ethernet_connected) { // Make sure we run on UI thread. if (!ChromeThread::CurrentlyOn(ChromeThread::UI)) { - MessageLoop* loop = ChromeThread::GetMessageLoop(ChromeThread::UI); - if (loop) - loop->PostTask(FROM_HERE, NewRunnableMethod(this, - &CrosNetworkLibrary::UpdateNetworkStatus, networks, - ethernet_connected)); + ChromeThread::PostTask( + ChromeThread::UI, FROM_HERE, + NewRunnableMethod(this, + &CrosNetworkLibrary::UpdateNetworkStatus, networks, + ethernet_connected)); return; } diff --git a/chrome/browser/chromeos/cros_power_library.cc b/chrome/browser/chromeos/cros_power_library.cc index d3188e8..d8a8e2b 100644 --- a/chrome/browser/chromeos/cros_power_library.cc +++ b/chrome/browser/chromeos/cros_power_library.cc @@ -86,10 +86,9 @@ void CrosPowerLibrary::Init() { void CrosPowerLibrary::UpdatePowerStatus(const chromeos::PowerStatus& status) { // Make sure we run on UI thread. if (!ChromeThread::CurrentlyOn(ChromeThread::UI)) { - MessageLoop* loop = ChromeThread::GetMessageLoop(ChromeThread::UI); - if (loop) - loop->PostTask(FROM_HERE, NewRunnableMethod(this, - &CrosPowerLibrary::UpdatePowerStatus, status)); + ChromeThread::PostTask( + ChromeThread::UI, FROM_HERE, + NewRunnableMethod(this, &CrosPowerLibrary::UpdatePowerStatus, status)); return; } diff --git a/chrome/browser/dom_ui/chrome_url_data_manager.cc b/chrome/browser/dom_ui/chrome_url_data_manager.cc index d937e7a..2f2959a 100644 --- a/chrome/browser/dom_ui/chrome_url_data_manager.cc +++ b/chrome/browser/dom_ui/chrome_url_data_manager.cc @@ -273,7 +273,8 @@ void ChromeURLDataManager::DataAvailable( void ChromeURLDataManager::DataSource::SendResponse( RequestID request_id, RefCountedMemory* bytes) { - ChromeThread::GetMessageLoop(ChromeThread::IO)->PostTask(FROM_HERE, + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, NewRunnableMethod(&chrome_url_data_manager, &ChromeURLDataManager::DataAvailable, request_id, scoped_refptr<RefCountedMemory>(bytes))); diff --git a/chrome/browser/download/download_file.cc b/chrome/browser/download/download_file.cc index 6bb0866..48d7b31 100644 --- a/chrome/browser/download/download_file.cc +++ b/chrome/browser/download/download_file.cc @@ -254,8 +254,10 @@ void DownloadFileManager::StartDownload(DownloadCreateInfo* info) { // about this download so we have to clean up 'info'. We need to get back // to the IO thread to cancel the network request and CancelDownloadRequest // on the UI thread is the safe way to do that. - ui_loop_->PostTask(FROM_HERE, - NewRunnableFunction(&DownloadManager::CancelDownloadRequest, + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + NewRunnableFunction(&DownloadManager::OnCancelDownloadRequest, + resource_dispatcher_host_, info->child_id, info->request_id)); delete info; @@ -385,7 +387,12 @@ void DownloadFileManager::OnStartDownload(DownloadCreateInfo* info) { DownloadManager* manager = DownloadManagerFromRenderIds(info->child_id, info->render_view_id); if (!manager) { - DownloadManager::CancelDownloadRequest(info->child_id, info->request_id); + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + NewRunnableFunction(&DownloadManager::OnCancelDownloadRequest, + resource_dispatcher_host_, + info->child_id, + info->request_id)); delete info; return; } @@ -607,8 +614,10 @@ void DownloadFileManager::OnFinalDownloadName(int id, &DownloadManager::DownloadCancelled, id)); } else { - ui_loop_->PostTask(FROM_HERE, - NewRunnableFunction(&DownloadManager::CancelDownloadRequest, + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + NewRunnableFunction(&DownloadManager::OnCancelDownloadRequest, + resource_dispatcher_host_, download->child_id(), download->request_id())); } diff --git a/chrome/browser/download/download_manager.cc b/chrome/browser/download/download_manager.cc index f412e33..e2f998a 100644 --- a/chrome/browser/download/download_manager.cc +++ b/chrome/browser/download/download_manager.cc @@ -931,26 +931,10 @@ void DownloadManager::DangerousDownloadRenamed(int64 download_handle, } // static -// We have to tell the ResourceDispatcherHost to cancel the download from this -// thread, since we can't forward tasks from the file thread to the IO thread -// reliably (crash on shutdown race condition). -void DownloadManager::CancelDownloadRequest(int render_process_id, - int request_id) { - ResourceDispatcherHost* rdh = g_browser_process->resource_dispatcher_host(); - base::Thread* io_thread = g_browser_process->io_thread(); - if (!io_thread || !rdh) - return; - io_thread->message_loop()->PostTask(FROM_HERE, - NewRunnableFunction(&DownloadManager::OnCancelDownloadRequest, - rdh, - render_process_id, - request_id)); -} - -// static void DownloadManager::OnCancelDownloadRequest(ResourceDispatcherHost* rdh, int render_process_id, int request_id) { + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); rdh->CancelRequest(render_process_id, request_id, false); } @@ -975,8 +959,13 @@ void DownloadManager::DownloadCancelled(int32 download_id) { void DownloadManager::DownloadCancelledInternal(int download_id, int render_process_id, int request_id) { - // Cancel the network request. - CancelDownloadRequest(render_process_id, request_id); + // Cancel the network request. RDH is guaranteed to outlive the IO thread. + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + NewRunnableFunction(&DownloadManager::OnCancelDownloadRequest, + g_browser_process->resource_dispatcher_host(), + render_process_id, + request_id)); // Tell the file manager to cancel the download. file_manager_->RemoveDownload(download_id, this); // On the UI thread @@ -988,24 +977,21 @@ void DownloadManager::DownloadCancelledInternal(int download_id, void DownloadManager::PauseDownload(int32 download_id, bool pause) { DownloadMap::iterator it = in_progress_.find(download_id); - if (it != in_progress_.end()) { - DownloadItem* download = it->second; - if (pause == download->is_paused()) - return; - - // Inform the ResourceDispatcherHost of the new pause state. - base::Thread* io_thread = g_browser_process->io_thread(); - ResourceDispatcherHost* rdh = g_browser_process->resource_dispatcher_host(); - if (!io_thread || !rdh) - return; - - io_thread->message_loop()->PostTask(FROM_HERE, - NewRunnableFunction(&DownloadManager::OnPauseDownloadRequest, - rdh, - download->render_process_id(), - download->request_id(), - pause)); - } + if (it == in_progress_.end()) + return; + + DownloadItem* download = it->second; + if (pause == download->is_paused()) + return; + + // Inform the ResourceDispatcherHost of the new pause state. + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + NewRunnableFunction(&DownloadManager::OnPauseDownloadRequest, + g_browser_process->resource_dispatcher_host(), + download->render_process_id(), + download->request_id(), + pause)); } // static diff --git a/chrome/browser/download/download_manager.h b/chrome/browser/download/download_manager.h index b2d6b3c..34a2e42 100644 --- a/chrome/browser/download/download_manager.h +++ b/chrome/browser/download/download_manager.h @@ -349,11 +349,7 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>, void UpdateDownload(int32 download_id, int64 size); void DownloadFinished(int32 download_id, int64 size); - // Helper method for cancelling the network request associated with a - // download. - static void CancelDownloadRequest(int render_process_id, int request_id); - - // Called from a view when a user clicks a UI button or link. + // Called from a view when a user clicks a UI button or link. void DownloadCancelled(int32 download_id); void PauseDownload(int32 download_id, bool pause); void RemoveDownload(int64 download_handle); @@ -461,6 +457,11 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>, static bool IsExtensionInstall(const DownloadItem* item); static bool IsExtensionInstall(const DownloadCreateInfo* info); + // Runs the network cancel. Must be called on the IO thread. + static void OnCancelDownloadRequest(ResourceDispatcherHost* rdh, + int render_process_id, + int request_id); + private: // Opens a download via the Windows shell. void OpenDownloadInShell(const DownloadItem* download, @@ -510,11 +511,6 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>, int render_process_id, int request_id); - // Runs the network cancel on the IO thread. - static void OnCancelDownloadRequest(ResourceDispatcherHost* rdh, - int render_process_id, - int request_id); - // Runs the pause on the IO thread. static void OnPauseDownloadRequest(ResourceDispatcherHost* rdh, int render_process_id, diff --git a/chrome/browser/extensions/autoupdate_interceptor.cc b/chrome/browser/extensions/autoupdate_interceptor.cc index b908133..f8f9bfd 100644 --- a/chrome/browser/extensions/autoupdate_interceptor.cc +++ b/chrome/browser/extensions/autoupdate_interceptor.cc @@ -68,8 +68,7 @@ void AutoUpdateInterceptor::SetResponse(const std::string url, void AutoUpdateInterceptor::SetResponseOnIOThread(const std::string url, const FilePath& path) { - MessageLoop* io_loop = ChromeThread::GetMessageLoop(ChromeThread::IO); - io_loop->PostTask(FROM_HERE, - NewRunnableMethod(this, &AutoUpdateInterceptor::SetResponse, - url, path)); + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + NewRunnableMethod(this, &AutoUpdateInterceptor::SetResponse, url, path)); } diff --git a/chrome/browser/extensions/extension_disabled_infobar_delegate.cc b/chrome/browser/extensions/extension_disabled_infobar_delegate.cc index 8e4de1a..64a1534 100644 --- a/chrome/browser/extensions/extension_disabled_infobar_delegate.cc +++ b/chrome/browser/extensions/extension_disabled_infobar_delegate.cc @@ -33,7 +33,8 @@ class ExtensionDisabledDialogDelegate install_icon_resource_ = extension_->GetIconPath(Extension::EXTENSION_ICON_LARGE); - ChromeThread::GetMessageLoop(ChromeThread::FILE)->PostTask(FROM_HERE, + ChromeThread::PostTask( + ChromeThread::FILE, FROM_HERE, NewRunnableMethod(this, &ExtensionDisabledDialogDelegate::Start)); } diff --git a/chrome/browser/extensions/extension_message_service.cc b/chrome/browser/extensions/extension_message_service.cc index 9107c4b..cd011f1 100644 --- a/chrome/browser/extensions/extension_message_service.cc +++ b/chrome/browser/extensions/extension_message_service.cc @@ -198,8 +198,7 @@ int ExtensionMessageService::OpenChannelToExtension( int routing_id, const std::string& source_extension_id, const std::string& target_extension_id, const std::string& channel_name, ResourceMessageFilter* source) { - DCHECK_EQ(MessageLoop::current(), - ChromeThread::GetMessageLoop(ChromeThread::IO)); + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); // Create a channel ID for both sides of the channel. int port1_id = -1; @@ -222,8 +221,7 @@ int ExtensionMessageService::OpenChannelToTab(int routing_id, const std::string& extension_id, const std::string& channel_name, ResourceMessageFilter* source) { - DCHECK_EQ(MessageLoop::current(), - ChromeThread::GetMessageLoop(ChromeThread::IO)); + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); // Create a channel ID for both sides of the channel. int port1_id = -1; diff --git a/chrome/browser/extensions/extension_updater_unittest.cc b/chrome/browser/extensions/extension_updater_unittest.cc index 69f2460..600ae71 100644 --- a/chrome/browser/extensions/extension_updater_unittest.cc +++ b/chrome/browser/extensions/extension_updater_unittest.cc @@ -9,6 +9,7 @@ #include "base/stl_util-inl.h" #include "base/string_util.h" #include "base/thread.h" +#include "chrome/browser/chrome_thread.h" #include "chrome/browser/extensions/extension_updater.h" #include "chrome/browser/extensions/extensions_service.h" #include "chrome/browser/net/test_url_fetcher_factory.h" @@ -234,9 +235,12 @@ class ExtensionUpdaterTest : public testing::Test { service.set_extensions(tmp); // Setup and start the updater. + MessageLoop message_loop; + ChromeThread io_thread(ChromeThread::IO); + io_thread.Start(); + TestURLFetcherFactory factory; URLFetcher::set_factory(&factory); - MessageLoop message_loop; ScopedTempPrefService prefs; scoped_refptr<ExtensionUpdater> updater = new ExtensionUpdater(&service, prefs.get(), 60*60*24, NULL, NULL); @@ -277,9 +281,12 @@ class ExtensionUpdaterTest : public testing::Test { ServiceForManifestTests service; // Setup and start the updater. + MessageLoop message_loop; + ChromeThread io_thread(ChromeThread::IO); + io_thread.Start(); + TestURLFetcherFactory factory; URLFetcher::set_factory(&factory); - MessageLoop message_loop; ScopedTempPrefService prefs; scoped_refptr<ExtensionUpdater> updater = new ExtensionUpdater(&service, prefs.get(), 60*60*24, NULL, NULL); @@ -349,15 +356,16 @@ class ExtensionUpdaterTest : public testing::Test { } static void TestMultipleManifestDownloading() { + MessageLoop ui_loop; + ChromeThread file_thread(ChromeThread::FILE); + file_thread.Start(); + ChromeThread io_thread(ChromeThread::IO); + io_thread.Start(); + TestURLFetcherFactory factory; TestURLFetcher* fetcher = NULL; URLFetcher::set_factory(&factory); ServiceForDownloadTests service; - MessageLoop ui_loop; - base::Thread file_thread("File Thread"); - ASSERT_TRUE(file_thread.Start()); - base::Thread io_thread("IO Thread"); - ASSERT_TRUE(io_thread.Start()); ScopedTempPrefService prefs; scoped_refptr<ExtensionUpdater> updater = new ExtensionUpdater(&service, prefs.get(), kUpdateFrequencySecs, @@ -410,8 +418,10 @@ class ExtensionUpdaterTest : public testing::Test { static void TestSingleExtensionDownloading() { MessageLoop ui_loop; - base::Thread file_thread("File Thread"); - ASSERT_TRUE(file_thread.Start()); + ChromeThread file_thread(ChromeThread::FILE); + file_thread.Start(); + ChromeThread io_thread(ChromeThread::IO); + io_thread.Start(); TestURLFetcherFactory factory; TestURLFetcher* fetcher = NULL; @@ -457,6 +467,9 @@ class ExtensionUpdaterTest : public testing::Test { static void TestBlacklistDownloading() { MessageLoop message_loop; + ChromeThread io_thread(ChromeThread::IO); + io_thread.Start(); + TestURLFetcherFactory factory; TestURLFetcher* fetcher = NULL; URLFetcher::set_factory(&factory); @@ -501,6 +514,9 @@ class ExtensionUpdaterTest : public testing::Test { static void TestMultipleExtensionDownloading() { MessageLoopForUI message_loop; + ChromeThread io_thread(ChromeThread::IO); + io_thread.Start(); + TestURLFetcherFactory factory; TestURLFetcher* fetcher = NULL; URLFetcher::set_factory(&factory); diff --git a/chrome/browser/extensions/extensions_ui.cc b/chrome/browser/extensions/extensions_ui.cc index 7bae618..ef05466 100644 --- a/chrome/browser/extensions/extensions_ui.cc +++ b/chrome/browser/extensions/extensions_ui.cc @@ -10,7 +10,6 @@ #include "base/thread.h" #include "chrome/browser/browser.h" #include "chrome/browser/browser_process.h" -#include "chrome/browser/chrome_thread.h" #include "chrome/browser/debugger/devtools_manager.h" #include "chrome/browser/extensions/extension_function_dispatcher.h" #include "chrome/browser/extensions/extension_message_service.h" @@ -265,8 +264,7 @@ void ExtensionsDOMHandler::HandlePackMessage(const Value* value) { return; } - pack_job_ = new PackExtensionJob(this, root_directory, key_file, - ChromeThread::GetMessageLoop(ChromeThread::FILE)); + pack_job_ = new PackExtensionJob(this, root_directory, key_file); } void ExtensionsDOMHandler::OnPackSuccess(const FilePath& crx_file, diff --git a/chrome/browser/extensions/file_reader.cc b/chrome/browser/extensions/file_reader.cc index 955e664..e848c2e 100644 --- a/chrome/browser/extensions/file_reader.cc +++ b/chrome/browser/extensions/file_reader.cc @@ -16,7 +16,8 @@ FileReader::FileReader(const ExtensionResource& resource, Callback* callback) } void FileReader::Start() { - ChromeThread::GetMessageLoop(ChromeThread::FILE)->PostTask(FROM_HERE, + ChromeThread::PostTask( + ChromeThread::FILE, FROM_HERE, NewRunnableMethod(this, &FileReader::ReadFileOnBackgroundThread)); } diff --git a/chrome/browser/extensions/pack_extension_job.cc b/chrome/browser/extensions/pack_extension_job.cc index c9c26cf..497f1d1 100644 --- a/chrome/browser/extensions/pack_extension_job.cc +++ b/chrome/browser/extensions/pack_extension_job.cc @@ -7,16 +7,17 @@ #include "base/message_loop.h" #include "base/string_util.h" #include "base/task.h" +#include "chrome/browser/chrome_thread.h" #include "chrome/browser/extensions/extension_creator.h" PackExtensionJob::PackExtensionJob(Client* client, const FilePath& root_directory, - const FilePath& key_file, - MessageLoop* file_loop) - : ui_loop_(MessageLoop::current()), file_loop_(file_loop), client_(client), + const FilePath& key_file) + : ui_loop_(MessageLoop::current()), client_(client), root_directory_(root_directory), key_file_(key_file) { - file_loop_->PostTask(FROM_HERE, NewRunnableMethod(this, - &PackExtensionJob::RunOnFileThread)); + ChromeThread::PostTask( + ChromeThread::FILE, FROM_HERE, + NewRunnableMethod(this, &PackExtensionJob::RunOnFileThread)); } void PackExtensionJob::ClearClient() { diff --git a/chrome/browser/extensions/pack_extension_job.h b/chrome/browser/extensions/pack_extension_job.h index 8c3cf26..b82e9d0 100644 --- a/chrome/browser/extensions/pack_extension_job.h +++ b/chrome/browser/extensions/pack_extension_job.h @@ -27,8 +27,7 @@ class PackExtensionJob : public base::RefCountedThreadSafe<PackExtensionJob> { PackExtensionJob(Client* client, const FilePath& root_directory, - const FilePath& key_file, - MessageLoop* file_loop); + const FilePath& key_file); // The client should call this when it is destroyed to prevent // PackExtensionJob from attempting to access it. @@ -40,7 +39,6 @@ class PackExtensionJob : public base::RefCountedThreadSafe<PackExtensionJob> { void ReportFailureOnUIThread(const std::string& error); MessageLoop* ui_loop_; - MessageLoop* file_loop_; Client* client_; FilePath root_directory_; FilePath key_file_; diff --git a/chrome/browser/extensions/sandboxed_extension_unpacker.cc b/chrome/browser/extensions/sandboxed_extension_unpacker.cc index 8756450..d3d9f61 100644 --- a/chrome/browser/extensions/sandboxed_extension_unpacker.cc +++ b/chrome/browser/extensions/sandboxed_extension_unpacker.cc @@ -73,8 +73,10 @@ void SandboxedExtensionUnpacker::Start() { #endif if (use_utility_process) { - ChromeThread::GetMessageLoop(ChromeThread::IO)->PostTask(FROM_HERE, - NewRunnableMethod(this, + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + NewRunnableMethod( + this, &SandboxedExtensionUnpacker::StartProcessOnIOThread, temp_crx_path)); } else { diff --git a/chrome/browser/extensions/user_script_listener_unittest.cc b/chrome/browser/extensions/user_script_listener_unittest.cc index fff1002..59357de 100644 --- a/chrome/browser/extensions/user_script_listener_unittest.cc +++ b/chrome/browser/extensions/user_script_listener_unittest.cc @@ -7,6 +7,7 @@ #include "base/message_loop.h" #include "base/thread.h" +#include "chrome/browser/chrome_thread.h" #include "chrome/browser/extensions/extensions_service.h" #include "chrome/browser/renderer_host/resource_dispatcher_host.h" #include "chrome/common/chrome_paths.h" @@ -41,9 +42,9 @@ class MockUserScriptMaster : public UserScriptMaster { } }; -class MockIOThread : public base::Thread { +class MockIOThread : public ChromeThread { public: - MockIOThread() : base::Thread("IO") {} + MockIOThread() : ChromeThread(ChromeThread::IO) {} virtual ~MockIOThread() { Stop(); } private: diff --git a/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.cc b/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.cc index c3dc389..9380cd3 100644 --- a/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.cc +++ b/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.cc @@ -36,16 +36,13 @@ DOMStorageDispatcherHost::DOMStorageDispatcherHost( : webkit_context_(webkit_context), webkit_thread_(webkit_thread), message_sender_(message_sender), - process_handle_(0), - ever_used_(false), - shutdown_(false) { + process_handle_(0) { DCHECK(webkit_context_.get()); DCHECK(webkit_thread_); DCHECK(message_sender_); } DOMStorageDispatcherHost::~DOMStorageDispatcherHost() { - DCHECK(shutdown_); } void DOMStorageDispatcherHost::Init(base::ProcessHandle process_handle) { @@ -62,23 +59,16 @@ void DOMStorageDispatcherHost::Shutdown() { if (process_handle_) // Init() was called Context()->UnregisterDispatcherHost(this); message_sender_ = NULL; - if (!ever_used_) { - // No need to (possibly) spin up the WebKit thread for a no-op. - shutdown_ = true; - return; - } - - MessageLoop* webkit_loop = webkit_thread_->GetMessageLoop(); - webkit_loop->PostTask(FROM_HERE, NewRunnableMethod(this, - &DOMStorageDispatcherHost::Shutdown)); + + // The task will only execute if the WebKit thread is already running. + ChromeThread::PostTask( + ChromeThread::WEBKIT, FROM_HERE, + NewRunnableMethod(this, &DOMStorageDispatcherHost::Shutdown)); return; } DCHECK(ChromeThread::CurrentlyOn(ChromeThread::WEBKIT)); - DCHECK(ever_used_); DCHECK(!message_sender_); - DCHECK(!shutdown_); - shutdown_ = true; // TODO(jorlow): Do stuff that needs to be run on the WebKit thread. Locks // and others will likely need this, so let's not delete this @@ -91,15 +81,16 @@ void DOMStorageDispatcherHost::DispatchStorageEvent(const string16& key, const string16& origin, bool is_local_storage) { DCHECK(ChromeThread::CurrentlyOn(ChromeThread::WEBKIT)); DCHECK(current_); - current_->webkit_thread_->PostIOThreadTask(FROM_HERE, NewRunnableMethod( - current_, &DOMStorageDispatcherHost::OnStorageEvent, key, old_value, - new_value, origin, is_local_storage)); + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + NewRunnableMethod( + current_, &DOMStorageDispatcherHost::OnStorageEvent, key, old_value, + new_value, origin, is_local_storage)); } bool DOMStorageDispatcherHost::OnMessageReceived(const IPC::Message& message, bool *msg_is_ok) { DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); - DCHECK(!shutdown_); DCHECK(process_handle_); bool handled = true; @@ -118,13 +109,10 @@ bool DOMStorageDispatcherHost::OnMessageReceived(const IPC::Message& message, IPC_MESSAGE_HANDLER(ViewHostMsg_DOMStorageClear, OnClear) IPC_MESSAGE_UNHANDLED(handled = false) IPC_END_MESSAGE_MAP() - if (handled) - ever_used_ = true; return handled; } void DOMStorageDispatcherHost::Send(IPC::Message* message) { - DCHECK(!shutdown_); if (!message_sender_) { delete message; return; @@ -137,16 +125,15 @@ void DOMStorageDispatcherHost::Send(IPC::Message* message) { // The IO thread can't go away while the WebKit thread is still running. DCHECK(ChromeThread::CurrentlyOn(ChromeThread::WEBKIT)); - webkit_thread_->PostIOThreadTask(FROM_HERE, NewRunnableMethod(this, - &DOMStorageDispatcherHost::Send, message)); + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + NewRunnableMethod(this, &DOMStorageDispatcherHost::Send, message)); } void DOMStorageDispatcherHost::OnNamespaceId(DOMStorageType storage_type, IPC::Message* reply_msg) { - DCHECK(!shutdown_); if (ChromeThread::CurrentlyOn(ChromeThread::IO)) { - MessageLoop* webkit_loop = webkit_thread_->GetMessageLoop(); - webkit_loop->PostTask(FROM_HERE, NewRunnableMethod(this, + PostTaskToWebKitThread(FROM_HERE, NewRunnableMethod(this, &DOMStorageDispatcherHost::OnNamespaceId, storage_type, reply_msg)); return; @@ -165,10 +152,8 @@ void DOMStorageDispatcherHost::OnNamespaceId(DOMStorageType storage_type, void DOMStorageDispatcherHost::OnCloneNamespaceId(int64 namespace_id, IPC::Message* reply_msg) { - DCHECK(!shutdown_); if (ChromeThread::CurrentlyOn(ChromeThread::IO)) { - MessageLoop* webkit_loop = webkit_thread_->GetMessageLoop(); - webkit_loop->PostTask(FROM_HERE, NewRunnableMethod(this, + PostTaskToWebKitThread(FROM_HERE, NewRunnableMethod(this, &DOMStorageDispatcherHost::OnCloneNamespaceId, namespace_id, reply_msg)); return; @@ -192,10 +177,8 @@ void DOMStorageDispatcherHost::OnCloneNamespaceId(int64 namespace_id, void DOMStorageDispatcherHost::OnStorageAreaId(int64 namespace_id, const string16& origin, IPC::Message* reply_msg) { - DCHECK(!shutdown_); if (ChromeThread::CurrentlyOn(ChromeThread::IO)) { - MessageLoop* webkit_loop = webkit_thread_->GetMessageLoop(); - webkit_loop->PostTask(FROM_HERE, NewRunnableMethod(this, + PostTaskToWebKitThread(FROM_HERE, NewRunnableMethod(this, &DOMStorageDispatcherHost::OnStorageAreaId, namespace_id, origin, reply_msg)); return; @@ -218,10 +201,8 @@ void DOMStorageDispatcherHost::OnStorageAreaId(int64 namespace_id, void DOMStorageDispatcherHost::OnLength(int64 storage_area_id, IPC::Message* reply_msg) { - DCHECK(!shutdown_); if (ChromeThread::CurrentlyOn(ChromeThread::IO)) { - MessageLoop* webkit_loop = webkit_thread_->GetMessageLoop(); - webkit_loop->PostTask(FROM_HERE, NewRunnableMethod(this, + PostTaskToWebKitThread(FROM_HERE, NewRunnableMethod(this, &DOMStorageDispatcherHost::OnLength, storage_area_id, reply_msg)); return; } @@ -241,10 +222,8 @@ void DOMStorageDispatcherHost::OnLength(int64 storage_area_id, void DOMStorageDispatcherHost::OnKey(int64 storage_area_id, unsigned index, IPC::Message* reply_msg) { - DCHECK(!shutdown_); if (ChromeThread::CurrentlyOn(ChromeThread::IO)) { - MessageLoop* webkit_loop = webkit_thread_->GetMessageLoop(); - webkit_loop->PostTask(FROM_HERE, NewRunnableMethod(this, + PostTaskToWebKitThread(FROM_HERE, NewRunnableMethod(this, &DOMStorageDispatcherHost::OnKey, storage_area_id, index, reply_msg)); return; } @@ -265,10 +244,8 @@ void DOMStorageDispatcherHost::OnKey(int64 storage_area_id, unsigned index, void DOMStorageDispatcherHost::OnGetItem(int64 storage_area_id, const string16& key, IPC::Message* reply_msg) { - DCHECK(!shutdown_); if (ChromeThread::CurrentlyOn(ChromeThread::IO)) { - MessageLoop* webkit_loop = webkit_thread_->GetMessageLoop(); - webkit_loop->PostTask(FROM_HERE, NewRunnableMethod(this, + PostTaskToWebKitThread(FROM_HERE, NewRunnableMethod(this, &DOMStorageDispatcherHost::OnGetItem, storage_area_id, key, reply_msg)); return; @@ -289,10 +266,8 @@ void DOMStorageDispatcherHost::OnGetItem(int64 storage_area_id, void DOMStorageDispatcherHost::OnSetItem(int64 storage_area_id, const string16& key, const string16& value, IPC::Message* reply_msg) { - DCHECK(!shutdown_); if (ChromeThread::CurrentlyOn(ChromeThread::IO)) { - MessageLoop* webkit_loop = webkit_thread_->GetMessageLoop(); - webkit_loop->PostTask(FROM_HERE, NewRunnableMethod(this, + PostTaskToWebKitThread(FROM_HERE, NewRunnableMethod(this, &DOMStorageDispatcherHost::OnSetItem, storage_area_id, key, value, reply_msg)); return; @@ -315,10 +290,8 @@ void DOMStorageDispatcherHost::OnSetItem(int64 storage_area_id, void DOMStorageDispatcherHost::OnRemoveItem(int64 storage_area_id, const string16& key) { - DCHECK(!shutdown_); if (ChromeThread::CurrentlyOn(ChromeThread::IO)) { - MessageLoop* webkit_loop = webkit_thread_->GetMessageLoop(); - webkit_loop->PostTask(FROM_HERE, NewRunnableMethod(this, + PostTaskToWebKitThread(FROM_HERE, NewRunnableMethod(this, &DOMStorageDispatcherHost::OnRemoveItem, storage_area_id, key)); return; } @@ -336,10 +309,8 @@ void DOMStorageDispatcherHost::OnRemoveItem(int64 storage_area_id, } void DOMStorageDispatcherHost::OnClear(int64 storage_area_id) { - DCHECK(!shutdown_); if (ChromeThread::CurrentlyOn(ChromeThread::IO)) { - MessageLoop* webkit_loop = webkit_thread_->GetMessageLoop(); - webkit_loop->PostTask(FROM_HERE, NewRunnableMethod(this, + PostTaskToWebKitThread(FROM_HERE, NewRunnableMethod(this, &DOMStorageDispatcherHost::OnClear, storage_area_id)); return; } @@ -372,3 +343,9 @@ void DOMStorageDispatcherHost::OnStorageEvent(const string16& key, ++cur; } } + +void DOMStorageDispatcherHost::PostTaskToWebKitThread( + const tracked_objects::Location& from_here, Task* task) { + webkit_thread_->EnsureInitialized(); + ChromeThread::PostTask(ChromeThread::WEBKIT, FROM_HERE, task); +} diff --git a/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.h b/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.h index ad3206f..94a7c00 100644 --- a/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.h +++ b/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.h @@ -7,12 +7,14 @@ #include "base/process.h" #include "base/ref_counted.h" +#include "base/tracked.h" #include "chrome/browser/in_process_webkit/storage_area.h" #include "chrome/browser/in_process_webkit/webkit_context.h" #include "chrome/common/dom_storage_type.h" #include "ipc/ipc_message.h" class DOMStorageContext; +class Task; class WebKitThread; // This class handles the logistics of DOM Storage within the browser process. @@ -69,10 +71,13 @@ class DOMStorageDispatcherHost : // A shortcut for accessing our context. DOMStorageContext* Context() { - DCHECK(!shutdown_); return webkit_context_->dom_storage_context(); } + // Posts a task to the WebKit thread, initializing it if necessary. + void PostTaskToWebKitThread( + const tracked_objects::Location& from_here, Task* task); + // Use whenever there's a chance OnStorageEvent will be called. class AutoSetCurrentDispatcherHost { public: @@ -96,16 +101,6 @@ class DOMStorageDispatcherHost : // handle. base::ProcessHandle process_handle_; - // Has this dispatcher ever handled a message. If not, then we can skip - // the entire shutdown procedure. This is only set to true on the IO thread - // and must be true if we're reading it on the WebKit thread. - bool ever_used_; - - // This is set once the Shutdown routine runs on the WebKit thread. Once - // set, we should not process any more messages because storage_area_map_ - // and storage_namespace_map_ contain pointers to deleted objects. - bool shutdown_; - DISALLOW_IMPLICIT_CONSTRUCTORS(DOMStorageDispatcherHost); }; diff --git a/chrome/browser/in_process_webkit/webkit_context.cc b/chrome/browser/in_process_webkit/webkit_context.cc index a46d53f..384e491 100644 --- a/chrome/browser/in_process_webkit/webkit_context.cc +++ b/chrome/browser/in_process_webkit/webkit_context.cc @@ -14,13 +14,15 @@ WebKitContext::WebKitContext(const FilePath& data_path, bool is_incognito) } WebKitContext::~WebKitContext() { - // If the WebKit thread was ever spun up, delete the object there. If we're - // on the IO thread, this is safe because the WebKit thread goes away after - // the IO. If we're on the UI thread, we're safe because the UI thread kills - // the WebKit thread. - MessageLoop* webkit_loop = ChromeThread::GetMessageLoop(ChromeThread::WEBKIT); - if (webkit_loop) - webkit_loop->DeleteSoon(FROM_HERE, dom_storage_context_.release()); + // If the WebKit thread was ever spun up, delete the object there. The task + // will just get deleted if the WebKit thread isn't created. + DOMStorageContext* dom_storage_context = dom_storage_context_.release(); + if (!ChromeThread::DeleteSoon( + ChromeThread::WEBKIT, FROM_HERE, dom_storage_context)) { + // The WebKit thread wasn't created, and the task got deleted without + // freeing the DOMStorageContext, so delete it manually. + delete dom_storage_context; + } } void WebKitContext::PurgeMemory() { @@ -28,18 +30,15 @@ void WebKitContext::PurgeMemory() { // thread. // // Note that if there is no WebKit thread, then there's nothing in - // LocalStorage and it's OK to no-op here. Further note that in a unittest, - // there may be no threads at all, in which case MessageLoop::current() will - // also be NULL and we'll go ahead and call PurgeMemory() directly, which is - // probably what the test wants. - MessageLoop* webkit_loop = ChromeThread::GetMessageLoop(ChromeThread::WEBKIT); - if (MessageLoop::current() == webkit_loop) { + // LocalStorage and it's OK to no-op here. + if (ChromeThread::CurrentlyOn(ChromeThread::WEBKIT)) { dom_storage_context_->PurgeMemory(); - } else if (webkit_loop) { + } else { // Since we're not on the WebKit thread, proxy the call over to it. We // can't post a task to call DOMStorageContext::PurgeMemory() directly // because that class is not refcounted. - webkit_loop->PostTask(FROM_HERE, - NewRunnableMethod(this, &WebKitContext::PurgeMemory)); + ChromeThread::PostTask( + ChromeThread::WEBKIT, FROM_HERE, + NewRunnableMethod(this, &WebKitContext::PurgeMemory)); } } diff --git a/chrome/browser/in_process_webkit/webkit_thread.cc b/chrome/browser/in_process_webkit/webkit_thread.cc index 46b62f4..77ad9b4 100644 --- a/chrome/browser/in_process_webkit/webkit_thread.cc +++ b/chrome/browser/in_process_webkit/webkit_thread.cc @@ -10,39 +10,24 @@ #include "webkit/api/public/WebKit.h" // This happens on the UI thread before the IO thread has been shut down. -WebKitThread::WebKitThread() - : io_message_loop_(ChromeThread::GetMessageLoop(ChromeThread::IO)) { +WebKitThread::WebKitThread() { // The thread is started lazily by InitializeThread() on the IO thread. } // This happens on the UI thread after the IO thread has been shut down. WebKitThread::~WebKitThread() { - // There's no good way to see if we're on the UI thread. + // We can't just check CurrentlyOn(ChromeThread::UI) because in unit tests, + // MessageLoop::Current is sometimes NULL and other times valid and there's + // no ChromeThread object. Can't check that CurrentlyOn is not IO since + // some unit tests set that ChromeThread for other checks. DCHECK(!ChromeThread::CurrentlyOn(ChromeThread::WEBKIT)); - DCHECK(!ChromeThread::CurrentlyOn(ChromeThread::IO)); - DCHECK(!io_message_loop_); } -void WebKitThread::Shutdown() { +void WebKitThread::EnsureInitialized() { DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); - DCHECK(io_message_loop_); - - AutoLock lock(io_message_loop_lock_); - io_message_loop_ = NULL; -} - -bool WebKitThread::PostIOThreadTask( - const tracked_objects::Location& from_here, Task* task) { - DCHECK(ChromeThread::CurrentlyOn(ChromeThread::WEBKIT)); - { - AutoLock lock(io_message_loop_lock_); - if (io_message_loop_) { - io_message_loop_->PostTask(from_here, task); - return true; - } - } - delete task; - return false; + if (webkit_thread_.get()) + return; + InitializeThread(); } WebKitThread::InternalWebKitThread::InternalWebKitThread() @@ -69,7 +54,6 @@ MessageLoop* WebKitThread::InitializeThread() { if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kSingleProcess)) return NULL; - DCHECK(io_message_loop_); DCHECK(!webkit_thread_.get()); webkit_thread_.reset(new InternalWebKitThread); bool started = webkit_thread_->Start(); diff --git a/chrome/browser/in_process_webkit/webkit_thread.h b/chrome/browser/in_process_webkit/webkit_thread.h index 828277c..97b1856 100644 --- a/chrome/browser/in_process_webkit/webkit_thread.h +++ b/chrome/browser/in_process_webkit/webkit_thread.h @@ -18,31 +18,16 @@ class BrowserWebKitClientImpl; // process. It should be instantiated and destroyed on the UI thread // before/after the IO thread is created/destroyed. All other usage should be // on the IO thread. If the browser is being run in --single-process mode, a -// thread will never be spun up, and GetMessageLoop() will always return NULL. +// thread will never be spun up. class WebKitThread { public: // Called from the UI thread. WebKitThread(); ~WebKitThread(); - // Returns the message loop for the WebKit thread unless we're in - // --single-processuntil mode, in which case it'll return NULL. Only call + // Creates the WebKit thread if it hasn't been already created. Only call // from the IO thread. Only do fast-path work here. - MessageLoop* GetMessageLoop() { - DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); - if (!webkit_thread_.get()) - return InitializeThread(); - return webkit_thread_->message_loop(); - } - - // Called from the IO thread. Notifies us that it's no longer safe to post - // tasks to the IO thread. - void Shutdown(); - - // Post a task to the IO thread if we haven't yet been told to shut down. - // Only call from the WebKit thread. - bool PostIOThreadTask(const tracked_objects::Location& from_here, - Task* task); + void EnsureInitialized(); private: // Must be private so that we can carefully control its lifetime. @@ -68,11 +53,6 @@ class WebKitThread { // from the IO thread while the WebKit thread is not running. scoped_ptr<InternalWebKitThread> webkit_thread_; - // A pointer to the IO message loop. This is nulled out when Shutdown() is - // called. Only access under the io_message_loop_lock_. - MessageLoop* io_message_loop_; - Lock io_message_loop_lock_; - DISALLOW_COPY_AND_ASSIGN(WebKitThread); }; diff --git a/chrome/browser/memory_details.cc b/chrome/browser/memory_details.cc index d96afdc..eed3bc9 100644 --- a/chrome/browser/memory_details.cc +++ b/chrome/browser/memory_details.cc @@ -49,8 +49,7 @@ void MemoryDetails::StartFetch() { } void MemoryDetails::CollectChildInfoOnIOThread() { - DCHECK(MessageLoop::current() == - ChromeThread::GetMessageLoop(ChromeThread::IO)); + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); std::vector<ProcessMemoryInformation> child_info; @@ -67,7 +66,8 @@ void MemoryDetails::CollectChildInfoOnIOThread() { } // Now go do expensive memory lookups from the file thread. - ChromeThread::GetMessageLoop(ChromeThread::FILE)->PostTask(FROM_HERE, + ChromeThread::PostTask( + ChromeThread::FILE, FROM_HERE, NewRunnableMethod(this, &MemoryDetails::CollectProcessData, child_info)); } diff --git a/chrome/browser/memory_details_linux.cc b/chrome/browser/memory_details_linux.cc index a1ed5e9..0c815ab 100644 --- a/chrome/browser/memory_details_linux.cc +++ b/chrome/browser/memory_details_linux.cc @@ -187,8 +187,7 @@ static void GetAllChildren(const std::vector<Process>& processes, void MemoryDetails::CollectProcessData( std::vector<ProcessMemoryInformation> child_info) { - DCHECK(MessageLoop::current() == - ChromeThread::GetMessageLoop(ChromeThread::FILE)); + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE)); std::vector<Process> processes; GetProcesses(&processes); diff --git a/chrome/browser/memory_details_win.cc b/chrome/browser/memory_details_win.cc index 56aa9fb..db5cce0 100644 --- a/chrome/browser/memory_details_win.cc +++ b/chrome/browser/memory_details_win.cc @@ -62,8 +62,7 @@ ProcessData* MemoryDetails::ChromeBrowser() { void MemoryDetails::CollectProcessData( std::vector<ProcessMemoryInformation> child_info) { - DCHECK(MessageLoop::current() == - ChromeThread::GetMessageLoop(ChromeThread::FILE)); + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE)); // Clear old data. for (int index = 0; index < arraysize(g_process_template); index++) diff --git a/chrome/browser/net/chrome_url_request_context.cc b/chrome/browser/net/chrome_url_request_context.cc index 33b6f61..a9e2cd8 100644 --- a/chrome/browser/net/chrome_url_request_context.cc +++ b/chrome/browser/net/chrome_url_request_context.cc @@ -257,9 +257,7 @@ ChromeURLRequestContext* FactoryForOriginal::Create() { DCHECK(!cookie_store_path_.empty()); scoped_refptr<SQLitePersistentCookieStore> cookie_db = - new SQLitePersistentCookieStore( - cookie_store_path_, - db_loop_); + new SQLitePersistentCookieStore(cookie_store_path_); context->set_cookie_store(new net::CookieMonster(cookie_db.get())); } @@ -318,7 +316,7 @@ ChromeURLRequestContext* FactoryForExtensions::Create() { DCHECK(!cookie_store_path_.empty()); scoped_refptr<SQLitePersistentCookieStore> cookie_db = - new SQLitePersistentCookieStore(cookie_store_path_, db_loop_); + new SQLitePersistentCookieStore(cookie_store_path_); net::CookieMonster* cookie_monster = new net::CookieMonster(cookie_db.get()); // Enable cookies for extension URLs only. diff --git a/chrome/browser/net/sqlite_persistent_cookie_store.cc b/chrome/browser/net/sqlite_persistent_cookie_store.cc index 7f19610..c79fa42 100644 --- a/chrome/browser/net/sqlite_persistent_cookie_store.cc +++ b/chrome/browser/net/sqlite_persistent_cookie_store.cc @@ -14,6 +14,7 @@ #include "base/scoped_ptr.h" #include "base/string_util.h" #include "base/thread.h" +#include "chrome/browser/chrome_thread.h" #include "chrome/browser/diagnostics/sqlite_diagnostics.h" using base::Time; @@ -25,9 +26,8 @@ class SQLitePersistentCookieStore::Backend public: // The passed database pointer must be already-initialized. This object will // take ownership. - explicit Backend(sql::Connection* db, MessageLoop* loop) + explicit Backend(sql::Connection* db) : db_(db), - background_loop_(loop), num_pending_(0) { DCHECK(db_) << "Database must exist."; } @@ -87,7 +87,6 @@ class SQLitePersistentCookieStore::Backend void InternalBackgroundClose(); sql::Connection* db_; - MessageLoop* background_loop_; typedef std::list<PendingOperation*> PendingOperationsList; PendingOperationsList pending_; @@ -121,7 +120,7 @@ void SQLitePersistentCookieStore::Backend::BatchOperation( static const int kCommitIntervalMs = 30 * 1000; // Commit right away if we have more than 512 outstanding operations. static const size_t kCommitAfterBatchSize = 512; - DCHECK(MessageLoop::current() != background_loop_); + DCHECK(!ChromeThread::CurrentlyOn(ChromeThread::DB)); // We do a full copy of the cookie here, and hopefully just here. scoped_ptr<PendingOperation> po(new PendingOperation(op, key, cc)); @@ -134,20 +133,20 @@ void SQLitePersistentCookieStore::Backend::BatchOperation( num_pending = ++num_pending_; } - // TODO(abarth): What if the DB thread is being destroyed on the UI thread? if (num_pending == 1) { // We've gotten our first entry for this batch, fire off the timer. - background_loop_->PostDelayedTask(FROM_HERE, + ChromeThread::PostDelayedTask( + ChromeThread::DB, FROM_HERE, NewRunnableMethod(this, &Backend::Commit), kCommitIntervalMs); } else if (num_pending == kCommitAfterBatchSize) { // We've reached a big enough batch, fire off a commit now. - background_loop_->PostTask(FROM_HERE, - NewRunnableMethod(this, &Backend::Commit)); + ChromeThread::PostTask( + ChromeThread::DB, FROM_HERE, NewRunnableMethod(this, &Backend::Commit)); } } void SQLitePersistentCookieStore::Backend::Commit() { - DCHECK(MessageLoop::current() == background_loop_); + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::DB)); PendingOperationsList ops; { AutoLock locked(pending_lock_); @@ -236,15 +235,15 @@ void SQLitePersistentCookieStore::Backend::Commit() { // pending commit timer that will be holding a reference on us, but if/when // this fires we will already have been cleaned up and it will be ignored. void SQLitePersistentCookieStore::Backend::Close() { - DCHECK(MessageLoop::current() != background_loop_); + DCHECK(!ChromeThread::CurrentlyOn(ChromeThread::DB)); // Must close the backend on the background thread. - // TODO(abarth): What if the DB thread is being destroyed on the UI thread? - background_loop_->PostTask(FROM_HERE, + ChromeThread::PostTask( + ChromeThread::DB, FROM_HERE, NewRunnableMethod(this, &Backend::InternalBackgroundClose)); } void SQLitePersistentCookieStore::Backend::InternalBackgroundClose() { - DCHECK(MessageLoop::current() == background_loop_); + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::DB)); // Commit any pending operations Commit(); @@ -252,12 +251,8 @@ void SQLitePersistentCookieStore::Backend::InternalBackgroundClose() { db_ = NULL; } -SQLitePersistentCookieStore::SQLitePersistentCookieStore( - const FilePath& path, - MessageLoop* background_loop) - : path_(path), - background_loop_(background_loop) { - DCHECK(background_loop) << "SQLitePersistentCookieStore needs a MessageLoop"; +SQLitePersistentCookieStore::SQLitePersistentCookieStore(const FilePath& path) + : path_(path) { } SQLitePersistentCookieStore::~SQLitePersistentCookieStore() { @@ -353,7 +348,7 @@ bool SQLitePersistentCookieStore::Load( } // Create the backend, this will take ownership of the db pointer. - backend_ = new Backend(db.release(), background_loop_); + backend_ = new Backend(db.release()); return true; } diff --git a/chrome/browser/net/sqlite_persistent_cookie_store.h b/chrome/browser/net/sqlite_persistent_cookie_store.h index fe7e3f0..5e3716c 100644 --- a/chrome/browser/net/sqlite_persistent_cookie_store.h +++ b/chrome/browser/net/sqlite_persistent_cookie_store.h @@ -17,13 +17,11 @@ #include "net/base/cookie_monster.h" class FilePath; -class MessageLoop; class SQLitePersistentCookieStore : public net::CookieMonster::PersistentCookieStore { public: - SQLitePersistentCookieStore(const FilePath& path, - MessageLoop* background_loop); + SQLitePersistentCookieStore(const FilePath& path); ~SQLitePersistentCookieStore(); virtual bool Load(std::vector<net::CookieMonster::KeyedCanonicalCookie>*); @@ -43,9 +41,6 @@ class SQLitePersistentCookieStore FilePath path_; scoped_refptr<Backend> backend_; - // Background MessageLoop on which to access the backend_; - MessageLoop* background_loop_; - sql::MetaTable meta_table_; DISALLOW_COPY_AND_ASSIGN(SQLitePersistentCookieStore); diff --git a/chrome/browser/net/test_url_fetcher_factory.cc b/chrome/browser/net/test_url_fetcher_factory.cc index b7bd523..7573646 100644 --- a/chrome/browser/net/test_url_fetcher_factory.cc +++ b/chrome/browser/net/test_url_fetcher_factory.cc @@ -18,8 +18,6 @@ URLFetcher* TestURLFetcherFactory::CreateURLFetcher( URLFetcher::Delegate* d) { TestURLFetcher* fetcher = new TestURLFetcher(url, request_type, d); fetchers_[id] = fetcher; - // URLFetcher's destructor requires the message loop. - fetcher->set_io_loop(MessageLoop::current()); return fetcher; } diff --git a/chrome/browser/net/url_fetcher.cc b/chrome/browser/net/url_fetcher.cc index ca73756..7d0aad7 100644 --- a/chrome/browser/net/url_fetcher.cc +++ b/chrome/browser/net/url_fetcher.cc @@ -65,7 +65,6 @@ class URLFetcher::Core RequestType request_type_; // What type of request is this? URLFetcher::Delegate* delegate_; // Object to notify on completion MessageLoop* delegate_loop_; // Message loop of the creating thread - MessageLoop* io_loop_; // Message loop of the IO thread URLRequest* request_; // The actual request this wraps int load_flags_; // Flags for the load operation int response_code_; // HTTP status code for the request @@ -126,7 +125,6 @@ URLFetcher::Core::Core(URLFetcher* fetcher, request_type_(request_type), delegate_(d), delegate_loop_(MessageLoop::current()), - io_loop_(ChromeThread::GetMessageLoop(ChromeThread::IO)), request_(NULL), load_flags_(net::LOAD_NORMAL), response_code_(-1), @@ -138,23 +136,24 @@ URLFetcher::Core::Core(URLFetcher* fetcher, void URLFetcher::Core::Start() { DCHECK(delegate_loop_); - DCHECK(io_loop_); - DCHECK(request_context_getter_) << "We need an URLRequestContextGetter!"; - io_loop_->PostDelayedTask(FROM_HERE, NewRunnableMethod( - this, &Core::StartURLRequest), - protect_entry_->UpdateBackoff(URLFetcherProtectEntry::SEND)); + DCHECK(request_context_getter_) << "We need an URLRequestContext!"; + ChromeThread::PostDelayedTask( + ChromeThread::IO, FROM_HERE, + NewRunnableMethod(this, &Core::StartURLRequest), + protect_entry_->UpdateBackoff(URLFetcherProtectEntry::SEND)); } void URLFetcher::Core::Stop() { DCHECK_EQ(MessageLoop::current(), delegate_loop_); delegate_ = NULL; - io_loop_->PostTask(FROM_HERE, NewRunnableMethod( - this, &Core::CancelURLRequest)); + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + NewRunnableMethod(this, &Core::CancelURLRequest)); } void URLFetcher::Core::OnResponseStarted(URLRequest* request) { DCHECK(request == request_); - DCHECK(MessageLoop::current() == io_loop_); + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); if (request_->status().is_success()) { response_code_ = request_->GetResponseCode(); response_headers_ = request_->response_headers(); @@ -172,7 +171,7 @@ void URLFetcher::Core::OnResponseStarted(URLRequest* request) { void URLFetcher::Core::OnReadCompleted(URLRequest* request, int bytes_read) { DCHECK(request == request_); - DCHECK(MessageLoop::current() == io_loop_); + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); url_ = request->url(); @@ -195,7 +194,7 @@ void URLFetcher::Core::OnReadCompleted(URLRequest* request, int bytes_read) { } void URLFetcher::Core::StartURLRequest() { - DCHECK(MessageLoop::current() == io_loop_); + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); DCHECK(!request_); request_ = new URLRequest(original_url_, this); @@ -238,7 +237,7 @@ void URLFetcher::Core::StartURLRequest() { } void URLFetcher::Core::CancelURLRequest() { - DCHECK(MessageLoop::current() == io_loop_); + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); if (request_) { request_->Cancel(); delete request_; @@ -264,8 +263,9 @@ void URLFetcher::Core::OnCompletedURLRequest(const URLRequestStatus& status) { // Restarts the request if we still need to notify the delegate. if (delegate_) { if (num_retries_ <= protect_entry_->max_retries()) { - io_loop_->PostDelayedTask(FROM_HERE, NewRunnableMethod( - this, &Core::StartURLRequest), wait); + ChromeThread::PostDelayedTask( + ChromeThread::IO, FROM_HERE, + NewRunnableMethod(this, &Core::StartURLRequest), wait); } else { delegate_->OnURLFetchComplete(fetcher_, url_, status, response_code_, cookies_, data_); @@ -279,10 +279,6 @@ void URLFetcher::Core::OnCompletedURLRequest(const URLRequestStatus& status) { } } -void URLFetcher::set_io_loop(MessageLoop* io_loop) { - core_->io_loop_ = io_loop; -} - void URLFetcher::set_upload_data(const std::string& upload_content_type, const std::string& upload_content) { core_->upload_content_type_ = upload_content_type; diff --git a/chrome/browser/net/url_fetcher.h b/chrome/browser/net/url_fetcher.h index d62536c..f81fd69 100644 --- a/chrome/browser/net/url_fetcher.h +++ b/chrome/browser/net/url_fetcher.h @@ -48,10 +48,6 @@ class HttpResponseHeaders; // You may create the URLFetcher instance on any thread; OnURLFetchComplete() // will be called back on the same thread you use to create the instance. // -// NOTE: Take extra care when using URLFetcher in services that live on the -// BrowserProcess; all URLFetcher instances need to be destroyed before -// the IO thread goes away, since the URLFetcher destructor requests an -// InvokeLater operation on that thread. // // NOTE: By default URLFetcher requests are NOT intercepted, except when // interception is explicitly enabled in tests. @@ -116,13 +112,6 @@ class URLFetcher { static URLFetcher* Create(int id, const GURL& url, RequestType request_type, Delegate* d); - // This should only be used by unittests, where g_browser_process->io_thread() - // does not exist and we must specify an alternate loop. Unfortunately, we - // can't put it under #ifdef UNIT_TEST since some callers (which themselves - // should only be reached in unit tests) use this. See - // chrome/browser/feeds/feed_manager.cc. - void set_io_loop(MessageLoop* io_loop); - // Sets data only needed by POSTs. All callers making POST requests should // call this before the request is started. |upload_content_type| is the MIME // type of the content, while |upload_content| is the data to be sent (the @@ -161,14 +150,6 @@ class URLFetcher { Delegate* delegate() const; private: - // This class is the real guts of URLFetcher. - // - // When created, delegate_loop_ is set to the message loop of the current - // thread, while io_loop_ is set to the message loop of the IO thread. These - // are used to ensure that all handling of URLRequests happens on the IO - // thread (since that class is not currently threadsafe and relies on - // underlying Microsoft APIs that we don't know to be threadsafe), while - // keeping the delegate callback on the delegate's thread. class Core; scoped_refptr<Core> core_; diff --git a/chrome/browser/net/url_fetcher_unittest.cc b/chrome/browser/net/url_fetcher_unittest.cc index f8537d9..25fca55 100644 --- a/chrome/browser/net/url_fetcher_unittest.cc +++ b/chrome/browser/net/url_fetcher_unittest.cc @@ -5,6 +5,7 @@ #include "base/thread.h" #include "base/time.h" #include "base/timer.h" +#include "chrome/browser/chrome_thread.h" #include "chrome/browser/net/url_fetcher.h" #include "chrome/browser/net/url_fetcher_protect.h" #include "chrome/browser/net/url_request_context_getter.h" @@ -34,7 +35,9 @@ class TestURLRequestContextGetter : public URLRequestContextGetter { class URLFetcherTest : public testing::Test, public URLFetcher::Delegate { public: - URLFetcherTest() : fetcher_(NULL) { } + URLFetcherTest() + : io_thread_(ChromeThread::IO, &io_loop_), + fetcher_(NULL) { } // Creates a URLFetcher, using the program's main thread to do IO. virtual void CreateFetcher(const GURL& url); @@ -60,6 +63,7 @@ class URLFetcherTest : public testing::Test, public URLFetcher::Delegate { // dispatches its requests to. When we wish to simulate being used from // a UI thread, we dispatch a worker thread to do so. MessageLoopForIO io_loop_; + ChromeThread io_thread_; URLFetcher* fetcher_; }; @@ -197,7 +201,6 @@ class FetcherWrapperTask : public Task { void URLFetcherTest::CreateFetcher(const GURL& url) { fetcher_ = new URLFetcher(url, URLFetcher::GET, this); fetcher_->set_request_context(new TestURLRequestContextGetter()); - fetcher_->set_io_loop(&io_loop_); fetcher_->Start(); } @@ -215,15 +218,15 @@ void URLFetcherTest::OnURLFetchComplete(const URLFetcher* source, // because the destructor won't necessarily run on the // same thread that CreateFetcher() did. - io_loop_.PostTask(FROM_HERE, new MessageLoop::QuitTask()); - // If MessageLoop::current() != io_loop_, it will be shut down when the - // main loop returns and this thread subsequently goes out of scope. + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, new MessageLoop::QuitTask()); + // If the current message loop is not the IO loop, it will be shut down when + // the main loop returns and this thread subsequently goes out of scope. } void URLFetcherPostTest::CreateFetcher(const GURL& url) { fetcher_ = new URLFetcher(url, URLFetcher::POST, this); fetcher_->set_request_context(new TestURLRequestContextGetter()); - fetcher_->set_io_loop(&io_loop_); fetcher_->set_upload_data("application/x-www-form-urlencoded", "bobsyeruncle"); fetcher_->Start(); @@ -258,7 +261,6 @@ void URLFetcherHeadersTest::OnURLFetchComplete( void URLFetcherProtectTest::CreateFetcher(const GURL& url) { fetcher_ = new URLFetcher(url, URLFetcher::GET, this); fetcher_->set_request_context(new TestURLRequestContextGetter()); - fetcher_->set_io_loop(&io_loop_); start_time_ = Time::Now(); fetcher_->Start(); } @@ -277,7 +279,8 @@ void URLFetcherProtectTest::OnURLFetchComplete(const URLFetcher* source, EXPECT_TRUE(status.is_success()); EXPECT_FALSE(data.empty()); delete fetcher_; - io_loop_.Quit(); + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, new MessageLoop::QuitTask()); } else { // Now running Overload test. static int count = 0; @@ -323,14 +326,14 @@ void URLFetcherBadHTTPSTest::OnURLFetchComplete( // The rest is the same as URLFetcherTest::OnURLFetchComplete. delete fetcher_; - io_loop_.Quit(); + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, new MessageLoop::QuitTask()); } void URLFetcherCancelTest::CreateFetcher(const GURL& url) { fetcher_ = new URLFetcher(url, URLFetcher::GET, this); fetcher_->set_request_context( new CancelTestURLRequestContextGetter(&context_released_)); - fetcher_->set_io_loop(&io_loop_); fetcher_->Start(); // Make sure we give the IO thread a chance to run. timer_.Start(TimeDelta::FromMilliseconds(300), this, @@ -346,7 +349,8 @@ void URLFetcherCancelTest::OnURLFetchComplete(const URLFetcher* source, // We should have cancelled the request before completion. ADD_FAILURE(); delete fetcher_; - io_loop_.PostTask(FROM_HERE, new MessageLoop::QuitTask()); + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, new MessageLoop::QuitTask()); } void URLFetcherCancelTest::CancelRequest() { @@ -360,7 +364,8 @@ void URLFetcherCancelTest::CancelRequest() { void URLFetcherCancelTest::TestContextReleased() { EXPECT_TRUE(context_released_); timer_.Stop(); - io_loop_.PostTask(FROM_HERE, new MessageLoop::QuitTask()); + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, new MessageLoop::QuitTask()); } TEST_F(URLFetcherTest, SameThreadsTest) { diff --git a/chrome/browser/notifications/notification_object_proxy.cc b/chrome/browser/notifications/notification_object_proxy.cc index f230d19..a18e0d7 100644 --- a/chrome/browser/notifications/notification_object_proxy.cc +++ b/chrome/browser/notifications/notification_object_proxy.cc @@ -51,11 +51,9 @@ void NotificationObjectProxy::Close(bool by_user) { void NotificationObjectProxy::DeliverMessage(IPC::Message* message) { DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI)); - MessageLoop* io_loop = ChromeThread::GetMessageLoop(ChromeThread::IO); - if (io_loop) { - io_loop->PostTask(FROM_HERE, - NewRunnableMethod(this, &NotificationObjectProxy::Send, message)); - } + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + NewRunnableMethod(this, &NotificationObjectProxy::Send, message)); } // Deferred method which runs on the IO thread and sends a message to the diff --git a/chrome/browser/plugin_process_host.cc b/chrome/browser/plugin_process_host.cc index fe4583a..96bf5a2 100644 --- a/chrome/browser/plugin_process_host.cc +++ b/chrome/browser/plugin_process_host.cc @@ -338,11 +338,11 @@ PluginProcessHost::~PluginProcessHost() { for (window_index = plugin_fullscreen_windows_set_.begin(); window_index != plugin_fullscreen_windows_set_.end(); window_index++) { - if (MessageLoop::current() == - ChromeThread::GetMessageLoop(ChromeThread::UI)) { + if (ChromeThread::CurrentlyOn(ChromeThread::UI)) { mac_util::ReleaseFullScreen(); } else { - ChromeThread::GetMessageLoop(ChromeThread::UI)->PostTask(FROM_HERE, + ChromeThread::PostTask( + ChromeThread::UI, FROM_HERE, NewRunnableFunction(mac_util::ReleaseFullScreen)); } } @@ -670,8 +670,7 @@ void PluginProcessHost::OnGetPluginFinderUrl(std::string* plugin_finder_url) { void PluginProcessHost::OnPluginMessage( const std::vector<uint8>& data) { - DCHECK(MessageLoop::current() == - ChromeThread::GetMessageLoop(ChromeThread::IO)); + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); ChromePluginLib *chrome_plugin = ChromePluginLib::Find(info_.path); if (chrome_plugin) { diff --git a/chrome/browser/plugin_process_host_mac.cc b/chrome/browser/plugin_process_host_mac.cc index 2c64245..6e6bb18 100644 --- a/chrome/browser/plugin_process_host_mac.cc +++ b/chrome/browser/plugin_process_host_mac.cc @@ -30,7 +30,8 @@ void PluginProcessHost::OnPluginShowWindow(uint32 window_id, plugin_fullscreen_windows_set_.insert(window_id); // If the plugin has just shown a window that's the same dimensions as // the main display, hide the menubar so that it has the whole screen. - ChromeThread::GetMessageLoop(ChromeThread::UI)->PostTask(FROM_HERE, + ChromeThread::PostTask( + ChromeThread::UI, FROM_HERE, NewRunnableFunction(mac_util::RequestFullScreen)); } } @@ -41,7 +42,8 @@ void PluginProcessHost::OnPluginHideWindow(uint32 window_id, if (plugin_fullscreen_windows_set_.find(window_id) != plugin_fullscreen_windows_set_.end()) { plugin_fullscreen_windows_set_.erase(window_id); - ChromeThread::GetMessageLoop(ChromeThread::UI)->PostTask(FROM_HERE, + ChromeThread::PostTask( + ChromeThread::UI, FROM_HERE, NewRunnableFunction(mac_util::ReleaseFullScreen)); } } diff --git a/chrome/browser/plugin_service.cc b/chrome/browser/plugin_service.cc index 78867e4..54577a7 100644 --- a/chrome/browser/plugin_service.cc +++ b/chrome/browser/plugin_service.cc @@ -104,8 +104,7 @@ const std::wstring& PluginService::GetUILocale() { PluginProcessHost* PluginService::FindPluginProcess( const FilePath& plugin_path) { - DCHECK(MessageLoop::current() == - ChromeThread::GetMessageLoop(ChromeThread::IO)); + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); if (plugin_path.value().empty()) { NOTREACHED() << "should only be called if we have a plugin to load"; @@ -124,8 +123,7 @@ PluginProcessHost* PluginService::FindPluginProcess( PluginProcessHost* PluginService::FindOrStartPluginProcess( const FilePath& plugin_path) { - DCHECK(MessageLoop::current() == - ChromeThread::GetMessageLoop(ChromeThread::IO)); + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); PluginProcessHost *plugin_host = FindPluginProcess(plugin_path); if (plugin_host) @@ -155,8 +153,7 @@ void PluginService::OpenChannelToPlugin( const std::string& mime_type, const std::wstring& locale, IPC::Message* reply_msg) { - DCHECK(MessageLoop::current() == - ChromeThread::GetMessageLoop(ChromeThread::IO)); + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); // We don't need a policy URL here because that was already checked by a // previous call to GetPluginPath. GURL policy_url; diff --git a/chrome/browser/power_save_blocker_common.cc b/chrome/browser/power_save_blocker_common.cc index b6ff2ac..47efe55 100644 --- a/chrome/browser/power_save_blocker_common.cc +++ b/chrome/browser/power_save_blocker_common.cc @@ -35,9 +35,8 @@ void PowerSaveBlocker::Disable() { void PowerSaveBlocker::PostAdjustBlockCount(int delta) { - MessageLoop *ml = ChromeThread::GetMessageLoop(ChromeThread::UI); - - ml->PostTask(FROM_HERE, + ChromeThread::PostTask( + ChromeThread::UI, FROM_HERE, NewRunnableFunction(&PowerSaveBlocker::AdjustBlockCount, delta)); } diff --git a/chrome/browser/printing/print_job_worker.cc b/chrome/browser/printing/print_job_worker.cc index 27a553d..3a91f0c 100644 --- a/chrome/browser/printing/print_job_worker.cc +++ b/chrome/browser/printing/print_job_worker.cc @@ -77,10 +77,11 @@ void PrintJobWorker::GetSettings(bool ask_user_for_settings, if (ask_user_for_settings) { #if defined(OS_MACOSX) - ChromeThread::GetMessageLoop(ChromeThread::UI)->PostTask( - FROM_HERE, NewRunnableMethod(this, &PrintJobWorker::GetSettingsWithUI, - parent_window, document_page_count, - has_selection)); + ChromeThread::PostTask( + ChromeThread::UI, FROM_HERE, + NewRunnableMethod(this, &PrintJobWorker::GetSettingsWithUI, + parent_window, document_page_count, + has_selection)); #else PrintingContext::Result result = printing_context_.AskUserForSettings( parent_window, document_page_count, has_selection); @@ -111,8 +112,7 @@ void PrintJobWorker::GetSettingsDone(PrintingContext::Result result) { void PrintJobWorker::GetSettingsWithUI(gfx::NativeWindow parent_window, int document_page_count, bool has_selection) { - DCHECK_EQ(ChromeThread::GetMessageLoop(ChromeThread::UI), - MessageLoop::current()); + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI)); PrintingContext::Result result = printing_context_.AskUserForSettings( parent_window, document_page_count, has_selection); diff --git a/chrome/browser/process_singleton_linux.cc b/chrome/browser/process_singleton_linux.cc index 0268693..af2a519 100644 --- a/chrome/browser/process_singleton_linux.cc +++ b/chrome/browser/process_singleton_linux.cc @@ -466,8 +466,7 @@ void ProcessSingleton::LinuxWatcher::OnFileCanReadWithoutBlocking(int fd) { } void ProcessSingleton::LinuxWatcher::StartListening(int socket) { - DCHECK(ChromeThread::GetMessageLoop(ChromeThread::IO) == - MessageLoop::current()); + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); // Watch for client connections on this socket. MessageLoopForIO* ml = MessageLoopForIO::current(); ml->AddDestructionObserver(this); diff --git a/chrome/browser/profile_manager.cc b/chrome/browser/profile_manager.cc index 94bbfe9..4526911 100644 --- a/chrome/browser/profile_manager.cc +++ b/chrome/browser/profile_manager.cc @@ -236,8 +236,7 @@ void ProfileManager::OnResume() { void ProfileManager::SuspendProfile(Profile* profile) { DCHECK(profile); - DCHECK(MessageLoop::current() == - ChromeThread::GetMessageLoop(ChromeThread::IO)); + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); for (URLRequestJobTracker::JobIterator i = g_url_request_job_tracker.begin(); i != g_url_request_job_tracker.end(); ++i) @@ -249,14 +248,11 @@ void ProfileManager::SuspendProfile(Profile* profile) { void ProfileManager::ResumeProfile(Profile* profile) { DCHECK(profile); - DCHECK(MessageLoop::current() == - ChromeThread::GetMessageLoop(ChromeThread::IO)); + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); profile->GetRequestContext()->GetURLRequestContext()-> http_transaction_factory()->Suspend(false); } - - // static bool ProfileManager::IsProfile(const FilePath& path) { FilePath prefs_path = GetDefaultProfilePath(path); diff --git a/chrome/browser/renderer_host/buffered_resource_handler.cc b/chrome/browser/renderer_host/buffered_resource_handler.cc index 68514ad..45dec61 100644 --- a/chrome/browser/renderer_host/buffered_resource_handler.cc +++ b/chrome/browser/renderer_host/buffered_resource_handler.cc @@ -402,7 +402,8 @@ bool BufferedResourceHandler::ShouldWaitForPlugins() { // file thread. This breaks assumptions in other message handlers (i.e. when // unregistering with NotificationService in the destructor). AddRef(); - ChromeThread::GetMessageLoop(ChromeThread::FILE)->PostTask(FROM_HERE, + ChromeThread::PostTask( + ChromeThread::FILE, FROM_HERE, NewRunnableFunction(&BufferedResourceHandler::LoadPlugins, this, host_->ui_loop())); return true; diff --git a/chrome/browser/renderer_host/file_system_accessor.cc b/chrome/browser/renderer_host/file_system_accessor.cc index 295b1f7..de5fc7b 100644 --- a/chrome/browser/renderer_host/file_system_accessor.cc +++ b/chrome/browser/renderer_host/file_system_accessor.cc @@ -21,8 +21,8 @@ void FileSystemAccessor::RequestFileSize(const FilePath& path, FileSizeCallback* callback) { // Getting file size could take long time if it lives on a network share, // so run it on FILE thread. - ChromeThread::GetMessageLoop(ChromeThread::FILE)->PostTask( - FROM_HERE, + ChromeThread::PostTask( + ChromeThread::FILE, FROM_HERE, NewRunnableMethod(new FileSystemAccessor(param, callback), &FileSystemAccessor::GetFileSize, path)); } diff --git a/chrome/browser/renderer_host/file_system_accessor_unittest.cc b/chrome/browser/renderer_host/file_system_accessor_unittest.cc index 81b8b94..62f1008 100644 --- a/chrome/browser/renderer_host/file_system_accessor_unittest.cc +++ b/chrome/browser/renderer_host/file_system_accessor_unittest.cc @@ -18,7 +18,6 @@ class FileSystemAccessorTest : public testing::Test { // Create FILE thread which is used to do file access. file_thread_.reset(new ChromeThread(ChromeThread::FILE)); - EXPECT_TRUE(ChromeThread::GetMessageLoop(ChromeThread::FILE) == NULL); // Start FILE thread and verify FILE message loop exists. file_thread_->Start(); @@ -27,7 +26,6 @@ class FileSystemAccessorTest : public testing::Test { virtual void TearDown() { file_thread_->Stop(); - EXPECT_TRUE(ChromeThread::GetMessageLoop(ChromeThread::FILE) == NULL); } void TestGetFileSize(const FilePath& path, diff --git a/chrome/browser/renderer_host/resource_dispatcher_host.cc b/chrome/browser/renderer_host/resource_dispatcher_host.cc index fd42868..0a495df8 100644 --- a/chrome/browser/renderer_host/resource_dispatcher_host.cc +++ b/chrome/browser/renderer_host/resource_dispatcher_host.cc @@ -322,9 +322,6 @@ void ResourceDispatcherHost::OnShutdown() { // runs if the timer is still running the Task is deleted twice (once by // the MessageLoop and the second time by RepeatingTimer). update_load_states_timer_.Stop(); - // Let the WebKit thread know the IO thread is going away soon and that it - // should prepare for its own shutdown soon after. - webkit_thread_->Shutdown(); } bool ResourceDispatcherHost::HandleExternalProtocol(int request_id, diff --git a/chrome/browser/renderer_host/resource_dispatcher_host_unittest.cc b/chrome/browser/renderer_host/resource_dispatcher_host_unittest.cc index 6c08025..0b704b9 100644 --- a/chrome/browser/renderer_host/resource_dispatcher_host_unittest.cc +++ b/chrome/browser/renderer_host/resource_dispatcher_host_unittest.cc @@ -8,6 +8,7 @@ #include "base/message_loop.h" #include "base/process_util.h" #include "chrome/browser/child_process_security_policy.h" +#include "chrome/browser/chrome_thread.h" #include "chrome/browser/renderer_host/resource_dispatcher_host.h" #include "chrome/common/chrome_plugin_lib.h" #include "chrome/common/render_messages.h" @@ -144,7 +145,9 @@ class ResourceDispatcherHostTest : public testing::Test, public ResourceDispatcherHost::Receiver { public: ResourceDispatcherHostTest() - : Receiver(ChildProcessInfo::RENDER_PROCESS, -1), host_(NULL), + : Receiver(ChildProcessInfo::RENDER_PROCESS, -1), + io_thread_(ChromeThread::IO, &message_loop_), + host_(NULL), old_factory_(NULL) { set_handle(base::GetCurrentProcessHandle()); } @@ -241,6 +244,7 @@ class ResourceDispatcherHostTest : public testing::Test, } MessageLoopForIO message_loop_; + ChromeThread io_thread_; ResourceDispatcherHost host_; ResourceIPCAccumulator accum_; std::string response_headers_; diff --git a/chrome/browser/renderer_host/resource_message_filter.cc b/chrome/browser/renderer_host/resource_message_filter.cc index dc49500..ce16753 100644 --- a/chrome/browser/renderer_host/resource_message_filter.cc +++ b/chrome/browser/renderer_host/resource_message_filter.cc @@ -550,7 +550,8 @@ void ResourceMessageFilter::OnGetPlugins(bool refresh, // file thread. We need this object to be destructed on the IO thread, so do // the refcounting manually. AddRef(); - ChromeThread::GetMessageLoop(ChromeThread::FILE)->PostTask(FROM_HERE, + ChromeThread::PostTask( + ChromeThread::FILE, FROM_HERE, NewRunnableFunction(&ResourceMessageFilter::OnGetPluginsOnFileThread, this, refresh, reply_msg)); } @@ -561,20 +562,9 @@ void ResourceMessageFilter::OnGetPluginsOnFileThread( IPC::Message* reply_msg) { std::vector<WebPluginInfo> plugins; NPAPI::PluginList::Singleton()->GetPlugins(refresh, &plugins); - ViewHostMsg_GetPlugins::WriteReplyParams(reply_msg, plugins); - // Note, we want to get to the IO thread now, but the file thread outlives it - // so we can't post a task to it directly as it might be in the middle of - // destruction. So hop through the main thread, where the destruction of the - // IO thread happens and hence no race conditions exist. - filter->ui_loop()->PostTask(FROM_HERE, - NewRunnableFunction(&ResourceMessageFilter::OnNotifyPluginsLoaded, - filter, reply_msg)); -} - -void ResourceMessageFilter::OnNotifyPluginsLoaded(ResourceMessageFilter* filter, - IPC::Message* reply_msg) { - ChromeThread::GetMessageLoop(ChromeThread::IO)->PostTask(FROM_HERE, + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, NewRunnableMethod(filter, &ResourceMessageFilter::OnPluginsLoaded, reply_msg)); } diff --git a/chrome/browser/renderer_host/resource_message_filter.h b/chrome/browser/renderer_host/resource_message_filter.h index f0029a5..958c642 100644 --- a/chrome/browser/renderer_host/resource_message_filter.h +++ b/chrome/browser/renderer_host/resource_message_filter.h @@ -143,8 +143,6 @@ class ResourceMessageFilter : public IPC::ChannelProxy::MessageFilter, static void OnGetPluginsOnFileThread(ResourceMessageFilter* filter, bool refresh, IPC::Message* reply_msg); - static void OnNotifyPluginsLoaded(ResourceMessageFilter* filter, - IPC::Message* reply_msg); void OnPluginsLoaded(IPC::Message* reply_msg); void OnGetPluginPath(const GURL& url, const GURL& policy_url, diff --git a/chrome/browser/renderer_host/resource_message_filter_gtk.cc b/chrome/browser/renderer_host/resource_message_filter_gtk.cc index c1b29e6..f8c8986 100644 --- a/chrome/browser/renderer_host/resource_message_filter_gtk.cc +++ b/chrome/browser/renderer_host/resource_message_filter_gtk.cc @@ -54,8 +54,9 @@ void ResourceMessageFilter::DoOnGetScreenInfo(gfx::NativeViewId view, WebScreenInfo results = WebScreenInfoFactory::screenInfo(display, screen); ViewHostMsg_GetScreenInfo::WriteReplyParams(reply_msg, results); - ChromeThread::GetMessageLoop(ChromeThread::IO)->PostTask( - FROM_HERE, NewRunnableMethod( + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + NewRunnableMethod( this, &ResourceMessageFilter::SendDelayedReply, reply_msg)); } @@ -78,8 +79,9 @@ void ResourceMessageFilter::DoOnGetWindowRect(gfx::NativeViewId view, ViewHostMsg_GetWindowRect::WriteReplyParams(reply_msg, rect); - ChromeThread::GetMessageLoop(ChromeThread::IO)->PostTask( - FROM_HERE, NewRunnableMethod( + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + NewRunnableMethod( this, &ResourceMessageFilter::SendDelayedReply, reply_msg)); } @@ -117,8 +119,9 @@ void ResourceMessageFilter::DoOnGetRootWindowRect(gfx::NativeViewId view, ViewHostMsg_GetRootWindowRect::WriteReplyParams(reply_msg, rect); - ChromeThread::GetMessageLoop(ChromeThread::IO)->PostTask( - FROM_HERE, NewRunnableMethod( + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + NewRunnableMethod( this, &ResourceMessageFilter::SendDelayedReply, reply_msg)); } @@ -130,8 +133,9 @@ void ResourceMessageFilter::DoOnClipboardIsFormatAvailable( ViewHostMsg_ClipboardIsFormatAvailable::WriteReplyParams(reply_msg, result); - ChromeThread::GetMessageLoop(ChromeThread::IO)->PostTask( - FROM_HERE, NewRunnableMethod( + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + NewRunnableMethod( this, &ResourceMessageFilter::SendDelayedReply, reply_msg)); } @@ -143,8 +147,9 @@ void ResourceMessageFilter::DoOnClipboardReadText(Clipboard::Buffer buffer, ViewHostMsg_ClipboardReadText::WriteReplyParams(reply_msg, result); - ChromeThread::GetMessageLoop(ChromeThread::IO)->PostTask( - FROM_HERE, NewRunnableMethod( + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + NewRunnableMethod( this, &ResourceMessageFilter::SendDelayedReply, reply_msg)); } @@ -156,8 +161,9 @@ void ResourceMessageFilter::DoOnClipboardReadAsciiText( ViewHostMsg_ClipboardReadAsciiText::WriteReplyParams(reply_msg, result); - ChromeThread::GetMessageLoop(ChromeThread::IO)->PostTask( - FROM_HERE, NewRunnableMethod( + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + NewRunnableMethod( this, &ResourceMessageFilter::SendDelayedReply, reply_msg)); } @@ -171,8 +177,9 @@ void ResourceMessageFilter::DoOnClipboardReadHTML(Clipboard::Buffer buffer, ViewHostMsg_ClipboardReadHTML::WriteReplyParams(reply_msg, markup, src_url); - ChromeThread::GetMessageLoop(ChromeThread::IO)->PostTask( - FROM_HERE, NewRunnableMethod( + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + NewRunnableMethod( this, &ResourceMessageFilter::SendDelayedReply, reply_msg)); } @@ -203,32 +210,36 @@ void ResourceMessageFilter::DoOnAllocateTempFileForPrinting( ViewHostMsg_AllocateTempFileForPrinting::WriteReplyParams( reply_msg, temp_file_fd, fd_in_browser); - ChromeThread::GetMessageLoop(ChromeThread::IO)->PostTask( - FROM_HERE, NewRunnableMethod( + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + NewRunnableMethod( this, &ResourceMessageFilter::SendDelayedReply, reply_msg)); } // Called on the IO thread. void ResourceMessageFilter::OnGetScreenInfo(gfx::NativeViewId view, IPC::Message* reply_msg) { - ChromeThread::GetMessageLoop(ChromeThread::BACKGROUND_X11)->PostTask( - FROM_HERE, NewRunnableMethod( + ChromeThread::PostTask( + ChromeThread::BACKGROUND_X11, FROM_HERE, + NewRunnableMethod( this, &ResourceMessageFilter::DoOnGetScreenInfo, view, reply_msg)); } // Called on the IO thread. void ResourceMessageFilter::OnGetWindowRect(gfx::NativeViewId view, IPC::Message* reply_msg) { - ChromeThread::GetMessageLoop(ChromeThread::BACKGROUND_X11)->PostTask( - FROM_HERE, NewRunnableMethod( + ChromeThread::PostTask( + ChromeThread::BACKGROUND_X11, FROM_HERE, + NewRunnableMethod( this, &ResourceMessageFilter::DoOnGetWindowRect, view, reply_msg)); } // Called on the IO thread. void ResourceMessageFilter::OnGetRootWindowRect(gfx::NativeViewId view, IPC::Message* reply_msg) { - ChromeThread::GetMessageLoop(ChromeThread::BACKGROUND_X11)->PostTask( - FROM_HERE, NewRunnableMethod( + ChromeThread::PostTask( + ChromeThread::BACKGROUND_X11, FROM_HERE, + NewRunnableMethod( this, &ResourceMessageFilter::DoOnGetRootWindowRect, view, reply_msg)); } @@ -269,8 +280,9 @@ void ResourceMessageFilter::OnClipboardReadHTML(Clipboard::Buffer buffer, // Called on the IO thread. void ResourceMessageFilter::OnAllocateTempFileForPrinting( IPC::Message* reply_msg) { - ChromeThread::GetMessageLoop(ChromeThread::FILE)->PostTask( - FROM_HERE, NewRunnableMethod( + ChromeThread::PostTask( + ChromeThread::FILE, FROM_HERE, + NewRunnableMethod( this, &ResourceMessageFilter::DoOnAllocateTempFileForPrinting, reply_msg)); } diff --git a/chrome/browser/spellchecker.cc b/chrome/browser/spellchecker.cc index 596c253..c3829b3 100644 --- a/chrome/browser/spellchecker.cc +++ b/chrome/browser/spellchecker.cc @@ -200,8 +200,8 @@ void SaveDictionaryTask::Run() { } // Unsuccessful save is taken care of in SpellChecker::Initialize(). // Set Flag that dictionary is not downloading anymore. - MessageLoop* ui_loop = ChromeThread::GetMessageLoop(ChromeThread::UI); - ui_loop->PostTask(FROM_HERE, + ChromeThread::PostTask( + ChromeThread::UI, FROM_HERE, new UIProxyForIOTask(on_dictionary_save_complete_callback_task_, NULL)); } @@ -268,12 +268,13 @@ class ReadDictionaryTask : public Task { Task* task = NewRunnableMethod(spellchecker_, &SpellChecker::HunspellInited, hunspell_, bdict_file_, file_existed); if (spellchecker_->file_loop_) { - MessageLoop* ui_loop = ChromeThread::GetMessageLoop(ChromeThread::UI); // We were called on the file loop. Post back to the IO loop. // If this never gets posted to the IO loop, then we will leak |hunspell_| // and |bdict_file_|. But that can only happen during shutdown, so it's // not worth caring about. - ui_loop->PostTask(FROM_HERE, new UIProxyForIOTask(task, spellchecker_)); + ChromeThread::PostTask( + ChromeThread::UI, FROM_HERE, + new UIProxyForIOTask(task, spellchecker_)); } else { // We were called directly (e.g., during testing). Run the task directly. task->Run(); diff --git a/chrome/browser/sync/glue/http_bridge.cc b/chrome/browser/sync/glue/http_bridge.cc index 3448755..c019cba 100644 --- a/chrome/browser/sync/glue/http_bridge.cc +++ b/chrome/browser/sync/glue/http_bridge.cc @@ -10,6 +10,7 @@ #include "base/string_util.h" #include "chrome/browser/chrome_thread.h" #include "chrome/browser/profile.h" +#include "chrome/browser/chrome_thread.h" #include "net/base/cookie_monster.h" #include "net/base/load_flags.h" #include "net/http/http_cache.h" @@ -53,15 +54,15 @@ HttpBridgeFactory::HttpBridgeFactory( HttpBridgeFactory::~HttpBridgeFactory() { if (request_context_getter_) { // Clean up request context getter on IO thread. - ChromeThread::GetMessageLoop(ChromeThread::IO)->ReleaseSoon(FROM_HERE, - request_context_getter_); + bool posted = ChromeThread::ReleaseSoon( + ChromeThread::IO, FROM_HERE, request_context_getter_); + DCHECK(posted); request_context_getter_ = NULL; } } sync_api::HttpPostProviderInterface* HttpBridgeFactory::Create() { - HttpBridge* http = new HttpBridge(request_context_getter_, - ChromeThread::GetMessageLoop(ChromeThread::IO)); + HttpBridge* http = new HttpBridge(request_context_getter_); http->AddRef(); return http; } @@ -109,22 +110,21 @@ HttpBridge::RequestContext::~RequestContext() { delete http_transaction_factory_; } -HttpBridge::HttpBridge(HttpBridge::RequestContextGetter* context_getter, - MessageLoop* io_loop) +HttpBridge::HttpBridge(HttpBridge::RequestContextGetter* context_getter) : context_getter_for_request_(context_getter), url_poster_(NULL), created_on_loop_(MessageLoop::current()), - io_loop_(io_loop), request_completed_(false), request_succeeded_(false), http_response_code_(-1), - http_post_completed_(false, false), - use_io_loop_for_testing_(false) { + http_post_completed_(false, false) { context_getter_for_request_->AddRef(); } HttpBridge::~HttpBridge() { - io_loop_->ReleaseSoon(FROM_HERE, context_getter_for_request_); + bool posted = ChromeThread::ReleaseSoon( + ChromeThread::IO, FROM_HERE, context_getter_for_request_); + DCHECK(posted); } void HttpBridge::SetUserAgent(const char* user_agent) { @@ -176,8 +176,9 @@ bool HttpBridge::MakeSynchronousPost(int* os_error_code, int* response_code) { DCHECK(url_for_request_.is_valid()) << "Invalid URL for request"; DCHECK(!content_type_.empty()) << "Payload not set"; - io_loop_->PostTask(FROM_HERE, NewRunnableMethod(this, - &HttpBridge::CallMakeAsynchronousPost)); + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, + NewRunnableMethod(this, &HttpBridge::CallMakeAsynchronousPost)); if (!http_post_completed_.Wait()) // Block until network request completes. NOTREACHED(); // See OnURLFetchComplete. @@ -189,17 +190,13 @@ bool HttpBridge::MakeSynchronousPost(int* os_error_code, int* response_code) { } void HttpBridge::MakeAsynchronousPost() { - DCHECK_EQ(MessageLoop::current(), io_loop_); + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); DCHECK(!request_completed_); url_poster_ = new URLFetcher(url_for_request_, URLFetcher::POST, this); url_poster_->set_request_context(context_getter_for_request_); url_poster_->set_upload_data(content_type_, request_content_); url_poster_->set_extra_request_headers(extra_headers_); - - if (use_io_loop_for_testing_) - url_poster_->set_io_loop(io_loop_); - url_poster_->Start(); } @@ -220,7 +217,7 @@ void HttpBridge::OnURLFetchComplete(const URLFetcher *source, const GURL &url, int response_code, const ResponseCookies &cookies, const std::string &data) { - DCHECK_EQ(MessageLoop::current(), io_loop_); + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); request_completed_ = true; request_succeeded_ = (URLRequestStatus::SUCCESS == status.status()); @@ -229,10 +226,10 @@ void HttpBridge::OnURLFetchComplete(const URLFetcher *source, const GURL &url, response_content_ = data; - // End of the line for url_poster_. It lives only on the io_loop. + // End of the line for url_poster_. It lives only on the IO loop. // We defer deletion because we're inside a callback from a component of the // URLFetcher, so it seems most natural / "polite" to let the stack unwind. - io_loop_->DeleteSoon(FROM_HERE, url_poster_); + MessageLoop::current()->DeleteSoon(FROM_HERE, url_poster_); url_poster_ = NULL; // Wake the blocked syncer thread in MakeSynchronousPost. diff --git a/chrome/browser/sync/glue/http_bridge.h b/chrome/browser/sync/glue/http_bridge.h index 7e1d967..12500ec 100644 --- a/chrome/browser/sync/glue/http_bridge.h +++ b/chrome/browser/sync/glue/http_bridge.h @@ -97,7 +97,7 @@ class HttpBridge : public base::RefCountedThreadSafe<HttpBridge>, DISALLOW_COPY_AND_ASSIGN(RequestContextGetter); }; - HttpBridge(RequestContextGetter* context, MessageLoop* io_loop); + HttpBridge(RequestContextGetter* context); virtual ~HttpBridge(); // sync_api::HttpPostProvider implementation. @@ -135,7 +135,7 @@ class HttpBridge : public base::RefCountedThreadSafe<HttpBridge>, private: friend class ::HttpBridgeTest; - // Called on the io_loop_ to issue the network request. The extra level + // Called on the IO loop to issue the network request. The extra level // of indirection is so that the unit test can override this behavior but we // still have a function to statically pass to PostTask. void CallMakeAsynchronousPost() { MakeAsynchronousPost(); } @@ -148,7 +148,7 @@ class HttpBridge : public base::RefCountedThreadSafe<HttpBridge>, // so we can block created_on_loop_ while the fetch is in progress. // NOTE: This is not a scoped_ptr for a reason. It must be deleted on the same // thread that created it, which isn't the same thread |this| gets deleted on. - // We must manually delete url_poster_ on the io_loop_. + // We must manually delete url_poster_ on the IO loop. URLFetcher* url_poster_; // The message loop of the thread we were created on. This is the thread that @@ -158,10 +158,6 @@ class HttpBridge : public base::RefCountedThreadSafe<HttpBridge>, // on network IO through curl_easy_perform. MessageLoop* const created_on_loop_; - // Member variable for the IO loop instead of asking ChromeThread directly, - // done this way for testability. - MessageLoop* const io_loop_; - // The URL to POST to. GURL url_for_request_; @@ -178,15 +174,10 @@ class HttpBridge : public base::RefCountedThreadSafe<HttpBridge>, std::string response_content_; // A waitable event we use to provide blocking semantics to - // MakeSynchronousPost. We block created_on_loop_ while the io_loop_ fetches + // MakeSynchronousPost. We block created_on_loop_ while the IO loop fetches // network request. base::WaitableEvent http_post_completed_; - // This is here so that the unit test subclass can force our URLFetcher to - // use the io_loop_ passed on construction for network requests, rather than - // ChromeThread::IO's message loop (which won't exist in testing). - bool use_io_loop_for_testing_; - DISALLOW_COPY_AND_ASSIGN(HttpBridge); }; diff --git a/chrome/browser/sync/glue/http_bridge_unittest.cc b/chrome/browser/sync/glue/http_bridge_unittest.cc index da3eb67..c34ad25 100644 --- a/chrome/browser/sync/glue/http_bridge_unittest.cc +++ b/chrome/browser/sync/glue/http_bridge_unittest.cc @@ -5,6 +5,7 @@ #if defined(BROWSER_SYNC) #include "base/thread.h" +#include "chrome/browser/chrome_thread.h" #include "chrome/browser/sync/glue/http_bridge.h" #include "net/url_request/url_request_unittest.h" #include "testing/gtest/include/gtest/gtest.h" @@ -32,7 +33,7 @@ class HttpBridgeTest : public testing::Test { public: HttpBridgeTest() : fake_default_request_context_getter_(NULL), - io_thread_("HttpBridgeTest IO thread") { + io_thread_(ChromeThread::IO) { } virtual void SetUp() { @@ -55,9 +56,7 @@ class HttpBridgeTest : public testing::Test { } HttpBridge* bridge = new HttpBridge( new HttpBridge::RequestContextGetter( - fake_default_request_context_getter_), - io_thread_loop()); - bridge->use_io_loop_for_testing_ = true; + fake_default_request_context_getter_)); return bridge; } @@ -74,7 +73,8 @@ class HttpBridgeTest : public testing::Test { TestURLRequestContextGetter* fake_default_request_context_getter_; // Separate thread for IO used by the HttpBridge. - base::Thread io_thread_; + ChromeThread io_thread_; + }; // An HttpBridge that doesn't actually make network requests and just calls @@ -82,10 +82,10 @@ class HttpBridgeTest : public testing::Test { class ShuntedHttpBridge : public HttpBridge { public: ShuntedHttpBridge(URLRequestContextGetter* baseline_context_getter, - MessageLoop* io_loop, HttpBridgeTest* test) + HttpBridgeTest* test) : HttpBridge(new HttpBridge::RequestContextGetter( - baseline_context_getter), - io_loop), test_(test) { } + baseline_context_getter)), + test_(test) { } protected: virtual void MakeAsynchronousPost() { ASSERT_TRUE(MessageLoop::current() == test_->io_thread_loop()); @@ -124,7 +124,7 @@ TEST_F(HttpBridgeTest, TestMakeSynchronousPostShunted) { scoped_refptr<URLRequestContextGetter> ctx_getter( new TestURLRequestContextGetter()); scoped_refptr<HttpBridge> http_bridge(new ShuntedHttpBridge( - ctx_getter, io_thread_loop(), this)); + ctx_getter, this)); http_bridge->SetUserAgent("bob"); http_bridge->SetURL("http://www.google.com", 9999); http_bridge->SetPostPayload("text/plain", 2, " "); diff --git a/chrome/browser/user_data_manager.cc b/chrome/browser/user_data_manager.cc index 5e9e0e6..bd2ba88 100644 --- a/chrome/browser/user_data_manager.cc +++ b/chrome/browser/user_data_manager.cc @@ -210,8 +210,7 @@ void UserDataManager::LaunchChromeForProfile(int index) const { void UserDataManager::GetProfiles(std::vector<std::wstring>* profiles) const { // This function should be called on the file thread. - DCHECK(MessageLoop::current() == - ChromeThread::GetMessageLoop(ChromeThread::FILE)); + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE)); file_util::FileEnumerator file_enum( FilePath::FromWStringHack(user_data_root_), false, file_util::FileEnumerator::DIRECTORIES); @@ -281,9 +280,8 @@ void GetProfilesHelper::GetProfiles(MessageLoop* target_loop) { message_loop_ = MessageLoop::current(); } DCHECK(message_loop_); - MessageLoop* file_loop = ChromeThread::GetMessageLoop(ChromeThread::FILE); - file_loop->PostTask( - FROM_HERE, + ChromeThread::PostTask( + ChromeThread::FILE, FROM_HERE, NewRunnableMethod(this, &GetProfilesHelper::GetProfilesFromManager)); } @@ -294,8 +292,7 @@ void GetProfilesHelper::OnDelegateDeleted() { void GetProfilesHelper::GetProfilesFromManager() { // This function should be called on the file thread. - DCHECK(MessageLoop::current() == - ChromeThread::GetMessageLoop(ChromeThread::FILE)); + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE)); // If the delegate is gone by now, no need to do any work. if (!delegate_) diff --git a/chrome/browser/web_resource/web_resource_service.cc b/chrome/browser/web_resource/web_resource_service.cc index b056822..7e56c9f 100644 --- a/chrome/browser/web_resource/web_resource_service.cc +++ b/chrome/browser/web_resource/web_resource_service.cc @@ -115,7 +115,8 @@ class WebResourceService::UnpackerClient #endif if (use_utility_process) { - ChromeThread::GetMessageLoop(ChromeThread::IO)->PostTask(FROM_HERE, + ChromeThread::PostTask( + ChromeThread::IO, FROM_HERE, NewRunnableMethod(this, &UnpackerClient::StartProcessOnIOThread, web_resource_service_->resource_dispatcher_host_, MessageLoop::current())); diff --git a/chrome/common/child_process_host.cc b/chrome/common/child_process_host.cc index 7327769..e3077a39 100644 --- a/chrome/common/child_process_host.cc +++ b/chrome/common/child_process_host.cc @@ -242,8 +242,7 @@ void ChildProcessHost::ListenerHook::OnChannelError() { ChildProcessHost::Iterator::Iterator() : all_(true), type_(UNKNOWN_PROCESS) { - DCHECK(MessageLoop::current() == - ChromeThread::GetMessageLoop(ChromeThread::IO)) << + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)) << "ChildProcessInfo::Iterator must be used on the IO thread."; iterator_ = Singleton<ChildProcessList>::get()->begin(); } @@ -251,9 +250,8 @@ ChildProcessHost::Iterator::Iterator() ChildProcessHost::Iterator::Iterator(ProcessType type) : all_(false), type_(type) { // IO loop can be NULL in unit tests. - DCHECK(!ChromeThread::GetMessageLoop(ChromeThread::IO) || - MessageLoop::current() == - ChromeThread::GetMessageLoop(ChromeThread::IO)) << + DCHECK(!MessageLoop::current() || + ChromeThread::CurrentlyOn(ChromeThread::IO)) << "ChildProcessInfo::Iterator must be used on the IO thread."; iterator_ = Singleton<ChildProcessList>::get()->begin(); if (!Done() && (*iterator_)->type() != type_) diff --git a/chrome/common/chrome_plugin_unittest.cc b/chrome/common/chrome_plugin_unittest.cc index 537c585..854092c 100644 --- a/chrome/common/chrome_plugin_unittest.cc +++ b/chrome/common/chrome_plugin_unittest.cc @@ -7,6 +7,7 @@ #include "base/path_service.h" #include "base/string_util.h" #include "chrome/browser/chrome_plugin_host.h" +#include "chrome/browser/chrome_thread.h" #include "chrome/browser/net/url_request_context_getter.h" #include "chrome/browser/profile.h" #include "chrome/common/chrome_plugin_lib.h" @@ -37,7 +38,8 @@ class TestURLRequestContextGetter : public URLRequestContextGetter { class ChromePluginTest : public testing::Test, public URLRequest::Delegate { public: ChromePluginTest() - : request_(NULL), + : io_thread_(ChromeThread::IO, &message_loop_), + request_(NULL), response_buffer_(new net::IOBuffer(kResponseBufferSize)), plugin_(NULL), expected_payload_(NULL), @@ -85,6 +87,7 @@ class ChromePluginTest : public testing::Test, public URLRequest::Delegate { } protected: MessageLoopForIO message_loop_; + ChromeThread io_thread_; // Note: we use URLRequest (instead of URLFetcher) because this allows the // request to be intercepted. |