sql::MetaTable.next_download_id, DownloadManager::GetNextId()

chrome/browser/download/download_manager.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.
 //
 // The DownloadManager object manages the process of downloading, including
 // updates to the history system and providing the information for displaying
 // the downloads view in the Destinations tab. There is one DownloadManager per
 // active profile in Chrome.
 //
 // Download observers:
@@ skipping 16 matching lines @@
 #ifndef CHROME_BROWSER_DOWNLOAD_DOWNLOAD_MANAGER_H_
 #define CHROME_BROWSER_DOWNLOAD_DOWNLOAD_MANAGER_H_
 #pragma once
 
 #include <map>
 #include <set>
 #include <string>
 #include <vector>
 
 #include "base/basictypes.h"
+#include "base/callback.h"
 #include "base/file_path.h"
 #include "base/gtest_prod_util.h"
 #include "base/hash_tables.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
 #include "base/time.h"
 #include "chrome/browser/download/download_item.h"
 #include "chrome/browser/download/download_request_handle.h"
+#include "chrome/browser/download/download_state_info.h"
 #include "chrome/browser/download/download_status_updater_delegate.h"
 #include "chrome/browser/ui/shell_dialogs.h"
 #include "content/browser/browser_thread.h"
 
 class DownloadFileManager;
 class DownloadHistory;
 class DownloadPrefs;
 class DownloadStatusUpdater;
 class GURL;
 class Profile;
 class ResourceDispatcherHost;
 class TabContents;
 struct DownloadCreateInfo;
 struct DownloadHistoryInfo;
 struct DownloadSaveInfo;
+namespace base {
+class Lock;
+}  // namespace base
 
 // Browser's download manager: manages all downloads and destination view.
 class DownloadManager
     : public base::RefCountedThreadSafe<DownloadManager,
                                         BrowserThread::DeleteOnUIThread>,
       public DownloadStatusUpdaterDelegate,
       public SelectFileDialog::Listener {
  public:
   explicit DownloadManager(DownloadStatusUpdater* status_updater);
 
@@ skipping 33 matching lines @@
   // Return all non-temporary downloads in the specified directory that are
   // in-progress (including dangerous downloads waiting for user confirmation).
   void GetCurrentDownloads(const FilePath& dir_path,
                            std::vector<DownloadItem*>* result);
 
   // Returns all non-temporary downloads matching |query|. Empty query matches
   // everything.
   void SearchDownloads(const string16& query,
                        std::vector<DownloadItem*>* result);
 
+  // Returns the next download id in a DownloadId and increments the counter.
+  // May be called on any thread. The incremented counter is not persisted until
+  // your new DownloadItem is stored in the history.

[Randy Smith (Not in Mondays), 2011/07/25 20:20:12]

nit: It's not really the incremented counter that's being persisted.  I'd be
tempted to phrase this as "The incremented counter is not persisted, but the
base counter for this accessor is initialized from the largest id actually saved
to the download history database."  But your call.

[benjhayden, 2011/07/27 19:40:54]

Done.

+  DownloadId GetNextId();
+
+  // Instead of passing a DownloadManager* between threads and hoping users only
+  // call GetNextId(), you can pass this thunk around instead.  Use it like "id
+  // = next_id_thunk.Run();" Pass the thunk around by const ref and store it by

[Randy Smith (Not in Mondays), 2011/07/25 20:20:12]

nit: I may be targeting a less-smart class of developers than we actually have,
but could you make this a bit clearer?  Specifically, I'd suggest saying:

---
Use it like:
    const GetNextIdThunkType& next_id_thunk =
download_manager->GetNextIdThunk();
    // next_id_thunk may be safely copied, including between threads.
    id = next_id_thunk.Run();
----

[Randy Smith (Not in Mondays), 2011/07/25 20:20:12]

I'm a bit worried by this.  My understanding was that when passed between
threads, refs were silently copied (to handle lifetime management
issues--whatever created the object the ref was to might destroy it on the
original thread without warning).  But that obviously won't work if you're
holding on to a reference to a base class, which is what this sounds like. 
Enlighten me as to why this is ok?

[benjhayden, 2011/07/27 19:40:54]

Done.

[benjhayden, 2011/07/27 19:40:54]

I'm not sure exactly what you're asking.
If you're asking about destroying DownloadManager, it's ref-counted, so you
don't destroy DM, you just decref it and trust that it will be destroyed when
everybody has decref'd it.
If you're asking about destroying the thunk, that's why it should be stored by
copy instead of by ref. A copy of this thunk is stored in ProfileIOData, which
is created on the UI thread and used on the IO thread. Another copy is stored in
DownloadResourceHandler.
Were you asking about something else?

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

I think I wasn't understanding how base::Callback and base::Bind were
implemented.  I was worried that when you created an object of
base::Calback<DownloadId(void)> you were actually creating an object of a
(hidden) superclass of that type.  If that was the case, passing the object
around by copy would "sheer off" everything except for the base class contents. 
I no longer think that, so I'm no longer worried about this issue :-}.

[benjhayden, 2011/08/03 17:44:46]

Done.

+  // copy per the base::Callback interface.  If you change GetNextIdThunkType
+  // from base::Callback, then you should think about how you're changing the
+  // ref-count of DownloadManager.
+  typedef base::Callback<DownloadId(void)> GetNextIdThunkType;
+  GetNextIdThunkType GetNextIdThunk();

[Randy Smith (Not in Mondays), 2011/07/25 20:20:12]

If you want to pass it around by const ref, is there a reason it doesn't return
a const ref?  

[benjhayden, 2011/07/27 19:40:54]

Error: returning reference to temporary.

+
   // Returns true if initialized properly.
   bool Init(Profile* profile);
 
   // Notifications sent from the download thread to the UI thread
   void StartDownload(int32 id);
   void UpdateDownload(int32 download_id, int64 size);
   // |hash| is sha256 hash for the downloaded file. It is empty when the hash
   // is not available.
   void OnResponseCompleted(int32 download_id, int64 size, int os_error,
                            const std::string& hash);
@@ skipping 218 matching lines @@
   // yet in the history map.
   DownloadItem* GetActiveDownloadItem(int id);
 
   // Debugging routine to confirm relationship between below
   // containers; no-op if NDEBUG.
   void AssertContainersConsistent() const;
 
   // Add a DownloadItem to history_downloads_.
   void AddDownloadItemToHistory(DownloadItem* item, int64 db_handle);
 
+  // The DownloadHistory grabbed the next_id counter from the sql MetaTable.
+  void OnHistoryGetNextId(int next_id);
+
   // |downloads_| is the owning set for all downloads known to the
   // DownloadManager.  This includes downloads started by the user in
   // this session, downloads initialized from the history system, and
   // "save page as" downloads.  All other DownloadItem containers in
   // the DownloadManager are maps; they do not own the DownloadItems.
   // Note that this is the only place (with any functional implications;
   // see save_page_as_downloads_ below) that "save page as" downloads are
   // kept, as the DownloadManager's only job is to hold onto those
   // until destruction.
   //
@@ skipping 33 matching lines @@
 #if !defined(NDEBUG)
   DownloadSet save_page_as_downloads_;
 #endif
 
   // True if the download manager has been initialized and requires a shutdown.
   bool shutdown_needed_;
 
   // Observers that want to be notified of changes to the set of downloads.
   ObserverList<Observer> observers_;
 
+  base::Lock next_id_lock_;
+  int next_id_;
+
   // The current active profile.
   Profile* profile_;
 
   scoped_ptr<DownloadHistory> download_history_;
 
   scoped_ptr<DownloadPrefs> download_prefs_;
 
   // Non-owning pointer for handling file writing on the download_thread_.
   DownloadFileManager* file_manager_;
 
   // Non-owning pointer for updating the download status.
   base::WeakPtr<DownloadStatusUpdater> status_updater_;
 
   // The user's last choice for download directory. This is only used when the
   // user wants us to prompt for a save location for each download.
   FilePath last_download_path_;
 
   // The "Save As" dialog box used to ask the user where a file should be
   // saved.
   scoped_refptr<SelectFileDialog> select_file_dialog_;
 
   scoped_ptr<OtherDownloadManagerObserver> other_download_manager_observer_;
 
   DISALLOW_COPY_AND_ASSIGN(DownloadManager);
 };
 
 #endif  // CHROME_BROWSER_DOWNLOAD_DOWNLOAD_MANAGER_H_