Don't accidentally add downloads with db_handle = kUninitializedHandle, which can happen if the sqlite history db is corrupt / offline.

BUG=25492
TEST=none
Review URL: http://codereview.chromium.org/492017

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@36736 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 32 insertions, 13 deletions

diff --git a/chrome/browser/download/download_manager.cc b/chrome/browser/download/download_manager.cc
index 6c798ba..c0b9e82 100644
--- a/chrome/browser/download/download_manager.cc
+++ b/chrome/browser/download/download_manager.cc
@@ -16,6 +16,7 @@
 #include "base/task.h"
 #include "base/thread.h"
 #include "base/timer.h"
+#include "build/build_config.h"
 #include "chrome/browser/browser_list.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_thread.h"
@@ -29,8 +30,8 @@
 #include "chrome/browser/renderer_host/render_process_host.h"
 #include "chrome/browser/renderer_host/render_view_host.h"
 #include "chrome/browser/renderer_host/resource_dispatcher_host.h"
-#include "chrome/browser/tab_contents/tab_util.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
+#include "chrome/browser/tab_contents/tab_util.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/extensions/extension.h"
@@ -53,10 +54,6 @@
 #include "base/win_util.h"
 #endif
 
-#if defined(OS_LINUX)
-#include <gtk/gtk.h>
-#endif
-
 // Periodically update our observers.
 class DownloadItemUpdateTask : public Task {
  public:
@@ -361,7 +358,8 @@ void DownloadManager::RegisterUserPrefs(PrefService* prefs) {
 DownloadManager::DownloadManager()
     : shutdown_needed_(false),
       profile_(NULL),
-      file_manager_(NULL) {
+      file_manager_(NULL),
+      fake_db_handle_(kUninitializedHandle - 1) {
 }
 
 DownloadManager::~DownloadManager() {
@@ -767,8 +765,7 @@ void DownloadManager::ContinueStartDownload(DownloadCreateInfo* info,
   // the neck first. YMMV.
   if (profile_->IsOffTheRecord() || download->is_extension_install() ||
       download->is_temporary()) {
-    static int64 fake_db_handle = kUninitializedHandle - 1;
-    OnCreateDownloadEntryComplete(*info, fake_db_handle--);
+    OnCreateDownloadEntryComplete(*info, fake_db_handle_.GetNext());
   } else {
     // Update the history system with the new download.
     // FIXME(paulg) see bug 958058. EXPLICIT_ACCESS below is wrong.
@@ -1586,6 +1583,15 @@ void DownloadManager::OnCreateDownloadEntryComplete(DownloadCreateInfo info,
   DCHECK(it != in_progress_.end());
 
   DownloadItem* download = it->second;
+
+  // It's not immediately obvious, but HistoryBackend::CreateDownload() can
+  // call this function with an invalid |db_handle|. For instance, this can
+  // happen when the history database is offline. We cannot have multiple
+  // DownloadItems with the same invalid db_handle, so we need to assign a
+  // unique |db_handle| here.
+  if (db_handle == kUninitializedHandle)
+    db_handle = fake_db_handle_.GetNext();
+
   DCHECK(download->db_handle() == kUninitializedHandle);
   download->set_db_handle(db_handle);
 
@@ -1655,4 +1661,3 @@ void DownloadManager::ShowDownloadInBrowser(const DownloadCreateInfo& info,
 void DownloadManager::ClearLastDownloadPath() {
   last_download_path_ = FilePath();
 }
-
diff --git a/chrome/browser/download/download_manager.h b/chrome/browser/download/download_manager.h
index 5a5bda2..b562bc2 100644
--- a/chrome/browser/download/download_manager.h
+++ b/chrome/browser/download/download_manager.h
@@ -35,11 +35,9 @@
 #ifndef CHROME_BROWSER_DOWNLOAD_DOWNLOAD_MANAGER_H_
 #define CHROME_BROWSER_DOWNLOAD_DOWNLOAD_MANAGER_H_
 
-#include "build/build_config.h"
-
-#include <string>
 #include <map>
 #include <set>
+#include <string>
 #include <vector>
 
 #include "base/basictypes.h"
@@ -381,7 +379,7 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>,
   void UpdateDownload(int32 download_id, int64 size);
   void DownloadFinished(int32 download_id, int64 size);
 
- // Called from a view when a user clicks a UI button or link.
+  // Called from a view when a user clicks a UI button or link.
   void DownloadCancelled(int32 download_id);
   void PauseDownload(int32 download_id, bool pause);
   void RemoveDownload(int64 download_handle);
@@ -510,6 +508,16 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>,
                                FilePath* generated_name);
 
  private:
+
+  class FakeDbHandleGenerator {
+   public:
+    explicit FakeDbHandleGenerator(int64 start_value) : value_(start_value) {}
+
+    int64 GetNext() { return value_--; }
+   private:
+    int64 value_;
+  };
+
   friend class base::RefCountedThreadSafe<DownloadManager>;
 
   ~DownloadManager();
@@ -671,6 +679,12 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>,
   // saved.
   scoped_refptr<SelectFileDialog> select_file_dialog_;
 
+  // In case we don't have a valid db_handle, we use |fake_db_handle_| instead.
+  // This is useful for incognito mode or when the history database is offline.
+  // Downloads are expected to have unique handles, so FakeDbHandleGenerator
+  // automatically decrement the handle value on every use.
+  FakeDbHandleGenerator fake_db_handle_;
+
   DISALLOW_COPY_AND_ASSIGN(DownloadManager);
 };