Persist download interrupt reason, both target and current paths, and url_chain.

chrome/browser/download/download_browsertest.cc

Index: chrome/browser/download/download_browsertest.cc
diff --git a/chrome/browser/download/download_browsertest.cc b/chrome/browser/download/download_browsertest.cc
index 0e906c218c1d4ea50c4296f332b034ad2c78e8a2..646090c5128e71ab220434aff9a322053fea3eed 100644
--- a/chrome/browser/download/download_browsertest.cc
+++ b/chrome/browser/download/download_browsertest.cc
@@ -93,6 +93,43 @@ const FilePath kGoodCrxPath(FILE_PATH_LITERAL("extensions/good.crx"));
 const char kLargeThemeCrxId[] = "pjpgmfcmabopnnfonnhmdjglfpjjfkbf";
 const FilePath kLargeThemePath(FILE_PATH_LITERAL("extensions/theme2.crx"));
 
+// Get History Information.
+class DownloadsHistoryDataCollector {
+ public:
+  explicit DownloadsHistoryDataCollector(Profile* profile)
+      : profile_(profile), result_valid_(false) {}
+
+  bool WaitForDownloadInfo(std::vector<history::DownloadRow>* results) {
+    HistoryService* hs = HistoryServiceFactory::GetForProfile(
+        profile_, Profile::EXPLICIT_ACCESS);
+    DCHECK(hs);
+    hs->QueryDownloads(
+        base::Bind(&DownloadsHistoryDataCollector::OnQueryDownloadsComplete,
+                   base::Unretained(this)));
+
+    content::RunMessageLoop();
+    if (result_valid_) {
+      *results = results_;

[benjhayden, 2012/12/11 19:48:13]

Any reason not to use scoped_ptr<>.Pass() instead of copying the vector twice?

[Randy Smith (Not in Mondays), 2012/12/12 19:19:17]

Done.

+    }
+    return result_valid_;
+  }
+
+ private:
+  void OnQueryDownloadsComplete(
+      scoped_ptr<std::vector<history::DownloadRow> > entries) {
+    result_valid_ = true;
+    results_ = *entries.get();
+    MessageLoopForUI::current()->Quit();
+  }
+
+  Profile* profile_;
+  std::vector<history::DownloadRow> results_;
+  bool result_valid_;
+  CancelableRequestConsumer callback_consumer_;
+
+  DISALLOW_COPY_AND_ASSIGN(DownloadsHistoryDataCollector);
+};
+
 // Mock that simulates a permissions dialog where the user denies
 // permission to install.  TODO(skerner): This could be shared with
 // extensions tests.  Find a common place for this class.
@@ -164,6 +201,11 @@ void SetHiddenDownloadCallback(DownloadItem* item, net::Error error) {
   download_util::SetShouldShowInShelf(item, false);
 }
 
+// Callback for HistoryObserver; used in DownloadHistoryCheck
+bool HasDataAndName(const history::DownloadRow& row) {
+  return row.received_bytes > 0 && !row.target_path.empty();
+}
+
 }  // namespace
 
 // While an object of this class exists, it will mock out download
@@ -192,6 +234,8 @@ class MockDownloadOpeningObserver : public DownloadManager::Observer {
 
 class HistoryObserver : public DownloadHistory::Observer {
  public:
+  typedef base::Callback<bool(const history::DownloadRow&)> FilterCallback;
+
   explicit HistoryObserver(Profile* profile)
       : profile_(profile),
         waiting_(false),
@@ -206,9 +250,16 @@ class HistoryObserver : public DownloadHistory::Observer {
       service->GetDownloadHistory()->RemoveObserver(this);
   }
 
+  void SetFilterCallback(const FilterCallback& callback) {
+    callback_ = callback;
+  }
+
   virtual void OnDownloadStored(
       content::DownloadItem* item,
       const history::DownloadRow& info) OVERRIDE {
+    if (!callback_.is_null() && (!callback_.Run(info)))
+        return;
+
     seen_stored_ = true;
     if (waiting_)
       MessageLoopForUI::current()->Quit();
@@ -231,6 +282,7 @@ class HistoryObserver : public DownloadHistory::Observer {
   Profile* profile_;
   bool waiting_;
   bool seen_stored_;
+  FilterCallback callback_;
 
   DISALLOW_COPY_AND_ASSIGN(HistoryObserver);
 };
@@ -1403,11 +1455,82 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, NewWindow) {
 }
 
 IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadHistoryCheck) {
-  FilePath file(FILE_PATH_LITERAL("download-test1.lib"));
-  GURL download_url(URLRequestMockHTTPJob::GetMockUrl(file));
+  GURL download_url(URLRequestSlowDownloadJob::kKnownSizeUrl);
+  std::string url_path(download_url.path());
+  FilePath file(
+      FILE_PATH_LITERAL(url_path.substr(url_path.find_last_of('/') + 1)));
+
+  // We use the server so that we can get a redirect and test url_chain
+  // persistence.
+  ASSERT_TRUE(test_server()->Start());
+  GURL redirect_url = test_server()->GetURL(
+      "server-redirect?" + download_url.spec());
+
+  // Download the url and wait until the object has been stored.
+  base::Time start(base::Time::Now());
   HistoryObserver observer(browser()->profile());
-  DownloadAndWait(browser(), download_url);
+  observer.SetFilterCallback(base::Bind(&HasDataAndName));
+  ui_test_utils::NavigateToURL(browser(), redirect_url);
   observer.WaitForStored();
+
+  // Get the details on what was stored into the history.
+  std::vector<history::DownloadRow> downloads_in_database;
+  ASSERT_TRUE(DownloadsHistoryDataCollector(
+      browser()->profile()).WaitForDownloadInfo(&downloads_in_database));
+  ASSERT_EQ(1u, downloads_in_database.size());
+
+  // Confirm history storage is what you expect for a partially completed
+  // slow download job.
+  history::DownloadRow& row(downloads_in_database[0]);
+  EXPECT_EQ(DestinationFile(browser(), file), row.target_path);
+  EXPECT_EQ(download_util::GetCrDownloadPath(DestinationFile(browser(), file)),
+            row.current_path);
+  ASSERT_EQ(2u, row.url_chain.size());
+  EXPECT_EQ(redirect_url.spec(), row.url_chain[0].spec());
+  EXPECT_EQ(download_url.spec(), row.url_chain[1].spec());
+  EXPECT_LE(start, row.start_time);
+  EXPECT_EQ(URLRequestSlowDownloadJob::kFirstDownloadSize, row.received_bytes);
+  EXPECT_EQ(URLRequestSlowDownloadJob::kFirstDownloadSize
+            + URLRequestSlowDownloadJob::kSecondDownloadSize, row.total_bytes);
+  EXPECT_EQ(content::DownloadItem::IN_PROGRESS, row.state);
+  EXPECT_FALSE(row.opened);
+
+  // Finish the download.  We're ok relying on the history to be flushed
+  // at this point as our queries will be behind the history updates
+  // invoked by completion.
+  scoped_ptr<content::DownloadTestObserver> download_observer(
+      CreateWaiter(browser(), 1));
+  ui_test_utils::NavigateToURL(browser(),
+      GURL(URLRequestSlowDownloadJob::kErrorDownloadUrl));
+  download_observer->WaitForFinished();
+  EXPECT_EQ(1u, download_observer->NumDownloadsSeenInState(
+      DownloadItem::INTERRUPTED));
+  base::Time end(base::Time::Now());
+
+  // Get what was stored in the history.
+  ASSERT_TRUE(DownloadsHistoryDataCollector(
+      browser()->profile()).WaitForDownloadInfo(&downloads_in_database));
+  ASSERT_EQ(1u, downloads_in_database.size());
+
+  // Confirm history storage is what you expect for a completed
+  // slow download job.
+  history::DownloadRow& row1(downloads_in_database[0]);
+  EXPECT_EQ(DestinationFile(browser(), file), row1.target_path);
+  EXPECT_EQ(download_util::GetCrDownloadPath(DestinationFile(browser(), file)),
+            row.current_path);
+  ASSERT_EQ(2u, row1.url_chain.size());
+  EXPECT_EQ(redirect_url.spec(), row1.url_chain[0].spec());
+  EXPECT_EQ(download_url.spec(), row1.url_chain[1].spec());
+  EXPECT_LE(start, row1.start_time);
+  EXPECT_GE(end, row1.end_time);
+  EXPECT_EQ(URLRequestSlowDownloadJob::kFirstDownloadSize,
+            row1.received_bytes);
+  EXPECT_EQ(URLRequestSlowDownloadJob::kFirstDownloadSize
+            + URLRequestSlowDownloadJob::kSecondDownloadSize, row1.total_bytes);
+  EXPECT_EQ(content::DownloadItem::INTERRUPTED, row1.state);
+  EXPECT_EQ(content::DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED,
+            row.interrupt_reason);
+  EXPECT_FALSE(row1.opened);
 }
 
 // Test for crbug.com/14505. This tests that chrome:// urls are still functional