Use a DownloadRequestHandle pointer in construction to allow mocking for tests.

content/browser/download/download_item.cc

Index: content/browser/download/download_item.cc
diff --git a/content/browser/download/download_item.cc b/content/browser/download/download_item.cc
index 84387b667353a3727d369273445e656fed360f2b..c81fa401c7041133e208c5ab0d5ab9bcfda4dcca 100644
--- a/content/browser/download/download_item.cc
+++ b/content/browser/download/download_item.cc
@@ -154,7 +154,7 @@ DownloadItem::DownloadItem(DownloadManager* download_manager,
 // Constructing for a regular download:
 DownloadItem::DownloadItem(DownloadManager* download_manager,
                            const DownloadCreateInfo& info,
-                           const DownloadRequestHandle& request_handle,
+                           DownloadRequestHandleInterface* request_handle,
                            bool is_otr)
     : state_info_(info.original_name, info.save_info.file_path,
                   info.has_user_gesture, info.transition_type,
@@ -576,9 +576,9 @@ void DownloadItem::TogglePause() {
 
   DCHECK(IsInProgress());
   if (is_paused_)
-    request_handle_.ResumeRequest();
+    request_handle_->ResumeRequest();
   else
-    request_handle_.PauseRequest();
+    request_handle_->PauseRequest();
   is_paused_ = !is_paused_;
   UpdateObservers();
 }
@@ -642,7 +642,7 @@ bool DownloadItem::MatchesQuery(const string16& query) const {
   //   L"/\x4f60\x597d\x4f60\x597d",
   //   "/%E4%BD%A0%E5%A5%BD%E4%BD%A0%E5%A5%BD"
   std::string languages;
-  TabContents* tab = request_handle_.GetTabContents();
+  TabContents* tab = request_handle_->GetTabContents();
   if (tab)
     languages = content::GetContentClient()->browser()->GetAcceptLangs(tab);
   string16 url_formatted(net::FormatUrl(GetURL(), languages));
@@ -702,6 +702,12 @@ DownloadPersistentStoreInfo DownloadItem::GetPersistentStoreInfo() const {
                                      opened());
 }
 
+TabContents* DownloadItem::GetTabContents() const {
+  if (request_handle_.get())

[cbentzel, 2011/10/27 17:20:39]

Why is this needed? DownloadItem::MatchesQuery doesn't use it, for example, and
it looks like other places don't check for NULL either.

[Randy Smith (Not in Mondays), 2011/10/27 18:21:22]

See DownloadManager::RestartDownload.  I ran into this because in some tests, a
null DownloadRequestHandle is now represented by a null pointer rather than a
default constructed download handle.  But for cleanliness of mocking, I'd rather
avoid cases where something external to a class gets a reference to an object
contained within the class, so I prefer the wrapping pattern anyway.

If you'd like me to merge in the switch over to mocks for DownloadRequestHandle
in this CL, I will, but I'd like to keep this wrapper.

[cbentzel, 2011/10/27 18:45:34]

I have no problem with the wrapper - just was more confused about why we were
checking for NULL here but not in other cases.

+    return request_handle_->GetTabContents();
+  return NULL;
+}
+
 FilePath DownloadItem::GetTargetFilePath() const {
   return full_path_.DirName().Append(state_info_.target_name);
 }
@@ -722,7 +728,7 @@ FilePath DownloadItem::GetUserVerifiedFilePath() const {
 
 void DownloadItem::OffThreadCancel(DownloadFileManager* file_manager) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  request_handle_.CancelRequest();
+  request_handle_->CancelRequest();
 
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,