Add methods to allow DownloadEntry from InProgressCache to get migrated to DownloadDB

This CL adds methods to get all DownloadEntry from InProgressCache,
convert them, and adding them back to the DownloadDB.
This migration step allows us to move from InProgressCache to DownloadDB.

BUG=870502

Change-Id: Id56346886b1cea2df6fa25954c89cdfb4db167d4
Reviewed-on: https://chromium-review.googlesource.com/1161487
Commit-Queue: Min Qin <qinmin@chromium.org>
Reviewed-by: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580695}

components/download/database/download_db_conversions.cc

@@ -311,4 +311,22 @@ download_pb::DownloadDBEntry DownloadDBConversions::DownloadDBEntryToProto(
   return proto;
 }
 
+DownloadDBEntry DownloadDBConversions::DownloadDBEntryFromDownloadEntry(
+    const DownloadEntry& entry) {
+  DownloadDBEntry db_entry;
+  DownloadInfo download_info;
+  download_info.guid = entry.guid;
+
+  UkmInfo ukm_info(entry.download_source, entry.ukm_download_id);
+
+  InProgressInfo in_progress_info;
+  in_progress_info.fetch_error_body = entry.fetch_error_body;
+  in_progress_info.request_headers = entry.request_headers;
+
+  download_info.ukm_info = ukm_info;
+  download_info.in_progress_info = in_progress_info;
+  db_entry.download_info = download_info;
+  return db_entry;
+}
+
 }  // namespace download

components/download/database/download_db_conversions.h

@@ -64,6 +64,9 @@ class DownloadDBConversions {
 
   static DownloadDBEntry DownloadDBEntryFromProto(
       const download_pb::DownloadDBEntry& proto);
+
+  static DownloadDBEntry DownloadDBEntryFromDownloadEntry(
+      const DownloadEntry& entry);
 };
 
 }  // namespace download

components/download/database/in_progress/in_progress_cache.h

@@ -6,6 +6,7 @@
 #define COMPONENTS_DOWNLOAD_DATABASE_IN_PROGRESS_IN_PROGRESS_CACHE_H_
 
 #include <string>
+#include <vector>
 
 #include "base/optional.h"
 #include "components/download/database/in_progress/download_entry.h"
@@ -34,6 +35,9 @@ class InProgressCache {
 
   // Removes an entry.
   virtual void RemoveEntry(const std::string& guid) = 0;
+
+  // Returns all entries.
+  virtual std::vector<DownloadEntry> GetAllEntries() = 0;
 };
 
 }  // namespace download

components/download/database/in_progress/in_progress_cache_impl.cc

@@ -212,4 +212,12 @@ void InProgressCacheImpl::RemoveEntry(const std::string& guid) {
                                                    entries_string, file_path_));
 }
 
+std::vector<DownloadEntry> InProgressCacheImpl::GetAllEntries() {
+  if (initialization_status_ != CACHE_INITIALIZED) {
+    LOG(ERROR) << "Cache is not initialized, cannot get all entries.";
+    return std::vector<DownloadEntry>();
+  }
+  return DownloadDBConversions::DownloadEntriesFromProto(entries_);
+}
+
 }  // namespace download

components/download/database/in_progress/in_progress_cache_impl.h

@@ -33,6 +33,7 @@ class InProgressCacheImpl : public InProgressCache {
   void AddOrReplaceEntry(const DownloadEntry& entry) override;
   base::Optional<DownloadEntry> RetrieveEntry(const std::string& guid) override;
   void RemoveEntry(const std::string& guid) override;
+  std::vector<DownloadEntry> GetAllEntries() override;
 
  private:
   // States to keep track of initialization status.

components/download/internal/common/download_db_cache.cc

@@ -5,7 +5,9 @@
 #include "components/download/internal/common/download_db_cache.h"
 
 #include "components/download/database/download_db.h"
+#include "components/download/database/download_db_conversions.h"
 #include "components/download/database/download_db_entry.h"
+#include "components/download/database/in_progress/download_entry.h"
 #include "components/download/public/common/download_utils.h"
 
 namespace download {
@@ -85,18 +87,11 @@ DownloadUrlParameters::RequestHeadersType GetRequestHeadersType(
   return in_progress_info->request_headers;
 }
 
-DownloadSource GetDownloadSource(base::Optional<DownloadDBEntry> entry) {
-  if (!entry)
-    return DownloadSource::UNKNOWN;
-
-  if (!entry->download_info)
-    return DownloadSource::UNKNOWN;
-
-  base::Optional<UkmInfo>& ukm_info = entry->download_info->ukm_info;
-  if (!ukm_info)
-    return DownloadSource::UNKNOWN;
+UkmInfo GetUkmInfo(base::Optional<DownloadDBEntry> entry) {
+  if (entry && entry->download_info && entry->download_info->ukm_info)
+    return entry->download_info->ukm_info.value();
 
-  return ukm_info->download_source;
+  return UkmInfo(DownloadSource::UNKNOWN, GetUniqueDownloadId());
 }
 
 void CleanUpInProgressEntry(DownloadDBEntry& entry) {
@@ -201,9 +196,9 @@ void DownloadDBCache::OnDownloadUpdated(DownloadItem* download) {
   bool fetch_error_body = GetFetchErrorBody(current);
   DownloadUrlParameters::RequestHeadersType request_header_type =
       GetRequestHeadersType(current);
-  DownloadSource download_source = GetDownloadSource(current);
+  UkmInfo ukm_info = GetUkmInfo(current);
   DownloadDBEntry entry = CreateDownloadDBEntryFromItem(
-      *download, download_source, fetch_error_body, request_header_type);
+      *download, ukm_info, fetch_error_body, request_header_type);
   AddOrReplaceEntry(entry);
 }
 
@@ -240,4 +235,17 @@ void DownloadDBCache::SetTimerTaskRunnerForTesting(
   update_timer_.SetTaskRunner(task_runner);
 }
 
+void DownloadDBCache::MigrateFromInProgressCache(
+    const std::vector<DownloadEntry>& entries) {
+  DCHECK(initialized_);
+  DCHECK(entries_.empty());
+  std::vector<DownloadDBEntry> db_entries;
+  for (const auto& entry : entries) {
+    entries_[entry.guid] =
+        DownloadDBConversions::DownloadDBEntryFromDownloadEntry(entry);
+    db_entries.emplace_back(entries_[entry.guid]);
+  }
+  download_db_->AddOrReplaceEntries(db_entries);
+}
+
 }  //  namespace download

components/download/internal/common/download_db_cache.h

@@ -21,6 +21,7 @@ namespace download {
 
 class DownloadDB;
 struct DownloadDBEntry;
+struct DownloadEntry;
 
 // Responsible for caching the metadata of all in progress downloads.
 class COMPONENTS_DOWNLOAD_EXPORT DownloadDBCache
@@ -39,6 +40,9 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadDBCache
   // Remove an entry from the DownloadDB.
   void RemoveEntry(const std::string& guid);
 
+  // Migrate DownloadEntry from in-progress cache.
+  void MigrateFromInProgressCache(const std::vector<DownloadEntry>& entries);
+
  private:
   friend class DownloadDBCacheTest;
   friend class InProgressDownloadManager;

components/download/internal/common/download_db_cache_unittest.cc

@@ -14,11 +14,17 @@
 #include "components/download/database/download_db_conversions.h"
 #include "components/download/database/download_db_entry.h"
 #include "components/download/database/download_db_impl.h"
+#include "components/download/database/in_progress/download_entry.h"
+#include "components/download/public/common/download_item_impl.h"
+#include "components/download/public/common/download_utils.h"
+#include "components/download/public/common/mock_download_item.h"
 #include "components/leveldb_proto/testing/fake_db.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using testing::_;
+using testing::Return;
+using testing::ReturnRefOfCopy;
 
 namespace download {
 
@@ -38,6 +44,44 @@ std::string GetKey(const std::string& guid) {
          "," + guid;
 }
 
+std::unique_ptr<DownloadItem> CreateDownloadItem(const std::string& guid) {
+  std::unique_ptr<MockDownloadItem> item(
+      new ::testing::NiceMock<MockDownloadItem>());
+  ON_CALL(*item, GetGuid()).WillByDefault(ReturnRefOfCopy(guid));
+  ON_CALL(*item, GetUrlChain())
+      .WillByDefault(ReturnRefOfCopy(std::vector<GURL>()));
+  ON_CALL(*item, GetReferrerUrl()).WillByDefault(ReturnRefOfCopy(GURL()));
+  ON_CALL(*item, GetSiteUrl()).WillByDefault(ReturnRefOfCopy(GURL()));
+  ON_CALL(*item, GetTabUrl()).WillByDefault(ReturnRefOfCopy(GURL()));
+  ON_CALL(*item, GetTabReferrerUrl()).WillByDefault(ReturnRefOfCopy(GURL()));
+  ON_CALL(*item, GetETag()).WillByDefault(ReturnRefOfCopy(std::string("etag")));
+  ON_CALL(*item, GetLastModifiedTime())
+      .WillByDefault(ReturnRefOfCopy(std::string("last-modified")));
+  ON_CALL(*item, GetMimeType()).WillByDefault(Return("text/html"));
+  ON_CALL(*item, GetOriginalMimeType()).WillByDefault(Return("text/html"));
+  ON_CALL(*item, GetTotalBytes()).WillByDefault(Return(1000));
+  ON_CALL(*item, GetFullPath())
+      .WillByDefault(ReturnRefOfCopy(base::FilePath()));
+  ON_CALL(*item, GetTargetFilePath())
+      .WillByDefault(ReturnRefOfCopy(base::FilePath()));
+  ON_CALL(*item, GetReceivedBytes()).WillByDefault(Return(1000));
+  ON_CALL(*item, GetStartTime()).WillByDefault(Return(base::Time()));
+  ON_CALL(*item, GetEndTime()).WillByDefault(Return(base::Time()));
+  ON_CALL(*item, GetReceivedSlices())
+      .WillByDefault(
+          ReturnRefOfCopy(std::vector<DownloadItem::ReceivedSlice>()));
+  ON_CALL(*item, GetHash()).WillByDefault(ReturnRefOfCopy(std::string("hash")));
+  ON_CALL(*item, IsTransient()).WillByDefault(Return(false));
+  ON_CALL(*item, GetDangerType())
+      .WillByDefault(Return(DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS));
+  ON_CALL(*item, GetLastReason())
+      .WillByDefault(Return(DOWNLOAD_INTERRUPT_REASON_NONE));
+  ON_CALL(*item, GetState()).WillByDefault(Return(DownloadItem::IN_PROGRESS));
+  ON_CALL(*item, IsPaused()).WillByDefault(Return(false));
+  ON_CALL(*item, GetBytesWasted()).WillByDefault(Return(10));
+  return std::move(item);
+}
+
 }  // namespace
 
 class DownloadDBCacheTest : public testing::Test {
@@ -82,6 +126,10 @@ class DownloadDBCacheTest : public testing::Test {
 
   DownloadDB* GetDownloadDB() { return db_cache_->download_db_.get(); }
 
+  void OnDownloadUpdated(DownloadItem* item) {
+    db_cache_->OnDownloadUpdated(item);
+  }
+
  protected:
   std::map<std::string, download_pb::DownloadDBEntry> db_entries_;
   leveldb_proto::test::FakeDB<download_pb::DownloadDBEntry>* db_;
@@ -233,4 +281,52 @@ TEST_F(DownloadDBCacheTest, RemoveEntry) {
   ASSERT_EQ(guid2, loaded_entries[0].GetGuid());
 }
 
+// Tests that Migrating a DownloadEntry from InProgressCache should store
+// a DownloadDBEntry in the DownloadDB.
+TEST_F(DownloadDBCacheTest, MigrateFromInProgressCache) {
+  CreateDBCache();
+  std::vector<DownloadDBEntry> loaded_entries;
+  db_cache_->Initialize(base::BindOnce(&DownloadDBCacheTest::InitCallback,
+                                       base::Unretained(this), &loaded_entries,
+                                       true));
+  db_->InitCallback(true);
+  db_->LoadCallback(true);
+  ASSERT_TRUE(loaded_entries.empty());
+
+  std::vector<DownloadEntry> download_entries;
+  download_entries.emplace_back(
+      "guid1", "foo.com", DownloadSource::DRAG_AND_DROP, true,
+      DownloadUrlParameters::RequestHeadersType(), 100);
+  download_entries.emplace_back(
+      "guid2", "foobar.com", DownloadSource::UNKNOWN, false,
+      DownloadUrlParameters::RequestHeadersType(), 200);
+
+  db_cache_->MigrateFromInProgressCache(download_entries);
+  db_->UpdateCallback(true);
+
+  std::unique_ptr<DownloadItem> item = CreateDownloadItem("guid1");
+  OnDownloadUpdated(item.get());
+
+  ASSERT_EQ(task_runner_->GetPendingTaskCount(), 1u);
+  ASSERT_GT(task_runner_->NextPendingTaskDelay(), base::TimeDelta());
+  task_runner_->FastForwardUntilNoTasksRemain();
+  db_->UpdateCallback(true);
+
+  DownloadDBEntry entry1 = CreateDownloadDBEntryFromItem(
+      *item, UkmInfo(DownloadSource::DRAG_AND_DROP, 100), true,
+      DownloadUrlParameters::RequestHeadersType());
+  DownloadDBEntry entry2 =
+      DownloadDBConversions::DownloadDBEntryFromDownloadEntry(
+          download_entries[1]);
+
+  DownloadDB* download_db = GetDownloadDB();
+  download_db->LoadEntries(base::BindOnce(&DownloadDBCacheTest::InitCallback,
+                                          base::Unretained(this),
+                                          &loaded_entries));
+  db_->LoadCallback(true);
+  ASSERT_EQ(loaded_entries.size(), 2u);
+  ASSERT_TRUE(entry1 == loaded_entries[0]);
+  ASSERT_TRUE(entry2 == loaded_entries[1]);
+}
+
 }  // namespace download

components/download/internal/common/download_utils.cc

@@ -361,7 +361,7 @@ DownloadEntry CreateDownloadEntryFromItem(
 
 DownloadDBEntry CreateDownloadDBEntryFromItem(
     const DownloadItem& item,
-    DownloadSource download_source,
+    const UkmInfo& ukm_info,
     bool fetch_error_body,
     const DownloadUrlParameters::RequestHeadersType& request_headers) {
   DownloadDBEntry entry;
@@ -397,7 +397,6 @@ DownloadDBEntry CreateDownloadDBEntryFromItem(
 
   download_info.in_progress_info = in_progress_info;
 
-  UkmInfo ukm_info(download_source, GetUniqueDownloadId());
   download_info.ukm_info = ukm_info;
   entry.download_info = download_info;
   return entry;

components/download/internal/common/in_progress_download_manager.cc

@@ -37,6 +37,10 @@ std::unique_ptr<DownloadItemImpl> CreateDownloadItemImpl(
   if (!entry.download_info)
     return nullptr;
 
+  // DownloadDBEntry migrated from in-progress cache doesn't have Ids.
+  if (!entry.download_info->id)
+    return nullptr;
+
   base::Optional<InProgressInfo> in_progress_info =
       entry.download_info->in_progress_info;
   if (!in_progress_info)
@@ -452,8 +456,8 @@ void InProgressDownloadManager::StartDownloadWithItem(
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableDownloadDB)) {
     download_db_cache_->AddOrReplaceEntry(CreateDownloadDBEntryFromItem(
-        *download, info->download_source, info->fetch_error_body,
-        info->request_headers));
+        *download, UkmInfo(info->download_source, GetUniqueDownloadId()),
+        info->fetch_error_body, info->request_headers));
     download->RemoveObserver(download_db_cache_.get());
     download->AddObserver(download_db_cache_.get());
   } else {
@@ -492,6 +496,8 @@ void InProgressDownloadManager::OnInitialized(
     std::unique_ptr<std::vector<DownloadDBEntry>> entries) {
   for (const auto& entry : *entries) {
     auto item = CreateDownloadItemImpl(this, entry);
+    if (!item)
+      continue;
     item->AddObserver(download_db_cache_.get());
     in_progress_downloads_.emplace_back(std::move(item));
   }
@@ -525,4 +531,5 @@ std::vector<std::unique_ptr<download::DownloadItemImpl>>
 InProgressDownloadManager::TakeInProgressDownloads() {
   return std::move(in_progress_downloads_);
 }
+
 }  // namespace download

components/download/public/common/download_utils.h

@@ -72,7 +72,7 @@ COMPONENTS_DOWNLOAD_EXPORT DownloadEntry CreateDownloadEntryFromItem(
 // Helper functions for DownloadItem -> DownloadDBEntry for DownloadDB.
 COMPONENTS_DOWNLOAD_EXPORT DownloadDBEntry CreateDownloadDBEntryFromItem(
     const DownloadItem& item,
-    DownloadSource download_source,
+    const UkmInfo& ukm_info,
     bool fetch_error_body,
     const DownloadUrlParameters::RequestHeadersType& request_headers);