Fix instances of passing raw pointers to RefCounted objects in tasks.

Some of these manually handled it correctly by using AddRef()/Release() pairs.  I switched them to make_scoped_refptr() to be more consistent.  This also makes them cleanup properly on MessageLoop shutdown if we start deleting tasks.

BUG=28083
TEST=builds

Review URL: http://codereview.chromium.org/3581008

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

15 files changed, 142 insertions, 75 deletions

diff --git a/chrome/browser/automation/automation_resource_message_filter.cc b/chrome/browser/automation/automation_resource_message_filter.cc
index 7dac439..92c4a17 100644
--- a/chrome/browser/automation/automation_resource_message_filter.cc
+++ b/chrome/browser/automation/automation_resource_message_filter.cc
@@ -226,7 +226,11 @@ bool AutomationResourceMessageFilter::RegisterRenderView(
       BrowserThread::IO, FROM_HERE,
       NewRunnableFunction(
           AutomationResourceMessageFilter::RegisterRenderViewInIOThread,
-          renderer_pid, renderer_id, tab_handle, filter, pending_view));
+          renderer_pid,
+          renderer_id,
+          tab_handle,
+          make_scoped_refptr(filter),
+          pending_view));
   return true;
 }
 
@@ -251,7 +255,10 @@ bool AutomationResourceMessageFilter::ResumePendingRenderView(
       BrowserThread::IO, FROM_HERE,
       NewRunnableFunction(
           AutomationResourceMessageFilter::ResumePendingRenderViewInIOThread,
-          renderer_pid, renderer_id, tab_handle, filter));
+          renderer_pid,
+          renderer_id,
+          tab_handle,
+          make_scoped_refptr(filter)));
   return true;
 }
 
diff --git a/chrome/browser/debugger/devtools_http_protocol_handler.cc b/chrome/browser/debugger/devtools_http_protocol_handler.cc
index 4ab1a79..e064018 100644
--- a/chrome/browser/debugger/devtools_http_protocol_handler.cc
+++ b/chrome/browser/debugger/devtools_http_protocol_handler.cc
@@ -95,7 +95,7 @@ void DevToolsHttpProtocolHandler::OnHttpRequest(
         FROM_HERE,
         NewRunnableMethod(this,
                           &DevToolsHttpProtocolHandler::OnHttpRequestUI,
-                          socket,
+                          scoped_refptr<HttpListenSocket>(socket),
                           info));
     return;
   }
@@ -123,7 +123,7 @@ void DevToolsHttpProtocolHandler::OnWebSocketRequest(
       NewRunnableMethod(
           this,
           &DevToolsHttpProtocolHandler::OnWebSocketRequestUI,
-          socket,
+          make_scoped_refptr(socket),
           request));
 }
 
@@ -135,7 +135,7 @@ void DevToolsHttpProtocolHandler::OnWebSocketMessage(HttpListenSocket* socket,
       NewRunnableMethod(
           this,
           &DevToolsHttpProtocolHandler::OnWebSocketMessageUI,
-          socket,
+          make_scoped_refptr(socket),
           data));
 }
 
@@ -160,7 +160,7 @@ void DevToolsHttpProtocolHandler::OnClose(HttpListenSocket* socket) {
       NewRunnableMethod(
           this,
           &DevToolsHttpProtocolHandler::OnCloseUI,
-          socket));
+          make_scoped_refptr(socket)));
 }
 
 void DevToolsHttpProtocolHandler::OnHttpRequestUI(
diff --git a/chrome/browser/download/download_file_manager.cc b/chrome/browser/download/download_file_manager.cc
index fc94314..10fe2fe 100644
--- a/chrome/browser/download/download_file_manager.cc
+++ b/chrome/browser/download/download_file_manager.cc
@@ -174,9 +174,13 @@ void DownloadFileManager::StartDownload(DownloadCreateInfo* info) {
     return;
   }
 
-  ChromeThread::PostTask(ChromeThread::FILE, FROM_HERE,
-      NewRunnableMethod(this, &DownloadFileManager::CreateDownloadFile,
-                        info, manager));
+  ChromeThread::PostTask(
+      ChromeThread::FILE,
+      FROM_HERE,
+      NewRunnableMethod(this,
+                        &DownloadFileManager::CreateDownloadFile,
+                        info,
+                        make_scoped_refptr(manager)));
 }
 
 // We don't forward an update to the UI thread here, since we want to throttle
diff --git a/chrome/browser/download/download_manager.cc b/chrome/browser/download/download_manager.cc
index 4e0f4ea..bb9aeea 100644
--- a/chrome/browser/download/download_manager.cc
+++ b/chrome/browser/download/download_manager.cc
@@ -76,7 +76,7 @@ void DownloadManager::Shutdown() {
     ChromeThread::PostTask(ChromeThread::FILE, FROM_HERE,
         NewRunnableMethod(file_manager_,
                           &DownloadFileManager::OnDownloadManagerShutdown,
-                          this));
+                          make_scoped_refptr(this)));
   }
 
   // 'in_progress_' may contain DownloadItems that have not finished the start
@@ -446,8 +446,12 @@ void DownloadManager::CreateDownloadItem(DownloadCreateInfo* info,
     ChromeThread::PostTask(
         ChromeThread::FILE, FROM_HERE,
         NewRunnableMethod(
-            file_manager_, &DownloadFileManager::OnFinalDownloadName,
-            download->id(), target_path, !info->is_dangerous, this));
+            file_manager_,
+            &DownloadFileManager::OnFinalDownloadName,
+            download->id(),
+            target_path,
+            !info->is_dangerous,
+            make_scoped_refptr(this)));
   } else {
     // The download hasn't finished and it is a safe download.  We need to
     // rename it to its intermediate '.crdownload' path.
@@ -455,8 +459,11 @@ void DownloadManager::CreateDownloadItem(DownloadCreateInfo* info,
     ChromeThread::PostTask(
         ChromeThread::FILE, FROM_HERE,
         NewRunnableMethod(
-            file_manager_, &DownloadFileManager::OnIntermediateDownloadName,
-            download->id(), download_path, this));
+            file_manager_,
+            &DownloadFileManager::OnIntermediateDownloadName,
+            download->id(),
+            download_path,
+            make_scoped_refptr(this)));
     download->set_need_final_rename(true);
   }
 
@@ -541,8 +548,12 @@ void DownloadManager::OnAllDataSaved(int32 download_id, int64 size) {
     ChromeThread::PostTask(
         ChromeThread::FILE, FROM_HERE,
         NewRunnableMethod(
-            file_manager_, &DownloadFileManager::OnFinalDownloadName,
-            download->id(), download->full_path(), false, this));
+            file_manager_,
+            &DownloadFileManager::OnFinalDownloadName,
+            download->id(),
+            download->full_path(),
+            false,
+            make_scoped_refptr(this)));
     return;
   }
 
diff --git a/chrome/browser/download/save_file_manager.cc b/chrome/browser/download/save_file_manager.cc
index 00e9251..b1c59a9 100644
--- a/chrome/browser/download/save_file_manager.cc
+++ b/chrome/browser/download/save_file_manager.cc
@@ -131,14 +131,16 @@ void SaveFileManager::SaveURL(const GURL& url,
     DCHECK(url.is_valid());
 
     ChromeThread::PostTask(
-        ChromeThread::IO, FROM_HERE,
-        NewRunnableMethod(this,
-                          &SaveFileManager::OnSaveURL,
-                          url,
-                          referrer,
-                          render_process_host_id,
-                          render_view_id,
-                          request_context_getter));
+        ChromeThread::IO,
+        FROM_HERE,
+        NewRunnableMethod(
+            this,
+            &SaveFileManager::OnSaveURL,
+            url,
+            referrer,
+            render_process_host_id,
+            render_view_id,
+            make_scoped_refptr(request_context_getter)));
   } else {
     // We manually start the save job.
     SaveFileCreateInfo* info = new SaveFileCreateInfo(file_full_path,
@@ -250,7 +252,6 @@ void SaveFileManager::UpdateSaveProgress(int save_id,
             this, &SaveFileManager::OnUpdateSaveProgress, save_file->save_id(),
             save_file->bytes_so_far(), write_success));
   }
-  data->Release();
 }
 
 // The IO thread will call this when saving is completed or it got error when
diff --git a/chrome/browser/download/save_package.cc b/chrome/browser/download/save_package.cc
index 4ce3e36..071e95b 100644
--- a/chrome/browser/download/save_package.cc
+++ b/chrome/browser/download/save_package.cc
@@ -1025,8 +1025,7 @@ void SavePackage::OnReceivedSerializedHtmlData(const GURL& frame_url,
 
   if (!data.empty()) {
     // Prepare buffer for saving HTML data.
-    net::IOBuffer* new_data = new net::IOBuffer(data.size());
-    new_data->AddRef();  // We'll pass the buffer to SaveFileManager.
+    scoped_refptr<net::IOBuffer> new_data = new net::IOBuffer(data.size());
     memcpy(new_data->data(), data.data(), data.size());
 
     // Call write file functionality in file thread.
diff --git a/chrome/browser/importer/importer_unittest.cc b/chrome/browser/importer/importer_unittest.cc
index 879d7cb..d99b1e9 100644
--- a/chrome/browser/importer/importer_unittest.cc
+++ b/chrome/browser/importer/importer_unittest.cc
@@ -120,9 +120,15 @@ class ImporterTest : public testing::Test {
     int items = HISTORY | PASSWORDS | FAVORITES;
     if (import_search_plugins)
       items = items | SEARCH_ENGINES;
-    loop->PostTask(FROM_HERE, NewRunnableMethod(host.get(),
-        &ImporterHost::StartImportSettings, profile_info,
-        static_cast<Profile*>(NULL), items, writer, true));
+    loop->PostTask(
+        FROM_HERE,
+        NewRunnableMethod(host.get(),
+                          &ImporterHost::StartImportSettings,
+                          profile_info,
+                          static_cast<Profile*>(NULL),
+                          items,
+                          make_scoped_refptr(writer),
+                          true));
     loop->Run();
   }
 
@@ -698,17 +704,23 @@ TEST_F(ImporterTest, MAYBE(Firefox2Importer)) {
 
   MessageLoop* loop = MessageLoop::current();
   scoped_refptr<ImporterHost> host = new ImporterHost();
-  FirefoxObserver* observer = new FirefoxObserver();
+  scoped_refptr<FirefoxObserver> observer = new FirefoxObserver();
   host->SetObserver(observer);
   ProfileInfo profile_info;
   profile_info.browser_type = FIREFOX2;
   profile_info.app_path = app_path_;
   profile_info.source_path = profile_path_;
 
-  loop->PostTask(FROM_HERE, NewRunnableMethod(host.get(),
-      &ImporterHost::StartImportSettings, profile_info,
-      static_cast<Profile*>(NULL),
-      HISTORY | PASSWORDS | FAVORITES | SEARCH_ENGINES, observer, true));
+  loop->PostTask(
+      FROM_HERE,
+      NewRunnableMethod(
+          host.get(),
+          &ImporterHost::StartImportSettings,
+          profile_info,
+          static_cast<Profile*>(NULL),
+          HISTORY | PASSWORDS | FAVORITES | SEARCH_ENGINES,
+          observer,
+          true));
   loop->Run();
 }
 
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 17e99c4..6dc08d0 100644
--- a/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.cc
+++ b/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.cc
@@ -151,9 +151,17 @@ void DOMStorageDispatcherHost::OnStorageAreaId(int64 namespace_id,
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
   ChromeURLRequestContext* url_request_context =
       resource_message_filter_->GetRequestContextForURL(GURL(origin));
-  ChromeThread::PostTask(ChromeThread::WEBKIT, FROM_HERE, NewRunnableMethod(
-      this, &DOMStorageDispatcherHost::OnStorageAreaIdWebKit, namespace_id,
-      origin, reply_msg, url_request_context->host_content_settings_map()));
+  ChromeThread::PostTask(
+      ChromeThread::WEBKIT,
+      FROM_HERE,
+      NewRunnableMethod(
+          this,
+          &DOMStorageDispatcherHost::OnStorageAreaIdWebKit,
+          namespace_id,
+          origin,
+          reply_msg,
+          make_scoped_refptr(
+              url_request_context->host_content_settings_map())));
 }
 
 void DOMStorageDispatcherHost::OnStorageAreaIdWebKit(
diff --git a/chrome/browser/notifications/desktop_notification_service_unittest.cc b/chrome/browser/notifications/desktop_notification_service_unittest.cc
index f88466b..0e52cb4 100644
--- a/chrome/browser/notifications/desktop_notification_service_unittest.cc
+++ b/chrome/browser/notifications/desktop_notification_service_unittest.cc
@@ -41,8 +41,10 @@ class ThreadProxy : public base::RefCountedThreadSafe<ThreadProxy> {
   int CacheHasPermission(NotificationsPrefsCache* cache, const GURL& url) {
     DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
     ChromeThread::PostTask(ChromeThread::IO, FROM_HERE,
-        NewRunnableMethod(this, &ThreadProxy::CacheHasPermissionIO,
-                          cache, url));
+        NewRunnableMethod(this,
+                          &ThreadProxy::CacheHasPermissionIO,
+                          make_scoped_refptr(cache),
+                          url));
     io_event_.Signal();
     ui_event_.Wait();  // Wait for IO thread to be done.
     ChromeThread::PostTask(ChromeThread::IO, FROM_HERE,
diff --git a/chrome/browser/renderer_host/audio_renderer_host.cc b/chrome/browser/renderer_host/audio_renderer_host.cc
index 96b4999..efe0d95 100644
--- a/chrome/browser/renderer_host/audio_renderer_host.cc
+++ b/chrome/browser/renderer_host/audio_renderer_host.cc
@@ -121,39 +121,54 @@ void AudioRendererHost::IPCChannelClosing() {
 // media::AudioOutputController::EventHandler implementations.
 void AudioRendererHost::OnCreated(media::AudioOutputController* controller) {
   ChromeThread::PostTask(
-      ChromeThread::IO, FROM_HERE,
-      NewRunnableMethod(this, &AudioRendererHost::DoCompleteCreation,
-                        controller));
+      ChromeThread::IO,
+      FROM_HERE,
+      NewRunnableMethod(
+          this,
+          &AudioRendererHost::DoCompleteCreation,
+          make_scoped_refptr(controller)));
 }
 
 void AudioRendererHost::OnPlaying(media::AudioOutputController* controller) {
   ChromeThread::PostTask(
-      ChromeThread::IO, FROM_HERE,
-      NewRunnableMethod(this, &AudioRendererHost::DoSendPlayingMessage,
-                        controller));
+      ChromeThread::IO,
+      FROM_HERE,
+      NewRunnableMethod(
+          this,
+          &AudioRendererHost::DoSendPlayingMessage,
+          make_scoped_refptr(controller)));
 }
 
 void AudioRendererHost::OnPaused(media::AudioOutputController* controller) {
   ChromeThread::PostTask(
-      ChromeThread::IO, FROM_HERE,
-      NewRunnableMethod(this, &AudioRendererHost::DoSendPausedMessage,
-                        controller));
+      ChromeThread::IO,
+      FROM_HERE,
+      NewRunnableMethod(
+          this,
+          &AudioRendererHost::DoSendPausedMessage,
+          make_scoped_refptr(controller)));
 }
 
 void AudioRendererHost::OnError(media::AudioOutputController* controller,
                                 int error_code) {
   ChromeThread::PostTask(
-      ChromeThread::IO, FROM_HERE,
-      NewRunnableMethod(this, &AudioRendererHost::DoHandleError,
-                        controller, error_code));
+      ChromeThread::IO,
+      FROM_HERE,
+      NewRunnableMethod(this,
+                        &AudioRendererHost::DoHandleError,
+                        make_scoped_refptr(controller),
+                        error_code));
 }
 
 void AudioRendererHost::OnMoreData(media::AudioOutputController* controller,
                                    AudioBuffersState buffers_state) {
   ChromeThread::PostTask(
-      ChromeThread::IO, FROM_HERE,
-      NewRunnableMethod(this, &AudioRendererHost::DoRequestMoreData,
-                        controller, buffers_state));
+      ChromeThread::IO,
+      FROM_HERE,
+      NewRunnableMethod(this,
+                        &AudioRendererHost::DoRequestMoreData,
+                        make_scoped_refptr(controller),
+                        buffers_state));
 }
 
 void AudioRendererHost::DoCompleteCreation(
diff --git a/chrome/browser/renderer_host/save_file_resource_handler.cc b/chrome/browser/renderer_host/save_file_resource_handler.cc
index b71181a..17100bf 100644
--- a/chrome/browser/renderer_host/save_file_resource_handler.cc
+++ b/chrome/browser/renderer_host/save_file_resource_handler.cc
@@ -80,8 +80,8 @@ bool SaveFileResourceHandler::OnWillRead(int request_id, net::IOBuffer** buf,
 bool SaveFileResourceHandler::OnReadCompleted(int request_id, int* bytes_read) {
   DCHECK(read_buffer_);
   // We are passing ownership of this buffer to the save file manager.
-  net::IOBuffer* buffer = NULL;
-  read_buffer_.swap(&buffer);
+  scoped_refptr<net::IOBuffer> buffer = NULL;
+  read_buffer_.swap(buffer);
   ChromeThread::PostTask(
       ChromeThread::FILE, FROM_HERE,
       NewRunnableMethod(save_manager_,
diff --git a/chrome/browser/safe_browsing/safe_browsing_service.cc b/chrome/browser/safe_browsing/safe_browsing_service.cc
index da38f18..e1fbab8 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_service.cc
@@ -386,9 +386,6 @@ void SafeBrowsingService::OnIOInitialize(
                                                       mackey_url_prefix,
                                                       disable_auto_update);
 
-  // Balance the reference added by Start().
-  request_context_getter->Release();
-
   protocol_manager_->Initialize();
 }
 
@@ -648,14 +645,16 @@ void SafeBrowsingService::Start() {
   }
 
   // We will issue network fetches using the default profile's request context.
-  URLRequestContextGetter* request_context_getter =
+  scoped_refptr<URLRequestContextGetter> request_context_getter =
       GetDefaultProfile()->GetRequestContext();
-  request_context_getter->AddRef();  // Balanced in OnIOInitialize.
 
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       NewRunnableMethod(
-          this, &SafeBrowsingService::OnIOInitialize, client_key, wrapped_key,
+          this,
+          &SafeBrowsingService::OnIOInitialize,
+          client_key,
+          wrapped_key,
           request_context_getter));
 }
 
diff --git a/chrome/profile_import/profile_import_thread.cc b/chrome/profile_import/profile_import_thread.cc
index 6f097b3..5700823 100644
--- a/chrome/profile_import/profile_import_thread.cc
+++ b/chrome/profile_import/profile_import_thread.cc
@@ -32,6 +32,8 @@ ProfileImportThread::ProfileImportThread()
   ChildProcess::current()->AddRefProcess();  // Balanced in Cleanup().
 }
 
+ProfileImportThread::~ProfileImportThread() {}
+
 void ProfileImportThread::OnControlMessageReceived(const IPC::Message& msg) {
   IPC_BEGIN_MESSAGE_MAP(ProfileImportThread, msg)
     IPC_MESSAGE_HANDLER(ProfileImportProcessMsg_StartImport,
@@ -49,7 +51,6 @@ void ProfileImportThread::OnImportStart(
     const DictionaryValue& localized_strings,
     bool import_to_bookmark_bar) {
   bridge_ = new ExternalProcessImporterBridge(this, localized_strings);
-  bridge_->AddRef();  // Balanced in Cleanup().
 
   ImporterList importer_list;
   importer_ = importer_list.CreateImporterByType(profile_info.browser_type);
@@ -59,7 +60,6 @@ void ProfileImportThread::OnImportStart(
     return;
   }
 
-  importer_->AddRef();  // Balanced in Cleanup().
   importer_->set_import_to_bookmark_bar(import_to_bookmark_bar);
   items_to_import_ = items;
 
@@ -71,9 +71,14 @@ void ProfileImportThread::OnImportStart(
     NOTREACHED();
     Cleanup();
   }
-  import_thread_->message_loop()->PostTask(FROM_HERE,
-      NewRunnableMethod(importer_, &Importer::StartImport,
-          profile_info, items, bridge_));
+  import_thread_->message_loop()->PostTask(
+      FROM_HERE,
+      NewRunnableMethod(
+          importer_.get(),
+          &Importer::StartImport,
+          profile_info,
+          items,
+          bridge_));
 }
 
 void ProfileImportThread::OnImportCancel() {
@@ -184,7 +189,7 @@ void ProfileImportThread::NotifyKeywordsReady(
 
 void ProfileImportThread::Cleanup() {
   importer_->Cancel();
-  importer_->Release();
-  bridge_->Release();
+  importer_ = NULL;
+  bridge_ = NULL;
   ChildProcess::current()->ReleaseProcess();
 }
diff --git a/chrome/profile_import/profile_import_thread.h b/chrome/profile_import/profile_import_thread.h
index f780cc5..079fa4c 100644
--- a/chrome/profile_import/profile_import_thread.h
+++ b/chrome/profile_import/profile_import_thread.h
@@ -28,7 +28,7 @@ class InProcessImporterBridge;
 class ProfileImportThread : public ChildThread {
  public:
   ProfileImportThread();
-  virtual ~ProfileImportThread() {}
+  virtual ~ProfileImportThread();
 
   // Returns the one profile import thread.
   static ProfileImportThread* current() {
@@ -84,7 +84,7 @@ class ProfileImportThread : public ChildThread {
 
   // Bridge object is passed to importer, so that it can send IPC calls
   // directly back to the ProfileImportProcessHost.
-  ExternalProcessImporterBridge* bridge_;
+  scoped_refptr<ExternalProcessImporterBridge> bridge_;
 
   // importer::ProfileType enum from importer_list, stored in ProfileInfo
   // struct in importer.
@@ -94,7 +94,7 @@ class ProfileImportThread : public ChildThread {
   uint16 items_to_import_;
 
   // Importer of the appropriate type (Firefox, Safari, IE, etc.)
-  Importer* importer_;
+  scoped_refptr<Importer> importer_;
 
   DISALLOW_COPY_AND_ASSIGN(ProfileImportThread);
 };
diff --git a/chrome/service/service_process.cc b/chrome/service/service_process.cc
index db562c3..3aeaa88 100644
--- a/chrome/service/service_process.cc
+++ b/chrome/service/service_process.cc
@@ -386,8 +386,12 @@ void ServiceProcess::SaveChromotingConfig(
 
   // And then do the update.
   chromoting_config_->Update(
-      NewRunnableFunction(&SaveChromotingConfigFunc, chromoting_config_.get(),
-                          login, token, host_id, host_name));
+      NewRunnableFunction(&SaveChromotingConfigFunc,
+                          chromoting_config_,
+                          login,
+                          token,
+                          host_id,
+                          host_name));
 
   // And then save the key pair.
   host_key_pair->Save(chromoting_config_);