Create a Null DownloadRequestHandle for use by SavePackage DownloadItems.

content/browser/download/download_item.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_item.h"
 
 #include <vector>
 
 #include "base/bind.h"
 #include "base/basictypes.h"
@@ skipping 92 matching lines @@
 DownloadItem::DangerType GetDangerType(bool dangerous_file,
                                        bool dangerous_url) {
   if (dangerous_url) {
     // dangerous URL overweights dangerous file. We check dangerous URL first.
     return DownloadItem::DANGEROUS_URL;
   }
   return dangerous_file ?
       DownloadItem::DANGEROUS_FILE : DownloadItem::NOT_DANGEROUS;
 }
 
+// Classes to null out request handle calls (for SavePage DownloadItems, which
+// may have, e.g., Cancel() called on them without it doing anything)
+// and to DCHECK on them (for history DownloadItems, which should never have
+// any operation that implies an off-thread component, since they don't
+// have any).
+class NullDownloadRequestHandle : public DownloadRequestHandleInterface {
+ public:
+  NullDownloadRequestHandle() {}
+
+  // DownloadRequestHandleInterface calls
+  virtual TabContents* GetTabContents() const OVERRIDE {
+    return NULL;
+  }
+  virtual DownloadManager* GetDownloadManager() const OVERRIDE {
+    return NULL;
+  }
+  virtual void PauseRequest() const OVERRIDE {}
+  virtual void ResumeRequest() const OVERRIDE {}
+  virtual void CancelRequest() const OVERRIDE {}
+  virtual std::string DebugString() const OVERRIDE {
+    return "Null DownloadRequestHandle";
+  }
+};
+
+
 }  // namespace
 
 // Our download table ID starts at 1, so we use 0 to represent a download that
 // has started, but has not yet had its data persisted in the table. We use fake
 // database handles in incognito mode starting at -1 and progressively getting
 // more negative.
 // static
 const int DownloadItem::kUninitializedHandle = 0;
 
 const char DownloadItem::kEmptyFileHash[] = "";
@@ skipping 71 matching lines @@
       delegate_delayed_complete_(false) {
   Init(true /* actively downloading */);
 }
 
 // Constructing for the "Save Page As..." feature:
 DownloadItem::DownloadItem(DownloadManager* download_manager,
                            const FilePath& path,
                            const GURL& url,
                            bool is_otr,
                            DownloadId download_id)
-    : download_id_(download_id),
+    : request_handle_(new NullDownloadRequestHandle()),
+      download_id_(download_id),
       full_path_(path),
       url_chain_(1, url),
       referrer_url_(GURL()),
       total_bytes_(0),
       received_bytes_(0),
       last_reason_(DOWNLOAD_INTERRUPT_REASON_NONE),
       start_tick_(base::TimeTicks::Now()),
       state_(IN_PROGRESS),
       start_time_(base::Time::Now()),
       db_handle_(DownloadItem::kUninitializedHandle),
@@ skipping 462 matching lines @@
                                      start_time(),
                                      end_time(),
                                      received_bytes(),
                                      total_bytes(),
                                      state(),
                                      db_handle(),
                                      opened());
 }
 
 TabContents* DownloadItem::GetTabContents() const {
   if (request_handle_.get())

[cbentzel, 2011/11/15 11:47:56]

Can you remove the NULL check here?

[Randy Smith (Not in Mondays), 2011/11/15 19:25:39]

No, I can't, though I'd like to and I have another CL working in that direction.
 The problem is that this CL only puts in a null request handle for Save Page
download items, not History download items (see first comment for context).  And
GetTabContents() is called when a search through DownloadItems is done.  So
removing the null check would result in a repeat of the original release blocker
crasher.  

[cbentzel, 2011/11/15 19:53:23]

Thanks. It seems very confusing that it's checked for in some cases and not
others, just due to the knowledge that certain portions won't be called based on
the source of the downloaditem. At least put a comment in the code for why the
NULL check is needed here (or why it isn't needed in the other cases). 

[Randy Smith (Not in Mondays), 2011/11/15 21:49:49]

Done.

     return request_handle_->GetTabContents();
   return NULL;
 }
 
 FilePath DownloadItem::GetTargetFilePath() const {
   return full_path_.DirName().Append(state_info_.target_name);
 }
 
 FilePath DownloadItem::GetFileNameToReportUser() const {
   if (state_info_.path_uniquifier > 0) {
@@ skipping 99 matching lines @@
         state_info_.target_name.value().c_str(),
         full_path().value().c_str());
   } else {
     description += base::StringPrintf(" url = \"%s\"", url_list.c_str());
   }
 
   description += " }";
 
   return description;
 }