DownloadManager::Observer::OnDownloadCreated

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 09628b0268a217e273cd1e564cbe5d1a03d1422e..70056d3b4c33ebd2680319b353e0335f9214ac4f 100644
--- a/content/public/browser/download_manager.h
+++ b/content/public/browser/download_manager.h
@@ -79,9 +79,22 @@ class CONTENT_EXPORT DownloadManager
   // to the DownloadManager's collection of downloads.
   class CONTENT_EXPORT Observer {
    public:
+    // A DownloadItem was created.  Useful methods on the item that are
+    // guaranteed to work as advertised include GetId(), AddObserver(),
+    // SetExternalData(), GetURL(). The item is guaranteed to be accessible via
+    // DownloadManager::GetDownload(). No other guarantees are made about its
+    // state: it may have just been loaded from history, it may be an active
+    // download, or it may be a SavePage download, it may or may not already be
+    // persisted. If a consumer needs a DownloadItem's filename, it should wait
+    // for an OnDownloadUpdated() when the filename is not empty(). When the
+    // history is loaded on startup, this method may be called many (thousands)
+    // of times at once.

[Randy Smith (Not in Mondays), 2012/07/23 18:40:50]

Can you shorten this a bit?  I think we're ok just warning that this may make
visible an item before it's file name has been determined, and telling the
consumer how to figure out if it's in that state.  (+ the final warning about
history loading.)

[benjhayden, 2012/07/23 18:54:22]

Please just give me a string to copy-paste into here.

[Randy Smith (Not in Mondays), 2012/07/23 19:00:06]

Sure, though please confirm with Asanka that what I'm writing is accurate:

--
A DownloadItem was created.  Unlike ModelChanged, this item may be visible
before the filename is determined; in this case the return value of
GetTargetFileName() will be null.  This method may be called an arbitrary number
of times, e.g. when loading history on startup.  As a result, consumers should
avoid doing large amounts of work in OnDownloadCreated().
TODO(<whoever>): When we've fully specified the possible states of the
DownloadItem in download_item.h and removed ModelChanged, we should remove the
caveat above.

[benjhayden, 2012/07/25 16:24:38]

Done.

+    virtual void OnDownloadCreated(
+        DownloadManager* manager, DownloadItem* item) {}
+
     // New or deleted download, observers should query us for the current set
     // of downloads.
-    virtual void ModelChanged(DownloadManager* manager) = 0;
+    virtual void ModelChanged(DownloadManager* manager) {}
 
     // Called when the DownloadManager is being destroyed to prevent Observers
     // from calling back to a stale pointer.