Added DownloadProcessHandle class.

chrome/browser/download/download_file.cc

Index: chrome/browser/download/download_file.cc
diff --git a/chrome/browser/download/download_file.cc b/chrome/browser/download/download_file.cc
index 629ee117a95e44d2008f1fc727b722d965f4cffb..50af091568ffefc6587396f23992e7a1f8d668a8 100644
--- a/chrome/browser/download/download_file.cc
+++ b/chrome/browser/download/download_file.cc
@@ -21,8 +21,7 @@ DownloadFile::DownloadFile(const DownloadCreateInfo* info,
                info->received_bytes,
                info->save_info.file_stream),
       id_(info->download_id),
-      child_id_(info->child_id),
-      request_id_(info->request_id),
+      process_handle_(info->process_handle),
       download_manager_(download_manager) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
@@ -33,13 +32,7 @@ DownloadFile::~DownloadFile() {
 }
 
 void DownloadFile::CancelDownloadRequest(ResourceDispatcherHost* rdh) {

[Paweł Hajdan Jr., 2011/05/06 18:04:42]

nit: Please restore the BrowserThread DCHECK.

[ahendrickson, 2011/05/09 15:57:31]

It doesn't matter what thread this is called from, as it just posts to the IO
thread.

[Paweł Hajdan Jr., 2011/05/09 19:47:30]

Agreed, but it's very important to ensure that DownloadFile is only ever used on
the IO thread. It simplifies reasoning about the code a lot. Please restore the
DCHECK.

[ahendrickson, 2011/05/10 18:04:43]

Done.

-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
-  BrowserThread::PostTask(
-      BrowserThread::IO, FROM_HERE,
-      NewRunnableFunction(&download_util::CancelDownloadRequest,
-                          rdh,
-                          child_id_,
-                          request_id_));
+  download_util::CancelDownloadRequest(rdh, process_handle_);

[Randy Smith (Not in Mondays), 2011/05/09 00:57:45]

I don't want to put more stuff into download_util.  If Pawel's really not
willing to put the methods into DownloadProcessHandle, could you expand them
back out inline?

Pawel, did you have any response to my note to you early on this CL about
encapsulation on DownloadProcessHandle?

[Paweł Hajdan Jr., 2011/05/09 19:47:30]

Wait, what's wrong with this change? We're not adding anything to download_util,
it's just changing the implementation and adding an internal function. All of
that seems simple, clean, and good to me. Maybe I just don't get the idea behind
not touching download_util.
Yes, I did, sorry if I didn't respond. I think that the CL in current state is
the best solution. I see no need for a CancelDownload method in
DownloadProcessHandle, and fixing download_util like here seems more correct.
I'm not a fan of the DownloadProcessHandle::CancelDownload method because it was
mostly looking like "OK, I give you this RDH, and you call some method on it".
It seems backwards. Of course the current code is not ideal either, but it's not
a regression. I'd like all new code/classes to be extremely clean.

[Randy Smith (Not in Mondays), 2011/05/09 21:42:20]

Ah, whoops, I see.  Quite right.  I rescind my objection.
I'll follow up on this in email--I'm ok with the CL as it is.

 }
 
 DownloadManager* DownloadFile::GetDownloadManager() {
@@ -49,15 +42,15 @@ DownloadManager* DownloadFile::GetDownloadManager() {
 
 std::string DownloadFile::DebugString() const {
   return base::StringPrintf("{"
-                            " full_path_ = " "\"%s\""
                             " id_ = " "%d"
-                            " child_id_ = " "%d"
-                            " request_id_ = " "%d"
+                            " child_id = " "%d"
+                            " request_id = " "%d"
+                            " render_view_id = " "%d"
                             " Base File = %s"
                             " }",
-                            full_path_.value().c_str(),
                             id_,
-                            child_id_,
-                            request_id_,
+                            process_handle_.child_id(),
+                            process_handle_.request_id(),
+                            process_handle_.render_view_id(),
                             BaseFile::DebugString().c_str());
 }