Created an interface for DownloadFile, for use in unit tests.

content/browser/download/download_file_impl.h

Index: content/browser/download/download_file_impl.h
diff --git a/content/browser/download/download_file_impl.h b/content/browser/download/download_file_impl.h
new file mode 100644
index 0000000000000000000000000000000000000000..da406ee3a6752ab0a8de910c9143eea09b21092d
--- /dev/null
+++ b/content/browser/download/download_file_impl.h
@@ -0,0 +1,71 @@
+// 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 CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_FILE_IMPL_H_
+#define CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_FILE_IMPL_H_
+#pragma once
+
+#include "content/browser/download/download_file.h"
+
+#include <string>
+
+#include "base/basictypes.h"
+#include "base/memory/ref_counted.h"
+#include "content/browser/download/download_request_handle.h"
+#include "content/common/content_export.h"
+
+struct DownloadCreateInfo;
+class DownloadManager;
+
+// These objects live exclusively on the download thread and handle the writing

[Randy Smith (Not in Mondays), 2011/11/10 21:58:43]

nit: Could you change  "download thread" to "file thread" as long as you're
fooling around with these comments.

[ahendrickson, 2011/11/13 00:50:21]

Done.

+// operations for one download. These objects live only for the duration that
+// the download is 'in progress': once the download has been completed or
+// cancelled, the DownloadFile is destroyed.

[Randy Smith (Not in Mondays), 2011/11/10 21:58:43]

I'm ambivalent (so if you have a strong feeling, let me know and we can go with
that) but I think that the comment describing the lifetime of a download file
probably should be in the interface file rather than the implementation file. 
It's an audience targeting thing--the people who need to know about the
externally visible behavior of the DownloadFile read the interface file, and I
think this counts as externally visible behavior.

[ahendrickson, 2011/11/13 00:50:21]

Done.

+class CONTENT_EXPORT DownloadFileImpl : virtual public DownloadFile {
+ public:
+  // Takes ownership of the object pointed to by |request_handle|.
+  DownloadFileImpl(const DownloadCreateInfo* info,
+                   DownloadRequestHandleInterface* request_handle,
+                   DownloadManager* download_manager);
+  virtual ~DownloadFileImpl();
+
+  // BaseFile delegated functions.

[Randy Smith (Not in Mondays), 2011/11/10 21:58:43]

nit: The fact that these are delegated to the BaseFile is part of the
implementation, rather than the interface, and hence isn't as important to
remark on in the header.  I think the fact that these functions are inherited
from the DownloadFile ABC is more important.

[ahendrickson, 2011/11/13 00:50:21]

Done.

+  virtual net::Error Initialize(bool calculate_hash);
+  virtual net::Error AppendDataToFile(const char* data, size_t data_len);
+  virtual net::Error Rename(const FilePath& full_path);
+  virtual void Detach();
+  virtual void Cancel();
+  virtual void Finish();
+  virtual void AnnotateWithSourceInformation();
+  virtual FilePath FullPath() const;
+  virtual bool InProgress() const;
+  virtual int64 BytesSoFar() const;
+  virtual bool GetSha256Hash(std::string* hash);
+
+  // DownloadFile implementation.

[Randy Smith (Not in Mondays), 2011/11/10 21:58:43]

The above is part of the DownloadFile implementation as well, right?

[Randy Smith (Not in Mondays), 2011/11/10 21:58:43]

Shouldn't all functions that inherit from the DownloadFile interface be marked
with OVERRIDE?

[ahendrickson, 2011/11/13 00:50:21]

Yes

[ahendrickson, 2011/11/13 00:50:21]

Done.

+  virtual void CancelDownloadRequest();
+  virtual int Id() const;
+  virtual DownloadManager* GetDownloadManager();
+  virtual const DownloadId& GlobalId() const;
+  virtual std::string DebugString() const;
+
+ private:
+  // The base file instance.
+  BaseFile file_;
+
+  // The unique identifier for this download, assigned at creation by
+  // the DownloadFileManager for its internal record keeping.
+  DownloadId id_;
+
+  // The handle to the request information.  Used for operations outside the
+  // download system, specifically canceling a download.
+  scoped_ptr<DownloadRequestHandleInterface> request_handle_;
+
+  // DownloadManager this download belongs to.
+  scoped_refptr<DownloadManager> download_manager_;
+
+  DISALLOW_COPY_AND_ASSIGN(DownloadFileImpl);
+};
+
+#endif  // CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_FILE_IMPL_H_