Remove DownloadRow constructor.

This constructor takes 28 arguments. It's much clearer to just set the
members as desired.

Do some minor C++11 updates.

Change-Id: I357749f2631b6f6cd6fda2690a870918be769476
Reviewed-on: https://chromium-review.googlesource.com/804807
Commit-Queue: Brett Wilson <brettw@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521725}

chrome/browser/download/download_browsertest.cc

@@ -180,11 +180,7 @@ class CreatedObserver : public content::DownloadManager::Observer {
 
 class PercentWaiter : public content::DownloadItem::Observer {
  public:
-  explicit PercentWaiter(DownloadItem* item)
-    : item_(item),
-      waiting_(false),
-      error_(false),
-      prev_percent_(0) {
+  explicit PercentWaiter(DownloadItem* item) : item_(item) {
     item_->AddObserver(this);
   }
   ~PercentWaiter() override {
@@ -224,9 +220,9 @@ class PercentWaiter : public content::DownloadItem::Observer {
   }
 
   content::DownloadItem* item_;
-  bool waiting_;
-  bool error_;
-  int prev_percent_;
+  bool waiting_ = false;
+  bool error_ = false;
+  int prev_percent_ = 0;
 
   DISALLOW_COPY_AND_ASSIGN(PercentWaiter);
 };
@@ -352,12 +348,9 @@ bool DownloadTestObserverNotInProgress::IsDownloadInFinalState(
 
 class HistoryObserver : public DownloadHistory::Observer {
  public:
-  typedef base::Callback<bool(const history::DownloadRow&)> FilterCallback;
+  using FilterCallback = base::Callback<bool(const history::DownloadRow&)>;
 
-  explicit HistoryObserver(Profile* profile)
-      : profile_(profile),
-        waiting_(false),
-        seen_stored_(false) {
+  explicit HistoryObserver(Profile* profile) : profile_(profile) {
     DownloadCoreServiceFactory::GetForBrowserContext(profile_)
         ->GetDownloadHistory()
         ->AddObserver(this);
@@ -400,8 +393,8 @@ class HistoryObserver : public DownloadHistory::Observer {
 
  private:
   Profile* profile_;
-  bool waiting_;
-  bool seen_stored_;
+  bool waiting_ = false;
+  bool seen_stored_ = false;
   FilterCallback callback_;
 
   DISALLOW_COPY_AND_ASSIGN(HistoryObserver);

chrome/browser/download/download_history.cc

@@ -65,19 +65,15 @@ class DownloadHistoryData : public base::SupportsUserData::Data {
 
   static DownloadHistoryData* Get(content::DownloadItem* item) {
     base::SupportsUserData::Data* data = item->GetUserData(kKey);
-    return (data == NULL) ? NULL :
-      static_cast<DownloadHistoryData*>(data);
+    return static_cast<DownloadHistoryData*>(data);
   }
 
   static const DownloadHistoryData* Get(const content::DownloadItem* item) {
     const base::SupportsUserData::Data* data = item->GetUserData(kKey);
-    return (data == NULL) ? NULL
-                          : static_cast<const DownloadHistoryData*>(data);
+    return static_cast<const DownloadHistoryData*>(data);
   }
 
-  explicit DownloadHistoryData(content::DownloadItem* item)
-      : state_(NOT_PERSISTED),
-        was_restored_from_history_(false) {
+  explicit DownloadHistoryData(content::DownloadItem* item) {
     item->SetUserData(kKey, base::WrapUnique(this));
   }
 
@@ -107,9 +103,9 @@ class DownloadHistoryData : public base::SupportsUserData::Data {
  private:
   static const char kKey[];
 
-  PersistenceState state_;
+  PersistenceState state_ = NOT_PERSISTED;
   std::unique_ptr<history::DownloadRow> info_;
-  bool was_restored_from_history_;
+  bool was_restored_from_history_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(DownloadHistoryData);
 };
@@ -129,21 +125,38 @@ history::DownloadRow GetDownloadRow(
   }
 #endif
 
-  return history::DownloadRow(
-      item->GetFullPath(), item->GetTargetFilePath(), item->GetUrlChain(),
-      item->GetReferrerUrl(), item->GetSiteUrl(), item->GetTabUrl(),
-      item->GetTabReferrerUrl(),
-      std::string(),  // HTTP method (not available yet)
-      item->GetMimeType(), item->GetOriginalMimeType(), item->GetStartTime(),
-      item->GetEndTime(), item->GetETag(), item->GetLastModifiedTime(),
-      item->GetReceivedBytes(), item->GetTotalBytes(),
-      history::ToHistoryDownloadState(item->GetState()),
-      history::ToHistoryDownloadDangerType(item->GetDangerType()),
-      history::ToHistoryDownloadInterruptReason(item->GetLastReason()),
-      std::string(),  // Hash value (not available yet)
-      history::ToHistoryDownloadId(item->GetId()), item->GetGuid(),
-      item->GetOpened(), item->GetLastAccessTime(), item->IsTransient(),
-      by_ext_id, by_ext_name, history::GetHistoryDownloadSliceInfos(*item));
+  history::DownloadRow download;
+  download.current_path = item->GetFullPath();
+  download.target_path = item->GetTargetFilePath();
+  download.url_chain = item->GetUrlChain();
+  download.referrer_url = item->GetReferrerUrl();
+  download.site_url = item->GetSiteUrl();
+  download.tab_url = item->GetTabUrl();
+  download.tab_referrer_url = item->GetTabReferrerUrl();
+  download.http_method = std::string();  // HTTP method not available yet.
+  download.mime_type = item->GetMimeType();
+  download.original_mime_type = item->GetOriginalMimeType();
+  download.start_time = item->GetStartTime();
+  download.end_time = item->GetEndTime();
+  download.etag = item->GetETag();
+  download.last_modified = item->GetLastModifiedTime();
+  download.received_bytes = item->GetReceivedBytes();
+  download.total_bytes = item->GetTotalBytes();
+  download.state = history::ToHistoryDownloadState(item->GetState());
+  download.danger_type =
+      history::ToHistoryDownloadDangerType(item->GetDangerType());
+  download.interrupt_reason =
+      history::ToHistoryDownloadInterruptReason(item->GetLastReason());
+  download.hash = std::string();  // Hash value not available yet.
+  download.id = history::ToHistoryDownloadId(item->GetId());
+  download.guid = item->GetGuid();
+  download.opened = item->GetOpened();
+  download.last_access_time = item->GetLastAccessTime();
+  download.transient = item->IsTransient();
+  download.by_ext_id = by_ext_id;
+  download.by_ext_name = by_ext_name;
+  download.download_slice_info = history::GetHistoryDownloadSliceInfos(*item);
+  return download;
 }
 
 enum class ShouldUpdateHistoryResult {
@@ -340,7 +353,7 @@ void DownloadHistory::MaybeAddToHistory(content::DownloadItem* item) {
     return;
 
   data->SetState(DownloadHistoryData::PERSISTING);
-  if (data->info() == NULL) {
+  if (data->info() == nullptr) {
     // Keep the info here regardless of whether the item is in progress so that,
     // when ItemAdded() calls OnDownloadUpdated(), it can decide whether to
     // Update the db and/or clear the info.

chrome/browser/safe_browsing/incident_reporting/last_download_finder_unittest.cc

@@ -244,35 +244,32 @@ class LastDownloadFinderTest : public testing::Test {
   history::DownloadRow CreateTestDownloadRow(
       const base::FilePath::CharType* file_path) {
     base::Time now(base::Time::Now());
-    return history::DownloadRow(
-        base::FilePath(file_path), base::FilePath(file_path),
-        std::vector<GURL>(1, GURL("http://www.google.com")),  // url_chain
-        GURL("http://referrer.example.com/"),                 // referrer
-        GURL("http://site-url.example.com/"),        // site instance URL
-        GURL("http://tab-url.example.com/"),         // tab URL
-        GURL("http://tab-referrer.example.com/"),    // tab referrer URL
-        std::string(),                               // HTTP method
-        "application/octet-stream",                  // mime_type
-        "application/octet-stream",                  // original_mime_type
-        now - base::TimeDelta::FromMinutes(10),      // start
-        now - base::TimeDelta::FromMinutes(9),       // end
-        std::string(),                               // etag
-        std::string(),                               // last_modified
-        47LL,                                        // received
-        47LL,                                        // total
-        history::DownloadState::COMPLETE,            // download_state
-        history::DownloadDangerType::NOT_DANGEROUS,  // danger_type
-        history::ToHistoryDownloadInterruptReason(
-            content::DOWNLOAD_INTERRUPT_REASON_NONE),  // interrupt_reason,
-        std::string(),                                 // hash
-        download_id_++,                                // id
-        base::GenerateGUID(),                          // GUID
-        false,                                         // download_opened
-        now - base::TimeDelta::FromMinutes(5),         // last_access_time
-        false,                                         // transient
-        std::string(),                                 // ext_id
-        std::string(),                                 // ext_name
-        std::vector<history::DownloadSliceInfo>());    // download_slice_info
+
+    history::DownloadRow row;
+    row.current_path = base::FilePath(file_path);
+    row.target_path = base::FilePath(file_path);
+    row.url_chain.push_back(GURL("http://www.google.com/"));
+    row.referrer_url = GURL("http://referrer.example.com/");
+    row.site_url = GURL("http://site-url.example.com/");
+    row.tab_url = GURL("http://tab-url.example.com/");
+    row.tab_referrer_url = GURL("http://tab-referrer.example.com/");
+    row.mime_type = "application/octet-stream";
+    row.original_mime_type = "application/octet-stream";
+    row.start_time = now - base::TimeDelta::FromMinutes(10);
+    row.end_time = now - base::TimeDelta::FromMinutes(9);
+    row.received_bytes = 47;
+    row.total_bytes = 47;
+    row.state = history::DownloadState::COMPLETE;
+    row.danger_type = history::DownloadDangerType::NOT_DANGEROUS;
+    row.interrupt_reason = history::ToHistoryDownloadInterruptReason(
+        content::DOWNLOAD_INTERRUPT_REASON_NONE);
+    row.id = download_id_++;
+    row.guid = base::GenerateGUID();
+    row.opened = false;
+    row.last_access_time = now - base::TimeDelta::FromMinutes(5);
+    row.transient = false;
+
+    return row;
   }
 
  private:

components/history/core/browser/download_row.cc

@@ -8,80 +8,12 @@
 
 namespace history {
 
-DownloadRow::DownloadRow()
-    : received_bytes(0),
-      total_bytes(0),
-      state(DownloadState::IN_PROGRESS),
-      danger_type(DownloadDangerType::NOT_DANGEROUS),
-      interrupt_reason(0),
-      id(kInvalidDownloadId),
-      opened(false),
-      transient(false) {
-  // |interrupt_reason| is left undefined by this constructor as the value
-  // has no meaning unless |state| is equal to kStateInterrupted.
-}
-
-DownloadRow::DownloadRow(
-    const base::FilePath& current_path,
-    const base::FilePath& target_path,
-    const std::vector<GURL>& url_chain,
-    const GURL& referrer_url,
-    const GURL& site_url,
-    const GURL& tab_url,
-    const GURL& tab_referrer_url,
-    const std::string& http_method,
-    const std::string& mime_type,
-    const std::string& original_mime_type,
-    base::Time start,
-    base::Time end,
-    const std::string& etag,
-    const std::string& last_modified,
-    int64_t received,
-    int64_t total,
-    DownloadState download_state,
-    DownloadDangerType danger_type,
-    DownloadInterruptReason interrupt_reason,
-    const std::string& hash,
-    DownloadId id,
-    const std::string& guid,
-    bool download_opened,
-    base::Time last_access,
-    bool transient,
-    const std::string& ext_id,
-    const std::string& ext_name,
-    const std::vector<DownloadSliceInfo>& download_slice_info)
-    : current_path(current_path),
-      target_path(target_path),
-      url_chain(url_chain),
-      referrer_url(referrer_url),
-      site_url(site_url),
-      tab_url(tab_url),
-      tab_referrer_url(tab_referrer_url),
-      http_method(http_method),
-      mime_type(mime_type),
-      original_mime_type(original_mime_type),
-      start_time(start),
-      end_time(end),
-      etag(etag),
-      last_modified(last_modified),
-      received_bytes(received),
-      total_bytes(total),
-      state(download_state),
-      danger_type(danger_type),
-      interrupt_reason(interrupt_reason),
-      hash(hash),
-      id(id),
-      guid(guid),
-      opened(download_opened),
-      last_access_time(last_access),
-      transient(transient),
-      by_ext_id(ext_id),
-      by_ext_name(ext_name),
-      download_slice_info(download_slice_info) {}
-
+DownloadRow::DownloadRow() = default;
 DownloadRow::DownloadRow(const DownloadRow& other) = default;
+DownloadRow::DownloadRow(DownloadRow&& other) = default;
+DownloadRow::~DownloadRow() = default;
 
-DownloadRow::~DownloadRow() {}
+DownloadRow& DownloadRow::operator=(const DownloadRow& other) = default;
 
 bool DownloadRow::operator==(const DownloadRow& rhs) const {
   return current_path == rhs.current_path && target_path == rhs.target_path &&

components/history/core/browser/download_row.h

@@ -12,6 +12,7 @@
 
 #include "base/files/file_path.h"
 #include "base/time/time.h"
+#include "components/history/core/browser/download_constants.h"
 #include "components/history/core/browser/download_slice_info.h"
 #include "components/history/core/browser/download_types.h"
 #include "url/gurl.h"
@@ -23,37 +24,11 @@ namespace history {
 // DownloadDatabase through the HistoryService.
 struct DownloadRow {
   DownloadRow();
-  DownloadRow(const base::FilePath& current_path,
-              const base::FilePath& target_path,
-              const std::vector<GURL>& url_chain,
-              const GURL& referrer_url,
-              const GURL& site_url,
-              const GURL& tab_url,
-              const GURL& tab_referrer_url,
-              const std::string& http_method,
-              const std::string& mime_type,
-              const std::string& original_mime_type,
-              base::Time start,
-              base::Time end,
-              const std::string& etag,
-              const std::string& last_modified,
-              int64_t received,
-              int64_t total,
-              DownloadState download_state,
-              DownloadDangerType danger_type,
-              DownloadInterruptReason interrupt_reason,
-              const std::string& hash,
-              DownloadId id,
-              const std::string& guid,
-              bool download_opened,
-              base::Time last_access,
-              bool transient,
-              const std::string& ext_id,
-              const std::string& ext_name,
-              const std::vector<DownloadSliceInfo>& download_slice_info);
   DownloadRow(const DownloadRow& other);
+  DownloadRow(DownloadRow&& other);
   ~DownloadRow();
 
+  DownloadRow& operator=(const DownloadRow&);
   bool operator==(const DownloadRow&) const;
 
   // The current path to the download (potentially different from final if
@@ -106,20 +81,20 @@ struct DownloadRow {
   std::string last_modified;
 
   // The number of bytes received (so far).
-  int64_t received_bytes;
+  int64_t received_bytes = 0;
 
   // The total number of bytes in the download. Is not changed by
   // UpdateDownload().
-  int64_t total_bytes;
+  int64_t total_bytes = 0;
 
   // The current state of the download.
-  DownloadState state;
+  DownloadState state = DownloadState::IN_PROGRESS;
 
   // Whether and how the download is dangerous.
-  DownloadDangerType danger_type;
+  DownloadDangerType danger_type = DownloadDangerType::NOT_DANGEROUS;
 
   // The reason the download was interrupted, if state == kStateInterrupted.
-  DownloadInterruptReason interrupt_reason;
+  DownloadInterruptReason interrupt_reason = 0;
 
   // The raw SHA-256 hash of the complete or partial download contents. Not hex
   // encoded.
@@ -128,20 +103,20 @@ struct DownloadRow {
   // The id of the download in the database. Is not changed by UpdateDownload().
   // Note: This field should be considered deprecated in favor of |guid| below.
   // See http://crbug.com/593020.
-  DownloadId id;
+  DownloadId id = kInvalidDownloadId;
 
   // The GUID of the download in the database. Not changed by UpdateDownload().
   std::string guid;
 
   // Whether this download has ever been opened from the browser.
-  bool opened;
+  bool opened = false;
 
   // The time when the download was last accessed.
   base::Time last_access_time;
 
   // Whether this download is transient. Transient items are cleaned up after
   // completion and not shown in the UI.
-  bool transient;
+  bool transient = false;
 
   // The id and name of the extension that created this download.
   std::string by_ext_id;

components/history/core/browser/download_types.h

@@ -42,7 +42,7 @@ std::ostream& operator<<(std::ostream& stream, DownloadDangerType danger_type);
 // of a DownloadRow into the DownloadDatabase. The values must not be changed
 // as they are saved to disk in the database. They have no meaning for the
 // history component.
-typedef int32_t DownloadInterruptReason;
+using DownloadInterruptReason = int32_t;
 
 // Utility functions to convert between int and DownloadInterruptReason for
 // serialization to the download database.
@@ -52,7 +52,7 @@ int DownloadInterruptReasonToInt(DownloadInterruptReason interrupt_reason);
 // DownloadId represents the id of a DownloadRow into the DownloadDatabase.
 // The value is controlled by the embedder except for the reserved id
 // kInvalidDownloadId.
-typedef uint32_t DownloadId;
+using DownloadId = uint32_t;
 
 // Utility functions to convert between int and DownloadId for
 // serialization to the download database.

components/history/core/browser/history_backend.cc

@@ -2009,8 +2009,8 @@ bool HistoryBackend::SetFaviconBitmaps(favicon_base::FaviconID icon_id,
   std::vector<FaviconBitmapIDSize> bitmap_id_sizes;
   thumbnail_db_->GetFaviconBitmapIDSizes(icon_id, &bitmap_id_sizes);
 
-  typedef std::pair<scoped_refptr<base::RefCountedBytes>, gfx::Size>
-      PNGEncodedBitmap;
+  using PNGEncodedBitmap =
+      std::pair<scoped_refptr<base::RefCountedBytes>, gfx::Size>;
   std::vector<PNGEncodedBitmap> to_add;
   for (size_t i = 0; i < bitmaps.size(); ++i) {
     scoped_refptr<base::RefCountedBytes> bitmap_data(new base::RefCountedBytes);

components/history/core/test/history_backend_db_base_test.cc

@@ -125,20 +125,31 @@ bool HistoryBackendDBBaseTest::AddDownload(uint32_t id,
                                            const std::string& guid,
                                            DownloadState state,
                                            base::Time time) {
-  std::vector<GURL> url_chain;
-  url_chain.push_back(GURL("foo-url"));
-
-  DownloadRow download(
-      base::FilePath(FILE_PATH_LITERAL("current-path")),
-      base::FilePath(FILE_PATH_LITERAL("target-path")), url_chain,
-      GURL("http://referrer.example.com/"), GURL("http://site-url.example.com"),
-      GURL("http://tab-url.example.com/"),
-      GURL("http://tab-referrer-url.example.com/"), std::string(),
-      "application/vnd.oasis.opendocument.text", "application/octet-stream",
-      time, time, std::string(), std::string(), 0, 512, state,
-      DownloadDangerType::NOT_DANGEROUS, kTestDownloadInterruptReasonNone,
-      std::string(), id, guid, false, time, true, "by_ext_id", "by_ext_name",
-      std::vector<DownloadSliceInfo>());
+  DownloadRow download;
+  download.current_path = base::FilePath(FILE_PATH_LITERAL("current-path"));
+  download.target_path = base::FilePath(FILE_PATH_LITERAL("target-path"));
+  download.url_chain.push_back(GURL("foo-url"));
+  download.referrer_url = GURL("http://referrer.example.com/");
+  download.site_url = GURL("http://site-url.example.com");
+  download.tab_url = GURL("http://tab-url.example.com/");
+  download.tab_referrer_url = GURL("http://tab-referrer-url.example.com/");
+  download.http_method = std::string();
+  download.mime_type = "application/vnd.oasis.opendocument.text";
+  download.original_mime_type = "application/octet-stream";
+  download.start_time = time;
+  download.end_time = time;
+  download.received_bytes = 0;
+  download.total_bytes = 512;
+  download.state = state;
+  download.danger_type = DownloadDangerType::NOT_DANGEROUS;
+  download.interrupt_reason = kTestDownloadInterruptReasonNone;
+  download.id = id;
+  download.guid = guid;
+  download.opened = false;
+  download.last_access_time = time;
+  download.transient = true;
+  download.by_ext_id = "by_ext_id";
+  download.by_ext_name = "by_ext_name";
   return db_->CreateDownload(download);
 }