Improve download service scheduling logic

If two downloads have the same scheduling criteria and the same cancel
time, have the one that was created first be a better candidate to
start.

Bug: 772152
Change-Id: Ib1459a95a6703685b34d712cf5d00e188e5d5ccb
Reviewed-on: https://chromium-review.googlesource.com/703735Reviewed-by: Xing Liu <xingliu@chromium.org>
Commit-Queue: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506927}

components/download/internal/entry_utils.cc

@@ -54,10 +54,15 @@ Criteria GetSchedulingCriteria(const Model::EntryList& entries) {
 }
 
 bool EntryBetterThan(const Entry& lhs, const Entry& rhs) {
-  return lhs.scheduling_params.priority > rhs.scheduling_params.priority ||
-         (lhs.scheduling_params.priority == rhs.scheduling_params.priority &&
-          lhs.scheduling_params.cancel_time <
-              rhs.scheduling_params.cancel_time);
+  if (lhs.scheduling_params.priority != rhs.scheduling_params.priority)
+    return lhs.scheduling_params.priority > rhs.scheduling_params.priority;
+
+  if (lhs.scheduling_params.cancel_time != rhs.scheduling_params.cancel_time) {
+    return lhs.scheduling_params.cancel_time <
+           rhs.scheduling_params.cancel_time;
+  }
+
+  return lhs.create_time < rhs.create_time;
 }
 
 DownloadMetaData BuildDownloadMetaData(Entry* entry) {

components/download/internal/scheduler/scheduler_impl_unittest.cc

@@ -320,6 +320,31 @@ TEST_F(DownloadSchedulerImplTest, UIPriorityLoadBalancing) {
   MakeEntryActive(next);
 }
 
+TEST_F(DownloadSchedulerImplTest, PickOlderDownloadIfSameParameters) {
+  BuildScheduler(std::vector<DownloadClient>{DownloadClient::TEST,
+                                             DownloadClient::TEST_2});
+
+  // Client TEST: entry 0(Low priority, No Cancel Time, Newer).
+  // Client TEST: entry 1(Low priority, No Cancel Time, Older).
+  // Client TEST: entry 2(Low priority, No Cancel Time, Newer).
+  BuildDataEntries(3);
+  entries_[0].client = DownloadClient::TEST;
+  entries_[0].scheduling_params.priority = SchedulingParams::Priority::LOW;
+  entries_[0].create_time = base::Time::Now();
+  entries_[1].client = DownloadClient::TEST;
+  entries_[1].scheduling_params.priority = SchedulingParams::Priority::LOW;
+  entries_[1].create_time = base::Time::Now() - base::TimeDelta::FromDays(1);
+  entries_[2].client = DownloadClient::TEST;
+  entries_[2].scheduling_params.priority = SchedulingParams::Priority::LOW;
+  entries_[2].create_time = base::Time::Now();
+
+  Entry* next = scheduler_->Next(
+      entries(),
+      BuildDeviceStatus(BatteryStatus::CHARGING, NetworkStatus::UNMETERED));
+  EXPECT_EQ(&entries_[1], next);
+  MakeEntryActive(next);
+}
+
 // When multiple UI priority entries exist, the next entry is selected based on
 // cancel time and load balancing.
 TEST_F(DownloadSchedulerImplTest, MultipleUIPriorityEntries) {