[Downloads] Ask DownloadHistory if a download was from history.

chrome/browser/download/download_history.h

 // Copyright (c) 2012 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_HISTORY_H_
 #define CHROME_BROWSER_DOWNLOAD_DOWNLOAD_HISTORY_H_
 
 #include <set>
 #include <vector>
 
@@ skipping 49 matching lines @@
                                   const history::DownloadRow& info) {}
 
     // Fires when RemoveDownloads messages are sent to the DB thread.
     virtual void OnDownloadsRemoved(const IdSet& ids) {}
 
     // Fires when the DownloadHistory is being destroyed so that implementors
     // can RemoveObserver() and nullify their DownloadHistory*s.
     virtual void OnDownloadHistoryDestroyed() {}
   };
 
-  // Returns true if the item is persisted.
-  static bool IsPersisted(content::DownloadItem* item);
+  // Returns true if the download is persisted. Not reliable when called from
+  // within a DownloadManager::Observer::OnDownloadCreated handler since the
+  // persisted state may not have been updated yet a download that were restored

[Randy Smith (Not in Mondays), 2014/04/10 18:03:23]

nit: "yet *for* a download"?

[asanka, 2014/04/17 21:16:10]

Done.

+  // from history.
+  static bool IsPersisted(const content::DownloadItem* item);
 
   // Neither |manager| nor |history| may be NULL.
   // DownloadService creates DownloadHistory some time after DownloadManager is
   // created and destroys DownloadHistory as DownloadManager is shutting down.
   DownloadHistory(
       content::DownloadManager* manager,
       scoped_ptr<HistoryAdapter> history);
 
   virtual ~DownloadHistory();
 
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
 
+  // Returns true if the download was restored from history. Safe to call from
+  // within a DownloadManager::Observer::OnDownloadCreated handler and can be
+  // used to distinguish between downloads that were created due to new requests
+  // vs. downloads that were created due to being restored from history. Note
+  // that the return value is only reliable for downloads that were restored by
+  // this specific DownloadHistory instance.

[Randy Smith (Not in Mondays), 2014/04/10 18:03:23]

Hmmm.  Not relevant for this CL, but I'm inclined to think that we should decide
whether we support multiple DownloadHistory instances (and not unless there's a
use case) and enforce that in the code.  It feels like the kind of thing that
could bit rot very easily (if we allow multiple instances but don't test them).

[asanka, 2014/04/17 21:16:10]

Yeah. Currently there's nothing that prevents multiple instances of
DownloadHistory from being associated with a single DownloadManager. There's
only one in practice.

+  bool WasRestoredFromHistory(const content::DownloadItem* item) const;
+
  private:
   typedef std::set<content::DownloadItem*> ItemSet;
 
   // Callback from |history_| containing all entries in the downloads database
   // table.
   void QueryCallback(
       scoped_ptr<std::vector<history::DownloadRow> > infos);
 
   // May add |item| to |history_|.
   void MaybeAddToHistory(content::DownloadItem* item);
@@ skipping 43 matching lines @@
   int64 history_size_;
 
   ObserverList<Observer> observers_;
 
   base::WeakPtrFactory<DownloadHistory> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(DownloadHistory);
 };
 
 #endif  // CHROME_BROWSER_DOWNLOAD_DOWNLOAD_HISTORY_H_