Revert 61899 for breaking cookes on file:// URLs.

BUG=58553 ================================================= 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 TBR=willchan@chromium.org Review URL: http://codereview.chromium.org/3654001 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@62043 0039d316-1c4b-4281-b951-d872f2087c98
marcosholgado · Oct 8, 2010 · 0d7e79f · 0d7e79f
1 parent f941516
commit 0d7e79f
Show file tree

Hide file tree

Showing 22 changed files with 90 additions and 158 deletions.
diff --git a/chrome/browser/automation/automation_resource_message_filter.cc b/chrome/browser/automation/automation_resource_message_filter.cc
@@ -226,11 +226,7 @@ bool AutomationResourceMessageFilter::RegisterRenderView(
       BrowserThread::IO, FROM_HERE,
       NewRunnableFunction(
           AutomationResourceMessageFilter::RegisterRenderViewInIOThread,
-          renderer_pid,
-          renderer_id,
-          tab_handle,
-          make_scoped_refptr(filter),
-          pending_view));
+          renderer_pid, renderer_id, tab_handle, filter, pending_view));
   return true;
 }
 
@@ -255,10 +251,7 @@ bool AutomationResourceMessageFilter::ResumePendingRenderView(
       BrowserThread::IO, FROM_HERE,
       NewRunnableFunction(
           AutomationResourceMessageFilter::ResumePendingRenderViewInIOThread,
-          renderer_pid,
-          renderer_id,
-          tab_handle,
-          make_scoped_refptr(filter)));
+          renderer_pid, renderer_id, tab_handle, filter));
   return true;
 }
 

diff --git 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,
-                          scoped_refptr<HttpListenSocket>(socket),
+                          socket,
                           info));
     return;
   }
@@ -123,7 +123,7 @@ void DevToolsHttpProtocolHandler::OnWebSocketRequest(
       NewRunnableMethod(
           this,
           &DevToolsHttpProtocolHandler::OnWebSocketRequestUI,
-          make_scoped_refptr(socket),
+          socket,
           request));
 }
 
@@ -135,7 +135,7 @@ void DevToolsHttpProtocolHandler::OnWebSocketMessage(HttpListenSocket* socket,
       NewRunnableMethod(
           this,
           &DevToolsHttpProtocolHandler::OnWebSocketMessageUI,
-          make_scoped_refptr(socket),
+          socket,
           data));
 }
 
@@ -160,7 +160,7 @@ void DevToolsHttpProtocolHandler::OnClose(HttpListenSocket* socket) {
       NewRunnableMethod(
           this,
           &DevToolsHttpProtocolHandler::OnCloseUI,
-          make_scoped_refptr(socket)));
+          socket));
 }
 
 void DevToolsHttpProtocolHandler::OnHttpRequestUI(

diff --git a/chrome/browser/download/download_file_manager.cc b/chrome/browser/download/download_file_manager.cc
@@ -174,13 +174,9 @@ void DownloadFileManager::StartDownload(DownloadCreateInfo* info) {
     return;
   }
 
-  ChromeThread::PostTask(
-      ChromeThread::FILE,
-      FROM_HERE,
-      NewRunnableMethod(this,
-                        &DownloadFileManager::CreateDownloadFile,
-                        info,
-                        make_scoped_refptr(manager)));
+  ChromeThread::PostTask(ChromeThread::FILE, FROM_HERE,
+      NewRunnableMethod(this, &DownloadFileManager::CreateDownloadFile,
+                        info, 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
@@ -76,7 +76,7 @@ void DownloadManager::Shutdown() {
     ChromeThread::PostTask(ChromeThread::FILE, FROM_HERE,
         NewRunnableMethod(file_manager_,
                           &DownloadFileManager::OnDownloadManagerShutdown,
-                          make_scoped_refptr(this)));
+                          this));
   }
 
   // 'in_progress_' may contain DownloadItems that have not finished the start
@@ -437,24 +437,17 @@ void DownloadManager::CreateDownloadItem(DownloadCreateInfo* info,
     ChromeThread::PostTask(
         ChromeThread::FILE, FROM_HERE,
         NewRunnableMethod(
-            file_manager_,
-            &DownloadFileManager::OnFinalDownloadName,
-            download->id(),
-            target_path,
-            !info->is_dangerous,
-            make_scoped_refptr(this)));
+            file_manager_, &DownloadFileManager::OnFinalDownloadName,
+            download->id(), target_path, !info->is_dangerous, this));
   } else {
     // The download hasn't finished and it is a safe download.  We need to
     // rename it to its intermediate '.crdownload' path.
     FilePath download_path = download_util::GetCrDownloadPath(target_path);
     ChromeThread::PostTask(
         ChromeThread::FILE, FROM_HERE,
         NewRunnableMethod(
-            file_manager_,
-            &DownloadFileManager::OnIntermediateDownloadName,
-            download->id(),
-            download_path,
-            make_scoped_refptr(this)));
+            file_manager_, &DownloadFileManager::OnIntermediateDownloadName,
+            download->id(), download_path, this));
     download->set_need_final_rename(true);
   }
 
@@ -539,12 +532,8 @@ 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,
-            make_scoped_refptr(this)));
+            file_manager_, &DownloadFileManager::OnFinalDownloadName,
+            download->id(), download->full_path(), false, this));
     return;
   }
 

diff --git a/chrome/browser/download/save_file_manager.cc b/chrome/browser/download/save_file_manager.cc
@@ -131,16 +131,14 @@ 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,
-            make_scoped_refptr(request_context_getter)));
+        ChromeThread::IO, FROM_HERE,
+        NewRunnableMethod(this,
+                          &SaveFileManager::OnSaveURL,
+                          url,
+                          referrer,
+                          render_process_host_id,
+                          render_view_id,
+                          request_context_getter));
   } else {
     // We manually start the save job.
     SaveFileCreateInfo* info = new SaveFileCreateInfo(file_full_path,
@@ -252,6 +250,7 @@ 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
@@ -1025,7 +1025,8 @@ void SavePackage::OnReceivedSerializedHtmlData(const GURL& frame_url,
 
   if (!data.empty()) {
     // Prepare buffer for saving HTML data.
-    scoped_refptr<net::IOBuffer> new_data = new net::IOBuffer(data.size());
+    net::IOBuffer* new_data = new net::IOBuffer(data.size());
+    new_data->AddRef();  // We'll pass the buffer to SaveFileManager.
     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
@@ -120,15 +120,9 @@ 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,
-                          make_scoped_refptr(writer),
-                          true));
+    loop->PostTask(FROM_HERE, NewRunnableMethod(host.get(),
+        &ImporterHost::StartImportSettings, profile_info,
+        static_cast<Profile*>(NULL), items, writer, true));
     loop->Run();
   }
 
@@ -704,23 +698,17 @@ TEST_F(ImporterTest, MAYBE(Firefox2Importer)) {
 
   MessageLoop* loop = MessageLoop::current();
   scoped_refptr<ImporterHost> host = new ImporterHost();
-  scoped_refptr<FirefoxObserver> observer = new FirefoxObserver();
+  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
@@ -151,17 +151,9 @@ 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,
-          make_scoped_refptr(
-              url_request_context->host_content_settings_map())));
+  ChromeThread::PostTask(ChromeThread::WEBKIT, FROM_HERE, NewRunnableMethod(
+      this, &DOMStorageDispatcherHost::OnStorageAreaIdWebKit, namespace_id,
+      origin, reply_msg, 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
@@ -41,10 +41,8 @@ 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,
-                          make_scoped_refptr(cache),
-                          url));
+        NewRunnableMethod(this, &ThreadProxy::CacheHasPermissionIO,
+                          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
@@ -126,7 +126,7 @@ void AudioRendererHost::OnCreated(media::AudioOutputController* controller) {
       NewRunnableMethod(
           this,
           &AudioRendererHost::DoCompleteCreation,
-          make_scoped_refptr(controller)));
+          controller));
 }
 
 void AudioRendererHost::OnPlaying(media::AudioOutputController* controller) {
@@ -136,7 +136,7 @@ void AudioRendererHost::OnPlaying(media::AudioOutputController* controller) {
       NewRunnableMethod(
           this,
           &AudioRendererHost::DoSendPlayingMessage,
-          make_scoped_refptr(controller)));
+          controller));
 }
 
 void AudioRendererHost::OnPaused(media::AudioOutputController* controller) {
@@ -146,7 +146,7 @@ void AudioRendererHost::OnPaused(media::AudioOutputController* controller) {
       NewRunnableMethod(
           this,
           &AudioRendererHost::DoSendPausedMessage,
-          make_scoped_refptr(controller)));
+          controller));
 }
 
 void AudioRendererHost::OnError(media::AudioOutputController* controller,
@@ -156,7 +156,7 @@ void AudioRendererHost::OnError(media::AudioOutputController* controller,
       FROM_HERE,
       NewRunnableMethod(this,
                         &AudioRendererHost::DoHandleError,
-                        make_scoped_refptr(controller),
+                        controller,
                         error_code));
 }
 
@@ -167,7 +167,7 @@ void AudioRendererHost::OnMoreData(media::AudioOutputController* controller,
       FROM_HERE,
       NewRunnableMethod(this,
                         &AudioRendererHost::DoRequestMoreData,
-                        make_scoped_refptr(controller),
+                        controller,
                         buffers_state));
 }
 

diff --git 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.
-  scoped_refptr<net::IOBuffer> buffer = NULL;
-  read_buffer_.swap(buffer);
+  net::IOBuffer* buffer = NULL;
+  read_buffer_.swap(&buffer);
   BrowserThread::PostTask(
       BrowserThread::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
@@ -386,6 +386,9 @@ void SafeBrowsingService::OnIOInitialize(
                                                       mackey_url_prefix,
                                                       disable_auto_update);
 
+  // Balance the reference added by Start().
+  request_context_getter->Release();
+
   protocol_manager_->Initialize();
 }
 
@@ -645,16 +648,14 @@ void SafeBrowsingService::Start() {
   }
 
   // We will issue network fetches using the default profile's request context.
-  scoped_refptr<URLRequestContextGetter> request_context_getter =
+  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
@@ -32,8 +32,6 @@ 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,
@@ -51,6 +49,7 @@ 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);
@@ -60,6 +59,7 @@ void ProfileImportThread::OnImportStart(
     return;
   }
 
+  importer_->AddRef();  // Balanced in Cleanup().
   importer_->set_import_to_bookmark_bar(import_to_bookmark_bar);
   items_to_import_ = items;
 
@@ -71,14 +71,9 @@ void ProfileImportThread::OnImportStart(
     NOTREACHED();
     Cleanup();
   }
-  import_thread_->message_loop()->PostTask(
-      FROM_HERE,
-      NewRunnableMethod(
-          importer_.get(),
-          &Importer::StartImport,
-          profile_info,
-          items,
-          bridge_));
+  import_thread_->message_loop()->PostTask(FROM_HERE,
+      NewRunnableMethod(importer_, &Importer::StartImport,
+          profile_info, items, bridge_));
 }
 
 void ProfileImportThread::OnImportCancel() {
@@ -189,7 +184,7 @@ void ProfileImportThread::NotifyKeywordsReady(
 
 void ProfileImportThread::Cleanup() {
   importer_->Cancel();
-  importer_ = NULL;
-  bridge_ = NULL;
+  importer_->Release();
+  bridge_->Release();
   ChildProcess::current()->ReleaseProcess();
 }