only populate DownloadHistoryData for inprogress download

When download history is not fully loaded, onDownloadCreated() can get
called by downloads already created by the DownloadManager.
And DownloadHistory will call set_info() to attach DownloadRow
to the download item regardless of its state.
However, this is not needed for non in-progress download. For non in-progress
downloads, if set_info() is called, a later DownloadHistory::OnDownloadUpdated()
call could still result in clear_info() gets called before
DownloadHistory::ItemAdded(). So the assumption in the comments may not
hold.
This CL also removes the call to OnDownloadUpdated() in ItemAdded().
OnDownloadUpdated() can be called any time before ItemAdded(), and it
will queue a sql update to the history db. So there is no need
to call OnDownloadUpdated after ItemAdded().
This CL also exposes and fixes a bug in DownloadHistoryTest

BUG=842245

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

chrome/browser/download/download_history.cc

@@ -370,18 +370,15 @@ void DownloadHistory::MaybeAddToHistory(download::DownloadItem* item) {
     return;
 
   data->SetState(DownloadHistoryData::PERSISTING);
-  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.
-    data->set_info(GetDownloadRow(item));
-  }
-
-  history_->CreateDownload(*data->info(), base::Bind(
-      &DownloadHistory::ItemAdded, weak_ptr_factory_.GetWeakPtr(),
-      download_id));
-  for (Observer& observer : observers_)
-    observer.OnDownloadStored(item, *data->info());
+  // Keep the info for in-progress download, so we can check whether history DB
+  // update is needed when DownloadUpdated() is called.
+  history::DownloadRow download_row = GetDownloadRow(item);
+  if (item->GetState() == download::DownloadItem::IN_PROGRESS)
+    data->set_info(download_row);
+  history_->CreateDownload(
+      download_row,
+      base::BindRepeating(&DownloadHistory::ItemAdded,
+                          weak_ptr_factory_.GetWeakPtr(), download_id));
 }
 
 void DownloadHistory::ItemAdded(uint32_t download_id, bool success) {
@@ -431,9 +428,6 @@ void DownloadHistory::ItemAdded(uint32_t download_id, bool success) {
     for (Observer& observer : observers_)
       observer.OnDownloadStored(item, *data->info());
   }
-
-  // In case the item changed or became temporary while it was being added.
-  OnDownloadUpdated(notifier_.GetManager(), item);
 }
 
 void DownloadHistory::OnDownloadCreated(content::DownloadManager* manager,

chrome/browser/download/download_history_unittest.cc

@@ -871,6 +871,7 @@ TEST_F(DownloadHistoryTest, CreateWithDownloadDB) {
   // Completed download should be inserted.
   EXPECT_CALL(item(0), GetState())
       .WillRepeatedly(Return(download::DownloadItem::COMPLETE));
+  info.state = history::DownloadState::COMPLETE;
   item(0).NotifyObserversDownloadUpdated();
   ExpectDownloadCreated(info);
 }