Detect file system errors during downloads.

content/browser/download/download_manager.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/download/download_manager.h"
 
 #include <iterator>
 
 #include "base/callback.h"
 #include "base/file_util.h"
@@ skipping 332 matching lines @@
     if (download->IsInProgress()) {
       download->Update(size);
       UpdateDownloadProgress();  // Reflect size updates.
       delegate_->UpdateItemInPersistentStore(download);
     }
   }
 }
 
 void DownloadManager::OnResponseCompleted(int32 download_id,
                                           int64 size,
-                                          int os_error,
                                           const std::string& hash) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  // ERR_CONNECTION_CLOSED is allowed since a number of servers in the wild
-  // advertise a larger Content-Length than the amount of bytes in the message
-  // body, and then close the connection. Other browsers - IE8, Firefox 4.0.1,
-  // and Safari 5.0.4 - treat the download as complete in this case, so we
-  // follow their lead.
-  if (os_error == 0 || os_error == net::ERR_CONNECTION_CLOSED) {
-    OnAllDataSaved(download_id, size, hash);
-  } else {
-    OnDownloadError(download_id, size, os_error);
-  }
-}
-
-void DownloadManager::OnAllDataSaved(int32 download_id,
-                                     int64 size,
-                                     const std::string& hash) {
   VLOG(20) << __FUNCTION__ << "()" << " download_id = " << download_id
            << " size = " << size;
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   // If it's not in active_downloads_, that means it was cancelled; just
   // ignore the notification.
   if (active_downloads_.count(download_id) == 0)
     return;
 
   DownloadItem* download = active_downloads_[download_id];
@@ skipping 122 matching lines @@
           file_manager_, &DownloadFileManager::CompleteDownload, download_id));
 
   if (uniquifier)
     item->set_path_uniquifier(uniquifier);
 
   item->OnDownloadRenamedToFinalName(full_path);
   delegate_->UpdatePathForItemInPersistentStore(item, full_path);
 }
 
 void DownloadManager::CancelDownload(int32 download_id) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));

[Randy Smith (Not in Mondays), 2011/08/28 22:14:03]

Why remove the DCHECK?

[ahendrickson, 2011/08/29 16:30:53]

I removed it because GetActiveDownload() has it.

However, I don't feel strongly about it, so I'll put it back in.

Done.

-  DownloadItem* download = GetDownloadItem(download_id);
+  DownloadItem* download = GetActiveDownload(download_id);
+  // A cancel at the right time could remove the download from the
+  // |active_downloads_| map before we get here.
   if (!download)
     return;
 
   download->Cancel(true);
 }
 
 void DownloadManager::DownloadCancelledInternal(DownloadItem* download) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  int download_id = download->id();
 
   VLOG(20) << __FUNCTION__ << "()"
            << " download = " << download->DebugString(true);
 
-  // Clean up will happen when the history system create callback runs if we
-  // don't have a valid db_handle yet.
-  if (download->db_handle() != DownloadItem::kUninitializedHandle) {
-    in_progress_.erase(download_id);
-    active_downloads_.erase(download_id);
-    UpdateDownloadProgress();  // Reflect removal from in_progress_.
-    delegate_->UpdateItemInPersistentStore(download);
-
-    // This function is called from the DownloadItem, so DI state
-    // should already have been updated.
-    AssertQueueStateConsistent(download);
-  }
+  RemoveFromActiveList(download);
+  // This function is called from the DownloadItem, so DI state
+  // should already have been updated.
+  AssertQueueStateConsistent(download);
 
   download->OffThreadCancel(file_manager_);
 }
 
 void DownloadManager::OnDownloadError(int32 download_id,
                                       int64 size,
-                                      int os_error) {
+                                      int error) {
+  DownloadItem* download = GetActiveDownload(download_id);
+  if (!download)
+    return;
+
+  VLOG(20) << __FUNCTION__ << "()" << " Error " << error
+           << " at offset " << download->received_bytes()
+           << " size = " << size
+           << " download = " << download->DebugString(true);
+
+  RemoveFromActiveList(download);
+  download->Interrupted(size, error);
+  download->OffThreadCancel(file_manager_);
+}
+
+DownloadItem* DownloadManager::GetActiveDownload(int32 download_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DownloadMap::iterator it = active_downloads_.find(download_id);
-  // A cancel at the right time could remove the download from the
-  // |active_downloads_| map before we get here.
   if (it == active_downloads_.end())
-    return;
+    return NULL;
 
   DownloadItem* download = it->second;
 
-  VLOG(20) << __FUNCTION__ << "()" << " Error " << os_error
-           << " at offset " << download->received_bytes()
-           << " for download = " << download->DebugString(true);
+  DCHECK(download);
+  DCHECK_EQ(download_id, download->id());
 
-  download->Interrupted(size, os_error);
+  return download;
+}
 
-  // TODO(ahendrickson) - Remove this when we add resuming of interrupted
-  // downloads, as we will keep the download item around in that case.
-  //
+void DownloadManager::RemoveFromActiveList(DownloadItem* download) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(download);
+
   // Clean up will happen when the history system create callback runs if we
   // don't have a valid db_handle yet.
   if (download->db_handle() != DownloadItem::kUninitializedHandle) {
-    in_progress_.erase(download_id);
-    active_downloads_.erase(download_id);
+    in_progress_.erase(download->id());
+    active_downloads_.erase(download->id());
     UpdateDownloadProgress();  // Reflect removal from in_progress_.
     delegate_->UpdateItemInPersistentStore(download);
   }
-
-  BrowserThread::PostTask(
-      BrowserThread::FILE, FROM_HERE,
-      NewRunnableMethod(
-          file_manager_, &DownloadFileManager::CancelDownload, download_id));
 }
 
 void DownloadManager::UpdateDownloadProgress() {
   delegate_->DownloadProgressUpdated();
 }
 
 int DownloadManager::RemoveDownloadItems(
     const DownloadVector& pending_deletes) {
   if (pending_deletes.empty())
     return 0;
@@ skipping 168 matching lines @@
   int32 download_id = *id_ptr;
   delete id_ptr;
 
   DownloadItem* download = GetActiveDownloadItem(download_id);
   if (!download)
     return;
 
   VLOG(20) << __FUNCTION__ << "()"
            << " download = " << download->DebugString(true);
 
+  // TODO(ahendrickson) -- This currently has no effect, as the download is
+  // not put on the active list until the file selection is complete.  Need
+  // to put it on the active list earlier in the process.
+  RemoveFromActiveList(download);
+
   download->OffThreadCancel(file_manager_);
 }
 
 // Operations posted to us from the history service ----------------------------
 
 // The history service has retrieved all download entries. 'entries' contains
 // 'DownloadPersistentStoreInfo's in sorted order (by ascending start_time).
 void DownloadManager::OnPersistentStoreQueryComplete(
     std::vector<DownloadPersistentStoreInfo>* entries) {
   // TODO(rdsmith): Remove this and related logic when
@@ skipping 239 matching lines @@
     DCHECK(ContainsKey(save_page_downloads_, download->id()));
     save_page_downloads_.erase(download->id());
 
     if (download->IsComplete())
       NotificationService::current()->Notify(
           content::NOTIFICATION_SAVE_PACKAGE_SUCCESSFULLY_FINISHED,
           Source<DownloadManager>(this),
           Details<DownloadItem>(download));
   }
 }