Change DownloadProcessHandle to be more of an encapsulated class.

chrome/browser/download/download_process_handle.h

 // 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.
 
 #ifndef CHROME_BROWSER_DOWNLOAD_DOWNLOAD_PROCESS_HANDLE_H_
 #define CHROME_BROWSER_DOWNLOAD_DOWNLOAD_PROCESS_HANDLE_H_
 #pragma once
 
+#include <string>
+
 class DownloadManager;
 class ResourceDispatcherHost;
 class TabContents;
 
 // A handle used by the download system for operations on external
 // objects associated with the download (e.g. URLRequest, TabContents,
 // DownloadManager).
 // This class needs to be copyable, so we can pass it across threads and not
 // worry about lifetime or const-ness.
 class DownloadProcessHandle {
  public:
   DownloadProcessHandle();
-  DownloadProcessHandle(int child_id, int render_view_id, int request_id);
+  DownloadProcessHandle(ResourceDispatcherHost* rdh,
+                        int child_id,
+                        int render_view_id,
+                        int request_id);
 
   // These functions must be called on the UI thread.
   TabContents* GetTabContents();
   DownloadManager* GetDownloadManager();
 
-  int child_id() const { return child_id_; }
-  int render_view_id() const { return render_view_id_; }
-  int request_id() const { return request_id_; }
+  // Pause or resume (|pause| == false) the matching URL request.
+  // If the DownloadProcessHandle was constructed with a null

[Paweł Hajdan Jr., 2011/06/04 08:58:51]

I'm worried about this possibly silent no-op. Is |rdh| null sometimes? If
possible, we should require RDH to be non-null. Otherwise, make this return bool
to indicate success, and use WARN_UNUSED_RESULT from base/compiler_specific to
make sure people check it.

[Randy Smith (Not in Mondays), 2011/06/07 22:41:11]

Hmmm.  The silent no-op is copied from the download_util functions I'm
eliminating.  It's there because some unit tests are testing a subset of the
system that doesn't include the RDH.

I'm going to "solve" this by creating two special DownloadPRocessHandles.  One
(default constructor) is a Null handle, and it's a DCHECK violation to call any
of the action routines on it.  The other (special private static member function
only available to tests) is a Nop handle, and it silently drops the actions. 
Let me know if this addresses your concerns.

+  // ResourceDispatcherHost, this is a no-op.
+  void PauseRequest(bool pause);
+
+  // Cancel the request
+  void CancelRequest();
+
+  std::string DebugString() const;
 
  private:
+  // IO Thread indirections to resource dispatcher host.
+  // Provided as targets for PostTask from within this object
+  // only.  Safe to use as ResourceDispatcherHost outlives the
+  // IO thread.
+  static void RDHPauseRequest(ResourceDispatcherHost* rdh,
+                              int process_unique_id,
+                              int request_id,
+                              bool pause);
+  static void RDHCancelRequest(ResourceDispatcherHost* rdh,
+                               int process_unique_id,
+                               int request_id);
+
+  // The resource dispatcher host.
+  ResourceDispatcherHost* rdh_;

[hendrickson_a, 2011/06/04 16:01:45]

Nit: It seems unintuitive from the name of the class that there is a pointer to
ResourceDispatcherHost as a member.

[Randy Smith (Not in Mondays), 2011/06/07 22:41:11]

Yep.  I think of that as being part of the encpasulation; we need the
ResourceDispatcherHost to implement the functionality of the class, but ther'es
no need for it in the interface.  I couldn't offhand think of a way to get rid
of the pointer (the global pointer to RDH that I'm aware of is only available on
the UI thread) and I think this is much cleaner than passing it in all the time
(because however unintuitive it is to have it as part of the implementation,
it's more unintuitive to have it as part of the interface as well).  But if you
have some ideas for how to get rid of it, I'd love to hear them.

+
   // The ID of the child process that started the download.
   int child_id_;
 
   // The ID of the render view that started the download.
   int render_view_id_;
 
   // The ID associated with the request used for the download.
   int request_id_;
 };
 
 #endif  // CHROME_BROWSER_DOWNLOAD_DOWNLOAD_PROCESS_HANDLE_H_