Change DownloadProcessHandle to be more of an encapsulated class.

chrome/browser/download/download_request_handle.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 "chrome/browser/download/download_request_handle.h"
+
+#include "base/stringprintf.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/tab_contents/tab_util.h"
+#include "content/browser/browser_thread.h"
+#include "content/browser/renderer_host/resource_dispatcher_host.h"
+#include "content/browser/tab_contents/tab_contents.h"
+
+// IO Thread indirections to resource dispatcher host.
+// Provided as targets for PostTask from within this object
+// only.
+static void ResourceDispatcherHostPauseRequest(
+    ResourceDispatcherHost* rdh,
+    int process_unique_id,
+    int request_id,
+    bool pause)
+{

[Paweł Hajdan Jr., 2011/06/08 09:36:58]

nit: Shouldn't that be on the previous line?

[Randy Smith (Not in Mondays), 2011/06/09 13:17:43]

Done.

+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  rdh->PauseRequest(process_unique_id, request_id, pause);
+}
+
+static void ResourceDispatcherHostCancelRequest(
+    ResourceDispatcherHost* rdh,
+    int process_unique_id,
+    int request_id)
+{

[Paweł Hajdan Jr., 2011/06/08 09:36:58]

nit: Shouldn't that be on the previous line?

[Randy Smith (Not in Mondays), 2011/06/09 13:17:43]

Done.

+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  rdh->CancelRequest(process_unique_id, request_id, false);
+}
+
+DownloadRequestHandle::DownloadRequestHandle()
+    : rdh_(NULL), child_id_(-1), render_view_id_(-1), request_id_(-1) {

[Paweł Hajdan Jr., 2011/06/08 09:36:58]

nit: I think each initializer should be on its own line.

[Randy Smith (Not in Mondays), 2011/06/09 13:17:43]

Done.

+}
+
+DownloadRequestHandle::DownloadRequestHandle(ResourceDispatcherHost* rdh,
+                                             int child_id,
+                                             int render_view_id,
+                                             int request_id)
+    : rdh_(rdh),
+      child_id_(child_id),
+      render_view_id_(render_view_id),
+      request_id_(request_id) {
+  DCHECK(rdh);  // ResourceDispatcherHost may not be null in normal operation.
+}
+
+TabContents* DownloadRequestHandle::GetTabContents() const{
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  return tab_util::GetTabContentsByID(child_id_, render_view_id_);
+}
+
+DownloadManager* DownloadRequestHandle::GetDownloadManager() const {
+  TabContents* contents = GetTabContents();
+  if (!contents)
+    return NULL;
+
+  Profile* profile = contents->profile();
+  if (!profile)
+    return NULL;
+
+  return profile->GetDownloadManager();
+}
+
+void DownloadRequestHandle::PauseRequest() {
+  DCHECK(!IsNull());
+
+  // The post is safe because ResourceDispatcherHost is guaranteed
+  // to outlive the IO thread.
+  if (!IsNop()) {
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        NewRunnableFunction(&ResourceDispatcherHostPauseRequest,
+                            rdh_, child_id_, request_id_, true));
+  }
+}
+
+void DownloadRequestHandle::ResumeRequest() {
+  DCHECK(!IsNull());
+  // The post is safe because ResourceDispatcherHost is guaranteed
+  // to outlive the IO thread.
+  if (!IsNop()) {
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        NewRunnableFunction(&ResourceDispatcherHostPauseRequest,
+                            rdh_, child_id_, request_id_, false));
+  }
+}
+
+void DownloadRequestHandle::CancelRequest() {
+  // The post is safe because ResourceDispatcherHost is guaranteed
+  // to outlive the IO thread.
+  if (rdh_) {
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        NewRunnableFunction(&ResourceDispatcherHostCancelRequest,
+                            rdh_, child_id_, request_id_));
+  }
+}
+
+std::string DownloadRequestHandle::DebugString() const {
+  return base::StringPrintf("{"
+                            " child_id = %d"
+                            " render_view_id = %d"
+                            " request_id = %d"
+                            "}",
+                            child_id_,
+                            render_view_id_,
+                            request_id_);
+}
+
+// static
+DownloadRequestHandle DownloadRequestHandle::GetNopDownloadRequestHandle() {
+  DownloadRequestHandle result;
+  result.child_id_ = 0;
+  result.render_view_id_ = 0;
+  result.request_id_ = 0;
+  return result;
+}
+
+bool DownloadRequestHandle::IsNull() const {

[Paweł Hajdan Jr., 2011/06/08 09:36:58]

Ouch, I don't like this distinction between null and nop. So we want nop in
tests, and where is null used? How hard would it be to not require nop for tests
(for example by converting them to browser tests)?

[Randy Smith (Not in Mondays), 2011/06/09 13:17:43]

Went back to just making no RDH be a no-op, as discussed separately.

+  return (rdh_ == NULL && child_id_ == -1 &&
+          render_view_id_ == -1 && request_id_ == -1);
+}
+
+bool DownloadRequestHandle::IsNop() const {
+  return (rdh_ == NULL && child_id_ == 0 &&
+          render_view_id_ == 0 && request_id_ == 0);
+}