downloads: prevent "Clear all" from removing in progress downloads

content/browser/download/download_item_impl.cc

 // Copyright (c) 2012 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.
 
 // File method ordering: Methods in this file are in the same order as
 // in download_item_impl.h, with the following exception: The public
 // interface Start is placed in chronological order with the other
 // (private) routines that together define a DownloadItem's state
 // transitions as the download progresses.  See "Download progression
 // cascade" later in this file.
@@ skipping 396 matching lines @@
 
   RecordDownloadCount(CANCELLED_COUNT);
 
   // TODO(rdsmith/benjhayden): Remove condition as part of
   // |SavePackage| integration.
   // |download_file_| can be NULL if Interrupt() is called after the
   // download file has been released.
   if (!is_save_package_download_ && download_file_)
     ReleaseDownloadFile(true);
 
-  if (state_ == IN_PROGRESS_INTERNAL) {
+  // |request_handle_| may be NULL during tests.
+  if (state_ == IN_PROGRESS_INTERNAL && request_handle_) {

[benjhayden, 2014/12/11 23:36:24]

Should this be checked everywhere that request_handle_ is used?
Or would it be possible to construct a request handle, either real or mock?

[Dan Beam, 2014/12/12 00:07:02]

we talked about this on chat -- adding a way to mock something internal to the
download manager (a content/ feature) from here (chrome/) requires more effort
and fun than it's probably currently worth.  additionally, the more
|request_handle_ == NULL| checks I add, the less safe the code gets when it
*shouldn't* be null (which is currently only in this one case).

I've added a TODO() for now.

     // Cancel the originating URL request unless it's already been cancelled
     // by interrupt.
     request_handle_->CancelRequest();
   }
 
   // Remove the intermediate file if we are cancelling an interrupted download.
   // Continuable interruptions leave the intermediate file around.
   if ((state_ == INTERRUPTED_INTERNAL || state_ == RESUMING_INTERNAL) &&
       !current_path_.empty()) {
     BrowserThread::PostTask(
@@ skipping 1376 matching lines @@
     case RESUME_MODE_USER_CONTINUE:
       return "USER_CONTINUE";
     case RESUME_MODE_USER_RESTART:
       return "USER_RESTART";
   }
   NOTREACHED() << "Unknown resume mode " << mode;
   return "unknown";
 }
 
 }  // namespace content