Fix an issue that temp files are left permanently on storage after chrome crash

chrome/browser/download/download_history_unittest.cc

 // 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.
 
 #include "chrome/browser/download/download_history.h"
 
 #include <stddef.h>
 #include <stdint.h>
 #include <utility>
 #include <vector>
@@ skipping 31 matching lines @@
 
 typedef DownloadHistory::IdSet IdSet;
 typedef std::vector<history::DownloadRow> InfoVector;
 typedef testing::StrictMock<content::MockDownloadItem> StrictMockDownloadItem;
 
 class FakeHistoryAdapter : public DownloadHistory::HistoryAdapter {
  public:
   FakeHistoryAdapter()
     : DownloadHistory::HistoryAdapter(NULL),
       slow_create_download_(false),
-      fail_create_download_(false) {
+      fail_create_download_(false),
+      should_commit_immediately_(false) {
   }
 
   ~FakeHistoryAdapter() override {}
 
   void QueryDownloads(
       const history::HistoryService::DownloadQueryCallback& callback) override {
     DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
     content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE,
         base::Bind(&FakeHistoryAdapter::QueryDownloadsDone,
             base::Unretained(this), callback));
@@ skipping 20 matching lines @@
     if (!slow_create_download_)
       FinishCreateDownload();
   }
 
   void FinishCreateDownload() {
     DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
     create_download_callback_.Run();
     create_download_callback_.Reset();
   }
 
-  void UpdateDownload(const history::DownloadRow& info) override {
+  void UpdateDownload(const history::DownloadRow& info,
+                      bool should_commit_immediately) override {
     DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
     update_download_ = info;
+    should_commit_immediately_ = should_commit_immediately;
   }
 
   void RemoveDownloads(const IdSet& ids) override {
     DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
     for (IdSet::const_iterator it = ids.begin();
          it != ids.end(); ++it) {
       remove_downloads_.insert(*it);
     }
   }
 
@@ skipping 19 matching lines @@
     EXPECT_EQ(info, create_download_info_);
     create_download_info_ = history::DownloadRow();
   }
 
   void ExpectNoDownloadCreated() {
     DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
     content::RunAllPendingInMessageLoop(content::BrowserThread::UI);
     EXPECT_EQ(history::DownloadRow(), create_download_info_);
   }
 
-  void ExpectDownloadUpdated(const history::DownloadRow& info) {
+  void ExpectDownloadUpdated(const history::DownloadRow& info,
+                             bool should_commit_immediately) {
     DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
     content::RunAllPendingInMessageLoop(content::BrowserThread::UI);
     EXPECT_EQ(update_download_, info);
+    EXPECT_EQ(should_commit_immediately_, should_commit_immediately);
     update_download_ = history::DownloadRow();
+    should_commit_immediately_ = false;
   }
 
   void ExpectNoDownloadUpdated() {
     DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
     content::RunAllPendingInMessageLoop(content::BrowserThread::UI);
     EXPECT_EQ(history::DownloadRow(), update_download_);
   }
 
   void ExpectNoDownloadsRemoved() {
     DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
     content::RunAllPendingInMessageLoop(content::BrowserThread::UI);
     EXPECT_EQ(0, static_cast<int>(remove_downloads_.size()));
   }
 
   void ExpectDownloadsRemoved(const IdSet& ids) {
     DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
     content::RunAllPendingInMessageLoop(content::BrowserThread::UI);
     IdSet differences = base::STLSetDifference<IdSet>(ids, remove_downloads_);
     for (IdSet::const_iterator different = differences.begin();
          different != differences.end(); ++different) {
       EXPECT_TRUE(false) << *different;
     }
     remove_downloads_.clear();
   }
 
  private:
   bool slow_create_download_;
   bool fail_create_download_;
+  bool should_commit_immediately_;
   base::Closure create_download_callback_;
   history::DownloadRow update_download_;
   std::unique_ptr<InfoVector> expect_query_downloads_;
   IdSet remove_downloads_;
   history::DownloadRow create_download_info_;
 
   DISALLOW_COPY_AND_ASSIGN(FakeHistoryAdapter);
 };
 
 class TestDownloadHistoryObserver : public DownloadHistory::Observer {
@@ skipping 103 matching lines @@
       const history::DownloadRow& info) {
     DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
     history_->ExpectDownloadCreated(info);
   }
 
   void ExpectNoDownloadCreated() {
     DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
     history_->ExpectNoDownloadCreated();
   }
 
-  void ExpectDownloadUpdated(const history::DownloadRow& info) {
+  void ExpectDownloadUpdated(const history::DownloadRow& info,
+                             bool should_commit_immediately) {
     DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-    history_->ExpectDownloadUpdated(info);
+    history_->ExpectDownloadUpdated(info, should_commit_immediately);
   }
 
   void ExpectNoDownloadUpdated() {
     DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
     history_->ExpectNoDownloadUpdated();
   }
 
   void ExpectNoDownloadsRemoved() {
     DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
     history_->ExpectNoDownloadsRemoved();
@@ skipping 12 matching lines @@
                    expected_value);
     post_on_create_handler_ =
         base::Bind(&DownloadHistoryTest::CheckDownloadWasRestoredFromHistory,
                    base::Unretained(this),
                    expected_value);
   }
 
   void InitBasicItem(const base::FilePath::CharType* path,
                      const char* url_string,
                      const char* referrer_string,
+                     content::DownloadItem::DownloadState state,
                      history::DownloadRow* info) {
     GURL url(url_string);
     GURL referrer(referrer_string);
     std::vector<GURL> url_chain;
     url_chain.push_back(url);
     InitItem(base::GenerateGUID(), static_cast<uint32_t>(items_.size() + 1),
              base::FilePath(path), base::FilePath(path), url_chain, referrer,
              GURL("http://example.com"), GURL("http://example.com/tab-url"),
              GURL("http://example.com/tab-referrer-url"),
              "application/octet-stream", "application/octet-stream",
              (base::Time::Now() - base::TimeDelta::FromMinutes(10)),
              (base::Time::Now() - base::TimeDelta::FromMinutes(1)), "Etag",
-             "abc", 100, 100, content::DownloadItem::COMPLETE,
+             "abc", 100, 100, state,
              content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
              content::DOWNLOAD_INTERRUPT_REASON_NONE, false, std::string(),
              std::string(), info);
   }
 
   void InitItem(const std::string& guid,
                 uint32_t id,
                 const base::FilePath& current_path,
                 const base::FilePath& target_path,
                 const std::vector<GURL>& url_chain,
@@ skipping 125 matching lines @@
 
 // Test loading an item from the database, changing it, saving it back, removing
 // it.
 TEST_F(DownloadHistoryTest, DownloadHistoryTest_Load) {
   // Load a download from history, create the item, OnDownloadCreated,
   // OnDownloadUpdated, OnDownloadRemoved.
   history::DownloadRow info;
   InitBasicItem(FILE_PATH_LITERAL("/foo/bar.pdf"),
                 "http://example.com/bar.pdf",
                 "http://example.com/referrer.html",
+                content::DownloadItem::COMPLETE,
                 &info);
   {
     std::unique_ptr<InfoVector> infos(new InfoVector());
     infos->push_back(info);
     CreateDownloadHistory(std::move(infos));
     ExpectNoDownloadCreated();
   }
   EXPECT_TRUE(DownloadHistory::IsPersisted(&item(0)));
 
   // Pretend that something changed on the item.
   EXPECT_CALL(item(0), GetOpened()).WillRepeatedly(Return(true));
   item(0).NotifyObserversDownloadUpdated();
   info.opened = true;
-  ExpectDownloadUpdated(info);
+  ExpectDownloadUpdated(info, false);
 
   // Pretend that the user removed the item.
   IdSet ids;
   ids.insert(info.id);
   item(0).NotifyObserversDownloadRemoved();
   ExpectDownloadsRemoved(ids);
 }
 
 // Test that the OnHistoryQueryComplete() observer method is invoked for an
 // observer that was added before the initial history query completing.
@@ skipping 54 matching lines @@
   ExpectDownloadsRestoredFromHistory(true);
 
   // Construct a DownloadHistory with a single history download. This results in
   // DownloadManager::CreateDownload() being called for the restored download.
   // The above test expectation should verify that the value of
   // WasRestoredFromHistory is correct for this download.
   history::DownloadRow info;
   InitBasicItem(FILE_PATH_LITERAL("/foo/bar.pdf"),
                 "http://example.com/bar.pdf",
                 "http://example.com/referrer.html",
+                content::DownloadItem::COMPLETE,
                 &info);
   std::unique_ptr<InfoVector> infos(new InfoVector());
   infos->push_back(info);
   CreateDownloadHistory(std::move(infos));
 
   EXPECT_TRUE(DownloadHistory::IsPersisted(&item(0)));
 }
 
 // Test that WasRestoredFromHistory accurately identifies downloads that were
 // not created from history.
 TEST_F(DownloadHistoryTest, DownloadHistoryTest_WasRestoredFromHistory_False) {
   // This sets DownloadHistoryTest to call DH::WasRestoredFromHistory() both
   // before and after DH::OnDownloadCreated() is called. At each call, the
   // expected return value is |true| since the download was restored from
   // history.
   ExpectDownloadsRestoredFromHistory(false);
 
   // Create a DownloadHistory with no history downloads. No
   // DownloadManager::CreateDownload() calls are expected.
   CreateDownloadHistory(std::unique_ptr<InfoVector>(new InfoVector()));
 
   // Notify DownloadHistory that a new download was created. The above test
   // expecation should verify that WasRestoredFromHistory is correct for this
   // download.
   history::DownloadRow info;
   InitBasicItem(FILE_PATH_LITERAL("/foo/bar.pdf"),
                 "http://example.com/bar.pdf",
                 "http://example.com/referrer.html",
+                content::DownloadItem::COMPLETE,
                 &info);
   CallOnDownloadCreated(0);
   ExpectDownloadCreated(info);
 }
 
 // Test creating an item, saving it to the database, changing it, saving it
 // back, removing it.
 TEST_F(DownloadHistoryTest, DownloadHistoryTest_Create) {
   // Create a fresh item not from history, OnDownloadCreated, OnDownloadUpdated,
   // OnDownloadRemoved.
   CreateDownloadHistory(std::unique_ptr<InfoVector>(new InfoVector()));
 
   history::DownloadRow info;
   InitBasicItem(FILE_PATH_LITERAL("/foo/bar.pdf"),
                 "http://example.com/bar.pdf",
                 "http://example.com/referrer.html",
+                content::DownloadItem::COMPLETE,
                 &info);
 
   // Pretend the manager just created |item|.
   CallOnDownloadCreated(0);
   ExpectDownloadCreated(info);
   EXPECT_TRUE(DownloadHistory::IsPersisted(&item(0)));
 
   // Pretend that something changed on the item.
   EXPECT_CALL(item(0), GetOpened()).WillRepeatedly(Return(true));
   item(0).NotifyObserversDownloadUpdated();
   info.opened = true;
-  ExpectDownloadUpdated(info);
+  ExpectDownloadUpdated(info, false);
 
   // Pretend that the user removed the item.
   IdSet ids;
   ids.insert(info.id);
   item(0).NotifyObserversDownloadRemoved();
   ExpectDownloadsRemoved(ids);
 }
 
 // Test that changes to persisted fields in a DownloadItem triggers database
 // updates.
 TEST_F(DownloadHistoryTest, DownloadHistoryTest_Update) {
   CreateDownloadHistory(std::unique_ptr<InfoVector>(new InfoVector()));
 
   history::DownloadRow info;
   InitBasicItem(FILE_PATH_LITERAL("/foo/bar.pdf"),
                 "http://example.com/bar.pdf",
                 "http://example.com/referrer.html",
+                content::DownloadItem::IN_PROGRESS,
                 &info);
+
   CallOnDownloadCreated(0);
   ExpectDownloadCreated(info);
   EXPECT_TRUE(DownloadHistory::IsPersisted(&item(0)));
 
   base::FilePath new_path(FILE_PATH_LITERAL("/foo/baz.txt"));
   base::Time new_time(base::Time::Now());
   std::string new_etag("new etag");
   std::string new_last_modifed("new last modified");
 
   // current_path
   EXPECT_CALL(item(0), GetFullPath()).WillRepeatedly(ReturnRefOfCopy(new_path));
   info.current_path = new_path;
   item(0).NotifyObserversDownloadUpdated();
-  ExpectDownloadUpdated(info);
+  ExpectDownloadUpdated(info, true);

[asanka, 2016/11/21 20:17:57]

There's currently no test for the case where ShouldUpdateDownload is called on a
download with no stored previous state.

I won't block this CL on adding that test in this CL, but we should add one.

[qinmin, 2016/11/21 23:14:54]

Actually, we set the state INTERRUPTED in this test. That will cause the stored
state to get cleared. I added some new code to change the state back to
IN_PROGRESS, and that will hit the no stored state case, and also restore the
state so that the code after that can do the actual comparison.

 
   // target_path
   EXPECT_CALL(item(0), GetTargetFilePath())
       .WillRepeatedly(ReturnRefOfCopy(new_path));
   info.target_path = new_path;
   item(0).NotifyObserversDownloadUpdated();
-  ExpectDownloadUpdated(info);
+  ExpectDownloadUpdated(info, false);
 
   // end_time
   EXPECT_CALL(item(0), GetEndTime()).WillRepeatedly(Return(new_time));
   info.end_time = new_time;
   item(0).NotifyObserversDownloadUpdated();
-  ExpectDownloadUpdated(info);
+  ExpectDownloadUpdated(info, false);
 
   // received_bytes
   EXPECT_CALL(item(0), GetReceivedBytes()).WillRepeatedly(Return(101));
   info.received_bytes = 101;
   item(0).NotifyObserversDownloadUpdated();
-  ExpectDownloadUpdated(info);
+  ExpectDownloadUpdated(info, false);
 
   // total_bytes
   EXPECT_CALL(item(0), GetTotalBytes()).WillRepeatedly(Return(102));
   info.total_bytes = 102;
   item(0).NotifyObserversDownloadUpdated();
-  ExpectDownloadUpdated(info);
+  ExpectDownloadUpdated(info, false);
 
   // etag
   EXPECT_CALL(item(0), GetETag()).WillRepeatedly(ReturnRefOfCopy(new_etag));
   info.etag = new_etag;
   item(0).NotifyObserversDownloadUpdated();
-  ExpectDownloadUpdated(info);
+  ExpectDownloadUpdated(info, false);
 
   // last_modified
   EXPECT_CALL(item(0), GetLastModifiedTime())
       .WillRepeatedly(ReturnRefOfCopy(new_last_modifed));
   info.last_modified = new_last_modifed;
   item(0).NotifyObserversDownloadUpdated();
-  ExpectDownloadUpdated(info);
+  ExpectDownloadUpdated(info, false);
 
   // state
   EXPECT_CALL(item(0), GetState())
       .WillRepeatedly(Return(content::DownloadItem::INTERRUPTED));
   info.state = history::DownloadState::INTERRUPTED;
   item(0).NotifyObserversDownloadUpdated();
-  ExpectDownloadUpdated(info);
+  ExpectDownloadUpdated(info, false);
 
   // danger_type
   EXPECT_CALL(item(0), GetDangerType())
       .WillRepeatedly(Return(content::DOWNLOAD_DANGER_TYPE_DANGEROUS_CONTENT));
   info.danger_type = history::DownloadDangerType::DANGEROUS_CONTENT;
   item(0).NotifyObserversDownloadUpdated();
-  ExpectDownloadUpdated(info);
+  ExpectDownloadUpdated(info, false);
 
   // interrupt_reason
   EXPECT_CALL(item(0), GetLastReason())
       .WillRepeatedly(Return(content::DOWNLOAD_INTERRUPT_REASON_SERVER_FAILED));
   info.interrupt_reason = history::ToHistoryDownloadInterruptReason(
       content::DOWNLOAD_INTERRUPT_REASON_SERVER_FAILED);
   item(0).NotifyObserversDownloadUpdated();
-  ExpectDownloadUpdated(info);
+  ExpectDownloadUpdated(info, false);
 
   // opened
   EXPECT_CALL(item(0), GetOpened()).WillRepeatedly(Return(true));
   info.opened = true;
   item(0).NotifyObserversDownloadUpdated();
-  ExpectDownloadUpdated(info);
+  ExpectDownloadUpdated(info, false);
 }
 
 // Test creating a new item, saving it, removing it by setting it Temporary,
 // changing it without saving it back because it's Temporary, clearing
 // IsTemporary, saving it back, changing it, saving it back because it isn't
 // Temporary anymore.
 TEST_F(DownloadHistoryTest, DownloadHistoryTest_Temporary) {
   // Create a fresh item not from history, OnDownloadCreated, OnDownloadUpdated,
   // OnDownloadRemoved.
   CreateDownloadHistory(std::unique_ptr<InfoVector>(new InfoVector()));
 
   history::DownloadRow info;
   InitBasicItem(FILE_PATH_LITERAL("/foo/bar.pdf"),
                 "http://example.com/bar.pdf",
                 "http://example.com/referrer.html",
+                content::DownloadItem::COMPLETE,
                 &info);
 
   // Pretend the manager just created |item|.
   CallOnDownloadCreated(0);
   ExpectDownloadCreated(info);
   EXPECT_TRUE(DownloadHistory::IsPersisted(&item(0)));
 
   // Pretend the item was marked temporary. DownloadHistory should remove it
   // from history and start ignoring it.
   EXPECT_CALL(item(0), IsTemporary()).WillRepeatedly(Return(true));
@@ skipping 12 matching lines @@
   // DownloadHistory call CreateDownload.
   EXPECT_CALL(item(0), IsTemporary()).WillRepeatedly(Return(false));
   item(0).NotifyObserversDownloadUpdated();
   info.received_bytes = 4200;
   ExpectDownloadCreated(info);
   EXPECT_TRUE(DownloadHistory::IsPersisted(&item(0)));
 
   EXPECT_CALL(item(0), GetReceivedBytes()).WillRepeatedly(Return(100));
   item(0).NotifyObserversDownloadUpdated();
   info.received_bytes = 100;
-  ExpectDownloadUpdated(info);
+  ExpectDownloadUpdated(info, false);
 }
 
 // Test removing downloads while they're still being added.
 TEST_F(DownloadHistoryTest, DownloadHistoryTest_RemoveWhileAdding) {
   CreateDownloadHistory(std::unique_ptr<InfoVector>(new InfoVector()));
 
   history::DownloadRow info;
   InitBasicItem(FILE_PATH_LITERAL("/foo/bar.pdf"),
                 "http://example.com/bar.pdf",
                 "http://example.com/referrer.html",
+                content::DownloadItem::COMPLETE,
                 &info);
 
   // Instruct CreateDownload() to not callback to DownloadHistory immediately,
   // but to wait for FinishCreateDownload().
   set_slow_create_download(true);
 
   // Pretend the manager just created |item|.
   CallOnDownloadCreated(0);
   ExpectDownloadCreated(info);
   EXPECT_FALSE(DownloadHistory::IsPersisted(&item(0)));
@@ skipping 18 matching lines @@
 }
 
 // Test loading multiple items from the database and removing them all.
 TEST_F(DownloadHistoryTest, DownloadHistoryTest_Multiple) {
   // Load a download from history, create the item, OnDownloadCreated,
   // OnDownloadUpdated, OnDownloadRemoved.
   history::DownloadRow info0, info1;
   InitBasicItem(FILE_PATH_LITERAL("/foo/bar.pdf"),
                 "http://example.com/bar.pdf",
                 "http://example.com/referrer.html",
+                content::DownloadItem::COMPLETE,
                 &info0);
   InitBasicItem(FILE_PATH_LITERAL("/foo/qux.pdf"),
                 "http://example.com/qux.pdf",
                 "http://example.com/referrer1.html",
+                content::DownloadItem::COMPLETE,
                 &info1);
   {
     std::unique_ptr<InfoVector> infos(new InfoVector());
     infos->push_back(info0);
     infos->push_back(info1);
     CreateDownloadHistory(std::move(infos));
     ExpectNoDownloadCreated();
   }
 
   EXPECT_TRUE(DownloadHistory::IsPersisted(&item(0)));
@@ skipping 11 matching lines @@
 // Test what happens when HistoryService/CreateDownload::CreateDownload() fails.
 TEST_F(DownloadHistoryTest, DownloadHistoryTest_CreateFailed) {
   // Create a fresh item not from history, OnDownloadCreated, OnDownloadUpdated,
   // OnDownloadRemoved.
   CreateDownloadHistory(std::unique_ptr<InfoVector>(new InfoVector()));
 
   history::DownloadRow info;
   InitBasicItem(FILE_PATH_LITERAL("/foo/bar.pdf"),
                 "http://example.com/bar.pdf",
                 "http://example.com/referrer.html",
+                content::DownloadItem::COMPLETE,
                 &info);
 
   FailCreateDownload();
   // Pretend the manager just created |item|.
   CallOnDownloadCreated(0);
   ExpectDownloadCreated(info);
   EXPECT_FALSE(DownloadHistory::IsPersisted(&item(0)));
 
   EXPECT_CALL(item(0), GetReceivedBytes()).WillRepeatedly(Return(100));
   item(0).NotifyObserversDownloadUpdated();
   info.received_bytes = 100;
   ExpectDownloadCreated(info);
   EXPECT_TRUE(DownloadHistory::IsPersisted(&item(0)));
 }
 
 TEST_F(DownloadHistoryTest, DownloadHistoryTest_UpdateWhileAdding) {
   // Create a fresh item not from history, OnDownloadCreated, OnDownloadUpdated,
   // OnDownloadRemoved.
   CreateDownloadHistory(std::unique_ptr<InfoVector>(new InfoVector()));
 
   history::DownloadRow info;
   InitBasicItem(FILE_PATH_LITERAL("/foo/bar.pdf"),
                 "http://example.com/bar.pdf",
                 "http://example.com/referrer.html",
+                content::DownloadItem::COMPLETE,
                 &info);
 
   // Instruct CreateDownload() to not callback to DownloadHistory immediately,
   // but to wait for FinishCreateDownload().
   set_slow_create_download(true);
 
   // Pretend the manager just created |item|.
   CallOnDownloadCreated(0);
   ExpectDownloadCreated(info);
   EXPECT_FALSE(DownloadHistory::IsPersisted(&item(0)));
 
   // Pretend that something changed on the item.
   EXPECT_CALL(item(0), GetOpened()).WillRepeatedly(Return(true));
   item(0).NotifyObserversDownloadUpdated();
 
   FinishCreateDownload();
   EXPECT_TRUE(DownloadHistory::IsPersisted(&item(0)));
 
   // ItemAdded should call OnDownloadUpdated, which should detect that the item
   // changed while it was being added and call UpdateDownload immediately.
   info.opened = true;
-  ExpectDownloadUpdated(info);
+  ExpectDownloadUpdated(info, false);
 }
 
 }  // anonymous namespace