Added callback to DownloadUrl() so we can find download failures.

content/public/browser/download_manager.h

Index: content/public/browser/download_manager.h
diff --git a/content/public/browser/download_manager.h b/content/public/browser/download_manager.h
index 8d9845fd0a2fb62dbfa32559fdd65e34a945e832..d6571ba5da0cfd0acffa9f05c62eea6c13d5276e 100644
--- a/content/public/browser/download_manager.h
+++ b/content/public/browser/download_manager.h
@@ -32,6 +32,7 @@
 #include <vector>
 
 #include "base/basictypes.h"
+#include "base/callback.h"
 #include "base/file_path.h"
 #include "base/gtest_prod_util.h"
 #include "base/message_loop_helpers.h"
@@ -62,6 +63,9 @@ class WebContents;
 class CONTENT_EXPORT DownloadManager
     : public base::RefCountedThreadSafe<DownloadManager> {
  public:
+  // NOTE: It is OK for the DownloadId to be invalid if there is an error.

[Randy Smith (Not in Mondays), 2012/03/07 21:16:57]

wording nit: "Is it OK" sounds like telling the consumer what they can and can't
do.  This is telling them what they should expect, so I think something like "If
there is an error, the DownloadId will be invalid." would be better.  

[ahendrickson, 2012/03/08 21:33:07]

Done.

+  typedef base::Callback<void(DownloadId, net::Error)> OnStartedCallback;
+
   virtual ~DownloadManager() {}
 
   static DownloadManager* Create(
@@ -176,13 +180,16 @@ class CONTENT_EXPORT DownloadManager
   // saved, and whether the user should be prompted about the download.
   // |web_contents| is the web page that the download is done in context of,
   // and must be non-NULL.
+  // |callback| will be called when the download starts, or if an error
+  // occurs.

[Randy Smith (Not in Mondays), 2012/03/07 21:16:57]

This sentence is actually underspecified, as the download can start and an error
occur later.  I'm inclined to say that once we know the DownloadItem is being
created, we can call the callback with the DownloadId and let any other errors
be the DownloadItem::Observer interface's problem.  If you agree, this
documentation should be made more specific (and you should make sure that's the
functionality that's actually supported).

[jstritar, 2012/03/07 21:34:39]

What if we define a Delegate that has separate methods, where we guarantee that
one of them will be called?

class Delegate {
  public:
    void OnDownloadStarted(DownloadId id) = 0;
    void OnDownloadNotStarted(net::Error error) = 0;
};

Once OnDownloadStarted is invoked, the client can use DownloadItem::Observer.

[ahendrickson, 2012/03/08 21:33:07]

Done.

[Randy Smith (Not in Mondays), 2012/03/09 19:14:02]

typo: "prevents ad download".

[ahendrickson, 2012/03/09 20:50:08]

Done.

   virtual void DownloadUrl(const GURL& url,
                            const GURL& referrer,
                            const std::string& referrer_encoding,
                            bool prefer_cache,
                            int64 post_id,
                            const DownloadSaveInfo& save_info,
-                           content::WebContents* web_contents) = 0;
+                           content::WebContents* web_contents,
+                           const OnStartedCallback& callback) = 0;
 
   // Allow objects to observe the download creation process.
   virtual void AddObserver(Observer* observer) = 0;